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

Remove mentions of “home project” in documentation and comments #3547

Merged
merged 3 commits into from
Aug 13, 2023

Conversation

Socob
Copy link
Contributor

@Socob Socob commented Jul 12, 2023

The concept of “home project” was removed in JuliaLang/julia#36434. Also see #1891.

Fixes #3127.

src/Pkg.jl Outdated Show resolved Hide resolved
Co-authored-by: Ian Butterworth <i.r.butterworth@gmail.com>
@IanButterworth IanButterworth merged commit 6024c0f into JuliaLang:master Aug 13, 2023
13 checks passed
@danielwe
Copy link

danielwe commented Aug 13, 2023

If no argument is given to activate, then use the first project found in LOAD_PATH.

This is still inaccurate. The first entry in the default LOAD_PATH is "@", and according to the documentation,

"@" refers to the "current active environment", the initial value of which is initially determined by the JULIA_PROJECT environment variable or the --project command-line option.

However, what happens when calling activate without arguments is that the project set through JULIA_PROJECT or --project is deliberately forgotten (by setting Base.ACTIVE_PROJECT[] = nothing). This turns "@" into a simple redirect to the next LOAD_PATH entry. In other words, activate without arguments isn't simply looking at LOAD_PATH, it's changing the meaning of the entries in LOAD_PATH.

How about:

If no argument is given to activate, then discard any active project set via the JULIA_PROJECT environment variable, the --project command-line option, or any previous calls to activate. Unless LOAD_PATH has been modified, the result is to activate the default @v1.x environment.

@IanButterworth
Copy link
Member

Note that #1891 remains open, so it may be better to spend effort getting that merged?

@Socob Socob deleted the home-project-mentions branch August 14, 2023 20:56
@Socob
Copy link
Contributor Author

Socob commented Aug 15, 2023

@danielwe Come to think of it, the somewhat unclear meaning of "@" is probably why I worded things the way I did in the original version of the PR (which I wouldn’t say was inaccurate!). In fact, unless I’m missing something, both Julia Base and Pkg.jl always simply ignore "@" in LOAD_PATH! Or are you aware of any situation where having "@" in LOAD_PATH makes any difference compared to not having it? Based on the code for Base.active_project() and Pkg.activate(), I don’t see any case where it’d do anything:

julia/base/initdefs.jl

Pkg.jl/src/API.jl

Lines 1818 to 1836 in 46b7eef

function activate(;temp=false, shared=false, prev=false, io::IO=stderr_f())
shared && pkgerror("Must give a name for a shared environment")
temp && return activate(mktempdir(); io=io)
if prev
if isempty(PREV_ENV_PATH[])
pkgerror("No previously active environment found")
else
return activate(PREV_ENV_PATH[]; io=io)
end
end
if !isnothing(Base.active_project())
PREV_ENV_PATH[] = Base.active_project()
end
Base.ACTIVE_PROJECT[] = nothing
p = Base.active_project()
p === nothing || printpkgstyle(io, :Activating, "project at $(pathrepr(dirname(p)))")
add_snapshot_to_undo()
return nothing
end

It seems like this is more of a historical leftover without any actual remaining functionality and should probably be removed?

@IanButterworth
Copy link
Member

@StefanKarpinski perhaps you could weigh in here about the design and perhaps a post merge review of these changes?

@danielwe
Copy link

danielwe commented Aug 15, 2023

Or are you aware of any situation where having "@" in LOAD_PATH makes any difference compared to not having it?

Not the expert here, I had never looked at this code before I wrote the previous comment, but it looks like it makes a difference in Base.load_path_expand: https://github.com/JuliaLang/julia/blob/fd38d50cefd17c2e31df34c6b6c4a5b7c721c57d/base/initdefs.jl#L250-L252

Without it, code loading won't know about the active project:

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

(jl_ujp5um) pkg> add EnumX
   Resolving package versions...
    Updating `/tmp/jl_ujp5um/Project.toml`
  [4e289a0a] + EnumX v1.0.4
    Updating `/tmp/jl_ujp5um/Manifest.toml`
  [4e289a0a] + EnumX v1.0.4

julia> popfirst!(LOAD_PATH)
"@"

julia> using EnumX
 │ Package EnumX not found, but a package named EnumX is available from a
 │ registry.
 │ Install package?
 │   (jl_ujp5um) pkg> add EnumX
 └ (y/n/o) [y]:

@Socob
Copy link
Contributor Author

Socob commented Aug 15, 2023

