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

bug: fix installation of dependency by using the resolved_version instead of actual_version #266

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

shubhbapna
Copy link
Collaborator

@shubhbapna shubhbapna commented Jul 29, 2024

We noticed that dependencies that were resolving to some different version were not actually being installed using that version. Here is my analysis of the bug:

_maybe_install is called after handling each requirement. If the resolved version is not none, then it checks whether the actual version matches the resolved version and if it doesn't then it installs it using safe_install but in_maybe_install it seems like we are passing the old requirement to safe_install instead of the resolved one: https://github.com/python-wheel-build/fromager/blob/main/src/fromager/sdist.py#L514

Copy link
Collaborator

@Gregory-Pereira Gregory-Pereira left a comment

Choose a reason for hiding this comment

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

Overall /lgtm. This change makes sense, still want to wait for Doug to confirm on this one though.

Copy link
Member

@dhellmann dhellmann left a comment

Choose a reason for hiding this comment

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

Nice find!

@dhellmann dhellmann merged commit eb03139 into python-wheel-build:main Jul 29, 2024
53 checks passed
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.

3 participants