-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
WIP: Pkg3-style package loading #24405
Conversation
Thread a `from` module through all require-related calls so that we can look up the name of the module to be loaded in the context of the module into which it's being loaded. No behavior change yet. It's a bit unfortunate that `from` is a module and `sym` isn't, but we can improve argument names later on.
IIUC this will make the module its being loaded from part of the identifier for that module? I am trying to understand how this will affect |
This currently just passes through the module that a name is being loaded from because that's necessary for resolving what the name means. This is because writing |
ef44fd2
to
3434ff7
Compare
if !root_module_exists(mod) | ||
_require(mod) | ||
info("@$(getpid()): require($from, $mod)") | ||
_require(from, mod) | ||
# After successfully loading, notify downstream consumers | ||
if toplevel_load[] && myid() == 1 && nprocs() > 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This codepath is moving out of base with #25139, so the callback
below will need to also be changed to include from
.
@@ -295,16 +297,17 @@ then tries paths in the global array `LOAD_PATH`. `require` is case-sensitive on | |||
all platforms, including those with case-insensitive filesystems like macOS and | |||
Windows. | |||
""" | |||
function require(mod::Symbol) | |||
function require(from::Module, mod::Symbol) | |||
if !root_module_exists(mod) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this check is still sensible. It depends on what it means to require a root module from within an other module.
Currently we are using root-modules for the stdlib
packages, since we don't want to make them available under Main
immediately. As I noted on Slack this means that we can't update stdlib packages that are backed into the sysimage.
@@ -467,8 +467,8 @@ unshift!(Base._included_files, (@__MODULE__, joinpath(@__DIR__, "sysimg.jl"))) | |||
Base.init_load_path(ccall(:jl_get_julia_home, Any, ())) | |||
|
|||
# load some stdlib packages but don't put their names in Main | |||
Base.require(:DelimitedFiles) | |||
Base.require(:Test) | |||
Base.require(Base, :DelimitedFiles) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I am in favour of not loading stdlib packages into the sys-image at all and instead create cache-files for them during the build.
If we keep loading them they shouldn't go into Base
in my opinion since people might continue to use using Base.Test
which is something I think we don't want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since changing this is non-breaking, we do not need to figure out the perfect way to build and give people stdlib packages in 1.0. For now, building them into the sysimg is easiest.
Superseded by #25333. |
The first step here is to thread a
from
module through all require-related calls so that we can look up the name of the module to be loaded in the context of the module into which it's being loaded. Each module will have a map from package names to UUIDs/paths which indicate the identity of each dependency it loads.