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: let upgrade --dry-run instantiate prefix for pypi #2771

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lucascolley
Copy link
Contributor

@lucascolley lucascolley commented Dec 27, 2024

description: #2770 (comment)

Closes #2770

@Hofer-Julian
Copy link
Contributor

Thanks for your contribution @lucascolley.

Since you marked this as draft: what is still missing? :)

@lucascolley
Copy link
Contributor Author

I think I ready somewhere in the code that dry-run was about "writing to disk" or not - I guess we are breaking that model here, so maybe can adjust wherever I read that?

We can also remove the commented out line in the test.

The description of --dry-run in the docs says

Only show the changes that would be made, without actually updating the manifest, lock file, or environment.

Does this count as updating the environment? Either way, we should probably make it clear that users should also pass --no-install if they don't want this behaviour to happen.

@Hofer-Julian
Copy link
Contributor

Only show the changes that would be made, without actually updating the manifest, lock file, or environment.

Ah yes, that expectation still holds true. I think we need to find a different way to fix #2770

@lucascolley
Copy link
Contributor Author

A new --dry-run-pypi flag?

@Hofer-Julian
Copy link
Contributor

A new --dry-run-pypi flag?

What would that flag do? :)

@lucascolley
Copy link
Contributor Author

Well the "dry" part that I care about here is not performing the upgrade.

Behaviour Main This PR What would that flag do? :)
Completely dry --dry-run --dry-run --no-install --dry-run
Allow instantiation of prefix for PyPI, do not perform upgrade impossible --dry-run --dry-run-pypi

@Hofer-Julian
Copy link
Contributor

Understood. We'd probably have to think of a different name for this.

Let's wait for @ruben-arts to chime in on that one.

@ruben-arts
Copy link
Contributor

What if we make the no install logic, still install a solve prefix for the pypi work? This might be a little confusing at first but atleast it's going to do what you need to perform the least amount of work for the command you ask from pixi.

@lucascolley
Copy link
Contributor Author

That would work for me as long as it's documented clearly!

@lucascolley
Copy link
Contributor Author

What if we make the no install logic, still install a solve prefix for the pypi work? This might be a little confusing at first but atleast it's going to do what you need to perform the least amount of work for the command you ask from pixi.

actually, what do you think @baszalmstra ? That seems to conflict with

/// Defines if during the update-process it is allowed to create prefixes.
/// This might be required to solve pypi dependencies because those require
/// a python interpreter.
pub(crate) fn with_no_install(self, no_install: bool) -> Self {
Self { no_install, ..self }
}

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.

pixi upgrade --dry-run fails with "cannot update pypi dependencies"
3 participants