Add sdist support by TG1999 · Pull Request #31 · aboutcode-org/python-inspector

TG1999 · 2022-06-28T16:30:45Z

Signed-off-by: Tushar Goel tushar.goel.dav@gmail.com

AyanSinhaMahapatra · 2022-07-01T15:37:18Z

@TG1999 thanks! This looks great. Btw, which of the issues opened in scancode-toolkit does this fix, all of them? i.e. aboutcode-org/scancode-toolkit#3011 aboutcode-org/scancode-toolkit#3008 aboutcode-org/scancode-toolkit#3009?

TG1999 · 2022-07-01T18:44:28Z

@TG1999 thanks! This looks great. Btw, which of the issues opened in scancode-toolkit does this fix, all of them? i.e. nexB/scancode-toolkit#3011 nexB/scancode-toolkit#3008 nexB/scancode-toolkit#3009?

@AyanSinhaMahapatra Yes!
Scope is now not same but based on sections
The files that were not getting parsed previously are getting parsed
And the extracted_requirement is as expected

pombredanne

Thanks!
IMHO we need some more unit tests and try to see where you can use objects or object attributes rather than dictionaries

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

pombredanne · 2022-07-11T10:18:58Z

The PR title "Add failing test" is likely outdated now?

pombredanne

The changes to "src/_packagedcode/pypi.py" should be applied also to ScanCode TK.
You can refine them here alright, but in the end you must add the changes back to SCTK and also have tests in SCTK for the changes. The tests for your "src/_packagedcode/pypi.py" changes should all be in SCTK IMHO.

The current situation where we vendor and copy of pypi.py from SCTK is temporary and we need to be careful to keep the two in sync by hand for now.

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

pombredanne

Thanks for the updates. See some comments for your review.
IMHO we could also use a different design rather than the "Wheel" and "Sdist" formats data you use for now.

pombredanne · 2022-07-13T08:41:09Z

            parser.read_file(f)
-
        for section in parser.values():
+            if section.name == 'options':


You likely want to add also the legacy "setup_requires" and "test_requires" as well as the "python_requires"

@pombredanne can you provide me some sample setup.cfg file with legacy "setup_requires", "test_requires", "python_requires"

See https://github.com/search?l=INI&q="setup_requires"&type=Code and https://github.com/karpierz/QQt/blob/930e22d9612f814fa242b1547ad037037ef6b5c7/setup.cfg
And also https://github.com/search?l=INI&q="test_requires"&type=Code for test_requires

Other notes:

use python as a scope for the "python_requires" and use a Package URL of pkg:generic/python such that this is clear that this is not a PyPI package. That way you can spot this special package and scope as needed to use it in your processing

make sure these fixes are also applied to setup.py parsing as well as other PyPI metadata ... the important part is mostly for python_requires ... setup/test requires are legacy and could only be seen in setup.py/setup.cfg

you need tests also on the SCTK side... you may want to start there instead

for setup_requires, use setup as scope.

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

TG1999 · 2022-07-13T13:14:59Z

@pombredanne

IMHO we could also use a different design rather than the "Wheel" and "Sdist" formats data you use for now

I have changed the design please check if this looks good

pombredanne

Thanks. See my comments for your consideration.

pombredanne · 2022-07-13T17:35:33Z

            parser.read_file(f)
-
        for section in parser.values():
+            if section.name == 'options':


See https://github.com/search?l=INI&q="setup_requires"&type=Code and https://github.com/karpierz/QQt/blob/930e22d9612f814fa242b1547ad037037ef6b5c7/setup.cfg
And also https://github.com/search?l=INI&q="test_requires"&type=Code for test_requires

Other notes:

use python as a scope for the "python_requires" and use a Package URL of pkg:generic/python such that this is clear that this is not a PyPI package. That way you can spot this special package and scope as needed to use it in your processing

make sure these fixes are also applied to setup.py parsing as well as other PyPI metadata ... the important part is mostly for python_requires ... setup/test requires are legacy and could only be seen in setup.py/setup.cfg

you need tests also on the SCTK side... you may want to start there instead

for setup_requires, use setup as scope.

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

pombredanne

Thanks!
Please see some nitpickings for your review

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

pombredanne

LGTM! Thanks!

TG1999 force-pushed the supported_wheels_failure branch 3 times, most recently from 4766526 to 4acf8ce Compare July 1, 2022 14:39

TG1999 requested review from AyanSinhaMahapatra, JonoYang and pombredanne July 1, 2022 14:39

AyanSinhaMahapatra approved these changes Jul 1, 2022

View reviewed changes

TG1999 force-pushed the supported_wheels_failure branch from 4acf8ce to 726d8a8 Compare July 1, 2022 18:43

TG1999 force-pushed the supported_wheels_failure branch 4 times, most recently from 7c99106 to fd73eed Compare July 6, 2022 16:22

pombredanne requested changes Jul 7, 2022

View reviewed changes

TG1999 added 5 commits July 8, 2022 19:50

Add failing test

0a489ec

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Add sdist support

291a6a0

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Fix dependency parsing

1cbebd5

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Support both wheels and sdist

e7ec8f7

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Ignore resolved dependencies

a6cb7d4

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

pombredanne requested changes Jul 11, 2022

View reviewed changes

TG1999 changed the title ~~Add failing test~~ Add sdist support Jul 11, 2022

TG1999 marked this pull request as draft July 11, 2022 14:01

TG1999 force-pushed the supported_wheels_failure branch 2 times, most recently from e3e4253 to ce9d58a Compare July 11, 2022 16:01

TG1999 marked this pull request as ready for review July 11, 2022 16:04

Add tests and address review comments

a04a634

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

TG1999 force-pushed the supported_wheels_failure branch from ce9d58a to a04a634 Compare July 11, 2022 16:32

pombredanne requested changes Jul 13, 2022

View reviewed changes

TG1999 added 2 commits July 13, 2022 16:54

Address review comments

03b7b1c

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Change API structure

6e7391e

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

pombredanne requested changes Jul 13, 2022

View reviewed changes

TG1999 force-pushed the supported_wheels_failure branch from 2f31bdf to cbde1d7 Compare July 15, 2022 10:21

TG1999 requested a review from pombredanne July 15, 2022 10:21

TG1999 force-pushed the supported_wheels_failure branch 2 times, most recently from 090db29 to b69630e Compare July 15, 2022 13:56

Address review comment

2b8134a

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

TG1999 force-pushed the supported_wheels_failure branch from b69630e to 2b8134a Compare July 15, 2022 15:39

pombredanne requested changes Jul 18, 2022

View reviewed changes

Address review comments

065bdce

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

pombredanne approved these changes Jul 20, 2022

View reviewed changes

pombredanne merged commit 6dab7e1 into main Jul 20, 2022

pombredanne deleted the supported_wheels_failure branch July 20, 2022 06:31

Uh oh!

Uh oh!

Conversation

TG1999 commented Jun 28, 2022

Uh oh!

AyanSinhaMahapatra commented Jul 1, 2022

Uh oh!

TG1999 commented Jul 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pombredanne commented Jul 11, 2022

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pombredanne Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

TG1999 Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

pombredanne Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TG1999 commented Jul 13, 2022

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pombredanne Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

TG1999 commented Jul 1, 2022 •

edited

Loading