Merge #971

971: Free should resolve against the manifest r=00vareladavid a=00vareladavid

Resolving against the manifest first ensures that a package's UUID is the intended one.

This fixes a bug where attempting to free an unregistered package with the same name as a registered package will fail.

Co-authored-by: David Varela <00.varela.david@gmail.com>

src/API.jl

@@ -146,21 +146,18 @@ function free(ctx::Context, pkgs::Vector{PackageSpec}; kwargs...)
     pkgs = deepcopy(pkgs)  # deepcopy for avoid mutating PackageSpec members
     Context!(ctx; kwargs...)
     ctx.preview && preview_info()
-    registry_resolve!(ctx.env, pkgs)
-    uuids_in_registry = UUID[]
-    for pkg in pkgs
-        pkg.mode = PKGMODE_MANIFEST
-    end
-    for pkg in pkgs
-        has_uuid(pkg) && push!(uuids_in_registry, pkg.uuid)
-    end
+    foreach(pkg -> pkg.mode = PKGMODE_MANIFEST, pkgs)
     manifest_resolve!(ctx.env, pkgs)
     ensure_resolved(ctx.env, pkgs)
-    # Every non pinned package that is freed need to be in a registry
+    find_registered!(ctx.env, UUID[pkg.uuid for pkg in pkgs])
     for pkg in pkgs
         entry = manifest_info(ctx.env, pkg.uuid)
-        if !entry.pinned && !(pkg.uuid in uuids_in_registry)
-            pkgerror("cannot free an unpinned package that does not exist in a registry")
+        entry.pinned && continue
+        if entry.path !== nothing
+            isempty(registered_paths(ctx.env, pkg.uuid)) &&
+                pkgerror("cannot free an unpinned package that does not exist in a registry")
+        else
+            pkgerror("`free` is only a valid operation for `pin`ed or `dev`ed packages")
         end
     end
     Operations.free(ctx, pkgs)

test/api.jl

@@ -181,4 +181,21 @@ end
     end end
 end
 
+@testset "Pkg.free" begin
+    temp_pkg_dir() do project_path
+        # Assumes that `TOML` is a registered package name
+        # Can not free an un-`dev`ed un-`pin`ed package
+        with_temp_env() do; mktempdir() do tempdir;
+            p = git_init_package(tempdir, joinpath(@__DIR__, "test_packages", "TOML"))
+            Pkg.add(Pkg.PackageSpec(;path=p))
+            @test_throws PkgError Pkg.free("TOML")
+        end end
+        # Can not free an unregistered package
+        with_temp_env() do;
+            Pkg.develop(Pkg.PackageSpec(;url="https://github.com/00vareladavid/Unregistered.jl"))
+            @test_throws PkgError Pkg.free("Unregistered")
+        end
+    end
+end
+
 end # module APITests

test/repl.jl

@@ -11,17 +11,6 @@ import LibGit2
 
 include("utils.jl")
 
-function git_init_package(tmp, path)
-    base = basename(path)
-    pkgpath = joinpath(tmp, base)
-    cp(path, pkgpath)
-    LibGit2.with(LibGit2.init(pkgpath)) do repo
-        LibGit2.add!(repo, "*")
-        LibGit2.commit(repo, "initial commit"; author=TEST_SIG, committer=TEST_SIG)
-    end
-    return pkgpath
-end
-
 @testset "generate args" begin
     @test_throws PkgError pkg"generate"
 end

test/test_packages/TOML/Project.toml

@@ -0,0 +1,6 @@
+authors = ["Some Bot <b0t@b0tmail.com>"]
+name = "TOML"
+uuid = "4fbc28a8-0597-11e9-18ac-0d2e309904b0"
+version = "0.1.0"
+
+[deps]

test/test_packages/TOML/src/TOML.jl

@@ -0,0 +1,5 @@
+module TOML
+
+greet() = print("Hello World!")
+
+end # module

test/utils.jl

@@ -122,3 +122,14 @@ import LibGit2
 using UUIDs
 const TEST_SIG = LibGit2.Signature("TEST", "TEST@TEST.COM", round(time()), 0)
 const TEST_PKG = (name = "Example", uuid = UUID("7876af07-990d-54b4-ab0e-23690620f79a"))
+
+function git_init_package(tmp, path)
+    base = basename(path)
+    pkgpath = joinpath(tmp, base)
+    cp(path, pkgpath)
+    LibGit2.with(LibGit2.init(pkgpath)) do repo
+        LibGit2.add!(repo, "*")
+        LibGit2.commit(repo, "initial commit"; author=TEST_SIG, committer=TEST_SIG)
+    end
+    return pkgpath
+end