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

Warn if an already loaded package is attempted to be loaded from a different path #44329

Merged
merged 4 commits into from
Dec 23, 2023

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Feb 24, 2022

As a use case, let's say I want to try out some Revise version but I forgot that I have Revise loaded in my startup file. With this PR, one gets:

(@v1.9) pkg> activate --temp
  Activating new project at `/tmp/jl_gIUuRd`

(jl_gIUuRd) pkg> add Revise@v3.2
    Updating registry at `~/.julia/registries/General.toml`
   Resolving package versions...
    Updating `/tmp/jl_gIUuRd/Project.toml`
⌃ [295af30f] + Revise v3.2.1
...

julia> using Revise
┌ Warning: Package Revise already loaded from path "/home/kc/.julia/packages/Revise/wjNpr/src/Revise.jl", now attempted to be loaded from "/home/kc/.julia/packages/Revise/xRdlY/src/Revise.jl". This might indicate a change of environment between package loads and can mean that incompatible packages have been loaded.
└ @ Base REPL[2]:21

Oops, thanks Julia for warning me that I in fact did not load the version of the package that I thought I did.

This is a quite common source of confusion, for example, I remember IJulia loading some version of JSON that was incompatible with what many people had in their projects. Since IJulia loads the package into the same session as the user code, this lead to confusing behavior when users had a different JSON loaded than what they thought.

There is a cost to this in that we have to look up the path for the package even when it is already loaded. I have yet to do some benchmarks for this but I doubt it is significant.

I wonder how useful it is to write out the full path in the error message.

@KristofferC KristofferC added the packages Package management and loading label Feb 24, 2022
@KristofferC KristofferC force-pushed the kc/check_path_different branch 2 times, most recently from f2d1744 to a1428a9 Compare February 24, 2022 16:17
base/loading.jl Outdated Show resolved Hide resolved
@KristofferC KristofferC force-pushed the kc/check_path_different branch 2 times, most recently from a5e1bdb to 5cdb501 Compare February 24, 2022 19:06
Copy link
Member

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

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

I think this is a great improvement!

@giordano
Copy link
Contributor

I have yet to do some benchmarks for this but I doubt it is significant.

I may have a system in mind where the impact can be significant...

@KristofferC
Copy link
Member Author

I may have a system in mind where the impact can be significant...

I am not sure, you still have to do the UUID lookup which does a bunch of file system stuff that is checked for the duration of the require call, so the path lookup should be not very expensive. But would be interesting to measure it for sure.

@KristofferC
Copy link
Member Author

I'm not sure this will work well for packages in the sysimage.

@vtjnash
Copy link
Member

vtjnash commented Feb 28, 2022

I wonder if we should move this up a level in the call-stack (to require(::Module, ::Symbol)), nearer to the aforementioned UUID lookup, and to only emit this for the toplevel modules?

base/loading.jl Outdated Show resolved Hide resolved
@DilumAluthge
Copy link
Member

This will need a rebase on master so that it picks up the Buildkite CI config.

base/loading.jl Outdated Show resolved Hide resolved
@IanButterworth IanButterworth force-pushed the kc/check_path_different branch 3 times, most recently from 97f5b98 to abb7e30 Compare December 19, 2023 21:36
@IanButterworth
Copy link
Member

Latest demo

% ./julia                    
┌ Info: Precompiling Revise [295af30f-e4ad-537b-8983-00126c2a3abe]
└ @ Base loading.jl:2631
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.11.0-DEV.1149 (2023-12-23)
 _/ |\__'_|_|_|\__'_|  |  kc/check_path_different/02a632aa9a* (fork: 1 commits, 0 days)
|__/                   |

(@v1.11) pkg> activate --temp
  Activating new project at `/var/folders/1z/jf841bdj73bdj3vk7kc7f_3w0000gn/T/jl_pPDv9H`

(jl_pPDv9H) pkg> add JET JuliaInterpreter@0.9.25
   Resolving package versions...
    Updating `/private/var/folders/1z/jf841bdj73bdj3vk7kc7f_3w0000gn/T/jl_pPDv9H/Project.toml`
  [c3a54625] + JET v0.8.21
⌃ [aa1ae85d] + JuliaInterpreter v0.9.25
    Updating `/private/var/folders/1z/jf841bdj73bdj3vk7kc7f_3w0000gn/T/jl_pPDv9H/Manifest.toml`
...
Precompiling project...
  ✓ JuliaInterpreter
  6 dependencies successfully precompiled in 28 seconds. 32 already precompiled.
  1 dependency precompiled but a different version is currently loaded. Restart julia to access the new version

julia> using JET
[ Info: Precompiling JET [c3a54625-cd67-489e-a8e7-0a5a0ff4e31b]
┌ Warning: JuliaInterpreter [aa1ae85d-cabe-5617-a682-6adf51b2e16a] is already loaded from a different path:
│   loaded:    v0.9.27 "/Users/ian/.julia/packages/JuliaInterpreter/EsH1r/src/JuliaInterpreter.jl"
│   requested: v0.9.25 "/Users/ian/.julia/packages/JuliaInterpreter/ddLm7/src/JuliaInterpreter.jl"
│ This might indicate a change of environment has happened between package loads and can mean that
│ incompatible packages have been loaded. If this happened due to a startup.jl consider starting julia
│ directly in the project via the `--project` arg.
└ @ Base loading.jl:2051

julia>

@IanButterworth
Copy link
Member

Now following messages are shorter

julia> using JET
[ Info: Precompiling JET [c3a54625-cd67-489e-a8e7-0a5a0ff4e31b]
┌ Warning: JuliaInterpreter [aa1ae85d-cabe-5617-a682-6adf51b2e16a] is already loaded from a different path:
│   loaded:    v0.9.27 "/Users/ian/.julia/packages/JuliaInterpreter/EsH1r/src/JuliaInterpreter.jl"
│   requested: v0.9.25 "/Users/ian/.julia/packages/JuliaInterpreter/ddLm7/src/JuliaInterpreter.jl"
│ This might indicate a change of environment has happened between package loads and can mean that
│ incompatible packages have been loaded. If this happened due to a startup.jl consider starting julia
│ directly in the project via the `--project` arg.
└ @ Base loading.jl:2018

julia> using OrderedCollections
┌ Warning: OrderedCollections [bac558e1-5e72-5ebc-8fee-abe8a469f55d] is already loaded from a different path:
│   loaded:    v1.6.3 "/Users/ian/.julia/packages/OrderedCollections/9C4Uz/src/OrderedCollections.jl"
│   requested: v1.6.2 "/Users/ian/.julia/packages/OrderedCollections/SInLM/src/OrderedCollections.jl"
└ @ Base loading.jl:2018

julia> 

@IanButterworth IanButterworth changed the title RFC: warn if an already loaded package is attempted to be loaded from a different path Warn if an already loaded package is attempted to be loaded from a different path Dec 23, 2023
@IanButterworth IanButterworth added the merge me PR is reviewed. Merge when all tests are passing label Dec 23, 2023
@IanButterworth IanButterworth merged commit b51b809 into master Dec 23, 2023
9 checks passed
@IanButterworth IanButterworth deleted the kc/check_path_different branch December 23, 2023 21:10
@oscardssmith oscardssmith removed the merge me PR is reviewed. Merge when all tests are passing label Dec 24, 2023
IanButterworth added a commit to IanButterworth/julia that referenced this pull request Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages Package management and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants