ensure sources take prio over manifest information, take 2

Author: KristofferC

src/Operations.jl

@@ -200,19 +200,47 @@ function load_project_deps(
     end
 
     for (name::String, uuid::UUID) in project.deps
-        findfirst(pkg -> pkg.uuid == uuid, pkgs) === nothing || continue # do not duplicate packages
+        # Check if package already exists in pkgs (from up_load_manifest_info! etc)
+        existing_idx = findfirst(pkg -> pkg.uuid == uuid, pkgs)
+
         path, repo = get_path_repo(project, project_file, manifest_file, name)
         entry = manifest_info(manifest, uuid)
+
+        # Clear tree_hash if sources override path or repo
+        tree_hash = entry === nothing ? nothing :
+            path !== nothing ? nothing :
+            repo.source !== nothing || repo.rev !== nothing ? nothing :
+            entry.tree_hash
+        final_repo = repo == GitRepo() ? entry.repo : repo
+
+        if existing_idx !== nothing
+            # Package already in pkgs - update it with sources info
+            existing_pkg = pkgs[existing_idx]
+            # Update tree_hash if sources override
+            if path !== nothing || repo.source !== nothing || repo.rev !== nothing
+                existing_pkg.tree_hash = tree_hash
+            end
+            # Update path if from sources
+            if path !== nothing
+                existing_pkg.path = path
+            end
+            # Update repo if from sources
+            if repo != GitRepo()
+                existing_pkg.repo = final_repo
+            end
+            continue
+        end
+
         push!(
             pkgs_direct, entry === nothing ?
                 PackageSpec(; uuid, name, path, repo) :
                 PackageSpec(;
                     uuid = uuid,
                     name = name,
                     path = path === nothing ? entry.path : path,
-                    repo = repo == GitRepo() ? entry.repo : repo,
+                    repo = final_repo,
                     pinned = entry.pinned,
-                    tree_hash = entry.tree_hash, # TODO should tree_hash be changed too?
+                    tree_hash = tree_hash,
                     version = load_version(entry.version, isfixed(entry), preserve),
                 )
         )
@@ -248,24 +276,7 @@ function load_all_deps(
         preserve::PreserveLevel = PRESERVE_ALL
     )
     pkgs = load_manifest_deps(env.manifest, pkgs; preserve = preserve)
-    # Sources takes presedence over the manifest...
-    for pkg in pkgs
-        path, repo = get_path_repo(env.project, env.project_file, env.manifest_file, pkg.name)
-        if path !== nothing
-            # Path from [sources] takes precedence - clear tree_hash and repo from manifest
-            pkg.tree_hash = nothing
-            pkg.repo = GitRepo()  # Clear any repo info
-            pkg.path = path
-        end
-        if repo.source !== nothing
-            # Repo from [sources] takes precedence - clear path from manifest
-            pkg.path = nothing
-            pkg.repo.source = repo.source
-        end
-        if repo.rev !== nothing
-            pkg.repo.rev = repo.rev
-        end
-    end
+    # load_direct_deps will apply sources and update tree_hash via load_project_deps
     return load_direct_deps(env, pkgs; preserve = preserve)
 end
 

src/Types.jl

@@ -667,13 +667,18 @@ function load_workspace_weak_deps(env::EnvCache)
     return weakdeps
 end
 
-# only hash the deps and compat fields as they are the only fields that affect a resolve
+# hash the deps, compat, and sources fields as they affect a resolve
 function workspace_resolve_hash(env::EnvCache)
     # Handle deps in both [deps] and [weakdeps]
     deps = Dict(pkg.name => pkg.uuid for pkg in Pkg.Operations.load_direct_deps(env))
     weakdeps = load_workspace_weak_deps(env)
     alldeps = merge(deps, weakdeps)
     compats = Dict(name => Pkg.Operations.get_compat_workspace(env, name) for (name, uuid) in alldeps)
+    # Collect sources from project and workspace
+    sources = copy(env.project.sources)
+    for (_, proj) in env.workspace
+        merge!(sources, proj.sources)
+    end
     iob = IOBuffer()
     for (name, uuid) in sort!(collect(deps); by = first)
         println(iob, name, "=", uuid)
@@ -686,6 +691,13 @@ function workspace_resolve_hash(env::EnvCache)
     for (name, compat) in sort!(collect(compats); by = first)
         println(iob, name, "=", compat)
     end
+    println(iob)
+    for (name, source_dict) in sort!(collect(sources); by = first)
+        # Sort the source attributes for deterministic hashing
+        for (key, val) in sort!(collect(source_dict); by = first)
+            println(iob, name, ".", key, "=", val)
+        end
+    end
     str = String(take!(iob))
     return bytes2hex(sha1(str))
 end

test/sources.jl

@@ -238,6 +238,98 @@ temp_pkg_dir() do project_path
             end
         end
     end
+
+    @testset "changing rev in sources updates project_hash and git-tree-sha1 (#4157)" begin
+        isolate() do
+            mktempdir() do tmp
+                # Create a test package with two commits
+                test_pkg_dir = joinpath(tmp, "TestPkg")
+                mkpath(test_pkg_dir)
+                cd(test_pkg_dir) do
+                    write(
+                        "Project.toml", """
+                        name = "TestPkg"
+                        uuid = "b4017d7c-a742-4580-99f2-e286571e6290"
+                        version = "0.1.0"
+                        """
+                    )
+                    mkpath("src")
+                    write(
+                        "src/TestPkg.jl", """
+                        module TestPkg
+                        greet() = "Hello, World!"
+                        end
+                        """
+                    )
+                    run(`git init -q`)
+                    run(`git add .`)
+                    run(`git commit -q -m "Initial commit"`)
+                    first_commit = readchomp(`git rev-parse HEAD`)
+                    # Get tree hash by parsing git log
+                    first_tree_hash = match(r"tree ([0-9a-f]+)", readchomp(`git cat-file -p $first_commit`))[1]
+
+                    # Make a second commit
+                    write("README.md", "# TestPkg\n")
+                    run(`git add .`)
+                    run(`git commit -q -m "Add README"`)
+                    second_commit = readchomp(`git rev-parse HEAD`)
+                    second_tree_hash = match(r"tree ([0-9a-f]+)", readchomp(`git cat-file -p $second_commit`))[1]
+
+                    # Create consumer project
+                    consumer_dir = joinpath(tmp, "consumer")
+                    mkpath(consumer_dir)
+                    cd(consumer_dir) do
+                        # Start with first revision
+                        write(
+                            "Project.toml", """
+                            [deps]
+                            TestPkg = "b4017d7c-a742-4580-99f2-e286571e6290"
+
+                            [sources]
+                            TestPkg = { url = "$test_pkg_dir", rev = "$first_commit" }
+                            """
+                        )
+
+                        Pkg.activate(".")
+                        Pkg.resolve()
+                        @test isfile("Manifest.toml")
+                        manifest = Pkg.Types.read_manifest("Manifest.toml")
+                        first_project_hash = manifest.project_hash
+
+                        # Verify first state
+                        test_pkg_uuid = UUID("b4017d7c-a742-4580-99f2-e286571e6290")
+                        @test haskey(manifest.deps, test_pkg_uuid)
+                        test_pkg_entry = manifest[test_pkg_uuid]
+                        @test test_pkg_entry.tree_hash !== nothing
+                        @test string(test_pkg_entry.tree_hash) == first_tree_hash
+                        @test test_pkg_entry.repo.rev == first_commit
+
+                        # Change to second revision
+                        write(
+                            "Project.toml", """
+                            [deps]
+                            TestPkg = "b4017d7c-a742-4580-99f2-e286571e6290"
+
+                            [sources]
+                            TestPkg = { url = "$test_pkg_dir", rev = "$second_commit" }
+                            """
+                        )
+
+                        Pkg.resolve()
+                        manifest = Pkg.Types.read_manifest("Manifest.toml")
+                        second_project_hash = manifest.project_hash
+
+                        # Verify second state - both project_hash and git-tree-sha1 should change
+                        test_pkg_entry = manifest[test_pkg_uuid]
+                        @test test_pkg_entry.tree_hash !== nothing
+                        @test string(test_pkg_entry.tree_hash) == second_tree_hash
+                        @test test_pkg_entry.repo.rev == second_commit
+                        @test second_project_hash != first_project_hash
+                    end
+                end
+            end
+        end
+    end
 end
 
 end # module