From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Clemens Fruhwirth <clemens@endorphin.org>
Subject: cf/fetch-set-upstream-while-detached (was: What's cooking in git.git (Dec 2021, #01; Fri, 3))
Date: Tue, 07 Dec 2021 20:37:47 +0100 [thread overview]
Message-ID: <211207.86pmq8mbtu.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <xmqqh7bpqhf0.fsf@gitster.g>
On Fri, Dec 03 2021, Junio C Hamano wrote:
> * cf/fetch-set-upstream-while-detached (2021-07-06) 1 commit
> - fetch: fix segfault on --set-upstream while on a detached HEAD
>
> "git fetch --set-upstream" while on detached HEAD segfaulted
> instead of noticing that such an operation did not make sense.
>
> Expecting a reroll.
> A low hanging fruit to make this usable is an addition of a few tests;
> any takers?
> cf. <CAPig+cTRXTGe-MNTy=2gk1eX8G+0fa303nrLnEtX1uHUC2usmg@mail.gmail.com>
> cf. <xmqqsg0ri5mq.fsf@gitster.g>
> source: <20210706162238.575988-1-clemens@endorphin.org>
Yes, you can pick my [1] which solves the same issue, but has tests
(which I came up with independently, didn't notice the earlier fix due
to a broken In-Reply-To chain with that fix).
To refresh your memory (I had to refresh mine) you had the comment that
we shouldn't fix the segfault without also making that exit with
non-zero instead of warning().
I don't disagree, but if you read t/t5553-set-upstream.sh you'll see
that's how we do it now.
So i'd think that any proposed behavior change there should come
independent of a narrow segfault fix, let's fix that, and then change
how "fetch --set-upstream-to" works in general.
In any case, per your comment above there's no mention of the
s/warning/die/g, just that it needs tests. If that's all that's needed
you can just pick up [1].
You can even pick that [1] and combine it with
cf/fetch-set-upstream-while-detached and it'll pass the tests the former
adds if they're fixed up[2] to grep for the difference in the warning
text.
I think the warning I added is a bit better, but I really don't care. As
long as we finally fix this segfault we can tweak that wording, or
whether we use warning() or die() etc. as a folllow-up.
1. https://lore.kernel.org/git/patch-v4-1.1-0caa9a89a86-20210831T135212Z-avarab@gmail.com/
2.
diff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh
index 48050162c27..631bd5a9366 100755
--- a/t/t5553-set-upstream.sh
+++ b/t/t5553-set-upstream.sh
@@ -95,7 +95,7 @@ test_expect_success 'fetch --set-upstream with a detached HEAD' '
git checkout HEAD^0 &&
test_when_finished "git checkout -" &&
cat >expect <<-\EOF &&
- warning: could not set upstream of HEAD to '"'"'main'"'"' from '"'"'upstream'"'"' when it does not point to any branch.
+ warning: not on a branch to use --set-upstream with
EOF
git fetch --set-upstream upstream main 2>actual.raw &&
grep ^warning: actual.raw >actual &&
@@ -193,7 +193,7 @@ test_expect_success 'pull --set-upstream with a detached HEAD' '
git checkout HEAD^0 &&
test_when_finished "git checkout -" &&
cat >expect <<-\EOF &&
- warning: could not set upstream of HEAD to '"'"'main'"'"' from '"'"'upstream'"'"' when it does not point to any branch.
+ warning: not on a branch to use --set-upstream with
EOF
git pull --no-rebase --set-upstream upstream main 2>actual.raw &&
grep ^warning: actual.raw >actual &&
next prev parent reply other threads:[~2021-12-07 19:53 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-04 1:37 What's cooking in git.git (Dec 2021, #01; Fri, 3) Junio C Hamano
2021-12-06 19:13 ` Phillip Wood
2021-12-06 19:24 ` Junio C Hamano
2021-12-08 12:42 ` Johannes Schindelin
2021-12-09 1:02 ` Junio C Hamano
2021-12-09 10:39 ` Phillip Wood
2021-12-07 6:00 ` en/keep-cwd (Was: Re: What's cooking in git.git (Dec 2021, #01; Fri, 3)) Elijah Newren
2021-12-07 9:17 ` Ævar Arnfjörð Bjarmason
2021-12-07 20:33 ` Elijah Newren
2021-12-07 18:26 ` en/keep-cwd Junio C Hamano
2021-12-08 12:46 ` en/keep-cwd (Was: Re: What's cooking in git.git (Dec 2021, #01; Fri, 3)) Johannes Schindelin
2021-12-07 15:54 ` What's cooking in git.git (Dec 2021, #01; Fri, 3) Derrick Stolee
2021-12-07 19:37 ` Ævar Arnfjörð Bjarmason [this message]
2021-12-07 20:41 ` cf/fetch-set-upstream-while-detached Junio C Hamano
2021-12-07 22:04 ` [PATCH v5] pull, fetch: fix segfault in --set-upstream option Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=211207.86pmq8mbtu.gmgdl@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=clemens@endorphin.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).