| Message ID | 6cba1d950b013410ecc6ffc15bfcba02c51d6de2.1644612979.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Builtin FSMonitor Part 2 | expand |
Random follow-up on this: in message/patch 76b6216281e3463821e650495f3090c677905f73.1646041236.git.gitgitgadget@gmail.com I propose a fix to this issue. If accepted, that fix (and related changes to rely more heavily on chmtime in t7063) will cause *this* change to cause test failures. If that patch is accepted, this commit will simply need to be dropped as far as I understand. On Fri, Feb 11, 2022 at 9:57 PM Jeff Hostetler via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Jeff Hostetler <jeffhost@microsoft.com> > > Teach `test-tool.exe chmtime` to ignore errors when setting the mtime > on a directory on Windows. > > NEEDSWORK: The Windows version of `utime()` (aka `mingw_utime()`) does > not properly handle directories because it uses `_wopen()`. It should > be converted to using `CreateFileW()` and backup semantics at a minimum. > Since I'm already in the middle of a large patch series, I did not want > to destabilize other callers of `utime()` right now. The problem has > only been observed in the t/perf/p7519 test when the test repo contains > an empty directory on disk. > > Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> > --- > t/helper/test-chmtime.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/t/helper/test-chmtime.c b/t/helper/test-chmtime.c > index 524b55ca496..dc28890a183 100644 > --- a/t/helper/test-chmtime.c > +++ b/t/helper/test-chmtime.c > @@ -134,6 +134,21 @@ int cmd__chmtime(int argc, const char **argv) > } > > if (utb.modtime != sb.st_mtime && utime(argv[i], &utb) < 0) { > +#ifdef GIT_WINDOWS_NATIVE > + if (S_ISDIR(sb.st_mode)) { > + /* > + * NEEDSWORK: The Windows version of `utime()` > + * (aka `mingw_utime()`) does not correctly > + * handle directory arguments, since it uses > + * `_wopen()`. Ignore it for now since this > + * is just a test. > + */ > + fprintf(stderr, > + ("Failed to modify time on directory %s. " > + "Skipping\n"), argv[i]); > + continue; > + } > +#endif > fprintf(stderr, "Failed to modify time on %s: %s\n", > argv[i], strerror(errno)); > return 1; > -- > gitgitgadget >
On 2/28/22 4:43 AM, Tao Klerks wrote: > Random follow-up on this: in message/patch > 76b6216281e3463821e650495f3090c677905f73.1646041236.git.gitgitgadget@gmail.com > I propose a fix to this issue. If accepted, that fix (and related > changes to rely > more heavily on chmtime in t7063) will cause *this* change to cause test > failures. > > If that patch is accepted, this commit will simply need to be dropped > as far as I understand. > Thanks for the note. Yes, with your change we should be able to drop or revert this commit. Thanks Jeff
Jeff Hostetler <git@jeffhostetler.com> writes: > On 2/28/22 4:43 AM, Tao Klerks wrote: >> Random follow-up on this: in message/patch >> 76b6216281e3463821e650495f3090c677905f73.1646041236.git.gitgitgadget@gmail.com >> I propose a fix to this issue. If accepted, that fix (and related >> changes to rely >> more heavily on chmtime in t7063) will cause *this* change to cause test >> failures. >> If that patch is accepted, this commit will simply need to be >> dropped >> as far as I understand. >> > > Thanks for the note. Yes, with your change we should be able to > drop or revert this commit. That sounds nice. The fewer differences among platforms we have to maintain, the easier.
diff --git a/t/helper/test-chmtime.c b/t/helper/test-chmtime.c index 524b55ca496..dc28890a183 100644 --- a/t/helper/test-chmtime.c +++ b/t/helper/test-chmtime.c @@ -134,6 +134,21 @@ int cmd__chmtime(int argc, const char **argv) } if (utb.modtime != sb.st_mtime && utime(argv[i], &utb) < 0) { +#ifdef GIT_WINDOWS_NATIVE + if (S_ISDIR(sb.st_mode)) { + /* + * NEEDSWORK: The Windows version of `utime()` + * (aka `mingw_utime()`) does not correctly + * handle directory arguments, since it uses + * `_wopen()`. Ignore it for now since this + * is just a test. + */ + fprintf(stderr, + ("Failed to modify time on directory %s. " + "Skipping\n"), argv[i]); + continue; + } +#endif fprintf(stderr, "Failed to modify time on %s: %s\n", argv[i], strerror(errno)); return 1;