Re: [RFC PATCH] checkout: Force matching mtime between files
- From
Marc Branchaud <marcnarc@xiplink.com>
- Date
- Apr 27, 2018, 21:08 UTC
- Message-ID
- <38e961ca-e354-6390-982d-4d0c9c9e55da@xiplink.com>
- In-Reply-To
- <CACsJy8AVTkqDomQW7Np5opnK+bMk53pcMtCQRBr-=QXEXs8Nqg@mail.gmail.com>
On 2018-04-27 01:03 PM, Duy Nguyen wrote:
Show 51 quoted lines
> 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.
Show 12 quoted lines
>> * 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.
Show 14 quoted lines
>> * 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. >