Merge #1292

1292: Adjustments to `pin` API r=fredrikekre a=00vareladavid

#1217 suggests there is some confusing behavior in `pin`.

More consistent behavior would be:
- `pkg> pin Example@x` means "Try to pin the package to the given registered version, if it exists"
    - error out when either the package or the version is unregistered
- `pkg> pin Example` means "Preserve the package at whatever state it happens to be in"
    - error out for stdlibs since stdlibs can not really be mutated in the way other packages can



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

Author: bors[bot]

src/Operations.jl

@@ -1072,30 +1072,46 @@ function up(ctx::Context, pkgs::Vector{PackageSpec}, level::UpgradeLevel)
     # TODO what to do about repo packages?
 end
 
-function update_package_pin!(pkg::PackageSpec, ::Nothing)
+function update_package_pin!(ctx::Context, pkg::PackageSpec, ::Nothing)
     if pkg.version == VersionSpec() # no version to pin
         pkgerror("Can not `pin` a package which does not exist in the manifest")
     end
-    is_stdlib(pkg.uuid) && pkgerror("cannot `pin` stdlibs.")
+    if is_stdlib(pkg.uuid)
+        pkgerror("`pin` can not be applied to `$(pkg.name)` because it is a stdlib.")
+    end
     pkg.pinned = true
 end
 
-function update_package_pin!(pkg::PackageSpec, entry::PackageEntry)
-    is_stdlib(pkg.uuid) && pkgerror("cannot `pin` stdlibs.")
-    !entry.pinned || pkgerror("`$(entry.name)` already pinned, use `free` first.")
-    entry.path === nothing || pkgerror("Can not `pin` `dev`ed package")
+function update_package_pin!(ctx::Context, pkg::PackageSpec, entry::PackageEntry)
+    if is_stdlib(pkg.uuid)
+        pkgerror("`pin` can not be applied to `$(pkg.name)` because it is a stdlib.")
+    end
+    if entry.pinned && pkg.version == VersionSpec()
+        pkgerror("`$(entry.name)` is already pinned. Use `free` to remove a pin.")
+    end
+    # update pinned package
     pkg.pinned = true
-    pkg.version == VersionSpec() && (pkg.version = entry.version) # pin at current version
-    pkg.tree_hash = entry.tree_hash
-    pkg.repo = entry.repo
+    if pkg.version == VersionSpec()
+        pkg.version = entry.version # pin at current version
+        pkg.repo = entry.repo
+        pkg.tree_hash = entry.tree_hash
+        pkg.path = entry.path
+    else # given explicit registered version
+        if entry.repo.url !== nothing || entry.path !== nothing
+            # A pin in this case includes an implicit `free` to switch to tracking registered versions
+            # First, make sure the package is registered so we have something to free to
+            if isempty(registered_paths(ctx.env, pkg.uuid))
+                pkgerror("Unable to pin `$(pkg.name)` to an arbitrary version since it could not be found in a registry.")
+            end
+        end
+    end
 end
 
 function pin(ctx::Context, pkgs::Vector{PackageSpec})
-    foreach(pkg -> update_package_pin!(pkg, manifest_info(ctx.env, pkg.uuid)), pkgs)
+    foreach(pkg -> update_package_pin!(ctx, pkg, manifest_info(ctx.env, pkg.uuid)), pkgs)
     load_direct_deps!(ctx, pkgs)
     check_registered(ctx, pkgs)
 
-    # TODO check that versions exist ? -> I guess resolve_versions should check ?
     resolve_versions!(ctx, pkgs)
     update_manifest!(ctx, pkgs)
 

test/api.jl

@@ -3,7 +3,7 @@
 module APITests
 
 using Pkg, Test
-import Pkg.Types.PkgError
+import Pkg.Types.PkgError, Pkg.Types.ResolverError
 
 include("utils.jl")
 
@@ -247,12 +247,21 @@ end
     end
 end
 
-@testset "Pkg.free" begin
-    # `pin` should not override previous `pin`
+@testset "Pkg.pin" begin
+    # `pin` should detect unregistered packages
+    temp_pkg_dir() do project_path; with_temp_env() do env_path
+        Pkg.add(Pkg.PackageSpec(;url="https://github.com/00vareladavid/Unregistered.jl"))
+        @test_throws PkgError Pkg.pin(Pkg.PackageSpec(;name="Unregistered", version="0.1.0"))
+    end end
+    # when dealing with packages tracking a repo of a regsitered package, `pin` should do an implicit free
+    temp_pkg_dir() do project_path; with_temp_env() do env_path
+        Pkg.add(Pkg.PackageSpec(;name="Example",rev="master"))
+        Pkg.pin(Pkg.PackageSpec(;name="Example",version="0.1.0"))
+    end end
+    # pin should check for a valid version number
     temp_pkg_dir() do project_path; with_temp_env() do env_path
-        Pkg.add(Pkg.PackageSpec(;name="Example"))
-        Pkg.pin(Pkg.PackageSpec(;name="Example", version="0.3.0"))
-        @test_throws PkgError Pkg.pin(Pkg.PackageSpec(;name="Example", version="0.4.0"))
+        Pkg.add(Pkg.PackageSpec(;name="Example",rev="master"))
+        @test_throws ResolverError Pkg.pin(Pkg.PackageSpec(;name="Example",version="100.0.0"))
     end end
 end
 

test/pkg.jl

@@ -624,7 +624,7 @@ end
         # the following should not error
         Pkg.add("UUIDs")
         Pkg.test("UUIDs")
-        @test_throws PkgError("cannot `pin` stdlibs.") Pkg.pin("UUIDs")
+        @test_throws PkgError("`pin` can not be applied to `UUIDs` because it is a stdlib.") Pkg.pin("UUIDs")
     end
 end