Ah right, sorry. I think what’s confusing here is that LOAD_PATH is used for two different purposes: Its main purpose of giving the places to look for packages in the context of using/import (via Base.load_path()), and secondarily, also to determine what the “active project” is with Base.active_project(). But apart from being the project that Pkg.jl operates on, i. e. when it comes to Base, the only meaning of the “active project” is, in turn, to define what the placeholder "@" in LOAD_PATH means. So in that sense, Pkg.activate()/Base.active_project() must skip over "@" in LOAD_PATH, because the entire point of setting an active project is to define what "@" means in the first place!

Given all this, I think the clearest option would be to simply say

If no argument is given to activate, then use the first project found in LOAD_PATH (ignoring "@").

(By the way, the comment line

# `@` in JULIA_LOAD_PATH is expanded early (at startup time)

seems to be inaccurate?)

@danielwe
Copy link

danielwe commented Aug 15, 2023

If no argument is given to activate, then use the first project found in LOAD_PATH (ignoring "@").

Yes, that's simple and clear. Though I do think it's worth pointing out what this implies in the usual case, i.e., that it activates @v1.x. You shouldn't have to read the docs for LOAD_PATH to understand what activate does when you haven't touched LOAD_PATH.

(By the way, the comment line

# `@` in JULIA_LOAD_PATH is expanded early (at startup time)

seems to be inaccurate?)

You're right:

user@host:~/tmp$ ls
Manifest.toml  Project.toml
user@host:~/tmp$ JULIA_LOAD_PATH="@" julia -q --startup-file=no
julia> LOAD_PATH
1-element Vector{String}:
 "@"

pkg> status
ERROR: no active project
user@host:~/tmp$ JULIA_LOAD_PATH="@" julia -q --startup-file=no --project=.
julia> LOAD_PATH
1-element Vector{String}:
 "@"

(tmp) pkg> status
Status `~/tmp/Project.toml` (empty project)

In the first case, expanding "@" would be meaningless anyway. In the second case, it could have been expanded to "~/tmp/Project.toml", but isn't.

(EDIT: Replaced confused example that used "@." instead of "@" and reached the wrong conclusion. The string "@." in JULIA_LOAD_PATH is expanded early.)

@Socob
Copy link
Contributor Author

Socob commented Aug 15, 2023

I don’t have any objections against mentioning what happens for the default LOAD_PATH. It’s a bit unfortunate that this discussion only happened after the PR was merged.

@IanButterworth What’s the best way forward here? Should I make another PR incorporating the things that were discussed here?

@IanButterworth
Copy link
Member

Sounds good to me. I think this PR is an improvement, at least.

@danielwe
Copy link

It’s a bit unfortunate that this discussion only happened after the PR was merged.

Yeah, sorry about that, I'm just a passer-by and didn't know about the PR until I got the email notification when the linked issue was closed by the PR merge (since I had contributed an MWE to the issue way back).

@Socob
Copy link
Contributor Author

Socob commented Aug 15, 2023

Oh no worries, not blaming anyone for anything. That was just a general remark about the situation as a whole 🙂

IanButterworth pushed a commit that referenced this pull request Aug 18, 2023
Co-authored-by: Ian Butterworth <i.r.butterworth@gmail.com>
(cherry picked from commit 6024c0f)
IanButterworth pushed a commit that referenced this pull request Aug 18, 2023
Co-authored-by: Ian Butterworth <i.r.butterworth@gmail.com>
(cherry picked from commit 6024c0f)
@IanButterworth IanButterworth mentioned this pull request Aug 18, 2023
10 tasks
IanButterworth pushed a commit that referenced this pull request Aug 18, 2023
Co-authored-by: Ian Butterworth <i.r.butterworth@gmail.com>
(cherry picked from commit 6024c0f)
@IanButterworth IanButterworth mentioned this pull request Aug 18, 2023
14 tasks
IanButterworth pushed a commit that referenced this pull request Aug 18, 2023
Co-authored-by: Ian Butterworth <i.r.butterworth@gmail.com>
(cherry picked from commit 6024c0f)
KristofferC pushed a commit to JuliaLang/julia that referenced this pull request Aug 28, 2023
When the meaning of `"@"` in `LOAD_PATH` was changed and `"@."`
introduced as the new name for the previous meaning of `"@"` in
9a4ecaa, a comment in `load_path_expand()` wasn’t updated to reflect
this change. This left the comment being incorrect, leading to potential
confusion.

I hope I’m not creating more hassle than it’s worth with this super-tiny
PR, but since it’s caused some confusion (e. g.
JuliaLang/Pkg.jl#3547 (comment)),
I thought it might be worthwhile to fix it.
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.

Pkg.activate() doesn't seem to activate the home package
3 participants