Skip to content

fix: install specified version of --preinstall dependency instead of latest version #1379

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Apr 29, 2024
Merged

fix: install specified version of --preinstall dependency instead of latest version #1379

merged 8 commits into from
Apr 29, 2024

Conversation

davidpeckham
Copy link
Contributor

@davidpeckham davidpeckham commented Apr 26, 2024 •
edited
Loading

  • I have added a news fragment under changelog.d/ (if the patch affects the end users)

Summary of changes

Fixes #1377

chrysle found the problem -- commands/install.py calls package_name_from_spec when it shouldn't. I removed that call and passed the dependencies unmodified to venv.upgrade_package_no_metadata.

Test plan

I added this test case to tests/test_install.py:

def test_preinstall_specific_version(pipx_temp_env, caplog):
    assert not run_pipx_cli(["install", "--preinstall", "black==22.8.0", "nox"])
    assert "black==22.8.0" in caplog.text

Tested by running

pipx install --preinstall black==22.8.0 nox

@davidpeckham davidpeckham changed the title pipx --preinstall ignores dependency version specifier #1377 fix: install specified version of --preinstall dependency instead of latest version Apr 26, 2024
@chrysle
Copy link
Contributor

chrysle commented Apr 27, 2024

How about running pipx.package_specifier.parse_specifier_for_install over the deps instead?

@davidpeckham
Copy link
Contributor Author

How about running pipx.package_specifier.parse_specifier_for_install over the deps instead?

Thanks, I added that.

@chrysle
Copy link
Contributor

chrysle commented Apr 27, 2024

My bad, I've just seen the function is already called in pipx.venv.install_package. Could you revert the change?

@chrysle
Copy link
Contributor

chrysle commented Apr 28, 2024

@davidpeckham 7339ae0 seems to be empty...

@davidpeckham
Copy link
Contributor Author

@chrysle I think that last commit fixed it.

@chrysle chrysle merged commit ec2ee5c into pypa:main Apr 29, 2024
@chrysle
Copy link
Contributor

chrysle commented Apr 29, 2024

Thanks!

@davidpeckham davidpeckham deleted the preinstall-specific-version branch May 6, 2024 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrysle chrysle chrysle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pipx --preinstall ignores dependency version specifier
2 participants
@davidpeckham @chrysle