jhenderson added inline comments.
================ Comment at: llvm/utils/lit/lit/llvm/config.py:171 + # in the tests that do the same thing. + (_, try_touch_err) = self.get_process_output(["touch", "-a", "-t", "199505050555.55", f.name]) + if try_touch_err != "": ---------------- lenary wrote: > michaelplatings wrote: > > lenary wrote: > > > michaelplatings wrote: > > > > It looks like this command will be run on Windows. I think it will fail > > > > and cause False to be returned, which is the desired result, but this > > > > appears to be by accident rather than design. Therefore I'm inclined to > > > > agree with @int3 that a hard-coded check would be preferable. > > > I am going to add the hardcoded checks, but I think `touch` is available > > > in windows, it should be in the same directory as all the git binaries. > > This is definitely tangential to the change, but in case it's useful to > > know: conventionally only `C:\Program Files\Git\bin` is added to the path > > on Windows, not `C:\Program Files\Git\usr\bin`. `C:\Program Files\Git\bin` > > only contains `bash.exe`, `git.exe` and `sh.exe`. > Something weirder is happening in `_find_git_windows_unix_tools` (from > https://reviews.llvm.org/D84380), but I think it's probably right just to > early exit with false on Windows. I've got no objection to explicitly omitting Windows, but just wanted to point out that LLVM requires various Unix-like tools to be available according to https://llvm.org/docs/GettingStarted.html#software. Whilst `touch` isn't explicitly specified, I'd be surprised if you managed to get access to that explicit list without including `touch` inadvertently (it's also worth noting that there are several other tests that use `touch` and other utilities that are intended to work on Windows). I thought that `Git\usr\bin` was recommended to be included in people's paths for Windows, as the way to get access to these tools, but I couldn't find that recommendation, so maybe that's just our downstream recommendation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144638/new/ https://reviews.llvm.org/D144638 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
