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

tweak code loading to make Revise happy #53406

Closed
wants to merge 1 commit into from

Conversation

KristofferC
Copy link
Member

There is currently a bit of an inconsistency in an internal code loading function where with an stdlib it gives a path to the entry file while with the normal package we get the path to the folder:

julia> id_stdlib = Base.identify_package("SparseArrays")
SparseArrays [2f01184e-e22b-5df5-ae63-d93ebab69eaf]

julia> id_pkg = Base.identify_package("Example")
Example [7876af07-990d-54b4-ab0e-23690620f79a]

julia> Base.explicit_manifest_uuid_path(Base.active_project(), id_stdlib)
"/Users/kristoffercarlsson/.julia/juliaup/julia-1.10.1+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/SparseArrays/src/SparseArrays.jl"

julia> Base.explicit_manifest_uuid_path(Base.active_project(), id_pkg)
"/Users/kristoffercarlsson/.julia/packages/Example/aqsx3"

For Base itself this turns out to not matter but Revise uses some internals from code loading:

https://github.com/timholy/Revise.jl/blob/dbbeb3594a4ea3eebd861e8ceea8fc746efc0f82/src/pkgs.jl#L410

and this causes it to get upset.

I believe timholy/Revise.jl#806 will fix it in Revise but might as well add this here as well.

@@ -985,11 +985,7 @@ function explicit_manifest_entry_path(manifest_file::String, pkg::PkgId, entry::
end
Copy link
Member

Choose a reason for hiding this comment

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

This is inconsistent now, since the function is supposed to return the path to the file (due to this branch and the stdlib one), but this PR only changes the stdlib one

Copy link
Member Author

Choose a reason for hiding this comment

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

But path here will be the path to the directory, no (dirname(manifest_file))?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, is it? I was confused by the documentation implying it is the path to the source code:
https://docs.julialang.org/en/v1/manual/code-loading/#Project-environments

But okay, seems you are right that it is expected to be either the path to the source code exactly or a directory containing the source code:

"If any of these result in success, the path to the source code entry point will be that result or the relative path from that result plus src/X.jl")

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what I have changed to here is correct actually because I think in implicit envs you can just have the file Package.jl without putting it in any folder. So maybe this PR should be closed.

@giordano giordano deleted the kc/stdlib_path_internal branch June 19, 2024 19:43
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.

2 participants