Re: [PATCH RESEND] branch: allow deleting dangling branches with --force

5 messages, 4 authors, 2021-08-26 · open the first message on its own page

Re: [PATCH RESEND] branch: allow deleting dangling branches with --force

From: Junio C Hamano <hidden>
Date: 2021-08-25 21:37:44

René Scharfe [off-list ref] writes:
git branch only allows deleting branches that point to valid commits.
Skip that check if --force is given, as the caller is indicating with
it that they know what they are doing and accept the consequences.
This allows deleting dangling branches, which previously had to be
reset to a valid start-point using --force first.

Signed-off-by: René Scharfe <redacted>
---
Original submission:
http://public-inbox.org/git/52847a99-db7c-9634-b3b1-fd9b1342bc32@web.de/
Thanks.
+test_expect_success 'branch --delete --force removes dangling branch' '
+	test_when_finished "rm -f .git/refs/heads/dangling" &&
+	echo $ZERO_OID >.git/refs/heads/dangling &&
+	git branch --delete --force dangling &&
+	test_path_is_missing .git/refs/heads/dangling
+'
This goes against the spirit of the series merged at c9780bb2 (Merge
branch 'hn/prep-tests-for-reftable', 2021-07-13).

Can we creat the dangling ref and test the lack of "dangling" ref in
the end in a less transparent way?

An escape hatch is to make this test depend on the REFFILES
prerequisite, just like dc474899 (t4202: mark bogus head hash test
with REFFILES, 2021-05-31) did, which may be more appropriate.
 test_expect_success 'use --edit-description' '
 	write_script editor <<-\EOF &&
 		echo "New contents" >"$1"
--
2.32.0

Re: [PATCH RESEND] branch: allow deleting dangling branches with --force

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-08-25 23:30:04

On Wed, Aug 25 2021, Junio C Hamano wrote:
René Scharfe [off-list ref] writes:
quoted
git branch only allows deleting branches that point to valid commits.
Skip that check if --force is given, as the caller is indicating with
it that they know what they are doing and accept the consequences.
This allows deleting dangling branches, which previously had to be
reset to a valid start-point using --force first.

Signed-off-by: René Scharfe <redacted>
---
Original submission:
http://public-inbox.org/git/52847a99-db7c-9634-b3b1-fd9b1342bc32@web.de/
Thanks.
quoted
+test_expect_success 'branch --delete --force removes dangling branch' '
+	test_when_finished "rm -f .git/refs/heads/dangling" &&
+	echo $ZERO_OID >.git/refs/heads/dangling &&
+	git branch --delete --force dangling &&
+	test_path_is_missing .git/refs/heads/dangling
+'
This goes against the spirit of the series merged at c9780bb2 (Merge
branch 'hn/prep-tests-for-reftable', 2021-07-13).

Can we creat the dangling ref and test the lack of "dangling" ref in
the end in a less transparent way?

An escape hatch is to make this test depend on the REFFILES
prerequisite, just like dc474899 (t4202: mark bogus head hash test
with REFFILES, 2021-05-31) did, which may be more appropriate.
I'm not sure, but this may also be a good example of the sort of thing
that we should probably go beyond REFFILES with, i.e. is it even
possible under reftable to run into this sort of situation?

Not really a topic for this series, but something to make a mental note
of for the reftable topic, i.e. we may eventually want to edit the docs
etc. appropriately if and when the new backend is more mature.

Re: [PATCH RESEND] branch: allow deleting dangling branches with --force

From: Han-Wen Nienhuys <hidden>
Date: 2021-08-26 07:26:16

On Wed, Aug 25, 2021 at 11:37 PM Junio C Hamano [off-list ref] wrote:
quoted
+test_expect_success 'branch --delete --force removes dangling branch' '
+     test_when_finished "rm -f .git/refs/heads/dangling" &&
+     echo $ZERO_OID >.git/refs/heads/dangling &&
+     git branch --delete --force dangling &&
+     test_path_is_missing .git/refs/heads/dangling
+'
This goes against the spirit of the series merged at c9780bb2 (Merge
branch 'hn/prep-tests-for-reftable', 2021-07-13).

Can we creat the dangling ref and test the lack of "dangling" ref in
the end in a less transparent way?
agreed. Try the ref-store test-helper's update-ref command?


-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

Re: [PATCH RESEND] branch: allow deleting dangling branches with --force

From: René Scharfe <hidden>
Date: 2021-08-26 18:19:04

Am 26.08.21 um 09:26 schrieb Han-Wen Nienhuys:
On Wed, Aug 25, 2021 at 11:37 PM Junio C Hamano [off-list ref] wrote:
quoted
quoted
+test_expect_success 'branch --delete --force removes dangling branch' '
+     test_when_finished "rm -f .git/refs/heads/dangling" &&
+     echo $ZERO_OID >.git/refs/heads/dangling &&
+     git branch --delete --force dangling &&
+     test_path_is_missing .git/refs/heads/dangling
+'
This goes against the spirit of the series merged at c9780bb2 (Merge
branch 'hn/prep-tests-for-reftable', 2021-07-13).
I assume the file backend won't go away anytime soon.  So I guess the
idea is that the test suite is supposed to be run with the new backend
as default and exercise it?
quoted
Can we creat the dangling ref and test the lack of "dangling" ref in
the end in a less transparent way?
agreed. Try the ref-store test-helper's update-ref command?
It requires the new hash to refer to an existing object, so we can't
use it in this test.

René

Re: [PATCH RESEND] branch: allow deleting dangling branches with --force

From: René Scharfe <hidden>
Date: 2021-08-26 18:19:09

Am 26.08.21 um 01:28 schrieb Ævar Arnfjörð Bjarmason:
On Wed, Aug 25 2021, Junio C Hamano wrote:
quoted
René Scharfe [off-list ref] writes:
quoted
git branch only allows deleting branches that point to valid commits.
Skip that check if --force is given, as the caller is indicating with
it that they know what they are doing and accept the consequences.
This allows deleting dangling branches, which previously had to be
reset to a valid start-point using --force first.

Signed-off-by: René Scharfe <redacted>
---
Original submission:
http://public-inbox.org/git/52847a99-db7c-9634-b3b1-fd9b1342bc32@web.de/
Thanks.
quoted
+test_expect_success 'branch --delete --force removes dangling branch' '
+	test_when_finished "rm -f .git/refs/heads/dangling" &&
+	echo $ZERO_OID >.git/refs/heads/dangling &&
+	git branch --delete --force dangling &&
+	test_path_is_missing .git/refs/heads/dangling
+'
This goes against the spirit of the series merged at c9780bb2 (Merge
branch 'hn/prep-tests-for-reftable', 2021-07-13).

Can we creat the dangling ref and test the lack of "dangling" ref in
the end in a less transparent way?

An escape hatch is to make this test depend on the REFFILES
prerequisite, just like dc474899 (t4202: mark bogus head hash test
with REFFILES, 2021-05-31) did, which may be more appropriate.
I'm not sure, but this may also be a good example of the sort of thing
that we should probably go beyond REFFILES with, i.e. is it even
possible under reftable to run into this sort of situation?
Probably yes: A commit can disappear when its object file or pack or
alternate object database gets lost somehow, and a ref store could
only compensate for that loss if it kept a copy of the ref target,
which seems impractical.

René
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help