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
Member

@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.

@JonoYang JonoYang requested a review from pombredanne September 3, 2022 00:57
@JonoYang JonoYang force-pushed the 3083-no-pkg-info-deps branch from ef37a0f to 2eb59ac Compare September 3, 2022 01:20
Copy link
Member

@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
Member

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
Member

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
Member

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>
@JonoYang JonoYang force-pushed the 3083-no-pkg-info-deps branch from 2eb59ac to 99f5034 Compare September 6, 2022 17:44
    * Ensure extracted_requirement contains dependency name
    * Update test expectations

Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang JonoYang force-pushed the 3083-no-pkg-info-deps branch from 99f5034 to 9433eec Compare September 6, 2022 19:25
Signed-off-by: Jono Yang <jyang@nexb.com>
@JonoYang
Copy link
Member 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