On Mon, Nov 09, 2020 at 11:48:20AM -0800, Junio C Hamano wrote: > This is a tangent, but what makes this test doubly interesting is > that "git update-ref -d HEAD" would have allowed us to make it a > non-repository if HEAD were detached, and it seems that we do not > require "--force" to do so. We probably should forbid removing HEAD > that id detached without "--force", which is such a destructive > operation. Yeah, I'd agree that is a good direction (but it definitely is a tangent that should come in a separate series). > > cp -f .git/HEAD .git/HEAD.orig > > test_expect_success 'delete symref without dereference' ' > > test_when_finished "cp -f .git/HEAD.orig .git/HEAD" && > > git update-ref --no-deref -d HEAD && > > - test_path_is_missing .git/HEAD > > + test_must_fail git show-ref --verify -q HEAD > > ' > > This is an example of breaking the repository. I am not sure if the > test_must_fail is a good replacement--it would fail even if you say > "git show-ref --verify -q refs/heads/$branch" where $branch is a > name of a branch that exists, no? Perhaps we could more directly check that the repository is broken. Coupled with the ceiling-limit from earlier in the script, then: git rev-parse --git-dir should fail. Maybe that reveals the intent of what we're expecting a little more clearly (and more so than your "show-ref before and after restoring HEAD" example does, in my opinion). I do have to wonder if this test even cares about HEAD. Could it equally well work on another symref, like refs/remotes/origin/HEAD? -Peff
