Hi Peff, On 21 Sep 2023, at 17:40, Jeff King wrote: > On Thu, Sep 21, 2023 at 01:52:52AM -0700, Junio C Hamano wrote: > >> Jeff King <peff@xxxxxxxx> writes: >> >>> In an empty repository, "git log" will die anyway. So I think the more >>> interesting case is "I have a repository with stuff in it, but HEAD >>> points to an unborn branch". So: >>> >>> git --attr-source=HEAD diff foo^ foo >> >> This still looks like a made-up example. Who in the right mind >> would specify HEAD when both of the revs involved in the operation >> are from branch 'foo'? The history of HEAD may not have anything >> common with the operand of the operation 'foo' (or its parent), or >> worse, it may not even exist. > > I think it's unlikely for a user to write that. But if you are running a > server full of bare repositories that does diffs, you might end up with > a script that sticks "--attr-source=HEAD" at the beginning of every > command. > > It is true that HEAD may not be related. But that is what we use if you > are in a non-bare repository and run "git diff foo^ foo". > > Arguably: > > git --attr-source=$to diff $from $to > > is a better default for this command. But something like "git log -p" is > trickier, as you have many commits to show. You can try to use the tip > of the traversal, but there may be multiple. Using the attributes from > the destination of each commit is the most likely to avoid divergence > between the attributes and the code, but it may also not be what people > want. Using the modern attributes from the working tree often makes > showing historical commits much nicer. > > So I dunno. I could see arguments in both directions, but I think in > general people have been OK with pulling attributes from the working > tree. And --attr-source=HEAD is the bare equivalent. > >> But your "in this repository we never trust attributes from working >> tree, take it instead from this file or from this blob" example does >> make a lot more sense as a use case. > > I don't know that it was my example. :) But yes, if you do > "--attr-source=$to", you're overriding even the non-bare case. That may > be what you want for some cases, but as above, I think it's hard to > apply consistently (or even what you'd want for the general case). > >>> And there you really are saying "if there are attributes in HEAD, use >>> them; otherwise, don't worry about it". This is exactly what we do with >>> mailmap.blob: in a bare repository it is set to HEAD by default, but if >>> HEAD does not resolve, we just ignore it (just like a HEAD that does not >>> contain a .mailmap file). And those match the non-bare cases, where we'd >>> read those files from the working tree instead. >> >> "HEAD" -> "HEAD:.mailmap" if I recall correctly. > > True, yeah. We can't do that here because attributes are spread across > the tree. So all my mentions of attr.blob would really be attr.tree. > >> And if HEAD does not resolve, we pretend as if HEAD is an empty >> tree-ish (hence HEAD:.mailmap is missing). It becomes very tempting >> to do the same for the attribute sources and treat unborn HEAD as if >> it specifies an empty tree-ish, without any configuration or an >> extra option. >> >> Such a change would be an end-user observable behaviour change, but >> nobody sane would be running "git --attr-source=HEAD diff HEAD^ HEAD" >> to check and detect an unborn HEAD for its error exit code, so I do >> not think it is a horribly wrong thing to do. > > Yeah, that is basically what I am proposing. It sounds from the > discussion here that there are two interesting cases: > > 1. You want to use --attr-source=HEAD because you are trying to make a > bare repo behave like a non-bare one. You probably want the "don't > complain if it is missing" behavior. Yep, this is the primary use case for us. > > 2. You are trying to use the attributes from one side of the diff to > override the worktree ones (because the two trees are unrelated). > In which case it does not really matter if --attr-source complains, > because the diff will likewise complain if the tree cannot be > resolved. > > Just trying to play devil's advocate, though, I guess you could run a > non-diff operation like say: > > git --attr-source=my-branch check-attr foo > > and then you probably _do_ want to know if that source was ignored or > typo'd. > >> But again, as you said, --attr-source=<tree-ish> does not sound like >> a good fit for bare-repository hosted environment and a tentative >> hack waiting for a proper attr.blob support, or something like that, >> to appear. > > I think folks mentioned mailmap.blob in the original discussion for > --attr-source. I don't remember why the patch went with --attr-source > there. Maybe John can speak to that. Yeah I was looking for this, and I think I found it in [1], which was part of a separate patch having to do with gitattributes. If someone did mention it in the patch for --attr-source, then I can't remember why we decided to go with the comand line flag rather than the config. 1. https://lore.kernel.org/git/ZBMn5T6zfKK+PYUe@xxxxxxxxxxxxxxxxxxxxxxx/ > > -Peff
