Fix several issues with Pkg.pin()
#17196
Conversation
* Actually check out the branch after creation (fix #17176) * Activate test in test/pkg.jl * Avoid trying to update a pinned package (and emit info message) * Allow to pin a package several times without producing errors * Fix the cases in which resolve() is called after a pin()
| # no need to resolve, branch will be from HEAD | ||
| should_resolve = false | ||
| LibGit2.head_oid(repo) |
There was a problem hiding this comment.
what was the logic error here?
There was a problem hiding this comment.
resolve was being called when keeping the head fixed and not called when changing it, rather than the other way around.
The behaviour after the fix is the following (notice the info messages which tell you whether resolve was invoked or not):
julia> Pkg.pin("Example") # stay on head, resolve not invoked
INFO: Creating Example branch pinned.526e34fd.tmp
julia> Pkg.pin("Example", v"0.4.0") # resolve invoked
INFO: Creating Example branch pinned.b1990792.tmp
INFO: No packages to install, update or remove
julia> Pkg.pin("Example", v"0.4.0") # no changes, resolve not invoked
INFO: Package Example is already pinned to the selected commit
julia> Pkg.pin("Example", v"0.4.1") # change commit, resolve invoked
INFO: Package Example: checking out existing branch pinned.526e34fd.tmp
INFO: No packages to install, update or remove
julia> Pkg.pin("Example") # no changes, resolve not invoked
INFO: Package Example is already pinned
julia> Pkg.free("Example") # free always calls resolve (may be changed, but it's not in this PR)
INFO: Freeing Example
INFO: No packages to install, update or remove
|
Tests added. Should be ready to merge after review. Note: I wrote an ugly macro in the test file in order to grab the package manager outputs and check them. Is there a better way to achieve that? |
carlobaldassi
force-pushed
the
cb/fixpkgpin
branch
from
6e52f76
to
91e69c2
Compare
I think it might be doable without writing to a temp file, but I like what it achieves of making this test file less noisy - probably applicable for the rest of the file too. Maybe generically useful enough to put it somewhere more official later. |
|
Yes, I also thought about applying it to the rest of the file. It's quite some work though, and can be left to another PR. Avoiding temp files would certainly be an improvement. |
|
LGTM |
|
Thanks for the reviews! |
Pkg.pinbroken, test ineffective #17176)resolve()is called after apin()More tests should be added. I'm working on it, but in the meanwhilethis could already be merged, if everything seems in order to people who actually know LibGit2 well.cc @wildart