Re: [PATCH 1/2] t3200-branch: test setting branch as own upstream

4 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 1/2] t3200-branch: test setting branch as own upstream

From: Matthieu Moy <hidden>
Date: 2016-06-15 23:00:07

Jeff King [off-list ref] writes:

  Don't die to let the caller finish its
  job in such case.

[...]

Matthieu, can you remember anything else that
led to that decision?

Not at all, unfortunately. I don't remember if I did that "in case
there's something like some cleanup to do" or because I had something
more precise in mind.

A case to be carefull about is if you're using the same "git branch"
command for multiple actions (trying --set-upstream in combination with
other options). But I do not see a case where this would be possible.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

[PATCH v2] branch: die when setting branch as own upstream

From: Brian Gesiak <hidden>
Date: 2016-06-15 23:00:08

From: modocache <redacted>

Branch set as own upstream using one of the following commands returns
immediately with an exit code of 0:

- `git branch --set-upstream-to foo refs/heads/foo`
- `git branch --force --track foo foo`

Since neither of these actions currently set the upstream, an exit code
of 0 is misleading. Instead, exit with a status code indicating failure
by using the die function.

Signed-off-by: Brian Gesiak <redacted>
---
 branch.c          | 9 ++-------
 t/t3200-branch.sh | 6 +++---
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/branch.c b/branch.c
index e163f3c..9bac8b5 100644
--- a/branch.c
+++ b/branch.c

@@ -54,13 +54,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
 	struct strbuf key = STRBUF_INIT;
 	int rebasing = should_setup_rebase(origin);
 
-	if (shortname
-	    && !strcmp(local, shortname)
-	    && !origin) {
-		warning(_("Not setting branch %s as its own upstream."),
-			local);
-		return;
-	}
+	if (shortname && !strcmp(local, shortname) && !origin)
+		die(_("Not setting branch %s as its own upstream."), local);
 
 	strbuf_addf(&key, "branch.%s.remote", local);
 	git_config_set(key.buf, origin ? origin : ".");

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 6164126..3ac493f 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh

@@ -507,10 +507,10 @@ EOF
 	test_cmp expected actual
 '
 
-test_expect_success '--set-upstream-to shows warning if used to set branch as own upstream' '
-	git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
+test_expect_success '--set-upstream-to fails if used to set branch as own upstream' '
+	test_must_fail git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
 	cat >expected <<EOF &&
-warning: Not setting branch my13 as its own upstream.
+fatal: Not setting branch my13 as its own upstream.
 EOF
 	test_i18ncmp expected actual
 '

-- 
1.8.3.4 (Apple Git-47)

[PATCH 3/3] branch: die when setting branch as own upstream

From: Brian Gesiak <hidden>
Date: 2016-06-15 23:00:08

Branch set as own upstream using one of the following commands returns
immediately with an exit code of 0:

- `git branch --set-upstream-to foo refs/heads/foo`
- `git branch --force --track foo foo`

Since neither of these actions currently set the upstream, an exit code
of 0 is misleading. Instead, exit with a status code indicating failure
by using the die function.

Signed-off-by: Brian Gesiak <redacted>
---
 branch.c          | 9 ++-------
 t/t3200-branch.sh | 6 +++---
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/branch.c b/branch.c
index e163f3c..9bac8b5 100644
--- a/branch.c
+++ b/branch.c

@@ -54,13 +54,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
 	struct strbuf key = STRBUF_INIT;
 	int rebasing = should_setup_rebase(origin);
 
-	if (shortname
-	    && !strcmp(local, shortname)
-	    && !origin) {
-		warning(_("Not setting branch %s as its own upstream."),
-			local);
-		return;
-	}
+	if (shortname && !strcmp(local, shortname) && !origin)
+		die(_("Not setting branch %s as its own upstream."), local);
 
 	strbuf_addf(&key, "branch.%s.remote", local);
 	git_config_set(key.buf, origin ? origin : ".");

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 6164126..3ac493f 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh

@@ -507,10 +507,10 @@ EOF
 	test_cmp expected actual
 '
 
-test_expect_success '--set-upstream-to shows warning if used to set branch as own upstream' '
-	git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
+test_expect_success '--set-upstream-to fails if used to set branch as own upstream' '
+	test_must_fail git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
 	cat >expected <<EOF &&
-warning: Not setting branch my13 as its own upstream.
+fatal: Not setting branch my13 as its own upstream.
 EOF
 	test_i18ncmp expected actual
 '

-- 
1.8.3.4 (Apple Git-47)

Re: [PATCH 3/3] branch: die when setting branch as own upstream

From: Brian Gesiak <hidden>
Date: 2016-06-15 23:00:08

Sorry for the multiple patches--I noticed the commit author was off in
the first one.

This patch converts the warning to an error, should it be decided that
it's prudent to do so (I'm in favor of doing so). If not, I think the
other two patches I submitted are good to merge.

Thanks for all the feedback so far!

- Brian Gesiak


On Sat, Mar 1, 2014 at 9:23 PM, Brian Gesiak [off-list ref] wrote:

quoted hunk

Branch set as own upstream using one of the following commands returns
immediately with an exit code of 0:

- `git branch --set-upstream-to foo refs/heads/foo`
- `git branch --force --track foo foo`

Since neither of these actions currently set the upstream, an exit code
of 0 is misleading. Instead, exit with a status code indicating failure
by using the die function.

Signed-off-by: Brian Gesiak <redacted>
---
 branch.c          | 9 ++-------
 t/t3200-branch.sh | 6 +++---
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/branch.c b/branch.c
index e163f3c..9bac8b5 100644
--- a/branch.c
+++ b/branch.c

@@ -54,13 +54,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
        struct strbuf key = STRBUF_INIT;
        int rebasing = should_setup_rebase(origin);

-       if (shortname
-           && !strcmp(local, shortname)
-           && !origin) {
-               warning(_("Not setting branch %s as its own upstream."),
-                       local);
-               return;
-       }
+       if (shortname && !strcmp(local, shortname) && !origin)
+               die(_("Not setting branch %s as its own upstream."), local);

        strbuf_addf(&key, "branch.%s.remote", local);
        git_config_set(key.buf, origin ? origin : ".");

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 6164126..3ac493f 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh

@@ -507,10 +507,10 @@ EOF
        test_cmp expected actual
 '

-test_expect_success '--set-upstream-to shows warning if used to set branch as own upstream' '
-       git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
+test_expect_success '--set-upstream-to fails if used to set branch as own upstream' '
+       test_must_fail git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
        cat >expected <<EOF &&
-warning: Not setting branch my13 as its own upstream.
+fatal: Not setting branch my13 as its own upstream.
 EOF
        test_i18ncmp expected actual
 '
--

1.8.3.4 (Apple Git-47)

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help