ensure sources take prio over manifest information, take 2

src/Operations.jl

@@ -200,19 +200,50 @@ 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)
+
+        # Track when [sources] entries should override manifest data
+        sources_has_path = path !== nothing
+        sources_has_repo = repo != GitRepo()
+        repo_overrides_entry = sources_has_repo && (entry === nothing || repo != entry.repo)
+        clear_tree_hash = sources_has_path || repo_overrides_entry
+        tree_hash = entry === nothing ? nothing :
+            clear_tree_hash ? nothing :
+            entry.tree_hash
+        final_repo = repo == GitRepo() ? (entry === nothing ? 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 when we purposely cleared it for sources overrides
+            if clear_tree_hash
+                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 sources_has_repo
+                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 +279,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
 
@@ -2250,8 +2264,8 @@ function add(
 
         # if env is a package add compat entries
         add_compat_entries!(ctx, pkgs)
-        record_project_hash(ctx.env) # compat entries changed the hash after it was last recorded in update_manifest!
 
+        record_project_hash(ctx.env)
         write_env(ctx.env) # write env before building
         show_update(ctx.env, ctx.registries; io = ctx.io)
         build_versions(ctx, union(new_apply, new_git))
@@ -2282,6 +2296,7 @@ function develop(
     new_apply = download_source(ctx)
     fixups_from_projectfile!(ctx)
     download_artifacts(ctx; platform = platform, julia_version = ctx.julia_version)
+    record_project_hash(ctx.env)
     write_env(ctx.env) # write env before building
     show_update(ctx.env, ctx.registries; io = ctx.io)
     return build_versions(ctx, union(new_apply, new_git))
@@ -2436,6 +2451,7 @@ function up(
     new_apply = download_source(ctx)
     fixups_from_projectfile!(ctx)
     download_artifacts(ctx, julia_version = ctx.julia_version)
+    record_project_hash(ctx.env)
     write_env(ctx.env; skip_writing_project) # write env before building
     show_update(ctx.env, ctx.registries; io = ctx.io, hidden_upgrades_info = true)
 
@@ -2510,6 +2526,7 @@ function pin(ctx::Context, pkgs::Vector{PackageSpec})
     new = download_source(ctx)
     fixups_from_projectfile!(ctx)
     download_artifacts(ctx; julia_version = ctx.julia_version)
+    record_project_hash(ctx.env)
     write_env(ctx.env) # write env before building
     show_update(ctx.env, ctx.registries; io = ctx.io)
     return build_versions(ctx, new)
@@ -2560,6 +2577,7 @@ function free(ctx::Context, pkgs::Vector{PackageSpec}; err_if_free = true)
         new = download_source(ctx)
         fixups_from_projectfile!(ctx)
         download_artifacts(ctx)
+        record_project_hash(ctx.env)
         write_env(ctx.env) # write env before building
         show_update(ctx.env, ctx.registries; io = ctx.io)
         build_versions(ctx, new)

src/Types.jl

@@ -1373,12 +1373,8 @@ manifest_info(::Manifest, uuid::Nothing) = nothing
 function manifest_info(manifest::Manifest, uuid::UUID)::Union{PackageEntry, Nothing}
     return get(manifest, uuid, nothing)
 end
-function write_env(
-        env::EnvCache; update_undo = true,
-        skip_writing_project::Bool = false,
-        skip_readonly_check::Bool = false
-    )
-    # Verify that the generated manifest is consistent with `sources`
+function sync_sources_from_manifest!(env::EnvCache)
+    # Sync sources in the project with what's in the manifest
     for (pkg, uuid) in env.project.deps
         path, repo = get_path_repo(env.project, env.project_file, env.manifest_file, pkg)
         entry = manifest_info(env.manifest, uuid)
@@ -1407,7 +1403,14 @@ function write_env(
             end
         end
     end
+    return
+end
 
+function write_env(
+        env::EnvCache; update_undo = true,
+        skip_writing_project::Bool = false,
+        skip_readonly_check::Bool = false
+    )
     # Check if the environment is readonly before attempting to write
     if env.project.readonly && !skip_readonly_check
         pkgerror("Cannot modify a readonly environment. The project at $(env.project_file) is marked as readonly.")

test/sources.jl

@@ -4,6 +4,7 @@ import ..Pkg # ensure we are using the correct Pkg
 using Test, Pkg
 using ..Utils
 using UUIDs
+using LibGit2
 
 temp_pkg_dir() do project_path
     @testset "test Project.toml [sources]" begin
@@ -238,6 +239,97 @@ temp_pkg_dir() do project_path
             end
         end
     end
+
+    @testset "changing rev in sources updates 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
+                        """
+                    )
+
+                    first_commit = string(git_init_and_commit(test_pkg_dir; msg = "Initial commit"))
+                    first_tree_hash = LibGit2.with(LibGit2.GitRepo(test_pkg_dir)) do repo
+                        string(LibGit2.GitHash(LibGit2.peel(LibGit2.GitTree, LibGit2.GitCommit(repo, first_commit))))
+                    end
+
+                    # Make a second commit
+                    write("README.md", "# TestPkg\n")
+                    second_commit = LibGit2.with(LibGit2.GitRepo(test_pkg_dir)) do repo
+                        LibGit2.add!(repo, "README.md")
+                        string(LibGit2.commit(repo, "Add README"; author = TEST_SIG, committer = TEST_SIG))
+                    end
+                    second_tree_hash = LibGit2.with(LibGit2.GitRepo(test_pkg_dir)) do repo
+                        string(LibGit2.GitHash(LibGit2.peel(LibGit2.GitTree, LibGit2.GitCommit(repo, second_commit))))
+                    end
+
+                    # 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")
+
+                        # 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")
+
+                        # Verify second state - 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
+                    end
+                end
+            end
+        end
+    end
 end
 
 end # module