Re: [RFC PATCH] checkout: Force matching mtime between files

[Marc Branchaud <marcnarc@xiplink.com>]

On 2018-04-27 01:03 PM, Duy Nguyen wrote:
> On Wed, Apr 25, 2018 at 5:18 PM, Marc Branchaud <marcnarc@xiplink.com> wrote:
>>> The best approach to do so is to have those people do the "touch"
>>> thing in their own post-checkout hook.  People who use Git as the
>>> source control system won't have to pay runtime cost of doing the
>>> touch thing, and we do not have to maintain such a hook script.
>>> Only those who use the "feature" would.
>>
>>
>> The post-checkout hook approach is not exactly straightforward.
> 
> I am revisiting this because I'm not even happy with my
> post-checkout-modified hook suggestion, so..
> 
>>
>> Naively, it's simply
>>
>>          for F in `git diff --name-only $1 $2`; do touch "$F"; done
>>
>> But consider:
>>
>> * Symlinks can cause the wrong file to be touched.  (Granted, Michał's
>> proposed patch also doesn't deal with symlinks.)  Let's assume that a hook
>> can be crafted will all possible sophistication.  There are still some
>> fundamental problems:
> 
> OK so this one could be tricky to get right, but it's possible.
> 
>>
>> * In a "file checkout" ("git checkout -- path/to/file"), $1 and $2 are
>> identical so the above loop does nothing.  Offhand I'm not even sure how a
>> hook might get the right files in this case.
> 
> This is a limitation of the current post-checkout hook. $3==0 from the
> hook lets us know this is not a branch switch, but it does not really
> tell you the affected paths. If it somehow passes all the given
> pathspec to you, then you should be able to do "git ls-files --
> $pathspec" which gives you the exact same set of paths that
> git-checkout updates. We could do this by setting $4 to "--" and put
> all the pathspecs in $5+ [1] e.g. "HEAD@{1} HEAD 0 -- path/to/file" in
> the above example.
> 
> There is  third case here, if you do "git checkout <tree-ish> --
> path/to/file" then it cannot be covered by the current design. I guess
> we could set $3 to '2' (retrieve from a tree) to indicate this in
> addition to 0 (from index) and 1 (from switching branch) and then $1
> could be the tree in question (pathspecs are passed the same way
> above)
> 
> [1] I wonder if we could have a more generic approach to pass
> pathspecs via environment, which could work for more than just this
> one hook. Not sure if it's a good idea though.

I think there needs to be something other than listing all the paths in 
the command is viable, because it's too easy to hit some 
command-line-length limit.  It would also be good if hook authors didn't 
have to re-invent the wheel of determining the changed paths for every 
corner-case.

My first instinct is to write them one-per-line on the hook's stdin. 
That's probably not generic enough for most hooks, but it seems like a 
good approach for this proposal.

Throwing them into a temporary file with a known name is also good --- 
better, I think, than stuffing them into an environment variable.

		M.

>> * The hook has to be set up in every repo and submodule (at least until
>> something like Ævar's experiments come to fruition).
> 
> Either --template or core.hooksPath would solve this, or I'll try to
> get my "hooks.* config" patch in. I think that one is a good thing to
> do anyway because it allows more flexible hook management (and it
> could allow multiple hooks of the same name too). With this, we could
> avoid adding more command-specific config like core.fsmonitor or
> uploadpack.packObjectsHook which to me are hooks.
> 
> Another option is extend core.hooksPath for searching multiple places
> instead of just one like AEvar suggested.



>> * A fresh clone can't run the hook.  This is especially important when
>> dealing with submodules.  (In one case where we were bit by this, make
>> though that half of a fresh submodule clone's files were stale, and decided
>> to re-autoconf the entire thing.)
> 
> Both --template and config-based hooks should let you handle this case.
> 
> So, I think if we improve the hook system to give more information
> (which is something we definitely should do), we could do this without
> adding special cases in git. I'm not saying that we should never add
> special cases, but at least this lets us play with different kinds of
> post-checkout activities before we decide which one would be best
> implemented in git.
>