From 4d0901ed809a7b1f20056893282bafdaee7076ac Mon Sep 17 00:00:00 2001 From: David Varela <00.varela.david@gmail.com> Date: Mon, 9 Dec 2019 14:15:48 -0800 Subject: [PATCH 1/7] Dont throw error when autocompleting faulty input (#1530) --- src/REPLMode/REPLMode.jl | 5 ++--- test/repl.jl | 3 +++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/REPLMode/REPLMode.jl b/src/REPLMode/REPLMode.jl index 3d678e1294..84ec0991e3 100644 --- a/src/REPLMode/REPLMode.jl +++ b/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 diff --git a/test/repl.jl b/test/repl.jl index 904f45ba31..33410bffd6 100644 --- a/test/repl.jl +++ b/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 From 8a6387c3e431d8ed3cec4e204400900d5448c504 Mon Sep 17 00:00:00 2001 From: David Varela <00.varela.david@gmail.com> Date: Mon, 9 Dec 2019 14:18:31 -0800 Subject: [PATCH 2/7] Remove redundant precompile statement --- src/API.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/API.jl b/src/API.jl index 7ac13db5ab..5e105ec6ff 100644 --- a/src/API.jl +++ b/src/API.jl @@ -700,7 +700,6 @@ function precompile(ctx::Context) break end if stale - printpkgstyle(ctx, :Precompiling, pkg.name) try Base.compilecache(pkg, sourcepath) catch From 53fdf2460089f0b1cf2f61251033d1c2081d8e85 Mon Sep 17 00:00:00 2001 From: David Varela <00.varela.david@gmail.com> Date: Mon, 9 Dec 2019 16:36:10 -0800 Subject: [PATCH 3/7] Check for duplicate name/UUID input --- src/API.jl | 20 ++++++++++++++++++++ test/new.jl | 8 ++++++++ 2 files changed, 28 insertions(+) diff --git a/src/API.jl b/src/API.jl index 5e105ec6ff..06117fa3fe 100644 --- a/src/API.jl +++ b/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) diff --git a/test/new.jl b/test/new.jl index 71d66878ab..73687177e0 100644 --- a/test/new.jl +++ b/test/new.jl @@ -302,6 +302,10 @@ 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 @@ -789,6 +793,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 From 37b6853bd6356716e7e46aa85c3980bfc4051219 Mon Sep 17 00:00:00 2001 From: David Varela <00.varela.david@gmail.com> Date: Mon, 9 Dec 2019 18:22:55 -0800 Subject: [PATCH 4/7] handle primary depot as relative path --- src/Types.jl | 2 +- test/new.jl | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Types.jl b/src/Types.jl index 479198cd10..c771ba24d9 100644 --- a/src/Types.jl +++ b/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 diff --git a/test/new.jl b/test/new.jl index 73687177e0..81239beb25 100644 --- a/test/new.jl +++ b/test/new.jl @@ -877,6 +877,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 From 43f46f8d97b39d0776c06f0b3fcb5c989d8ef9f9 Mon Sep 17 00:00:00 2001 From: David Varela <00.varela.david@gmail.com> Date: Mon, 9 Dec 2019 18:23:45 -0800 Subject: [PATCH 5/7] check no overwrite is occuring when resolving from a project file --- src/Types.jl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Types.jl b/src/Types.jl index c771ba24d9..c42b7bb927 100644 --- a/src/Types.jl +++ b/src/Types.jl @@ -631,8 +631,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) = From 0766765e76f15dba0b47d6ae5817a3213fc0b75d Mon Sep 17 00:00:00 2001 From: David Varela <00.varela.david@gmail.com> Date: Tue, 10 Dec 2019 10:07:43 -0800 Subject: [PATCH 6/7] test: default environment should be created when the primary depot does not exist --- test/new.jl | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/new.jl b/test/new.jl index 81239beb25..5f9e53b658 100644 --- a/test/new.jl +++ b/test/new.jl @@ -408,6 +408,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. From 3f1cf40ad6320619d510974b3b9fb8de23dd6050 Mon Sep 17 00:00:00 2001 From: David Varela <00.varela.david@gmail.com> Date: Tue, 10 Dec 2019 12:17:57 -0800 Subject: [PATCH 7/7] it is invalid to `add` a package with no commits --- src/GitTools.jl | 7 +++++++ src/Types.jl | 3 +++ test/new.jl | 11 ++++++++++- 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/GitTools.jl b/src/GitTools.jl index 1e4ae100b0..403d9e6f32 100644 --- a/src/GitTools.jl +++ b/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 diff --git a/src/Types.jl b/src/Types.jl index c42b7bb927..41d5b06604 100644 --- a/src/Types.jl +++ b/src/Types.jl @@ -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))) diff --git a/test/new.jl b/test/new.jl index 5f9e53b658..c90a71d8de 100644 --- a/test/new.jl +++ b/test/new.jl @@ -312,7 +312,16 @@ end 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