From f189860ef8d0e62ba5cd9119378e9a5d20a3cdc9 Mon Sep 17 00:00:00 2001 From: Milan Bouchet-Valat Date: Sun, 8 Oct 2017 16:07:30 +0200 Subject: [PATCH 1/4] Add test for Pkg issue #17994 This test is supposed to fail on Windows only. Fix is coming. --- test/pkg.jl | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/pkg.jl b/test/pkg.jl index 4d2548f253d87..9fa980c041951 100644 --- a/test/pkg.jl +++ b/test/pkg.jl @@ -675,3 +675,17 @@ temp_pkg_dir(initialize=false) do "INFO: Building Normal") Pkg.Entry.build!(["Exit", "Normal", "Exit", "Normal"], errors) end end + +@testset "issue #17994" begin + temp_pkg_dir() do + @test Pkg.installed("Example.jl") === nothing + Pkg.add("Example.jl") + @test [keys(Pkg.installed())...] == ["Example"] + Pkg.checkout("Example.jl") + Pkg.rm("Example.jl") + Pkg.free("Example.jl") + iob = IOBuffer() + Pkg.status("Example.jl", iob) + @test String(take!(iob)) == "No packages installed\n" + end +end From 1b7f68c36aa93132d385992f7eb4c0a229089c37 Mon Sep 17 00:00:00 2001 From: Milan Bouchet-Valat Date: Sun, 8 Oct 2017 16:29:39 +0200 Subject: [PATCH 2/4] Only call resolve() after closing libgit2 handles in Pkg.free() On Windows, libgit2 can hold locks on files under the package directory which resolve() might want to remove, leading to corruption. --- base/pkg/entry.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/base/pkg/entry.jl b/base/pkg/entry.jl index ad160ae956b9a..cc1a25eb766f9 100644 --- a/base/pkg/entry.jl +++ b/base/pkg/entry.jl @@ -250,11 +250,11 @@ function free(pkg::AbstractString) for ver in vers sha1 = avail[ver].sha1 LibGit2.iscommit(sha1, repo) || continue - return LibGit2.transact(repo) do r + LibGit2.transact(repo) do r LibGit2.isdirty(repo) && throw(PkgError("$pkg is dirty, bailing")) LibGit2.checkout!(repo, sha1) - resolve() end + return resolve() end isempty(Cache.prefetch(pkg, Read.url(pkg), [a.sha1 for (v,a)=avail])) && continue throw(PkgError("can't find any registered versions of $pkg to checkout")) From 782a807a85afe47a4166d927a33e5e09ba3d6c10 Mon Sep 17 00:00:00 2001 From: Milan Bouchet-Valat Date: Sun, 8 Oct 2017 19:29:28 +0200 Subject: [PATCH 3/4] Make Pkg.Entry.free(::AbstractString) call the multi-package method No need to duplicate code here. It turns out the multi-package method taking a collection was actually correct, as it called resolve() only after freeing libgit2 handles. --- base/pkg/entry.jl | 27 ++------------------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/base/pkg/entry.jl b/base/pkg/entry.jl index cc1a25eb766f9..edc84ab98a5f4 100644 --- a/base/pkg/entry.jl +++ b/base/pkg/entry.jl @@ -237,31 +237,6 @@ function checkout(pkg::AbstractString, branch::AbstractString, do_merge::Bool, d end end -function free(pkg::AbstractString) - ispath(pkg,".git") || throw(PkgError("$pkg is not a git repo")) - Read.isinstalled(pkg) || throw(PkgError("$pkg cannot be freed – not an installed package")) - avail = Read.available(pkg) - isempty(avail) && throw(PkgError("$pkg cannot be freed – not a registered package")) - with(GitRepo, pkg) do repo - LibGit2.isdirty(repo) && throw(PkgError("$pkg cannot be freed – repo is dirty")) - info("Freeing $pkg") - vers = sort!(collect(keys(avail)), rev=true) - while true - for ver in vers - sha1 = avail[ver].sha1 - LibGit2.iscommit(sha1, repo) || continue - LibGit2.transact(repo) do r - LibGit2.isdirty(repo) && throw(PkgError("$pkg is dirty, bailing")) - LibGit2.checkout!(repo, sha1) - end - return resolve() - end - isempty(Cache.prefetch(pkg, Read.url(pkg), [a.sha1 for (v,a)=avail])) && continue - throw(PkgError("can't find any registered versions of $pkg to checkout")) - end - end -end - function free(pkgs) try for pkg in pkgs @@ -288,6 +263,8 @@ function free(pkgs) end end +free(pkg::AbstractString) = free([pkg]) + function pin(pkg::AbstractString, head::AbstractString) ispath(pkg,".git") || throw(PkgError("$pkg is not a git repo")) should_resolve = true From 3f31162af949dc6cf25fe394aa541a3a2a22991d Mon Sep 17 00:00:00 2001 From: Milan Bouchet-Valat Date: Sun, 8 Oct 2017 20:20:40 +0200 Subject: [PATCH 4/4] Ensure that package is listed in REQUIRES when calling Pkg.free() Else, resolve() will remove the package (potentially losing data), which is unlikely to be the intended effect. --- base/pkg/entry.jl | 3 +++ test/pkg.jl | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/base/pkg/entry.jl b/base/pkg/entry.jl index edc84ab98a5f4..78b1df245aafc 100644 --- a/base/pkg/entry.jl +++ b/base/pkg/entry.jl @@ -255,6 +255,9 @@ function free(pkgs) break end end + # Add package to REQUIRE if it wasn't present already + # so that resolve() does not remove it + edit(Reqs.add, pkg) isempty(Cache.prefetch(pkg, Read.url(pkg), [a.sha1 for (v,a)=avail])) && continue throw(PkgError("Can't find any registered versions of $pkg to checkout")) end diff --git a/test/pkg.jl b/test/pkg.jl index 9fa980c041951..d1e8f39312caa 100644 --- a/test/pkg.jl +++ b/test/pkg.jl @@ -684,8 +684,10 @@ end Pkg.checkout("Example.jl") Pkg.rm("Example.jl") Pkg.free("Example.jl") + @test [keys(Pkg.installed())...] == ["Example"] iob = IOBuffer() Pkg.status("Example.jl", iob) - @test String(take!(iob)) == "No packages installed\n" + str = chomp(String(take!(iob))) + @test endswith(str, string(Pkg.installed("Example.jl"))) end end