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.

Sign up for GitHub

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

Fix pkgdir for extensions #55720

Merged
merged 7 commits into from
Sep 10, 2024
Merged

Fix pkgdir for extensions #55720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
12bf8db
fix pkgdir for extensions
IanButterworth Sep 9, 2024
28e6162
add tests
IanButterworth Sep 9, 2024
b5d8328
tidy tests avoiding duplication
IanButterworth Sep 9, 2024
69a8795
fix dir check
IanButterworth Sep 9, 2024
2417012
more fix
IanButterworth Sep 9, 2024
5c3b0d0
simplify
IanButterworth Sep 9, 2024
41c47af
oh for the want of testing before pushing
IanButterworth Sep 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion base/loading.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,8 @@ package root.
To get the root directory of the package that implements the current module
the form `pkgdir(@__MODULE__)` can be used.

If an extension module is given, the root of the parent package is returned.
Copy link
Member

Choose a reason for hiding this comment

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

Why shouldn't it give the path to the extension?

Copy link
Contributor

Choose a reason for hiding this comment

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

Extensions aren't fully fledged packages themselves, right? The resulting path would not point to a package.

Copy link
Member

Choose a reason for hiding this comment

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

Well, they load like a normal package at least. But I see that pkgdir gives you the path to where the whole package is stored so it would be hard to have an analogy for that for an extension so maybe this is the best thing to do.


```julia-repl
julia> pkgdir(Foo)
"/path/to/Foo.jl"
Expand All @@ -525,7 +527,19 @@ function pkgdir(m::Module, paths::String...)
rootmodule = moduleroot(m)
path = pathof(rootmodule)
path === nothing && return nothing
return joinpath(dirname(dirname(path)), paths...)
original = path
path, base = splitdir(dirname(path))
if base == "src"
# package source in `../src/Foo.jl`
elseif base == "ext"
# extension source in `../ext/FooExt.jl`
elseif basename(path) == "ext"
# extension source in `../ext/FooExt/FooExt.jl`
path = dirname(path)
else
error("Unexpected path structure for module source: $original")
end
return joinpath(path, paths...)
end

function get_pkgversion_from_path(path)
Expand Down
19 changes: 16 additions & 3 deletions test/loading.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,6 +1034,16 @@ end
end

@testset "Extensions" begin
test_ext = """
function test_ext(parent::Module, ext::Symbol)
_ext = Base.get_extension(parent, ext)
_ext isa Module || error("expected extension \$ext to be loaded")
_pkgdir = pkgdir(_ext)
_pkgdir == pkgdir(parent) != nothing || error("unexpected extension \$ext pkgdir path: \$_pkgdir")
_pkgversion = pkgversion(_ext)
_pkgversion == pkgversion(parent) || error("unexpected extension \$ext version: \$_pkgversion")
end
"""
depot_path = mktempdir()
try
proj = joinpath(@__DIR__, "project", "Extensions", "HasDepWithExtensions.jl")
Expand All @@ -1044,13 +1054,15 @@ end
cmd = """
$load_distr
begin
$ew $test_ext
$ew push!(empty!(DEPOT_PATH), $(repr(depot_path)))
using HasExtensions
$ew using HasExtensions
$ew Base.get_extension(HasExtensions, :Extension) === nothing || error("unexpectedly got an extension")
$ew HasExtensions.ext_loaded && error("ext_loaded set")
using HasDepWithExtensions
$ew using HasDepWithExtensions
$ew test_ext(HasExtensions, :Extension)
$ew Base.get_extension(HasExtensions, :Extension).extvar == 1 || error("extvar in Extension not set")
$ew HasExtensions.ext_loaded || error("ext_loaded not set")
$ew HasExtensions.ext_folder_loaded && error("ext_folder_loaded set")
Expand Down Expand Up @@ -1102,13 +1114,14 @@ end

test_ext_proj = """
begin
$test_ext
using HasExtensions
using ExtDep
Base.get_extension(HasExtensions, :Extension) isa Module || error("expected extension to load")
test_ext(HasExtensions, :Extension)
using ExtDep2
Base.get_extension(HasExtensions, :ExtensionFolder) isa Module || error("expected extension to load")
test_ext(HasExtensions, :ExtensionFolder)
using ExtDep3
Base.get_extension(HasExtensions, :ExtensionDep) isa Module || error("expected extension to load")
test_ext(HasExtensions, :ExtensionDep)
end
"""
for compile in (`--compiled-modules=no`, ``)
Expand Down
Loading