From 7186b8be1fa83f31c4419e930eedeffca51ff7cd Mon Sep 17 00:00:00 2001 From: James Wrigley Date: Sat, 25 May 2024 01:48:51 +0200 Subject: [PATCH] Use Base.require() to load Distributed internally (#54571) TL;DR: we changed to loading stdlibs exclusively from the default sysimg in https://github.com/JuliaLang/julia/pull/53326, and this breaks developing Distributed.jl because the workers will load the builtin version instead of the development version. I think this should be backported to 1.11? CC @jpsamaroo --- `Base.require_stdlib()` will exclusively load a stdlib from whatever was shipped with Julia. The problem is that when developing Distributed.jl this will cause the workers to always load the builtin Distributed module instead of the development version, which can break everything because now the master and worker are running different (potentially incompatible) versions of Distributed. This commit changes Base and LinearAlgebra to use `Base.require()` instead, which will respect `Base.LOAD_PATH`. I argue that this is safe because unlike the other stdlibs like REPL or Pkg, Distributed is only loaded if explicitly requested by the user with `-p` or through `addprocs()` or something, so it shouldn't be possible to get into quite the same tools-are-broken-because-I-broke-my-tools situation that motivated using `Base.require_stdlib()` in the first place. An alternative design would be to: 1. Move the if-block loading Distributed in `exec_options()` below the for-loop where it will execute `-e` options. 2. Require any implementation of `Distributed.launch(::ClusterManager)` to pass `-e 'using Distributed'` in their command to ensure that Distributed is loaded in a way respecting `Base.LOAD_PATH`. This would be more consistent with how the other stdlibs must be developed, but it requires implementers (i.e. Distributed and ClusterManagers.jl) to opt-in to allowing development versions of Distributed, which feels very annoying and easy to miss so I decided not to implement that. --- base/client.jl | 2 +- stdlib/LinearAlgebra/src/LinearAlgebra.jl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/base/client.jl b/base/client.jl index 83998e0b0e22b3..53e946165469f7 100644 --- a/base/client.jl +++ b/base/client.jl @@ -260,7 +260,7 @@ function exec_options(opts) # Load Distributed module only if any of the Distributed options have been specified. distributed_mode = (opts.worker == 1) || (opts.nprocs > 0) || (opts.machine_file != C_NULL) if distributed_mode - let Distributed = require_stdlib(PkgId(UUID((0x8ba89e20_285c_5b6f, 0x9357_94700520ee1b)), "Distributed")) + let Distributed = require(PkgId(UUID((0x8ba89e20_285c_5b6f, 0x9357_94700520ee1b)), "Distributed")) Core.eval(MainInclude, :(const Distributed = $Distributed)) Core.eval(Main, :(using Base.MainInclude.Distributed)) end diff --git a/stdlib/LinearAlgebra/src/LinearAlgebra.jl b/stdlib/LinearAlgebra/src/LinearAlgebra.jl index 34605700f73cd8..5663f66eac8b38 100644 --- a/stdlib/LinearAlgebra/src/LinearAlgebra.jl +++ b/stdlib/LinearAlgebra/src/LinearAlgebra.jl @@ -743,7 +743,7 @@ function peakflops(n::Integer=4096; eltype::DataType=Float64, ntrials::Integer=3 end if parallel - let Distributed = Base.require_stdlib(Base.PkgId( + let Distributed = Base.require(Base.PkgId( Base.UUID((0x8ba89e20_285c_5b6f, 0x9357_94700520ee1b)), "Distributed")) nworkers = @invokelatest Distributed.nworkers() results = @invokelatest Distributed.pmap(peakflops, fill(n, nworkers))