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

3 messages, 3 authors, 2021-08-27 · 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-26 17:38:36

Junio C Hamano [off-list ref] writes:
Han-Wen Nienhuys [off-list ref] writes:
quoted
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).

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?
I thought the approach taken by dc474899 (t4202: mark bogus head
hash test with REFFILES, 2021-05-31) to hide it behind a
prerequisite was good enough, but if we can ensure the same
behaviour under the reftable backend, that is even better.

Thanks.
Having said that, there are a few observations to make about this
test script.

 * It is hopefully becoming harder and harder to check for behaviour
   in broken repositories in a "portable" way, simply because we are
   making it harder to corrupt repository.  We hopefully won't point
   a ref to point at a missing object, we hopefully won't prune an
   object away that is still pointed at by a ref, etc.

 * This script to test "branch" is full of tests that rely on direct
   manipulation of .git/refs/ filesystem hierarchy.

For these two reasons, it probably is OK to accept this patch as-is
and leave the "clean-up" to a later follow-on series, that would
cover both "what's our recommended approach to 'corrupt' the test
repository so that we can use different ref (and other) backends?"
and "make sure the tests in the script are happy with both ref
backends." issues.

Thanks.

Antw: [EXT] Re: [PATCH RESEND] branch: allow deleting dangling branches with ‑‑force

From: Ulrich Windl <hidden>
Date: 2021-08-27 07:30:04

quoted
quoted
Junio C Hamano [off-list ref] schrieb am 26.08.2021 um 19:38 in Nachricht
<xmqq1r6gf6ne.fsf@gitster.g>:

...
 * It is hopefully becoming harder and harder to check for behaviour
   in broken repositories in a "portable" way, simply because we are
   making it harder to corrupt repository.  We hopefully won't point
   a ref to point at a missing object, we hopefully won't prune an
   object away that is still pointed at by a ref, etc.
...

Maybe git needs a "--disarm-safety-belt" option that disables all those nice checks for testing purposes ;-)

Re: Antw: [EXT] Re: [PATCH RESEND] branch: allow deleting dangling branches with ‑‑force

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-08-27 07:55:35

On Fri, Aug 27 2021, Ulrich Windl wrote:
quoted
quoted
quoted
Junio C Hamano [off-list ref] schrieb am 26.08.2021 um 19:38 in Nachricht
<xmqq1r6gf6ne.fsf@gitster.g>:

...
quoted
 * It is hopefully becoming harder and harder to check for behaviour
   in broken repositories in a "portable" way, simply because we are
   making it harder to corrupt repository.  We hopefully won't point
   a ref to point at a missing object, we hopefully won't prune an
   object away that is still pointed at by a ref, etc.
...

Maybe git needs a "--disarm-safety-belt" option that disables all those nice checks for testing purposes ;-)
I haven't tested, but I think in both of those cases a way to accomplish
this corruption in a way that bypasses the safety of our tooling is also
to setup an alternate object directory with the relevant object(s), and
then simply drop that alternate to simulate the case of an object
disappearing or other such corruption.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help