Skip to content
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

Add test for preference sandboxing of implicit test dependencies #3392

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

carstenbauer
Copy link
Member

@carstenbauer carstenbauer commented Feb 27, 2023

As per request by @vchuravy, in this PR I add a test that tries to cover issue #3389, i.e. that preferences of implicit test dependencies are currently not sandboxed, but should be. Specifically, I tried to create a minimal fake dependency structure that mimics the MPI.jl / MPIPreferences.jl setup (MPI -> Foo, MPIPreferences -> FooDep). I marked the relevant test as @test_broken to indicate the broken status and the expected result.

(Note that one gets the desired result if one makes FooDeps an explicit test dependency of Sandbox_PreservePrefsImplicit.)

cc @staticfloat

@vchuravy
Copy link
Member

Thanks Carsten!

@staticfloat
Copy link
Member

So are we thinking that JuliaLang/julia#48796 will fix this (that is to say, break the broken tests), once it propagates through?

@vchuravy
Copy link
Member

So are we thinking...

Nope that's just something I noticed while thinking about this. The real issue is #3389 (comment)

I started writing a fix, but didn't get far. We need to collect all the UUIDs belonging to the Preferences and then inject them to the generated Project.toml into [extras].

diff --git a/src/Operations.jl b/src/Operations.jl
index 60a1baa6b..544df7c6d 100644
--- a/src/Operations.jl
+++ b/src/Operations.jl
@@ -1697,6 +1697,7 @@ end
 function sandbox(fn::Function, ctx::Context, target::PackageSpec, target_path::String,
                  sandbox_path::String, sandbox_project_override;
                  preferences::Union{Nothing,Dict{String,Any}} = nothing,
+                 preferences_uuids::Union{Nothing, Dict{String, UUID}} = nothing,
                  force_latest_compatible_version::Bool=false,
                  allow_earlier_backwards_compatible_versions::Bool=true,
                  allow_reresolve::Bool=true)
@@ -1715,6 +1716,14 @@ function sandbox(fn::Function, ctx::Context, target::PackageSpec, target_path::S
             cp(sandbox_project, tmp_project)
             chmod(tmp_project, 0o600)
         end
+        if preferences_uuids !== nothing
+            proj = Base.parse_toml(tmp_project)
+            extras = get!(proj, "extras") do
+                Dict{String, UUID}()
+            end
+            merge!(extras, preferences_uuids)
+        end
+
         # create merged manifest
         # - copy over active subgraph
         # - abspath! to maintain location of all deved nodes
@@ -1855,6 +1864,19 @@ function gen_target_project(ctx::Context, pkg::PackageSpec, source_path::String,
     return test_project
 end
 
+function get_preferences_uuids(prefs)
+    merged_uuids = Dict{String, UUID}()
+    for env in reverse(load_path())
+        project_toml = env_project_file(env)
+        if !isa(project_toml, String)
+            continue
+        end
+        project = Base.parsed_toml(project_toml)
+        # for section in ("deps", "extras")
+    end
+    return merged_uuids
+end
+
 testdir(source_path::String) = joinpath(source_path, "test")
 testfile(source_path::String) = joinpath(testdir(source_path), "runtests.jl")
 function test(ctx::Context, pkgs::Vector{PackageSpec};
@@ -1899,16 +1921,18 @@ 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"
-        local test_project_preferences, test_project_override
+        local test_project_preferences, test_project_override, test_project_preferences_uuids
         if isfile(projectfile_path(testdir(source_path)))
             test_project_override = nothing
             with_load_path([testdir(source_path), Base.LOAD_PATH...]) do
                 test_project_preferences = Base.get_preferences()
+                test_project_preferences_uuids = get_preferences_uuid(test_project_preferences)
             end
         else
             test_project_override = gen_target_project(ctx, pkg, source_path, "test")
             with_load_path([something(projectfile_path(source_path)), Base.LOAD_PATH...]) do
                 test_project_preferences = Base.get_preferences()
+                test_project_preferences_uuids = get_preferences_uuid(test_project_preferences)
             end
         end
         # now we sandbox

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants