Pkg: Bug fixes and doc improvements

stdlib/Pkg/docs/src/index.md

@@ -734,6 +734,7 @@ Compatibility for a dependency is entered in the `Project.toml` file as for exam
 
 ```toml
 [compat]
+julia = "1.0"
 Example = "0.4.3"
 ```
 

stdlib/Pkg/src/API.jl

@@ -29,6 +29,7 @@ add_or_develop(pkgs::Vector{String}; kwargs...)            = add_or_develop([che
 add_or_develop(pkgs::Vector{PackageSpec}; kwargs...)       = add_or_develop(Context(), pkgs; kwargs...)
 
 function add_or_develop(ctx::Context, pkgs::Vector{PackageSpec}; mode::Symbol, shared::Bool=true, kwargs...)
+    pkgs = deepcopy(pkgs)  # deepcopy for avoid mutating PackageSpec members
     Context!(ctx; kwargs...)
 
     # All developed packages should go through handle_repos_develop so just give them an empty repo
@@ -73,6 +74,7 @@ rm(pkgs::Vector{String}; kwargs...)            = rm([PackageSpec(pkg) for pkg in
 rm(pkgs::Vector{PackageSpec}; kwargs...)       = rm(Context(), pkgs; kwargs...)
 
 function rm(ctx::Context, pkgs::Vector{PackageSpec}; mode=PKGMODE_PROJECT, kwargs...)
+    pkgs = deepcopy(pkgs)  # deepcopy for avoid mutating PackageSpec members
     for pkg in pkgs
         # TODO only overwrite pkg.mode if default value ?
         pkg.mode = mode
@@ -166,6 +168,7 @@ up(pkgs::Vector{PackageSpec}; kwargs...)       = up(Context(), pkgs; kwargs...)
 
 function up(ctx::Context, pkgs::Vector{PackageSpec};
             level::UpgradeLevel=UPLEVEL_MAJOR, mode::PackageMode=PKGMODE_PROJECT, do_update_registry=true, kwargs...)
+    pkgs = deepcopy(pkgs)  # deepcopy for avoid mutating PackageSpec members
     for pkg in pkgs
         # TODO only override if they are not already set
         pkg.mode = mode
@@ -205,6 +208,7 @@ pin(pkgs::Vector{String}; kwargs...)            = pin([PackageSpec(pkg) for pkg
 pin(pkgs::Vector{PackageSpec}; kwargs...)       = pin(Context(), pkgs; kwargs...)
 
 function pin(ctx::Context, pkgs::Vector{PackageSpec}; kwargs...)
+    pkgs = deepcopy(pkgs)  # deepcopy for avoid mutating PackageSpec members
     Context!(ctx; kwargs...)
     ctx.preview && preview_info()
     project_deps_resolve!(ctx.env, pkgs)
@@ -219,6 +223,7 @@ free(pkgs::Vector{String}; kwargs...)            = free([PackageSpec(pkg) for pk
 free(pkgs::Vector{PackageSpec}; kwargs...)       = free(Context(), pkgs; kwargs...)
 
 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)
@@ -250,6 +255,7 @@ test(pkgs::Vector{String}; kwargs...)             = test([PackageSpec(pkg) for p
 test(pkgs::Vector{PackageSpec}; kwargs...)        = test(Context(), pkgs; kwargs...)
 
 function test(ctx::Context, pkgs::Vector{PackageSpec}; coverage=false, kwargs...)
+    pkgs = deepcopy(pkgs)  # deepcopy for avoid mutating PackageSpec members
     Context!(ctx; kwargs...)
     ctx.preview && preview_info()
     if isempty(pkgs)
@@ -423,6 +429,7 @@ build(pkg::Array{Union{}, 1}) = build(PackageSpec[])
 build(pkg::PackageSpec) = build([pkg])
 build(pkgs::Vector{PackageSpec}) = build(Context(), pkgs)
 function build(ctx::Context, pkgs::Vector{PackageSpec}; kwargs...)
+    pkgs = deepcopy(pkgs)  # deepcopy for avoid mutating PackageSpec members
     Context!(ctx; kwargs...)
 
     ctx.preview && preview_info()
@@ -489,7 +496,7 @@ function precompile(ctx::Context)
         sourcepath = Base.locate_package(pkg)
         if sourcepath == nothing
             # XXX: this isn't supposed to be fatal
-            pkgerror("couldn't find path to $(pkg.name) when trying to precompilie project")
+            pkgerror("couldn't find path to $(pkg.name) when trying to precompile project")
         end
         stale = true
         for path_to_try in paths::Vector{String}

stdlib/Pkg/src/GitTools.jl

@@ -75,8 +75,13 @@ setprotocol!(proto::Union{Nothing, AbstractString}=nothing) = GIT_PROTOCOL[] = p
 # TODO: extend this to more urls
 function normalize_url(url::AbstractString)
     m = match(GITHUB_REGEX, url)
-    (m === nothing || GIT_PROTOCOL[] === nothing) ?
-        url : "$(GIT_PROTOCOL[])://github.com/$(m.captures[1]).git"
+    if m === nothing || GIT_PROTOCOL[] === nothing
+        url
+    elseif GIT_PROTOCOL[] == "ssh"
+        "ssh://[email protected]/$(m.captures[1]).git"
+    else
+        "$(GIT_PROTOCOL[])://github.com/$(m.captures[1]).git"
+    end
 end
 
 function clone(url, source_path; header=nothing, kwargs...)

stdlib/Pkg/src/Pkg.jl

@@ -74,7 +74,7 @@ const UpgradeLevel = Types.UpgradeLevel
 
 # Define new variables so tab comleting Pkg. works.
 """
-    Pkg.add(pkg::Union{String, Vector{String})
+    Pkg.add(pkg::Union{String, Vector{String}})
     Pkg.add(pkg::Union{PackageSpec, Vector{PackageSpec}})
 
 Add a package to the current project. This package will be available using the
@@ -94,7 +94,7 @@ See also [`PackageSpec`](@ref).
 const add = API.add
 
 """
-    Pkg.rm(pkg::Union{String, Vector{String})
+    Pkg.rm(pkg::Union{String, Vector{String}})
     Pkg.rm(pkg::Union{PackageSpec, Vector{PackageSpec}})
 
 Remove a package from the current project. If the `mode` of `pkg` is
@@ -107,7 +107,7 @@ const rm = API.rm
 
 """
     Pkg.update(; level::UpgradeLevel=UPLEVEL_MAJOR, mode::PackageMode = PKGMODE_PROJECT)
-    Pkg.update(pkg::Union{String, Vector{String})
+    Pkg.update(pkg::Union{String, Vector{String}})
     Pkg.update(pkg::Union{PackageSpec, Vector{PackageSpec}})
 
 Update a package `pkg`. If no posistional argument is given, update all packages in the manifest if `mode` is `PKGMODE_MANIFEST` and packages in both manifest and project if `mode` is `PKGMODE_PROJECT`.
@@ -162,7 +162,7 @@ const gc = API.gc
 
 """
     Pkg.build()
-    Pkg.build(pkg::Union{String, Vector{String})
+    Pkg.build(pkg::Union{String, Vector{String}})
     Pkg.build(pkgs::Union{PackageSpec, Vector{PackageSpec}})
 
 Run the build script in `deps/build.jl` for `pkg` and all of the dependencies in
@@ -178,7 +178,7 @@ const build = API.build
 const installed = API.installed
 
 """
-    Pkg.pin(pkg::Union{String, Vector{String})
+    Pkg.pin(pkg::Union{String, Vector{String}})
     Pkg.pin(pkgs::Union{Packagespec, Vector{Packagespec}})
 
 Pin a package to the current version (or the one given in the `packagespec` or a certain
@@ -187,7 +187,7 @@ git revision. A pinned package is never updated.
 const pin = API.pin
 
 """
-    Pkg.free(pkg::Union{String, Vector{String})
+    Pkg.free(pkg::Union{String, Vector{String}})
     Pkg.free(pkgs::Union{Packagespec, Vector{Packagespec}})
 
 Free a package which removes a `pin` if it exists, or if the package is tracking a path,
@@ -203,7 +203,7 @@ const free = API.free
 
 
 """
-    Pkg.develop(pkg::Union{String, Vector{String})
+    Pkg.develop(pkg::Union{String, Vector{String}})
     Pkg.develop(pkgs::Union{Packagespec, Vector{Packagespec}})
 
 Make a package available for development by tracking it by path.
@@ -270,16 +270,16 @@ that is modified by executing package commands.
 The logic for what path is activated is as follows:
 
   * If `shared` is `true`, the first existing environment named `s` from the depots
-    in the depot stack will be activated. If no such environment exists yet,
-    activate it in the first depot.
-  * If `s` is a path that exist, that environment will be activated.
-  * If `s` is a package name in the current project activate that is tracking a path,
-    activate the environment at that path.
-  * If `s` is a non-existing path, activate that path.
-
-If no argument is given to `activate`, activate the home project,
-which is the one specified by either `--project` command line when starting julia,
-or `JULIA_PROJECT` environment variable.
+    in the depot stack will be activated. If no such environment exists,
+    create and activate that environment in the first depot.
+  * If `s` is an existing path, then activate the environment at that path.
+  * If `s` is a package in the current project and `s` is tracking a path, then
+    activate the environment at the tracked path.
+  * Else, `s` is interpreted as a non-existing path, activate that path.
+
+If no argument is given to `activate`, then activate the home project.
+The home project is specified by either the `--project` command line option to
+the julia executable, or the `JULIA_PROJECT` environment variable.
 
 # Examples
 ```

stdlib/Pkg/src/REPLMode.jl

@@ -764,7 +764,8 @@ end
 
 function complete_local_path(s, i1, i2)
     cmp = REPL.REPLCompletions.complete_path(s, i2)
-    [REPL.REPLCompletions.completion_text(p) for p in cmp[1]], cmp[2], !isempty(cmp[1])
+    completions = filter!(isdir, [REPL.REPLCompletions.completion_text(p) for p in cmp[1]])
+    return completions, cmp[2], !isempty(completions)
 end
 
 function complete_installed_package(s, i1, i2, project_opt)
@@ -1174,7 +1175,7 @@ The `startup.jl` file is disabled during building unless julia is started with `
     resolve
 
 Resolve the project i.e. run package resolution and update the Manifest. This is useful in case the dependencies of developed
-packages have changed causing the current Manifest to_indices be out of sync.
+packages have changed causing the current Manifest to be out of sync.
     """,
 ),( CMD_ACTIVATE,
     ["activate"],

stdlib/Pkg/src/Types.jl

@@ -240,13 +240,13 @@ function find_project_file(env::Union{Nothing,String}=nothing)
     project_file = nothing
     if env isa Nothing
         project_file = Base.active_project()
-        project_file == nothing && error("no active project")
+        project_file == nothing && pkgerror("no active project")
     elseif startswith(env, '@')
         project_file = Base.load_path_expand(env)
-        project_file === nothing && error("package environment does not exist: $env")
+        project_file === nothing && pkgerror("package environment does not exist: $env")
     elseif env isa String
         if isdir(env)
-            isempty(readdir(env)) || error("environment is a package directory: $env")
+            isempty(readdir(env)) || pkgerror("environment is a package directory: $env")
             project_file = joinpath(env, Base.project_names[end])
         else
             project_file = endswith(env, ".toml") ? abspath(env) :
@@ -708,14 +708,16 @@ function parse_package!(ctx, pkg, project_path)
         if !isempty(ctx.old_pkg2_clone_name) # remove when legacy CI script support is removed
             pkg.name = ctx.old_pkg2_clone_name
         else
-            # This is an old style package, get the name from src/PackageName
-            if isdir_windows_workaround(pkg.repo.url)
-                m = match(reg_pkg, abspath(pkg.repo.url))
-            else
-                m = match(reg_pkg, pkg.repo.url)
+            # This is an old style package, if not set, get the name from src/PackageName
+            if !has_name(pkg)
+                if isdir_windows_workaround(pkg.repo.url)
+                    m = match(reg_pkg, abspath(pkg.repo.url))
+                else
+                    m = match(reg_pkg, pkg.repo.url)
+                end
+                m === nothing && pkgerror("cannot determine package name from URL or path: $(pkg.repo.url), provide a name argument to `PackageSpec`")
+                pkg.name = m.captures[1]
             end
-            m === nothing && pkgerror("cannot determine package name from URL or path: $(pkg.repo.url)")
-            pkg.name = m.captures[1]
         end
         reg_uuids = registered_uuids(env, pkg.name)
         is_registered = !isempty(reg_uuids)
@@ -786,7 +788,7 @@ end
 # Disambiguate name/uuid package specifications using project info.
 function project_deps_resolve!(env::EnvCache, pkgs::AbstractVector{PackageSpec})
     uuids = env.project["deps"]
-    names = Dict(uuid => name for (uuid, name) in uuids)
+    names = Dict(uuid => name for (name, uuid) in uuids)
     length(uuids) < length(names) && # TODO: handle this somehow?
         pkgerror("duplicate UUID found in project file's [deps] section")
     for pkg in pkgs
@@ -1054,12 +1056,16 @@ function registered_uuid(env::EnvCache, name::String)::UUID
         end
     end
     length(choices_cache) == 1 && return choices_cache[1][1]
-    # prompt for which UUID was intended:
-    menu = RadioMenu(choices)
-    choice = request("There are multiple registered `$name` packages, choose one:", menu)
-    choice == -1 && return UUID(zero(UInt128))
-    env.paths[choices_cache[choice][1]] = [choices_cache[choice][2]]
-    return choices_cache[choice][1]
+    if isinteractive()
+        # prompt for which UUID was intended:
+        menu = RadioMenu(choices)
+        choice = request("There are multiple registered `$name` packages, choose one:", menu)
+        choice == -1 && return UUID(zero(UInt128))
+        env.paths[choices_cache[choice][1]] = [choices_cache[choice][2]]
+        return choices_cache[choice][1]
+    else
+        pkgerror("there are multiple registered `$name` packages, explicitly set the uuid")
+    end
 end
 
 # Determine current name for a given package UUID

stdlib/Pkg/src/versions.jl

@@ -66,7 +66,7 @@ stricterupper(a::VersionBound, b::VersionBound) = isless_uu(a, b) ? a : b
 # `2.3.4` can be joined with `2.3.5` etc.
 
 function isjoinable(up::VersionBound, lo::VersionBound)
-    up.n == 0 && up.lo == 0 && return true
+    up.n == 0 && lo.n == 0 && return true
     if up.n == lo.n
         n = up.n
         for i = 1:(n - 1)

stdlib/Pkg/test/pkg.jl

@@ -88,6 +88,9 @@ import Pkg.Types: semver_spec, VersionSpec
     @test_throws ErrorException semver_spec("^^0.2.3")
     @test_throws ErrorException semver_spec("^^0.2.3.4")
     @test_throws ErrorException semver_spec("0.0.0")
+
+    @test Pkg.Types.isjoinable(Pkg.Types.VersionBound((1,5)), Pkg.Types.VersionBound((1,6)))
+    @test !(Pkg.Types.isjoinable(Pkg.Types.VersionBound((1,5)), Pkg.Types.VersionBound((1,6,0))))
 end
 
 # TODO: Should rewrite these tests not to rely on internals like field names
@@ -259,6 +262,23 @@ temp_pkg_dir() do project_path
                 end
             end
         end
+        mktempdir() do devdir
+            withenv("JULIA_PKG_DEVDIR" => devdir) do
+                try
+                    https_url = "https://github.com/JuliaLang/Example.jl.git"
+                    ssh_url = "ssh://[email protected]/JuliaLang/Example.jl.git"
+                    @test Pkg.GitTools.normalize_url(https_url) == https_url
+                    Pkg.setprotocol!("ssh")
+                    @test Pkg.GitTools.normalize_url(https_url) == ssh_url
+                    # TODO: figure out how to test this without
+                    #       having to deploy a ssh key on github
+                    #Pkg.develop("Example")
+                    #@test isinstalled(TEST_PKG)
+                finally
+                    Pkg.setprotocol!()
+                end
+            end
+        end
     end
 
     @testset "check logging" begin
@@ -354,7 +374,6 @@ temp_pkg_dir() do project_path
     end
 end
 
-#=
 temp_pkg_dir() do project_path
     @testset "valid project file names" begin
         extract_uuid(toml_path) = begin
@@ -370,6 +389,8 @@ temp_pkg_dir() do project_path
         end
 
         cd(project_path) do
+            target_dir = mktempdir()
+            uuid = nothing
             mktempdir() do tmp; cd(tmp) do
                 pkg_name = "FooBar"
                 # create a project and grab its uuid
@@ -381,20 +402,21 @@ temp_pkg_dir() do project_path
                 Pkg.activate(abspath(pkg_name))
                 # add an example project to populate manifest file
                 Pkg.add("Example")
-                Pkg.activate()
                 # change away from default names
-                mv(joinpath(pkg_name, "Project.toml"), joinpath(pkg_name, "JuliaProject.toml"))
-                mv(joinpath(pkg_name, "Manifest.toml"), joinpath(pkg_name, "JuliaManifest.toml"))
-                # make sure things still work
-                Pkg.develop(PackageSpec(url = abspath(pkg_name)))
-                @test isinstalled((name=pkg_name, uuid=UUID(uuid)))
-                Pkg.rm(pkg_name)
-                @test !isinstalled((name=pkg_name, uuid=UUID(uuid)))
+                ## note: this is written awkwardly because a `mv` here causes failures on AppVeyor
+                cp(joinpath(pkg_name, "src"), joinpath(target_dir, "src"))
+                cp(joinpath(pkg_name, "Project.toml"), joinpath(target_dir, "JuliaProject.toml"))
+                cp(joinpath(pkg_name, "Manifest.toml"), joinpath(target_dir, "JuliaManifest.toml"))
             end end
+            Pkg.activate()
+            # make sure things still work
+            Pkg.REPLMode.pkgstr("dev $target_dir")
+            @test isinstalled((name="FooBar", uuid=UUID(uuid)))
+            Pkg.rm("FooBar")
+            @test !isinstalled((name="FooBar", uuid=UUID(uuid)))
         end # cd project_path
     end # @testset
 end
-=#
 
 temp_pkg_dir() do project_path
     @testset "invalid repo url" begin
@@ -506,6 +528,16 @@ end
     @test Pkg.Types.pathrepr(path) == "`@stdlib/Test`"
 end
 
+
+temp_pkg_dir() do project_path
+    @testset "Pkg.add should not mutate" begin
+        package_names = ["JSON"]
+        packages = PackageSpec.(package_names)
+        Pkg.add(packages)
+        @test [p.name for p in packages] == package_names
+    end
+end
+
 include("repl.jl")
 include("api.jl")
 

stdlib/Pkg/test/repl.jl

@@ -462,6 +462,10 @@ temp_pkg_dir() do project_path; cd(project_path) do
         @test apply_completion("add ./tes") == (Sys.iswindows() ? "add ./testdir\\\\" : "add ./testdir/")
         c, r = test_complete("dev ./")
         @test (Sys.iswindows() ? ("testdir\\\\" in c) : ("testdir/" in c))
+        # dont complete files
+        touch("README.md")
+        c, r = test_complete("add RE")
+        @test !("README.md" in c)
     end # testset
 end end