diff --git a/src/Operations.jl b/src/Operations.jl index 7fc4770234..d53b78e625 100644 --- a/src/Operations.jl +++ b/src/Operations.jl @@ -252,9 +252,14 @@ function load_all_deps( 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 @@ -2224,7 +2229,7 @@ function up_load_versions!(ctx::Context, pkg::PackageSpec, entry::PackageEntry, if pkg.path === nothing pkg.tree_hash = entry.tree_hash end - elseif entry.repo.source !== nothing || source_repo.source !== nothing # repo packages have a version but are treated specially + elseif source_path === nothing && pkg.path === nothing && (entry.repo.source !== nothing || source_repo.source !== nothing) # repo packages have a version but are treated specially if source_repo.source !== nothing pkg.repo = source_repo else @@ -2257,10 +2262,12 @@ end up_load_manifest_info!(pkg::PackageSpec, ::Nothing) = nothing function up_load_manifest_info!(pkg::PackageSpec, entry::PackageEntry) pkg.name = entry.name # TODO check name is same - if pkg.repo == GitRepo() + # Only restore repo from manifest if we don't already have a path set + if pkg.repo == GitRepo() && pkg.path === nothing pkg.repo = entry.repo # TODO check that repo is same end - if pkg.path === nothing + # Only set path if tree_hash is not already set (to avoid invalid state where both are set) + if pkg.path === nothing && pkg.repo == GitRepo() && pkg.tree_hash === nothing pkg.path = entry.path end return pkg.pinned = entry.pinned diff --git a/src/Types.jl b/src/Types.jl index 03c9ecbd00..6fea684e8f 100644 --- a/src/Types.jl +++ b/src/Types.jl @@ -1040,6 +1040,7 @@ function handle_repo_add!(ctx::Context, pkg::PackageSpec) pkgerror("Did not find subdirectory `$(pkg.repo.subdir)`") end end + @assert pkg.path === nothing pkg.tree_hash = SHA1(string(LibGit2.GitHash(tree_hash_object))) # If we already resolved a uuid, we can bail early if this package is already installed at the current tree_hash diff --git a/test/sources.jl b/test/sources.jl index 3edefd8dde..e5b5508ec3 100644 --- a/test/sources.jl +++ b/test/sources.jl @@ -3,6 +3,7 @@ module SourcesTest import ..Pkg # ensure we are using the correct Pkg using Test, Pkg using ..Utils +using UUIDs temp_pkg_dir() do project_path @testset "test Project.toml [sources]" begin @@ -152,6 +153,91 @@ temp_pkg_dir() do project_path end end end + + # Regression test for https://github.com/JuliaLang/Pkg.jl/issues/4337 + # Switching between path and repo sources should not cause assertion error + @testset "switching between path and repo sources (#4337)" begin + mktempdir() do tmp + cd(tmp) do + # Create a local package and initialize it as a git repo + local_pkg_uuid = UUID("00000000-0000-0000-0000-000000000001") + mkdir("LocalPkg") + write( + joinpath("LocalPkg", "Project.toml"), """ + name = "LocalPkg" + uuid = "$local_pkg_uuid" + version = "0.1.0" + """ + ) + mkdir(joinpath("LocalPkg", "src")) + write(joinpath("LocalPkg", "src", "LocalPkg.jl"), "module LocalPkg end") + + # Initialize as a git repo + git_init_and_commit("LocalPkg") + + # Get the absolute path for file:// URL + local_pkg_url = make_file_url(abspath("LocalPkg")) + + # Create test project with path source + write( + "Project.toml", """ + [deps] + LocalPkg = "$local_pkg_uuid" + + [sources] + LocalPkg = { path = "LocalPkg" } + """ + ) + + with_current_env() do + # Initial resolve with path source + Pkg.resolve() + manifest = Pkg.Types.read_manifest("Manifest.toml") + @test manifest[local_pkg_uuid].path !== nothing + @test manifest[local_pkg_uuid].tree_hash === nothing + @test manifest[local_pkg_uuid].repo.source === nothing + # Update should work without error + Pkg.update() + + # Switch to repo source using file:// protocol + write( + "Project.toml", """ + [deps] + LocalPkg = "$local_pkg_uuid" + + [sources] + LocalPkg = { url = "$local_pkg_url", rev = "HEAD" } + """ + ) + + # This should NOT cause an assertion error about tree_hash and path both being set + Pkg.update() + manifest = Pkg.Types.read_manifest("Manifest.toml") + @test manifest[local_pkg_uuid].path === nothing + @test manifest[local_pkg_uuid].tree_hash !== nothing + @test manifest[local_pkg_uuid].repo.source !== nothing + + # Switch back to path source + write( + "Project.toml", """ + [deps] + LocalPkg = "$local_pkg_uuid" + + [sources] + LocalPkg = { path = "LocalPkg" } + """ + ) + + # This should work and restore the path source without assertion error + Pkg.update() + manifest = Pkg.Types.read_manifest("Manifest.toml") + @test manifest[local_pkg_uuid].path !== nothing + @test manifest[local_pkg_uuid].tree_hash === nothing + @test manifest[local_pkg_uuid].repo.source === nothing + end + end + end + end end end # module diff --git a/test/subdir.jl b/test/subdir.jl index 7fa9c57c69..99420c714a 100644 --- a/test/subdir.jl +++ b/test/subdir.jl @@ -65,16 +65,6 @@ function setup_packages_repository(dir) return package_tree_hash, dep_tree_hash end -# Convert a path into a file URL. -function make_file_url(path) - # Turn the slashes on Windows. In case the path starts with a - # drive letter, an extra slash will be needed in the file URL. - path = replace(path, "\\" => "/") - if !startswith(path, "/") - path = "/" * path - end - return "file://$(path)" -end # Create a registry with the two packages `Package` and `Dep`. function setup_registry(dir, packages_dir_url, package_tree_hash, dep_tree_hash) diff --git a/test/utils.jl b/test/utils.jl index b752f5bbb7..fc151c6d3f 100644 --- a/test/utils.jl +++ b/test/utils.jl @@ -11,7 +11,7 @@ using UUIDs export temp_pkg_dir, cd_tempdir, isinstalled, write_build, with_current_env, with_temp_env, with_pkg_env, git_init_and_commit, copy_test_package, git_init_package, add_this_pkg, TEST_SIG, TEST_PKG, isolate, LOADED_DEPOT, - list_tarball_files, recursive_rm_cov_files, copy_this_pkg_cache + list_tarball_files, recursive_rm_cov_files, copy_this_pkg_cache, make_file_url const CACHE_DIRECTORY = realpath(mktempdir(; cleanup = true)) @@ -367,4 +367,15 @@ function recursive_rm_cov_files(rootdir::String) return end +# Convert a path into a file URL. +function make_file_url(path) + # Turn the slashes on Windows. In case the path starts with a + # drive letter, an extra slash will be needed in the file URL. + path = replace(path, "\\" => "/") + if !startswith(path, "/") + path = "/" * path + end + return "file://$(path)" +end + end