Fixes broken LibGit2.rebase! #19624
Fixes broken LibGit2.rebase! #19624
Conversation
LibGit2 was not rebasing correctly, causing problems with `Pkg.update()` after `PkgDev.tag()`, see https://discourse.julialang.org/t/how-to-properly-add-releases-with-pkgdev/961/15?u=simonbyrne There were two problems: - `upstream` was used as a variable name and a function call - if the patch was redundant (`EAPPLIED` return code), `rebase!` would throw an error.
tkelman
added
backport pending 0.5
bugfix
| (Ptr{Oid}, Ptr{Void}, Ptr{SignatureStruct}, Ptr{SignatureStruct}, Ptr{UInt8}, Ptr{UInt8}), | ||
| oid_ptr, rb.ptr, C_NULL, sig.ptr, C_NULL, C_NULL) | ||
| oid_ptr, rb.ptr, C_NULL, sig.ptr, C_NULL, C_NULL) |
There was a problem hiding this comment.
alignment probably shouldn't change here?
|
yay, Simon to the rescue. can we construct this situation locally and test for it in test/libgit2.jl or test/pkg.jl ? |
|
Yes, we probably should do it, and I imagine it should be possible, but I'm not sure how best to do it. Any suggestions? |
|
create 2 local test repos with the same content but via different series of commits so that merging from an on-disk remote that points to the other repo wouldn't be a fast forward but would trigger this bug? |
|
I'm still not sure why this went undiscovered for so long: it should have thrown a more informative error. |
|
I'm not surprised, the libgit2 code is full of this kind of thing, it could use a pretty thorough review, refactoring, and docs. |
|
I've added a test: it fails earlier and now passes. |
| (Ptr{Oid}, Ptr{Void}, Ptr{SignatureStruct}, Ptr{SignatureStruct}, Ptr{UInt8}, Ptr{UInt8}), | ||
| oid_ptr, rb.ptr, C_NULL, sig.ptr, C_NULL, C_NULL) | ||
| catch err | ||
| err.code == Error.EAPPLIED && return nothing |
There was a problem hiding this comment.
is returning nothing acceptable for callers? would returning the head be any more appropriate?
There was a problem hiding this comment.
probably, but I can't figure out how to do that.
There was a problem hiding this comment.
hm okay. leave a todo comment maybe?
|
cool. that test passes without internet access, right? looks like it should, but if not then it should be moved to libgit2-online. |
|
Yes, it just tests rebase locally (as it is a local operation). |
* Fixes broken LibGit2.rebase! LibGit2 was not rebasing correctly, causing problems with `Pkg.update()` after `PkgDev.tag()`, see https://discourse.julialang.org/t/how-to-properly-add-releases-with-pkgdev/961/15?u=simonbyrne There were two problems: - `upstream` was used as a variable name and a function call - if the patch was redundant (`EAPPLIED` return code), `rebase!` would throw an error. * alignment * revert check macro to avoid incompatibilities * add docstring * add testcase * add TODO (cherry picked from commit 474210f)
LibGit2.rebase!is broken, causing problems withPkg.update()after
PkgDev.tag(), seehttps://discourse.julialang.org/t/how-to-properly-add-releases-with-pkgdev/961/15?u=simonbyrne
There were two problems:
upstreamwas used as a variable name and a function callEAPPLIEDreturn code),rebase!wouldthrow an error.