From 7cd4ba1b17d579134ac6cc172fc858021d4aa407 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 22 May 2015 12:11:42 +0200 Subject: [PATCH 1/2] refspec: make sure matching refspecs have src, dst and input strings When we find out that we're dealing with a matching refspec, we set the flag and return immediately. This leaves the strings as NULL, which breaks the contract. Assign these pointers to a string with the correct values. --- src/refspec.c | 6 ++++++ tests/network/refspecs.c | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/refspec.c b/src/refspec.c index ad8141248..961f939c6 100644 --- a/src/refspec.c +++ b/src/refspec.c @@ -42,6 +42,12 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch) */ if (!is_fetch && rhs == lhs && rhs[1] == '\0') { refspec->matching = 1; + refspec->string = git__strdup(input); + GITERR_CHECK_ALLOC(refspec->string); + refspec->src = git__strdup(""); + GITERR_CHECK_ALLOC(refspec->src); + refspec->dst = git__strdup(""); + GITERR_CHECK_ALLOC(refspec->dst); return 0; } diff --git a/tests/network/refspecs.c b/tests/network/refspecs.c index c6bcb10e8..614d91d07 100644 --- a/tests/network/refspecs.c +++ b/tests/network/refspecs.c @@ -146,3 +146,13 @@ void test_network_refspecs__invalid_reverse(void) assert_invalid_rtransform("refs/heads/*:refs/remotes/origin/*", "master"); assert_invalid_rtransform("refs/heads/*:refs/remotes/origin/*", "refs/remotes/o/master"); } + +void test_network_refspecs__matching(void) +{ + git_refspec spec; + + cl_git_pass(git_refspec__parse(&spec, ":", false)); + cl_assert_equal_s(":", spec.string); + cl_assert_equal_s("", spec.src); + cl_assert_equal_s("", spec.dst); +} From 5014fe95f5691ec9c48e1bb33fa03d925a4159f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 22 May 2015 12:24:09 +0200 Subject: [PATCH 2/2] branch: error out if we cannot find the remote When we look for which remote corresponds to a remote-tracking branch, we look in the refspecs to see which ones matches. If none do, we should abort. We currently ignore the error message from this operation, so let's not do that anymore. As part of the test we're writing, let's test for the expected behaviour if we cannot find a refspec which tells us what the remote-tracking branch for a remote would look like. --- src/branch.c | 9 ++++++--- tests/refs/branches/upstream.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/branch.c b/src/branch.c index 10be6f70c..791d55106 100644 --- a/src/branch.c +++ b/src/branch.c @@ -551,7 +551,7 @@ int git_branch_set_upstream(git_reference *branch, const char *upstream_name) git_remote *remote = NULL; git_config *config; const char *name, *shortname; - int local; + int local, error; const git_refspec *fetchspec; name = git_reference_name(branch); @@ -586,9 +586,12 @@ int git_branch_set_upstream(git_reference *branch, const char *upstream_name) * that. */ if (local) - git_buf_puts(&value, "."); + error = git_buf_puts(&value, "."); else - git_branch_remote_name(&value, repo, git_reference_name(upstream)); + error = git_branch_remote_name(&value, repo, git_reference_name(upstream)); + + if (error < 0) + goto on_error; if (git_buf_printf(&key, "branch.%s.remote", shortname) < 0) goto on_error; diff --git a/tests/refs/branches/upstream.c b/tests/refs/branches/upstream.c index 351449416..8f2e7a2ca 100644 --- a/tests/refs/branches/upstream.c +++ b/tests/refs/branches/upstream.c @@ -159,3 +159,35 @@ void test_refs_branches_upstream__set_unset_upstream(void) cl_git_sandbox_cleanup(); } + +void test_refs_branches_upstream__no_fetch_refspec(void) +{ + git_reference *ref, *branch; + git_repository *repo; + git_remote *remote; + git_config *cfg; + + repo = cl_git_sandbox_init("testrepo.git"); + + cl_git_pass(git_remote_create_with_fetchspec(&remote, repo, "matching", ".", NULL)); + cl_git_pass(git_remote_add_push(repo, "matching", ":")); + + cl_git_pass(git_reference_lookup(&branch, repo, "refs/heads/test")); + cl_git_pass(git_reference_create(&ref, repo, "refs/remotes/matching/master", git_reference_target(branch), 1, "fetch")); + cl_git_fail(git_branch_set_upstream(branch, "matching/master")); + cl_assert_equal_s("Could not determine remote for 'refs/remotes/matching/master'", + giterr_last()->message); + + /* we can't set it automatically, so let's test the user setting it by hand */ + cl_git_pass(git_repository_config(&cfg, repo)); + cl_git_pass(git_config_set_string(cfg, "branch.test.remote", "matching")); + cl_git_pass(git_config_set_string(cfg, "branch.test.merge", "refs/heads/master")); + /* we still can't find it because there is no rule for that reference */ + cl_git_fail_with(GIT_ENOTFOUND, git_branch_upstream(&ref, branch)); + + git_reference_free(ref); + git_reference_free(branch); + git_remote_free(remote); + + cl_git_sandbox_cleanup(); +}