On Tue, 2015-07-28 at 12:00 -0700, Junio C Hamano wrote: > David Turner <dturner@xxxxxxxxxxxxxxxx> writes: > > > All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are > > per-worktree. We don't want multiple notes merges happening at once > > (in different worktrees), so we want to make these refs true refs. > > > > So, we lowercase NOTES_MERGE_REF and friends. That way, backends > > that distinguish between pseudorefs and real refs can correctly > > handle notes merges. > > > > This will also enable us to prevent pseudorefs from being updated by > > the ref update machinery e.g. git update-ref. > > > > Signed-off-by: David Turner <dturner@xxxxxxxxxxxxxxxx> > > --- > > This seems to do a bit more than what it claims to do. As this kind > of changes are very error-prone to review, I did a bulk replace of > the three all-caps NOTES_*THING* and compared the result with what > this patch gives to spot this: > > > # Fail to finalize merge > > test_must_fail git notes merge --commit >output 2>&1 && > > - # .git/NOTES_MERGE_* must remain > > - test -f .git/NOTES_MERGE_PARTIAL && > > - test -f .git/NOTES_MERGE_REF && > > - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha1 && > > - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha2 && > > - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 && > > - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 && > > + # .git/notes_merge_* must remain > > + git rev-parse --verify notes_merge_partial && > > + git rev-parse --verify notes_merge_ref && > > + test -f .git/notes_merge_worktree/$commit_sha1 && > > + test -f .git/notes_merge_worktree/$commit_sha2 && > > + test -f .git/notes_merge_worktree/$commit_sha3 && > > + test -f .git/notes_merge_worktree/$commit_sha4 && > > The two "rev-parse --verify" looks semi-sensible [*1*]; > notes_merge_partial is all lowercase and it refers to > $GIT_DIR/notes_merge_partial, because they are shared across working > tree. > > But then why are $GIT_DIR/notes_merge_worktree/* still checked with > "test -f"? If they are not refs or ref-like things, why should they > be downcased? If they are, why not "rev-parse --verify"? They are downcased for consistency with the other notes_merge_* stuff. > [Footnote] > > *1* I say "semi-" sensible, because it looks ugly. All ref-like > things immediately below $GIT_DIR/ are all-caps by convention. > Perhaps it is a better idea to move it under refs/; "everything > under refs/ is shared across working trees" is probably a much > better rule than "all caps but HEAD is special". Do you mean move all current pseudorefs? Or just the notes stuff? Moving MERGE_HEAD means that if someone upgrades in the middle of a merge, they'll end up confused. The same is true of the notes stuff, but I imagine that many fewer people use notes than use git merge, so I wasn't going to worry about that. What do you think? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html