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

Overrides.toml broken on master #54930

Closed
barche opened this issue Jun 25, 2024 · 6 comments · Fixed by #55218
Closed

Overrides.toml broken on master #54930

barche opened this issue Jun 25, 2024 · 6 comments · Fixed by #55218
Labels
needs more info Clarification or a reproducible example is required
Milestone

Comments

@barche
Copy link
Contributor

barche commented Jun 25, 2024

It seems that somehow (and in no way obvious to me how) commit a1a2ac6 (#54739) broke the use of the Overrides.toml file. Details of the testing I did:

/home/bjanssens/.julia/artifacts/Overrides.toml:

[3eaa8342-bff7-56a5-9981-c04077f7cee7]
libcxxwrap_julia = "/home/bjanssens/src/build/libcxxwrap_julia"

Normal Overrides.toml behavior (tested on Julia 1.10 here):

julia> using libcxxwrap_julia_jll, Libdl

julia> println.(filter(x -> contains(x, "cxx"), dllist()));
/home/bjanssens/.julia/compiled/v1.10/libcxxwrap_julia_jll/9VnAb_YexWl.so
/home/bjanssens/src/build/libcxxwrap_julia/lib/libcxxwrap_julia.so
/home/bjanssens/src/build/libcxxwrap_julia/lib/libcxxwrap_julia_stl.so

Behavior on Julia master at or after commit a1a2ac6:

julia> using libcxxwrap_julia_jll, Libdl
[ Info: Precompiling libcxxwrap_julia_jll [3eaa8342-bff7-56a5-9981-c04077f7cee7] (cache misses: invalid header (4))

julia> println.(filter(x -> contains(x, "cxx"), dllist()));
/home/bjanssens/.julia/compiled/v1.12/libcxxwrap_julia_jll/9VnAb_MRJjp.so
/home/bjanssens/.julia/artifacts/9a7be4f65e83c37ef55fd1b9e2e6b3361fd0a483/lib/libcxxwrap_julia.so
/home/bjanssens/.julia/artifacts/9a7be4f65e83c37ef55fd1b9e2e6b3361fd0a483/lib/libcxxwrap_julia_stl.so

I have tested this with an Overrides.toml both in ~/.julia/artifacts and in usr/local/share/julia/artifacts/ in the Julia build dir.

@giordano giordano modified the milestones: 1.11, 1.12 Jun 25, 2024
@KristofferC KristofferC modified the milestones: 1.12, 1.11 Jun 25, 2024
@staticfloat
Copy link
Member

This feature works as expected for me with latest master (f2558c461c). In particular, I:

  1. Built latest julia from a clean checkout
  2. Created an Overrides.toml file:
$ cat ~/.julia/artifacts/Overrides.toml 
[3eaa8342-bff7-56a5-9981-c04077f7cee7]
libcxxwrap_julia = "/home/bjanssens/src/build/libcxxwrap_julia"
  1. Created a new project in /tmp, and added libcxxwrap_julia_jll, which failed precompilation with the following error:
ERROR: LoadError: Artifact "libcxxwrap_julia" was not found by looking in the path "/home/bjanssens/src/build/libcxxwrap_julia". Check that your `Overrides.toml` file is correct (https://pkgdocs.julialang.org/v1/artifacts/#Overriding-artifact-locations).

@vchuravy vchuravy added the needs more info Clarification or a reproducible example is required label Jul 2, 2024
@barche
Copy link
Contributor Author

barche commented Jul 6, 2024

I confirm that I also get the same result if the library doesn't exist, but once I populate the overrides path the result is the same as reported in my original comment and the library from the downloaded artifact is used:

./julia                                                                                                                                                     ✔  2h 43m 9s  
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.12.0-DEV.825 (2024-07-06)
 _/ |\__'_|_|_|\__'_|  |  Commit 7d4afbae15 (0 days old master)
|__/                   |

julia> cd(mktempdir())

(@v1.12) pkg> activate .
  Activating new project at `/tmp/jl_oASp34`

(jl_oASp34) pkg> add libcxxwrap_julia_jll
   Resolving package versions...
    Updating `/tmp/jl_oASp34/Project.toml`
  [3eaa8342] + libcxxwrap_julia_jll v0.13.2+0
    Updating `/tmp/jl_oASp34/Manifest.toml`
  [692b3bcd] + JLLWrappers v1.5.0
  [21216c6a] + Preferences v1.4.3
  [3eaa8342] + libcxxwrap_julia_jll v0.13.2+0
  [56f22d72] + Artifacts v1.11.0
  [ade2ca70] + Dates v1.11.0
  [8f399da3] + Libdl v1.11.0
  [de0858da] + Printf v1.11.0
  [fa267f1f] + TOML v1.0.3
  [4ec0a83e] + Unicode v1.11.0
Precompiling all packages...
  1 dependency successfully precompiled in 1 seconds. 6 already precompiled.

julia> using libcxxwrap_julia_jll
[ Info: Precompiling libcxxwrap_julia_jll [3eaa8342-bff7-56a5-9981-c04077f7cee7] (cache misses: invalid header (18))

julia> libcxxwrap_julia_jll.artifact_dir
"/home/bjanssens/.julia/artifacts/9a7be4f65e83c37ef55fd1b9e2e6b3361fd0a483"

julia> using Libdl

julia> println.(filter(x->contains(x,"cxx"), dllist()))
/home/bjanssens/.julia/compiled/v1.12/libcxxwrap_julia_jll/9VnAb_Ksxy9.so
/home/bjanssens/.julia/artifacts/9a7be4f65e83c37ef55fd1b9e2e6b3361fd0a483/lib/libcxxwrap_julia.so
/home/bjanssens/.julia/artifacts/9a7be4f65e83c37ef55fd1b9e2e6b3361fd0a483/lib/libcxxwrap_julia_stl.so
3-element Vector{Nothing}:
 nothing
 nothing
 nothing

@barche
Copy link
Contributor Author

barche commented Jul 9, 2024

Putting back the register_root_module(M) that was removed here fixes the problem. This because Artifacts.jl checks the Base.module_keys dict here:

if haskey(Base.module_keys, moduleroot)
# Process overrides for this UUID, if we know what it is
process_overrides(artifact_dict, Base.module_keys[moduleroot].uuid)
end

I have no idea if this is a good fix however, that line probably was removed for a reason...

@staticfloat
Copy link
Member

Hmmm, that is quite convincing. @vtjnash did you accidentally break the module root tracking?

@vtjnash
Copy link
Member

vtjnash commented Jul 13, 2024

It looks like that Artifact code is just a buggy version of Base.PkgId(__module__).uuid, so we should perhaps delete the private implementation detail Base.module_keys used here, and the semi-private root_module_key, used elsewhere, in favor of that

@KristofferC
Copy link
Member

KristofferC commented Jul 16, 2024

@barche, could you try changing out the Artifacts.jl code that you linked to to instead use

 pkg = Base.PkgId(__module__)
 if pkg.uuid !== nothing
     # Process overrides for this UUID, if we know what it is 
     process_overrides(artifact_dict, pkg.uuid) 
 end 

?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs more info Clarification or a reproducible example is required
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants