Re: git smudge filter fails

From: Stephen Morton <stephen.c.morton@gmail.com>
Date: Mar 10, 2016, 14:45 UTC
Message-ID: <CAH8BJxFmAQtoF+1Q7Ub5qWnz5UewrPS4e8JQWms254hO_E05Hw@mail.gmail.com>
In-Reply-To: <20160310015939.GA12709@sigill.intra.peff.net>

I am a bit confused because this is basically the example used in ProGit [1] and it is fundamentally broken. In fact, if I understand correctly, this means that smudge filters cannot be relied upon to provide any 'keyword expansion' type tasks because they will all by nature have to query the file with 'git log'.

(Note that although in my example I used 'git checkout', with an only slightly more complicated example I can make it fail on 'git pull' which is perhaps a much more realistic use case. That was probably implied in your answer, I just wanted to mention it.)

Steve

[1] https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes

On Wed, Mar 9, 2016 at 8:59 PM, Jeff King <peff@peff.net> wrote:

Show 46 quoted lines

> On Wed, Mar 09, 2016 at 01:29:31PM -0500, Stephen Morton wrote:
>
>> git config --local filter.dater.smudge 'myDate=`git log
>> --pretty=format:"%cd" --date=iso -1 -- %f`; sed -e
>> "s/\(\\$\)Date[^\\$]*\\$/\1Date: $myDate \\$/g"'
>
> Your filter is running "git log" without a revision parameter, which
> means it is looking at HEAD.
>
> And here....
>
>> git checkout no_foo
>> git checkout master
>> cat foo.c
>> #observe keyword expansion lost
>
> You are expecting this second one to do:
>
>   1. Switch HEAD to "master".
>
>   2. Checkout files which need updating. Looking at HEAD in your filter
>      then examines "master", and you see the commit timestamp of the
>      destination.
>
> But that isn't how it is implemented. Checkout will handle the file
> checkout _first_, as that is the part that is likely to run into
> problems (e.g., rejecting a switch because it would lose changes in the
> working tree). Only at the very end, after the change to the working
> tree has succeeded, do we update HEAD.
>
> I think the order you are expecting is conceptually cleaner, but I don't
> think we would want to switch it around (for reasons of efficiency and
> robustness). And I don't think we would want to make a promise about the
> ordering to callers either way, as it binds our implementation.
>
> So is there a way to reliably know the destination of a checkout? My
> first thought was that we could add a placeholder similar to "%f" that
> your filter could use. I'm not sure how we would handle the corner cases
> there, though (e.g., do we always have a "destination" to report? If
> not, what do we give the script?).
>
> I suspect you could also hack something together with a post-checkout
> script, though it would probably be a lot less efficient (and might also
> have some weird corner cases).
>
> -Peff