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

fix: Use the fallback PKG-INFO file if the one generated manually is incomplete #523

Merged
merged 8 commits into from
Jul 25, 2023

Conversation

lucashuy
Copy link
Contributor

@lucashuy lucashuy commented Jul 13, 2023

Issue #, if available:
#526

Description of changes:
Lambda Builders will invoke the setup.py file of source packages to determine the name and version of a package. The PKG-INFO file generated could be incomplete (UNKNOWN name or 0.0.0 version). If either of these are found, then we'll fall back to the file that is generated by pip download.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@lucashuy lucashuy marked this pull request as ready for review July 14, 2023 20:50
@lucashuy lucashuy requested a review from a team as a code owner July 14, 2023 20:50
"""
# default values logic:
# https://github.com/pypa/setuptools/blob/6083e18f4afc40316c0112134c205c336afbcdfd/setuptools/_distutils/dist.py#L1185-L1189
return name == "UNKNOWN" or version == "0.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be and? Doc string indicates and.

Copy link
Contributor Author

@lucashuy lucashuy Jul 21, 2023

Choose a reason for hiding this comment

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

Hmm, thats a good call out. This is an or, so the doc string should be updated. Reason why the version check is also included here is that the assumption is being made that the packages wouldn't have version 0.0.0. In any case, if it did it would just grab the fall back file (the one pip download generates with the build deps) and use what it has.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Probably fine. I did a quick search and seems like most package versions start with 0.1.0 or 0.0.1. I am sure there are packages with 0.0.0 but we can always handle those in one offs.

The "safer" thing is probably doing a version check for 0.0.0 only if UNKNOWN is given but I think what you have is fine.

if not self._is_default_setuptools_values(fallback_name, fallback_version):
name = fallback_name
version = fallback_version

return name, version
Copy link
Contributor

Choose a reason for hiding this comment

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

in case they are not the fallback name and version, what are they?

Copy link
Contributor Author

@lucashuy lucashuy Jul 21, 2023

Choose a reason for hiding this comment

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

If the fallback name or version turn out to also be the default values, then it will use the the initially determined values. Basically, no replacement logic will happen.

@lucashuy lucashuy added this pull request to the merge queue Jul 25, 2023
Merged via the queue into aws:develop with commit 5b7daf6 Jul 25, 2023
123 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants