From f7f30ec136907a2aad5e58ba95081ac94fe6aba6 Mon Sep 17 00:00:00 2001 From: Ryan Pham Date: Thu, 28 Nov 2024 15:25:44 +0900 Subject: [PATCH] remote: Handle fetching 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. --- src/libgit2/refspec.c | 13 ++++++- src/libgit2/remote.c | 8 +++-- tests/libgit2/refs/normalize.c | 2 ++ tests/libgit2/remote/fetch.c | 64 ++++++++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 3 deletions(-) diff --git a/src/libgit2/refspec.c b/src/libgit2/refspec.c index 18ece6b70..7ce8ef4ba 100644 --- a/src/libgit2/refspec.c +++ b/src/libgit2/refspec.c @@ -22,6 +22,7 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch) const char *lhs, *rhs; int valid = 0; unsigned int flags; + bool is_neg_refspec = false; GIT_ASSERT_ARG(refspec); GIT_ASSERT_ARG(input); @@ -34,6 +35,9 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch) refspec->force = 1; lhs++; } + if (*lhs == '^') { + is_neg_refspec = true; + } rhs = strrchr(lhs, ':'); @@ -62,7 +66,14 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch) llen = (rhs ? (size_t)(rhs - lhs - 1) : strlen(lhs)); if (1 <= llen && memchr(lhs, '*', llen)) { - if ((rhs && !is_glob) || (!rhs && is_fetch)) + /* + * If the lefthand side contains a glob, then one of the following must be + * true, otherwise the spec is invalid + * 1) the rhs exists and also contains a glob + * 2) it is a negative refspec (i.e. no rhs) + * 3) the rhs doesn't exist and we're fetching + */ + if ((rhs && !is_glob) || (rhs && is_neg_refspec) || (!rhs && is_fetch && !is_neg_refspec)) goto invalid; is_glob = 1; } else if (rhs && is_glob) diff --git a/src/libgit2/remote.c b/src/libgit2/remote.c index 5e59ce894..5d87d4207 100644 --- a/src/libgit2/remote.c +++ b/src/libgit2/remote.c @@ -2628,17 +2628,21 @@ done: 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; } - return NULL; + return match; } git_refspec *git_remote__matching_dst_refspec(git_remote *remote, const char *refname) diff --git a/tests/libgit2/refs/normalize.c b/tests/libgit2/refs/normalize.c index f1850759d..c2f5684ef 100644 --- a/tests/libgit2/refs/normalize.c +++ b/tests/libgit2/refs/normalize.c @@ -402,6 +402,8 @@ void test_refs_normalize__negative_refspec_pattern(void) { ensure_refname_normalized( GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "^foo/bar", "^foo/bar"); + ensure_refname_normalized( + GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "^foo/bar/*", "^foo/bar/*"); ensure_refname_invalid( GIT_REFERENCE_FORMAT_REFSPEC_PATTERN, "foo/^bar"); } diff --git a/tests/libgit2/remote/fetch.c b/tests/libgit2/remote/fetch.c index 7d2d11e27..a24f09925 100644 --- a/tests/libgit2/remote/fetch.c +++ b/tests/libgit2/remote/fetch.c @@ -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, + }; + + 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); +}