Merge pull request #1539 from 00vareladavid/00/fixes

a set of small fixes

src/API.jl

@@ -79,6 +79,13 @@ function develop(ctx::Context, pkgs::Vector{PackageSpec}; shared::Bool=true,
             pkgerror("version specification invalid when calling `develop`:",
                      " `$(pkg.version)` specified for package $(err_rep(pkg))")
         end
+        # not strictly necessary to check these fields early, but it is more efficient
+        if pkg.name !== nothing && (length(findall(x -> x.name == pkg.name, pkgs)) > 1)
+            pkgerror("it is invalid to specify multiple packages with the same name: $(err_rep(pkg))")
+        end
+        if pkg.uuid !== nothing && (length(findall(x -> x.uuid == pkg.uuid, pkgs)) > 1)
+            pkgerror("it is invalid to specify multiple packages with the same UUID: $(err_rep(pkg))")
+        end
     end
 
     new_git = handle_repos_develop!(ctx, pkgs, shared)
@@ -87,6 +94,9 @@ function develop(ctx::Context, pkgs::Vector{PackageSpec}; shared::Bool=true,
         if Types.collides_with_project(ctx, pkg)
             pkgerror("package $(err_rep(pkg)) has the same name or UUID as the active project")
         end
+        if length(findall(x -> x.uuid == pkg.uuid, pkgs)) > 1
+            pkgerror("it is invalid to specify multiple packages with the same UUID: $(err_rep(pkg))")
+        end
     end
 
     Operations.develop(ctx, pkgs, new_git; strict=strict, platform=platform)
@@ -116,6 +126,13 @@ function add(ctx::Context, pkgs::Vector{PackageSpec}; preserve::PreserveLevel=PR
                          " `$(pkg.version)` specified for package $(err_rep(pkg))")
             end
         end
+        # not strictly necessary to check these fields early, but it is more efficient
+        if pkg.name !== nothing && (length(findall(x -> x.name == pkg.name, pkgs)) > 1)
+            pkgerror("it is invalid to specify multiple packages with the same name: $(err_rep(pkg))")
+        end
+        if pkg.uuid !== nothing && (length(findall(x -> x.uuid == pkg.uuid, pkgs)) > 1)
+            pkgerror("it is invalid to specify multiple packages with the same UUID: $(err_rep(pkg))")
+        end
     end
 
     repo_pkgs = [pkg for pkg in pkgs if (pkg.repo.source !== nothing || pkg.repo.rev !== nothing)]
@@ -134,6 +151,9 @@ function add(ctx::Context, pkgs::Vector{PackageSpec}; preserve::PreserveLevel=PR
         if Types.collides_with_project(ctx, pkg)
             pkgerror("package $(err_rep(pkg)) has same name or UUID as the active project")
         end
+        if length(findall(x -> x.uuid == pkg.uuid, pkgs)) > 1
+            pkgerror("it is invalid to specify multiple packages with the same UUID: $(err_rep(pkg))")
+        end
     end
 
     Operations.add(ctx, pkgs, new_git; preserve=preserve, platform=platform)
@@ -700,7 +720,6 @@ function precompile(ctx::Context)
             break
         end
         if stale
-            printpkgstyle(ctx, :Precompiling, pkg.name)
             try
                 Base.compilecache(pkg, sourcepath)
             catch

src/GitTools.jl

@@ -313,4 +313,11 @@ function tree_hash(root::AbstractString; HashType = SHA.SHA1_CTX)
     return SHA.digest!(ctx)
 end
 
+function check_valid_HEAD(repo)
+    try LibGit2.head(repo)
+    catch err
+        Pkg.Types.pkgerror("invalid git HEAD ($(err.msg))")
+    end
+end
+
 end # module

src/REPLMode/REPLMode.jl

@@ -296,6 +296,7 @@ function APIOptions(options::Vector{Option},
                     specs::Dict{String, OptionSpec},
                     )::APIOptions
     api_options = Dict{Symbol, Any}()
+    enforce_option(options, specs)
     for option in options
         spec = specs[option.val]
         api_options[spec.api.first] = spec.takes_arg ?
@@ -364,9 +365,7 @@ function Command(statement::Statement)::Command
         pkgerror("Wrong number of arguments")
     end
     # options
-    opt_spec = statement.spec.option_specs
-    enforce_option(statement.options, opt_spec)
-    options = APIOptions(statement.options, opt_spec)
+    options = APIOptions(statement.options, statement.spec.option_specs)
     return Command(statement.spec, options, arguments)
 end
 

src/Types.jl

@@ -436,7 +436,7 @@ function relative_project_path(ctx::Context, path::String)
 end
 
 function devpath(ctx::Context, name::String, shared::Bool)
-    dev_dir = shared ? Pkg.devdir() : joinpath(dirname(ctx.env.project_file), "dev")
+    dev_dir = shared ? abspath(Pkg.devdir()) : joinpath(dirname(ctx.env.project_file), "dev")
     return joinpath(dev_dir, name)
 end
 
@@ -556,6 +556,7 @@ function handle_repo_add!(ctx::Context, pkg::PackageSpec)
             if !isdir(joinpath(pkg.repo.source, ".git"))
                 pkgerror("Did not find a git repository at `$(pkg.repo.source)`")
             end
+            LibGit2.with(GitTools.check_valid_HEAD, LibGit2.GitRepo(pkg.repo.source)) # check for valid git HEAD
             pkg.repo.source = isabspath(pkg.repo.source) ? safe_realpath(pkg.repo.source) : relative_project_path(ctx, pkg.repo.source)
             repo_source = normpath(joinpath(dirname(ctx.env.project_file), pkg.repo.source))
         else
@@ -564,6 +565,8 @@ function handle_repo_add!(ctx::Context, pkg::PackageSpec)
     end
 
     LibGit2.with(GitTools.ensure_clone(ctx, add_repo_cache_path(repo_source), repo_source; isbare=true)) do repo
+        GitTools.check_valid_HEAD(repo)
+
         # If the user didn't specify rev, assume they want the default (master) branch if on a branch, otherwise the current commit
         if pkg.repo.rev === nothing
             pkg.repo.rev = LibGit2.isattached(repo) ? LibGit2.branch(repo) : string(LibGit2.GitHash(LibGit2.head(repo)))
@@ -631,8 +634,16 @@ function resolve_projectfile!(ctx, pkg, project_path)
     project_file === nothing && pkgerror(string("could not find project file in package at ",
                                                 pkg.repo.source !== nothing ? pkg.repo.source : (pkg.path)))
     project_data = read_package(project_file)
-    pkg.uuid = project_data.uuid # TODO check no overwrite
-    pkg.name = project_data.name # TODO check no overwrite
+    if pkg.uuid === nothing || pkg.uuid == project_data.uuid
+        pkg.uuid = project_data.uuid
+    else
+        pkgerror("UUID `$(project_data.uuid)` given by project file `$project_file` does not match given UUID `$(pkg.uuid)`")
+    end
+    if pkg.name === nothing || pkg.name == project_data.name
+        pkg.name = project_data.name
+    else
+        pkgerror("name `$(project_data.name)` given by project file `$project_file` does not match given name `$(pkg.name)`")
+    end
 end
 
 get_object_or_branch(repo, rev::SHA1) =

test/new.jl

@@ -302,13 +302,26 @@ end
         @test_throws PkgError Pkg.add(Pkg.PackageSpec("Example", UUID(UInt128(1))))
         # Missing UUID
         @test_throws PkgError Pkg.add(Pkg.PackageSpec(uuid = uuid4()))
+        # Two packages with the same name
+        @test_throws PkgError(
+            "it is invalid to specify multiple packages with the same name: `Example`"
+            ) Pkg.add([Pkg.PackageSpec(;name="Example"), Pkg.PackageSpec(;name="Example",version="0.5.0")])
     end
     # Unregistered UUID in manifest
     isolate(loaded_depot=true) do; mktempdir() do tempdir
         package_path = copy_test_package(tempdir, "UnregisteredUUID")
         Pkg.activate(package_path)
         @test_throws PkgError("expected package `Example [142fd7e7]` to be registered") Pkg.add("JSON")
-
+    end end
+    # empty git repo (no commits)
+    isolate(loaded_depot=true) do; mktempdir() do tempdir
+        close(LibGit2.init(tempdir))
+        try Pkg.add(Pkg.PackageSpec(;path=tempdir))
+            @assert false
+        catch err
+            @test err isa PkgError
+            @test match(r"^invalid git HEAD", err.msg) !== nothing
+        end
     end end
 end
 
@@ -404,6 +417,15 @@ end
         @test haskey(Pkg.project().dependencies, "SimplePackage")
         @test length(Pkg.project().dependencies) == 1
     end end
+    # add when depot does not exist should create the default project in the correct location
+    isolate() do; mktempdir() do tempdir
+        empty!(DEPOT_PATH)
+        push!(DEPOT_PATH, tempdir)
+        rm(tempdir; force=true, recursive=true)
+        @test !isdir(first(DEPOT_PATH))
+        Pkg.add("JSON")
+        @test dirname(dirname(Pkg.project().path)) == realpath(joinpath(tempdir, "environments"))
+    end end
 end
 
 # Here we can use a loaded depot becuase we are only checking changes to the active project.
@@ -789,6 +811,10 @@ end
         @test_throws PkgError Pkg.develop(Pkg.PackageSpec("Example", UUID(UInt128(1))))
         # Missing UUID
         @test_throws PkgError Pkg.develop(Pkg.PackageSpec(uuid = uuid4()))
+        # Two packages with the same name
+        @test_throws PkgError(
+            "it is invalid to specify multiple packages with the same UUID: `Example [7876af07]`"
+            ) Pkg.develop([Pkg.PackageSpec(;name="Example"), Pkg.PackageSpec(;uuid=exuuid)])
     end
 end
 
@@ -869,6 +895,15 @@ end
             @test pkg.source == joinpath(@__DIR__, "test_packages", "A", "dev", "D")
         end
     end
+    # primary depot is a relative path
+    isolate() do; cd_tempdir() do dir
+        empty!(DEPOT_PATH)
+        push!(DEPOT_PATH, "temp")
+        Pkg.develop("JSON")
+        Pkg.dependencies(json_uuid) do pkg
+            @test pkg.source == abspath(joinpath("temp", "dev", "JSON"))
+        end
+    end end
 end
 
 @testset "develop: interaction with `JULIA_PKG_DEVDIR`" begin

test/repl.jl

@@ -394,6 +394,9 @@ temp_pkg_dir() do project_path; cd(project_path) do
 
         # parse errors should not throw
         _ = test_complete("add \"Foo")
+        # invalid option should not throw
+        _ = test_complete("add -z Foo")
+        _ = test_complete("add --dontexist Foo")
     end # testset
 end end