Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer using PKG-INFO from .egg-info in assemble #3083 #3091

Merged
merged 3 commits into from
Sep 6, 2022

Conversation

JonoYang
Copy link
Contributor

@JonoYang JonoYang commented Sep 3, 2022

This PR addresses #3083, where we have no dependencies returned when we scanned an installable Python codebase. This issue is cause by BaseExtractedPythonLayout.assemble(), where it would use the package data from the PKG-INFO file from the root of a Python codebase rather than from the PKG-INFO file from the .egg-info directory (also located in the root of a Python codebase).

We want to use the PKG-INFO file from the .egg-info directory because the package data from that contains the dependency information.

Copy link
Contributor

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
There are a few nit that likely are leftover from before... we should better track the extracted requirements and treat PyPI extras as optional

{
"purl": "pkg:pypi/azure-storage-blob@12.9.0",
"extracted_requirement": "azure-storage-blob==12.9.0; extra == \"azureblockblob\"",
"scope": "azurebl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have an extracted requirement of "msgpack"

{
"purl": "pkg:pypi/azure-storage-blob@12.9.0",
"extracted_requirement": "azure-storage-blob==12.9.0; extra == \"azureblockblob\"",
"scope": "azurebl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have instead:

Suggested change
"scope": "azurebl
"extracted_requirement": "couchbase>=3.0.0",

"extracted_requirement": "pylibmc; platform_system != \"Windows\" and extra == \"memcache\"",
"scope": "memcache",
"is_runtime": true,
"is_optional": false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO we should be true for "extras" e.g. not an "install_requires" and therefore not in "install" scope:

Suggested change
"is_optional": false,
"is_optional": true,

    * Add test for checking that the .egg-info PKG-INFO is the only Package source reported
    * Update test expectations

Signed-off-by: Jono Yang <jyang@nexb.com>
    * Ensure extracted_requirement contains dependency name
    * Update test expectations

Signed-off-by: Jono Yang <jyang@nexb.com>
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Copy link
Contributor Author

JonoYang commented Sep 6, 2022

I've updated the code to return the dependency name in the extracted_requirements and to properly set the is_optional field on the dependencies listed in extra_requires.

Tests are passing, so I will merge this.

@JonoYang JonoYang merged commit f056c59 into develop Sep 6, 2022
@JonoYang JonoYang deleted the 3083-no-pkg-info-deps branch September 6, 2022 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants