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

Prompt the user on a manifest version mismatch #3775

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tecosaur
Copy link
Contributor

@tecosaur tecosaur commented Feb 1, 2024

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

  • says nothing, or
  • warns that there might be funny behaviour

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

  • refresh the manifest, by removing the current one and regenerating the manifest
  • version the manifest, by renaming the current one to (Julia)Manifest-v{major}.{minor}.toml and regenerating the manifest (corresponding to Add preference for version named manifest files julia#43845)
  • ignore the mismatch

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
Copy link
Contributor Author

tecosaur commented Feb 1, 2024

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
Copy link
Member

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 #3024

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

@IanButterworth
Copy link
Member

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
Copy link
Contributor

MasonProtter commented Feb 2, 2024

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
Copy link
Member

I am personally not a big fan of:

  • More environment variables to control small details of default behavior
  • Surprising interactive input from commands that are not always interactive.

@tecosaur
Copy link
Contributor Author

tecosaur commented Feb 2, 2024

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
Copy link
Member

KristofferC commented Feb 2, 2024

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
Copy link
Contributor

MasonProtter commented Feb 2, 2024

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
Copy link
Member

#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.

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.

4 participants