remote: Handle fetching negative refspecs #6962
Conversation
|
I'm not super familiar with the entire Git spec, so I'm not 100% sure, but I believe this should cover the rest of the functionality for negative refspecs. If there are other places that need to be updated, please let me know and I can open up a separate PR to handle those places. |
1ce1baa to
e99d721
Compare
|
edit: Resolved |
e99d721 to
3764c0b
Compare
Negative refspecs were added in Git v2.29.0 and are denoted by prefixing a refspec with a caret. This adds a way to distinguish if a refspec is negative and match negative refspecs.
Add support for fetching with negative refspecs. Fetching from the remote with a negative refspec will now validate any references fetched do not match _any_ negative refspecs and at least one non-negative refspec. As we must ensure that fetched references do not match any of the negative refspecs provided, we cannot short-circuit when checking for matching refspecs.
3764c0b to
f7f30ec
Compare
src/libgit2/remote.c
Outdated
| size_t i; | ||
|
|
||
| git_vector_foreach(&remote->active_refspecs, i, spec) { | ||
| if (spec->push) | ||
| continue; | ||
|
|
||
| if (git_refspec_is_negative(spec) && git_refspec_src_matches_negative(spec, refname)) |
There was a problem hiding this comment.
| if (git_refspec_is_negative(spec) && git_refspec_src_matches_negative(spec, refname)) | |
| if (git_refspec_src_matches_negative(spec, refname)) |
I think the is_negative check is superfluous, it looks like src_matches_negative does the check itself?
There was a problem hiding this comment.
Removed in 83994da
| if (git_refspec_src_matches(spec, refname)) | ||
| return spec; | ||
| match = spec; |
There was a problem hiding this comment.
Is this a subtle behavior change? Can there be multiple matches here? Previously, we exited the function as soon as we found a match, returning the first match; now, we would clobber the match, this returning the last match. 🤔
There was a problem hiding this comment.
Ah, right. Fixed in 6039d28
tests/libgit2/remote/fetch.c
Outdated
| git_strarray refspecs = { | ||
| .count = 1, | ||
| .strings = &refspec_strs, | ||
| }; |
There was a problem hiding this comment.
We don't use designated initializers; we try to keep support for very ancient compilers so that people can do clever things.
| git_strarray refspecs = { | |
| .count = 1, | |
| .strings = &refspec_strs, | |
| }; | |
| git_strarray refspecs = { &refspec_strs, 1 }; |
There was a problem hiding this comment.
Addressed in 099e62c
`git_refspec_is_negative()` is already called by `git_refspec_src_matches_negative()`, so should not be necessary here.
The previous behavior was to return the first match, so this reverts the clobbering behavior that was introduced in f7f30ec.
|
Thanks for this PR; another really nice change. And for thoughtfully responding to my nitpicking review requests. I'm very thankful for this change. 🙏 |
Add support for fetching with negative refspecs. Fetching from the
remote with a negative refspec will now validate any references fetched
do not match any negative refspecs and at least one non-negative
refspec. As we must ensure that fetched references do not match any of
the negative refspecs provided, we cannot short-circuit when checking
for matching refspecs.
#6741