From 58ab4c73088933dc53fd3e28325172bf97a0da8e Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Fri, 6 May 2022 22:18:40 -0400 Subject: [PATCH] Fix different .cachepath initialization points for precompile load (#45149) For quite some time I've been observing Revise randomly not pick up changes to my packages. Usually I just write that off to Revise getting into a bad state and restarting Julia fixes it. Today, I got very annoyed by this again and decided to finally track it down. What turns out to happen here is the packages in question are those which: A. Are being precompiled on load B. Use Requires C. Have an `@require`'d-dependency already loaded D. The change to be revised is made in a file not loaded via Requires.jl. In this case the `__init__` of the package triggers Requires, which in turn calls back to Revise, which tries to start watching the package. However, on the `compilecache` path (but not on the path where we just find a pre-existing cache file), we used to not set the .cachepath property of `pkgorigins` until after the `__init__` callbacks run, causing Revise not to be able to see those files. Infuriatingly, restarting julia fixes this because it just loads the .ji file that was just compiled. Fix this by unifying the point at which the .cachepath is set, always setting it just prior to the module initialization callback. --- base/loading.jl | 24 ++++++++++++------------ test/precompile.jl | 14 +++++++++++++- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/base/loading.jl b/base/loading.jl index 2f20850b1ab85..0c3004ab4569c 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -789,7 +789,7 @@ end # these return either the array of modules loaded from the path / content given # or an Exception that describes why it couldn't be loaded # and it reconnects the Base.Docs.META -function _include_from_serialized(path::String, depmods::Vector{Any}) +function _include_from_serialized(pkg::PkgId, path::String, depmods::Vector{Any}) sv = ccall(:jl_restore_incremental, Any, (Cstring, Any), path, depmods) if isa(sv, Exception) return sv @@ -805,6 +805,11 @@ function _include_from_serialized(path::String, depmods::Vector{Any}) register_root_module(M) end end + + # Register this cache path now - If Requires.jl is loaded, Revise may end + # up looking at the cache path during the init callback. + get!(PkgOrigin, pkgorigins, pkg).cachepath = path + inits = sv[2]::Vector{Any} if !isempty(inits) unlock(require_lock) # temporarily _unlock_ during these callbacks @@ -859,7 +864,7 @@ function _tryrequire_from_serialized(modkey::PkgId, build_id::UInt64, modpath::U return nothing end -function _require_from_serialized(path::String) +function _require_from_serialized(pkg::PkgId, path::String) # loads a precompile cache file, ignoring stale_cachfile tests # load all of the dependent modules first local depmodnames @@ -880,7 +885,7 @@ function _require_from_serialized(path::String) depmods[i] = dep::Module end # then load the file - return _include_from_serialized(path, depmods) + return _include_from_serialized(pkg, path, depmods) end # use an Int counter so that nested @time_imports calls all remain open @@ -924,7 +929,7 @@ const TIMING_IMPORTS = Threads.Atomic{Int}(0) if staledeps === true continue end - restored = _include_from_serialized(path_to_try, staledeps) + restored = _include_from_serialized(pkg, path_to_try, staledeps) if isa(restored, Exception) @debug "Deserialization checks failed while attempting to load cache from $path_to_try" exception=restored else @@ -1104,10 +1109,7 @@ require(uuidkey::PkgId) = @lock require_lock _require_prelocked(uuidkey) function _require_prelocked(uuidkey::PkgId) just_loaded_pkg = false if !root_module_exists(uuidkey) - cachefile = _require(uuidkey) - if cachefile !== nothing - get!(PkgOrigin, pkgorigins, uuidkey).cachepath = cachefile - end + _require(uuidkey) # After successfully loading, notify downstream consumers run_package_callbacks(uuidkey) just_loaded_pkg = true @@ -1244,11 +1246,11 @@ function _require(pkg::PkgId) end # fall-through to loading the file locally else - m = _require_from_serialized(cachefile) + m = _require_from_serialized(pkg, cachefile) if isa(m, Exception) @warn "The call to compilecache failed to create a usable precompiled cache file for $pkg" exception=m else - return cachefile + return end end end @@ -2037,8 +2039,6 @@ get_compiletime_preferences(::Nothing) = String[] @debug "Rejecting cache file $cachefile because preferences hash does not match 0x$(string(prefs_hash, base=16)) != 0x$(string(curr_prefs_hash, base=16))" return true end - - get!(PkgOrigin, pkgorigins, id).cachepath = cachefile end return depmods # fresh cachefile diff --git a/test/precompile.jl b/test/precompile.jl index e5e1c35ec34af..fb38f08dad93b 100644 --- a/test/precompile.jl +++ b/test/precompile.jl @@ -313,7 +313,7 @@ precompile_test_harness(false) do dir # the module doesn't reload from the image: @test_warn "@ccallable was already defined for this method name" begin @test_logs (:warn, "Replacing module `$Foo_module`") begin - ms = Base._require_from_serialized(cachefile) + ms = Base._require_from_serialized(Base.PkgId(Foo), cachefile) @test isa(ms, Array{Any,1}) end end @@ -1278,3 +1278,15 @@ end @test any(mi -> mi.specTypes.parameters[2] === Any, mis) @test all(mi -> isa(mi.cache, Core.CodeInstance), mis) end + +# Test that the cachepath is available in pkgorigins during the +# __init__ callback +precompile_test_harness("__init__ cachepath") do load_path + write(joinpath(load_path, "InitCachePath.jl"), + """ + module InitCachePath + __init__() = Base.pkgorigins[Base.PkgId(InitCachePath)] + end + """) + @test isa((@eval (using InitCachePath; InitCachePath)), Module) +end