JuliaLang / Pkg.jl

Pkg - Package manager for the Julia programming language
https://pkgdocs.julialang.org
Other
613 stars 251 forks source link

Prompt the user on a manifest version mismatch #3775

Open tecosaur opened 5 months ago

tecosaur commented 5 months ago

Currently when the current project's Manifest's julia_version doesn't match the current Julia version, Pkg either:

This PR adds a check that results in a prompt when the versions differ, offering three possible courses of action:

image

The default behaviour can be set via the new environment variable JULIA_PKG_MANIFEST_MISMATCH_ACTION

This PR was prompted by this Zulip gripe thread: https://julialang.zulipchat.com/#narrow/stream/225540-gripes/topic/Manifests.20annoyances

tecosaur commented 5 months ago

I have noticed one bug with this PR that I could do with help resolving, it seems that when using a mismatched manifest, and you just enter the pkg> repl and type rm<space> the prompt is shown, but you can't respond to it (and ^C does nothing). I'm not sure what's happening here?

image

IanButterworth commented 5 months ago

The bug you're seeing is because of the hint tab completion populating packages to tab complete by reading the manifest.

Note that adding just a warning at a similar code level stalled because it might be too annoying https://github.com/JuliaLang/Pkg.jl/pull/3024

Maybe this is more acceptable given it is actionable now we have Manifest-v1.11.toml handling, as your prompt suggests.

IanButterworth commented 5 months ago

To handle the tab completion hint issue, maybe suppress the error if it's coming from the completion handler and prevent the manifest being read so that it's read in during the next actual prompt execution, at which point an interactive prompt makes more sense IMO.

MasonProtter commented 5 months ago

Note that adding just a warning at a similar code level stalled because it might be too annoying #3024

Didn't some version of that end up making it's way in? I get that warning from time to time, e.g.:

$ julia-1.10 --project=TSc1D
┌ Warning: The active manifest file has dependencies that were resolved with a different julia version (1.7.2). Unexpected behavior may occur.
└ @ ~/TSc1D/Manifest.toml:0
julia> VERSION
v"1.10.0"
KristofferC commented 5 months ago

I am personally not a big fan of:

tecosaur commented 5 months ago

Understandable. Have you got your own thoughts on how the situation with mis-matched Manifests could be improved? Or is it just those two points you have quibbles with?

I am personally not a big fan of: More environment variables to control small details of default behavior

I thought this might be nice, but I don't mind getting rid of it at all.

Surprising interactive input from commands that are not always interactive.

The interactive ask is gated behind isinteractive(), so this shouldn't happen.

KristofferC commented 5 months ago

The interactive ask is gated behind isinteractive(), so this shouldn't happen.

My point is that add typically does not have interactive behavior so I often run it, tab away, and then expect it to actually do the thing I told it to during that time.

Also, how does it make sense to query this on add which will do a resolve and fix this itself?

Doing this in the manifest constructor is also not feasible since this is called in an API fashion in various places (and other packages) where the version the manifest is resolved in and the current julia version is irrelevant,

MasonProtter commented 5 months ago

I think add is probably just the wrong use-case for this, since as Kristoffer mentioned, the project will re-resolve. I think this is quite nice for activate though.

I think especially now in the days where we're transitioning from old code that didn't properly declare stdlib dependancies to new code that does declare those dependencies (or code where which sub-artifacts get installed depends on the version), I'm running into errors much more often from trying to activate Manifest with the 'wrong' julia version, so having an easy prompt to fix it would be great.

IanButterworth commented 5 months ago

https://github.com/JuliaLang/Pkg.jl/pull/3024 proposed adding the warning to activate. Note that julia/Pkg does not Pkg.activate the active_project that julia starts in.

So if your intention is to avoid nasty errors, they won't be avoided before a Pkg.activate unless you do something like add a hook to this check inside any possibly related error throw path.