Patrick Steinhardt <ps@xxxxxx> writes: > On Mon, Oct 07, 2024 at 02:02:02PM -0700, Junio C Hamano wrote: >> Patrick Steinhardt <ps@xxxxxx> writes: >> >> > `$GIT_DIR` _is_ defined during hook >> > execution. So in theory, if git-rev-parse(1) behaved exactly as >> > documented, it shouldn't even care whether or not it is executing in a >> > repository. >> >> I've always considered "git rev-parse --git-dir" and friends were to >> verify the validity of the repository before returning "Your GIT_DIR >> is here". Otherwise there is no easy way to ask "I have this directory. >> Is it a valid repository you can work with?". >> >> So, I am not sure I agree with the above. > > Well, I'm not sure either. I was merely pointing out that the documented > behaviour is different than the actual behaviour. Which of both is the > more sensible one is a different question. > >> For what is worth, I am skeptical to the "solution" that tentively >> creates a bogus HEAD file while the repository is being initialized. >> The code today may ignore certain bogosity in such a HEAD (like the >> ".invalid" magic used during "git clone"), but there is no guarantee >> that a random third-party add-on a hook script may invoke do the >> same as we do, and more importantly, what a repository with its >> initialization complete look like may change over time and it may >> not be enough to have HEAD pointing at "refs/heads/.invalid" to fool >> our bootstrap process. If we were to go that route, I would rather >> see us pick a pointee that is *not* bogus at the mechanical level >> (i.e., "git symbolic-ref HEAD refs/heads/.invalid" would fail) but >> is clearly a placeholder value to humans, soon to be updated. >> >> Let's say if we were to create a repository with the name of initial >> branch as 'main', we could create HEAD that points at refs/heads/main >> bypassing any hook intervention, then call the hook to see if it >> approves the name. We'd need to make sure that we fail the >> repository creation when the hook declines, as it is refusing to set >> a HEAD, one critical element in the repository that has to exist, >> and probably remove the directory if we are not reinitializing. >> >> Or we could use a name that is clearly bogus to humans but is still >> structurally OK, say "refs/heads/hook-failed-during-initialization", >> ask the hook if it is OK to repoint HEAD to "refs/heads/main" from >> that state, and (1) if it approves, HEAD will point at "refs/heads/main" >> and "hook-failed-during-initialization" will be seen nowhere but the >> reflog of HEAD, or (2) if it refuses, we stop, and the user will be >> left on an unborn branch with a long descriptive name that explains >> the situation. > > I dunno. It all feels rather complex. > >> A much simpler alternative would be to simply ignore any hooks, >> traces, or anything that want to look into the directory we are >> working to turn into a repository but haven't completed doing so, >> during repository initialization, I would think, though. > > That could work, yes, but it would limit the usefulness of the hook. In > theory, you can create a full log of all changes in the repository from > its inception. If we didn't log the first item, that log would be > incomplete. > > We have another solution that is even simpler: just do nothing. I do not > think that the behaviour we exhibit is wrong. Unwieldy? Maybe. But it is > merely stating facts: we are executing a transaction in a repository > that is not yet fully set up. If you don't want that, either don't set > up a global reference-transaction hook, or alternatively handle that > edge case in your script. This does seem like we're putting the onus on the users, but ultimately I agree with this. What the hook is doing is as expected. Thanks Patrick for responding while I was away. Karthik
Attachment:
signature.asc
Description: PGP signature
