-
Notifications
You must be signed in to change notification settings - Fork 2.5k
remote: Handle fetching negative refspecs #6962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
3d9f406
f7f30ec
83994da
6039d28
099e62c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
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.
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2628,17 +2628,21 @@ int git_remote_name_is_valid(int *valid, const char *remote_name) | |
git_refspec *git_remote__matching_refspec(git_remote *remote, const char *refname) | ||
{ | ||
git_refspec *spec; | ||
git_refspec *match = NULL; | ||
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)) | ||
return NULL; | ||
|
||
if (git_refspec_src_matches(spec, refname)) | ||
return spec; | ||
match = spec; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, right. Fixed in 6039d28 |
||
} | ||
|
||
return NULL; | ||
return match; | ||
} | ||
|
||
git_refspec *git_remote__matching_dst_refspec(git_remote *remote, const char *refname) | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -10,8 +10,11 @@ static char* repo2_path; | |||||||||||
|
||||||||||||
static const char *REPO1_REFNAME = "refs/heads/main"; | ||||||||||||
static const char *REPO2_REFNAME = "refs/remotes/repo1/main"; | ||||||||||||
static const char *REPO1_UNDERSCORE_REFNAME = "refs/heads/_branch"; | ||||||||||||
static const char *REPO2_UNDERSCORE_REFNAME = "refs/remotes/repo1/_branch"; | ||||||||||||
static char *FORCE_FETCHSPEC = "+refs/heads/main:refs/remotes/repo1/main"; | ||||||||||||
static char *NON_FORCE_FETCHSPEC = "refs/heads/main:refs/remotes/repo1/main"; | ||||||||||||
static char *NEGATIVE_SPEC = "^refs/heads/_*"; | ||||||||||||
|
||||||||||||
void test_remote_fetch__initialize(void) { | ||||||||||||
git_config *c; | ||||||||||||
|
@@ -170,3 +173,64 @@ void test_remote_fetch__do_update_refs_if_not_descendant_and_force(void) { | |||||||||||
|
||||||||||||
git_reference_free(ref); | ||||||||||||
} | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* This checks that negative refspecs are respected when fetching. We create a | ||||||||||||
* repository with a '_' prefixed reference. A second repository is configured | ||||||||||||
* with a negative refspec to ignore any refs prefixed with '_' and fetch the | ||||||||||||
* first repository into the second. | ||||||||||||
* | ||||||||||||
* @param commit1id A pointer to an OID which will be populated with the first | ||||||||||||
* commit. | ||||||||||||
*/ | ||||||||||||
static void do_fetch_repo_with_ref_matching_negative_refspec(git_oid *commit1id) { | ||||||||||||
/* create a commit in repo 1 and a reference to it with '_' prefix */ | ||||||||||||
{ | ||||||||||||
git_oid empty_tree_id; | ||||||||||||
git_tree *empty_tree; | ||||||||||||
git_signature *sig; | ||||||||||||
git_treebuilder *tb; | ||||||||||||
cl_git_pass(git_treebuilder_new(&tb, repo1, NULL)); | ||||||||||||
cl_git_pass(git_treebuilder_write(&empty_tree_id, tb)); | ||||||||||||
cl_git_pass(git_tree_lookup(&empty_tree, repo1, &empty_tree_id)); | ||||||||||||
cl_git_pass(git_signature_default(&sig, repo1)); | ||||||||||||
cl_git_pass(git_commit_create(commit1id, repo1, REPO1_UNDERSCORE_REFNAME, sig, | ||||||||||||
sig, NULL, "one", empty_tree, 0, NULL)); | ||||||||||||
|
||||||||||||
git_tree_free(empty_tree); | ||||||||||||
git_signature_free(sig); | ||||||||||||
git_treebuilder_free(tb); | ||||||||||||
} | ||||||||||||
|
||||||||||||
/* fetch the remote with negative refspec for references prefixed with '_' */ | ||||||||||||
{ | ||||||||||||
char *refspec_strs = { NEGATIVE_SPEC }; | ||||||||||||
git_strarray refspecs = { | ||||||||||||
.count = 1, | ||||||||||||
.strings = &refspec_strs, | ||||||||||||
}; | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't use designated initializers; we try to keep support for very ancient compilers so that people can do clever things.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 099e62c |
||||||||||||
|
||||||||||||
git_remote *remote; | ||||||||||||
|
||||||||||||
cl_git_pass(git_remote_create_anonymous(&remote, repo2, | ||||||||||||
git_repository_path(repo1))); | ||||||||||||
cl_git_pass(git_remote_fetch(remote, &refspecs, NULL, "some message")); | ||||||||||||
|
||||||||||||
git_remote_free(remote); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
void test_remote_fetch__skip_negative_refspec_match(void) { | ||||||||||||
git_oid commit1id; | ||||||||||||
git_reference *ref1; | ||||||||||||
git_reference *ref2; | ||||||||||||
|
||||||||||||
do_fetch_repo_with_ref_matching_negative_refspec(&commit1id); | ||||||||||||
|
||||||||||||
/* assert that the reference in exists in repo1 but not in repo2 */ | ||||||||||||
cl_git_pass(git_reference_lookup(&ref1, repo1, REPO1_UNDERSCORE_REFNAME)); | ||||||||||||
cl_assert_equal_b(git_reference_lookup(&ref2, repo2, REPO2_UNDERSCORE_REFNAME), GIT_ENOTFOUND); | ||||||||||||
|
||||||||||||
git_reference_free(ref1); | ||||||||||||
git_reference_free(ref2); | ||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the
is_negative
check is superfluous, it looks likesrc_matches_negative
does the check itself?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in 83994da