Improve handling of preferences for sandboxes used in Pkg

Project.toml

@@ -25,6 +25,7 @@ p7zip_jll = "3f19e933-33d8-53b3-aaab-bd5110c3b7a0"
 
 [extras]
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+Preferences = "21216c6a-2e73-6563-6e65-726566657250"
 
 [targets]
-test = ["Test"]
+test = ["Test", "Preferences"]

src/Operations.jl

@@ -604,8 +604,10 @@ function collect_artifacts(pkg_root::String; platform::AbstractPlatform=HostPlat
 
             # If there is a dynamic artifact selector, run that in an appropriate sandbox to select artifacts
             if isfile(selector_path)
-                select_cmd = Cmd(`$(gen_build_code(selector_path)) $(triplet(platform))`)
-                addenv(select_cmd, "JULIA_LOAD_PATH" => "@stdlib")
+                # Despite the fact that we inherit the project, since the in-memory manifest
+                # has not been updated yet, if we try to load any dependencies, it may fail.
+                # Therefore, this project inheritance is really only for Preferences, not dependencies.
+                select_cmd = Cmd(`$(gen_build_code(selector_path; inherit_project=true)) $(triplet(platform))`)
                 meta_toml = String(read(select_cmd))
                 push!(artifacts_tomls, (artifacts_toml, TOML.parse(meta_toml)))
             else
@@ -896,7 +898,7 @@ function dependency_order_uuids(env::EnvCache, uuids::Vector{UUID})::Dict{UUID,I
     return order
 end
 
-function gen_build_code(build_file::String)
+function gen_build_code(build_file::String; inherit_project::Bool = false)
     code = """
         $(Base.load_path_setup_code(false))
         cd($(repr(dirname(build_file))))
@@ -906,10 +908,22 @@ function gen_build_code(build_file::String)
         $(Base.julia_cmd()) -O0 --color=no --history-file=no
         --startup-file=$(Base.JLOptions().startupfile == 1 ? "yes" : "no")
         --compiled-modules=$(Bool(Base.JLOptions().use_compiled_modules) ? "yes" : "no")
+        $(inherit_project ? `--project=$(Base.active_project())` : ``)
         --eval $code
         ```
 end
 
+with_load_path(f::Function, new_load_path::String) = with_load_path(f, [new_load_path])
+function with_load_path(f::Function, new_load_path::Vector{String})
+    old_load_path = copy(Base.LOAD_PATH)
+    copy!(Base.LOAD_PATH, new_load_path)
+    try
+        f()
+    finally
+        copy!(LOAD_PATH, old_load_path)
+    end
+end
+
 const PkgUUID = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
 pkg_scratchpath() = joinpath(depots1(), "scratchspaces", PkgUUID)
 
@@ -959,9 +973,18 @@ function build_versions(ctx::Context, uuids::Set{UUID}; verbose=false)
         pkg = PackageSpec(;uuid=uuid, name=name, version=version)
         build_file = buildfile(source_path)
         # compatibility shim
-        build_project_override = isfile(projectfile_path(builddir(source_path))) ?
-            nothing :
-            gen_target_project(ctx.env, ctx.registries, pkg, source_path, "build")
+        local build_project_override, build_project_preferences
+        if isfile(projectfile_path(builddir(source_path)))
+            build_project_override = nothing
+            with_load_path([builddir(source_path)]) do
+                build_project_preferences = Base.get_preferences()
+            end
+        else
+            build_project_override = gen_target_project(ctx.env, ctx.registries, pkg, source_path, "build")
+            with_load_path([projectfile_path(source_path)]) do
+                build_project_preferences = Base.get_preferences()
+            end
+        end
 
         # Put log output in Pkg's scratchspace if the package is content adressed
         # by tree sha and in the build directory if it is tracked by path etc.
@@ -991,7 +1014,7 @@ function build_versions(ctx::Context, uuids::Set{UUID}; verbose=false)
         fancyprint && show_progress(ctx.io, bar)
 
         let log_file=log_file
-            sandbox(ctx, pkg, source_path, builddir(source_path), build_project_override) do
+            sandbox(ctx, pkg, source_path, builddir(source_path), build_project_override; preferences=build_project_preferences) do
                 flush(ctx.io)
                 ok = open(log_file, "w") do log
                     std = verbose ? ctx.io : log
@@ -1483,6 +1506,7 @@ end
 # ctx + pkg used to compute parent dep graph
 function sandbox(fn::Function, ctx::Context, target::PackageSpec, target_path::String,
                  sandbox_path::String, sandbox_project_override;
+                 preferences::Union{Nothing,Dict{String,Any}} = nothing,
                  force_latest_compatible_version::Bool=false,
                  allow_earlier_backwards_compatible_versions::Bool=true,
                  allow_reresolve::Bool=true)
@@ -1492,6 +1516,7 @@ function sandbox(fn::Function, ctx::Context, target::PackageSpec, target_path::S
     mktempdir() do tmp
         tmp_project  = projectfile_path(tmp)
         tmp_manifest = manifestfile_path(tmp)
+        tmp_preferences = joinpath(tmp, first(Base.preferences_names))
 
         # Copy env info over to temp env
         if sandbox_project_override !== nothing
@@ -1522,6 +1547,13 @@ function sandbox(fn::Function, ctx::Context, target::PackageSpec, target_path::S
         end
 
         Types.write_manifest(working_manifest, tmp_manifest)
+        # Copy over preferences
+        if preferences !== nothing
+            open(tmp_preferences, "w") do io
+                TOML.print(io, preferences)
+            end
+        end
+
         # sandbox
         with_temp_env(tmp) do
             temp_ctx = Context()
@@ -1671,12 +1703,21 @@ function test(ctx::Context, pkgs::Vector{PackageSpec};
     pkgs_errored = Tuple{String, Base.Process}[]
     for (pkg, source_path) in zip(pkgs, source_paths)
         # compatibility shim between "targets" and "test/Project.toml"
-        test_project_override = isfile(projectfile_path(testdir(source_path))) ?
-            nothing :
-            gen_target_project(ctx.env, ctx.registries, pkg, source_path, "test")
+        local test_project_preferences, test_project_override
+        if isfile(projectfile_path(testdir(source_path)))
+            test_project_override = nothing
+            with_load_path(testdir(source_path)) do
+                test_project_preferences = Base.get_preferences()
+            end
+        else
+            test_project_override = gen_target_project(ctx.env, ctx.registries, pkg, source_path, "test")
+            with_load_path(projectfile_path(source_path)) do
+                test_project_preferences = Base.get_preferences()
+            end
+        end
         # now we sandbox
         printpkgstyle(ctx.io, :Testing, pkg.name)
-        sandbox(ctx, pkg, source_path, testdir(source_path), test_project_override; force_latest_compatible_version, allow_earlier_backwards_compatible_versions, allow_reresolve) do
+        sandbox(ctx, pkg, source_path, testdir(source_path), test_project_override; preferences=test_project_preferences, force_latest_compatible_version, allow_earlier_backwards_compatible_versions, allow_reresolve) do
             test_fn !== nothing && test_fn()
             sandbox_ctx = Context(;io=ctx.io)
             status(sandbox_ctx.env, sandbox_ctx.registries; mode=PKGMODE_COMBINED, io=sandbox_ctx.io, ignore_indent = false)

test/artifacts.jl

@@ -6,6 +6,13 @@ import Pkg.Artifacts: pack_platform!, unpack_platform, with_artifacts_directory,
 using TOML, Dates
 import Base: SHA1
 
+# Order-dependence in the tests, so we delay this until we need it
+if Base.find_package("Preferences") === nothing
+    @info "Installing Preferences for Pkg tests"
+    Pkg.add("Preferences") # Needed for sandbox and artifacts tests
+end
+using Preferences
+
 using ..Utils
 
 # Helper function to create an artifact, then chmod() the whole thing to 0o644.  This is
@@ -429,11 +436,16 @@ end
                 wrong_hash = engaged_hash
             end
 
-            withenv("FLOOBLECRANK" => flooblecrank_status) do
-                add_this_pkg()
-                @test isdir(artifact_path(right_hash))
-                @test !isdir(artifact_path(wrong_hash))
-            end
+            # Set the flooblecrank via its preference
+            set_preferences!(
+                joinpath(ap_path, "LocalPreferences.toml"),
+                "AugmentedPlatform",
+                "flooblecrank" => flooblecrank_status,
+            )
+
+            add_this_pkg()
+            @test isdir(artifact_path(right_hash))
+            @test !isdir(artifact_path(wrong_hash))
 
             # Manual test that artifact is installed by instantiate()
             artifacts_toml = joinpath(ap_path, "Artifacts.toml")
@@ -470,15 +482,18 @@ end
         @test !isdir(artifact_path(engaged_hash))
         @test !isdir(artifact_path(disengaged_hash))
 
-        # Instantiate with the environment variable set, but with an explicit
-        # tag set in the platform object, which overrides.
-        withenv("FLOOBLECRANK" => "disengaged") do
-            p = HostPlatform()
-            p["flooblecrank"] = "engaged"
-            add_this_pkg(; platform=p)
-            @test isdir(artifact_path(engaged_hash))
-            @test !isdir(artifact_path(disengaged_hash))
-        end
+        # Set the flooblecrank via its preference
+        set_preferences!(
+            joinpath(ap_path, "LocalPreferences.toml"),
+            "AugmentedPlatform",
+            "flooblecrank" => "disengaged",
+        )
+
+        p = HostPlatform()
+        p["flooblecrank"] = "engaged"
+        add_this_pkg(; platform=p)
+        @test isdir(artifact_path(engaged_hash))
+        @test !isdir(artifact_path(disengaged_hash))
     end
 
     # Also run a test of "cross-installation" use `Pkg.API.instantiate(;platform)`
@@ -491,18 +506,21 @@ end
         @test !isdir(artifact_path(engaged_hash))
         @test !isdir(artifact_path(disengaged_hash))
 
-        # Instantiate with the environment variable set, but with an explicit
-        # tag set in the platform object, which overrides.
-        withenv("FLOOBLECRANK" => "disengaged") do
-            add_this_pkg()
+        # Set the flooblecrank via its preference
+        set_preferences!(
+            joinpath(ap_path, "LocalPreferences.toml"),
+            "AugmentedPlatform",
+            "flooblecrank" => "disengaged",
+        )
 
-            p = HostPlatform()
-            p["flooblecrank"] = "engaged"
-            Pkg.API.instantiate(; platform=p)
+        add_this_pkg()
 
-            @test isdir(artifact_path(engaged_hash))
-            @test isdir(artifact_path(disengaged_hash))
-        end
+        p = HostPlatform()
+        p["flooblecrank"] = "engaged"
+        Pkg.API.instantiate(; platform=p)
+
+        @test isdir(artifact_path(engaged_hash))
+        @test isdir(artifact_path(disengaged_hash))
     end
 end
 

test/sandbox.jl

@@ -1,9 +1,16 @@
 module SandboxTests
 import ..Pkg # ensure we are using the correct Pkg
 
+# Order-dependence in the tests, so we delay this until we need it
+if Base.find_package("Preferences") === nothing
+    @info "Installing Preferences for Pkg tests"
+    Pkg.add("Preferences") # Needed for sandbox and artifacts tests
+end
+
 using Test
 using UUIDs
 using Pkg
+using Preferences
 
 using ..Utils
 test_test(fn, name; kwargs...) = Pkg.test(name; test_fn=fn, kwargs...)
@@ -39,6 +46,67 @@ test_test(fn; kwargs...)       = Pkg.test(;test_fn=fn, kwargs...)
     end end
 end
 
+@testset "Preferences sandboxing without test/Project.toml" begin
+    # Preferences should be copied over into sandbox
+    temp_pkg_dir() do project_path; mktempdir() do tmp
+        copy_test_package(tmp, "Sandbox_PreservePreferences")
+        Pkg.activate(joinpath(tmp, "Sandbox_PreservePreferences"))
+        test_test() do
+            uuid =  UUID("3872bf94-3adb-11e9-01dc-bf80c7641364")
+            @test !Preferences.has_preference(uuid, "does_not_exist")
+            @test Preferences.load_preference(uuid, "tree") == "birch"
+            @test Preferences.load_preference(uuid, "default") === nothing
+        end
+    end end
+end
+
+@testset "Preferences sandboxing with test/Project.toml" begin
+    # Preferences should be copied over into sandbox
+    temp_pkg_dir() do project_path; mktempdir() do tmp
+        copy_test_package(tmp, "Sandbox_PreservePreferences")
+        Pkg.activate(joinpath(tmp, "Sandbox_PreservePreferences"))
+
+        # Create fake test/Project.toml and test/LocalPreferences.toml
+        open(joinpath(tmp, "Sandbox_PreservePreferences", "test", "Project.toml"), write=true) do io
+            print(io, """
+            [deps]
+            Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+            """)
+        end
+        Preferences.set_preferences!(
+            joinpath(tmp, "Sandbox_PreservePreferences", "test", "LocalPreferences.toml"),
+            "Sandbox_PreservePreferences",
+            "scent" => "juniper",
+        )
+
+        # This test should have completely different preferences
+        test_test() do
+            uuid =  UUID("3872bf94-3adb-11e9-01dc-bf80c7641364")
+            @test !Preferences.has_preference(uuid, "does_not_exist")
+            # `tree` has no mapping because we do not know anything about the project
+            # in the root directory; test environments are not stacked!
+            @test Preferences.load_preference(uuid, "tree") === nothing
+            @test Preferences.load_preference(uuid, "scent") == "juniper"
+            @test Preferences.load_preference(uuid, "default") === nothing
+        end
+    end end
+end
+
+@testset "Nested Preferences sandboxing" begin
+    # Preferences should be copied over into sandbox
+    temp_pkg_dir() do project_path; mktempdir() do tmp
+        copy_test_package(tmp, "Sandbox_PreservePreferences")
+        Pkg.activate(joinpath(tmp, "Sandbox_PreservePreferences"))
+        test_test("Foo") do
+            uuid =  UUID("48898bec-3adb-11e9-02a6-a164ba74aeae")
+            @test !Preferences.has_preference(uuid, "does_not_exist")
+            @test Preferences.load_preference(uuid, "toy") == "car"
+            @test Preferences.load_preference(uuid, "tree") == "birch"
+            @test Preferences.load_preference(uuid, "default") === nothing
+        end
+    end end
+end
+
 @testset "Basic `build` sandbox" begin
     temp_pkg_dir() do project_path; mktempdir() do tmp
         copy_test_package(tmp, "BasicSandbox")

test/test_packages/AugmentedPlatform/.pkg/platform_augmentation.jl

@@ -7,8 +7,9 @@ function augment_platform!(p::Platform)
         return p
     end
 
-    # If the tag is not set, autodetect through magic (in this case, checking environment variables)
-    flooblecrank_status = get(ENV, "FLOOBLECRANK", "disengaged")
+    # If the tag is not set, autodetect through magic (in this case, checking preferences)
+    ap_uuid = Base.UUID("4d5b37cf-bcfd-af76-759b-4d98ee7f9293")
+    flooblecrank_status = get(Base.get_preferences(ap_uuid), "flooblecrank", "disengaged")
     if flooblecrank_status == "engaged"
         p["flooblecrank"] = "engaged"
     else

test/test_packages/Sandbox_PreservePreferences/Manifest.toml

@@ -0,0 +1,13 @@
+# This file is machine-generated - editing it directly is not advised
+
+manifest_format = "2.0"
+
+[[deps.Example]]
+git-tree-sha1 = "686e1ac14b35cce47f90c91200f904c603ace29e"
+uuid = "7876af07-990d-54b4-ab0e-23690620f79a"
+version = "0.4.0"
+
+[[deps.Foo]]
+path = "dev/Foo"
+uuid = "48898bec-3adb-11e9-02a6-a164ba74aeae"
+version = "0.1.0"

test/test_packages/Sandbox_PreservePreferences/Project.toml

@@ -0,0 +1,11 @@
+name = "Sandbox_PreservePreferences"
+uuid = "3872bf94-3adb-11e9-01dc-bf80c7641364"
+authors = ["David Varela <[email protected]>"]
+version = "0.1.0"
+
+[deps]
+Example = "7876af07-990d-54b4-ab0e-23690620f79a"
+Foo = "48898bec-3adb-11e9-02a6-a164ba74aeae"
+
+[preferences]
+Sandbox_PreservePreferences.tree = "birch"

test/test_packages/Sandbox_PreservePreferences/dev/Foo/Manifest.toml

@@ -0,0 +1,25 @@
+# This file is machine-generated - editing it directly is not advised
+
+julia_version = "1.7.0"
+manifest_format = "2.0"
+
+[[deps.Dates]]
+deps = ["Printf"]
+uuid = "ade2ca70-3891-5945-98fb-dc099432e06a"
+
+[[deps.Preferences]]
+deps = ["TOML"]
+git-tree-sha1 = "2cf929d64681236a2e074ffafb8d568733d2e6af"
+uuid = "21216c6a-2e73-6563-6e65-726566657250"
+version = "1.2.3"
+
+[[deps.Printf]]
+deps = ["Unicode"]
+uuid = "de0858da-6303-5e67-8744-51eddeeeb8d7"
+
+[[deps.TOML]]
+deps = ["Dates"]
+uuid = "fa267f1f-6049-4f14-aa54-33bafae1ed76"
+
+[[deps.Unicode]]
+uuid = "4ec0a83e-493e-50e2-b9ac-8f72acf5a8f5"

test/test_packages/Sandbox_PreservePreferences/dev/Foo/Project.toml

@@ -0,0 +1,11 @@
+name = "Foo"
+uuid = "48898bec-3adb-11e9-02a6-a164ba74aeae"
+version = "0.1.0"
+
+[deps]
+Preferences = "21216c6a-2e73-6563-6e65-726566657250"
+
+[preferences]
+Foo.toy = "train"
+Foo.tree = "oak"
+Foo.default = "default"