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

(1.10) Extensions that trigger on the current project are ignored by ] precompile #56675

Open
topolarity opened this issue Nov 24, 2024 · 7 comments

Comments

@topolarity
Copy link
Member

... exposed a new bug on 1.10:

$ ./julia --project=test/project/Extensions/Parent.jl -q
(Parent) pkg> precompile
Precompiling project...
  ✓ DepWithParentExt
  ✓ Parent
  2 dependencies successfully precompiled in 2 seconds

julia> using Parent
[ Info: Precompiling ParentExt [3290166e-fa45-5082-a21d-a36e4f051b06]

] precompile doesn't realize it needs to compile the extension because it doesn't have Parent in the old manifest format.

Originally posted by @topolarity in #56666 (comment)

@topolarity
Copy link
Member Author

I got the Manifest-related details mixed up a bit here. I think the 1.10 behavior is affected by whether or not you use the new manifest with it (it doesn't complain if you do), but the new manifest is on nightly not 1.11 so that's not why this is working on 1.11

Anyway this is definitely an issue on 1.10 and seems to be OK on 1.11

@KristofferC
Copy link
Member

We (probably me) could bisect this on 1.11 to see what fixed it there (considering that the fact that the main package inclusion in the manifest came in 1.12 with the introduction of workspaces).

@topolarity
Copy link
Member Author

topolarity commented Nov 24, 2024

It looks like it came in somewhere around where we moved pre-compilation to Base:

On 1.10, deps is just from the Manifest: https://github.com/JuliaLang/Pkg.jl/blob/845909f49043c8c78ca2aebdcd9aee434ea70279/src/API.jl#L1169

But on 1.11, it includes entries from the Project:

if proj_name !== nothing && proj_uuid !== nothing
deps_expanded[proj_uuid] = filter!(!=(proj_uuid), collect(values(project_deps)))
extensions_expanded[proj_uuid] = project_extensions
path = get(project_d, "path", nothing)::Union{String, Nothing}
entry_point = path !== nothing ? path : dirname(envpath)
lookup_strategy[proj_uuid] = entry_point
end

@KristofferC
Copy link
Member

We could copy paste the current precompilation code back into Pkg :P.

@topolarity
Copy link
Member Author

Another problem I found...

If you change DepWithParentExt to have a strong dep on Parent (intentionally creating a cycle between the two), then you get this stack trace:

(Parent) pkg> precompile
ERROR: KeyError: key "Parent" not found
Stacktrace:
  [1] getindex
    @ ./dict.jl:498 [inlined]
  [2] precompile(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}; internal_call::Bool, strict::Bool, warn_loaded::Bool, already_instantiated::Bool, timing::Bool, _from_loading::Bool, kwargs::@Kwargs{io::Base.TTY})
    @ Pkg.API ~/repos/julia/usr/share/julia/stdlib/v1.10/Pkg/src/API.jl:1168
  [3] precompile(pkgs::Vector{Pkg.Types.PackageSpec}; io::Base.TTY, kwargs::@Kwargs{})
    @ Pkg.API ~/repos/julia/usr/share/julia/stdlib/v1.10/Pkg/src/API.jl:159
  [4] precompile(pkgs::Vector{Pkg.Types.PackageSpec})
    @ Pkg.API ~/repos/julia/usr/share/julia/stdlib/v1.10/Pkg/src/API.jl:148
  [5] precompile(pkgs::Vector{String})
    @ Pkg.API ~/repos/julia/usr/share/julia/stdlib/v1.10/Pkg/src/API.jl:147
...

This again works OK on 1.11

@topolarity
Copy link
Member Author

Oof. You also hit that error if you only add a symmetric extension Parent → DepWithParentExtExt:

diff --git a/test/project/Extensions/Parent.jl/Project.toml b/test/project/Extensions/Parent.jl/Project.toml
index a79ec8859b..9119864a47 100644
--- a/test/project/Extensions/Parent.jl/Project.toml
+++ b/test/project/Extensions/Parent.jl/Project.toml
@@ -4,3 +4,6 @@ version = "0.1.0"

 [deps]
 DepWithParentExt = "8a35c396-5ffc-40d2-b7ec-e8ed2248da32"
+
+[extensions]
+DepWithParentExtExt = "DepWithParentExt"
diff --git a/test/project/Extensions/Parent.jl/ext/DepWithParentExtExt.jl b/test/project/Extensions/Parent.jl/ext/DepWithParentExtExt.jl
new file mode 100644
index 0000000000..c9232efcbd
--- /dev/null
+++ b/test/project/Extensions/Parent.jl/ext/DepWithParentExtExt.jl
@@ -0,0 +1,6 @@
+module DepWithParentExtExt
+
+using DepWithParentExt
+using Parent
+
+end

That diff introduces an extension "cycle" of the kind that doesn't exist on 1.11, but is a cycle on 1.10.

We don't even get to print our warning though:

./julia --project=test/project/Extensions/tempenv -q
(tempenv) pkg> dev ./test/project/Extensions/DepWithParentExt.jl/ ./test/project/Extensions/Parent.jl/
   Resolving package versions...
    Updating `~/repos/julia/test/project/Extensions/tempenv/Project.toml`
  [8a35c396] + DepWithParentExt v0.1.0 `../DepWithParentExt.jl`
  [58cecb9c] + Parent v0.1.0 `../Parent.jl`
    Updating `~/repos/julia/test/project/Extensions/tempenv/Manifest.toml`
  [8a35c396] + DepWithParentExt v0.1.0 `../DepWithParentExt.jl`
  [58cecb9c] + Parent v0.1.0 `../Parent.jl`

(tempenv) pkg> precompile
ERROR: KeyError: key "DepWithParentExt" not found
Stacktrace:
  [1] getindex
    @ ./dict.jl:498 [inlined]
  [2] precompile(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}; internal_call::Bool, strict::Bool, warn_loaded::Bool, already_instantiated::Bool, timing::Bool, _from_loading::Bool, kwargs::@Kwargs{io::Base.TTY})
    @ Pkg.API ~/repos/julia/usr/share/julia/stdlib/v1.10/Pkg/src/API.jl:1168
  [3] precompile(pkgs::Vector{Pkg.Types.PackageSpec}; io::Base.TTY, kwargs::@Kwargs{})
    @ Pkg.API ~/repos/julia/usr/share/julia/stdlib/v1.10/Pkg/src/API.jl:159
  [4] precompile(pkgs::Vector{Pkg.Types.PackageSpec})
    @ Pkg.API ~/repos/julia/usr/share/julia/stdlib/v1.10/Pkg/src/API.jl:148
  [5] precompile(pkgs::Vector{String})
    @ Pkg.API ~/repos/julia/usr/share/julia/stdlib/v1.10/Pkg/src/API.jl:147

@topolarity
Copy link
Member Author

That second issue look like it's because we're only looking in weakdeps on 1.10: https://github.com/JuliaLang/Pkg.jl/blob/845909f49043c8c78ca2aebdcd9aee434ea70279/src/API.jl#L1168

instead of both weakdeps and deps: https://github.com/JuliaLang/julia/blame/94f542dcd2f67aa066fc17b08904125facc12c11/base/precompilation.jl#L187-L211

Another bug that was cleaned up with the new precompile code

@topolarity topolarity changed the title Extensions that trigger on the current project are ignored by ] precompile (1.10) Extensions that trigger on the current project are ignored by ] precompile Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants