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.

(cherry picked from commit 58ab4c7)

base/loading.jl

@@ -786,7 +786,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
@@ -802,6 +802,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
@@ -856,7 +861,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
@@ -877,7 +882,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
@@ -915,7 +920,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
@@ -1083,10 +1088,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
@@ -1208,11 +1210,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
@@ -2001,8 +2003,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

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