From: Marc Branchaud Date: Fri, 27 Apr 2018 21:08:10 GMT Subject: Re: [RFC PATCH] checkout: Force matching mtime between files Message-ID: <38e961ca-e354-6390-982d-4d0c9c9e55da@xiplink.com> In-Reply-To: On 2018-04-27 01:03 PM, Duy Nguyen wrote: > On Wed, Apr 25, 2018 at 5:18 PM, Marc Branchaud 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 -- > 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. >