On Fri, Dec 02 2022, Phillip Wood wrote: > On 01/12/2022 23:00, Junio C Hamano wrote: >> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: >> >>>> Junio please drop this series when you rebuild next as it breaks >>>> manually running individual test scripts when building with Visual >>>> Studio. >>> >>> I think the issue you've spotted is easily fixed on top. See below. >> Smells more like papering over than fixed, but let's see how folks >> who need cmake/ctest feel about it. > > As MSVC uses different directories for debug and release builds there > can be more than one build directory. I don't think selecting one of > them at random using 'find' is a good idea. I think the latest iteration submitted today should fix that issue: https://lore.kernel.org/git/cover-v5-00.15-00000000000-20221202T110947Z-avarab@xxxxxxxxx/ >> Let's mark the series never to graduate to 'master' for now, >> optionally revert it out of 'next'. >> Phillip, you asked about rebuilding 'next', which would not >> happen until 2.39.0 final---did you mean reverting the topic out >> of 'next'? Do you need 'next' without this topic, not just >> 'master'? > > I don't mind waiting but I'm not a Windows user. I only tested this > topic under Windows because I knew Ævar had not and a quick web search > for "MSVC CMake" made me worry it was broken. > > I'm afraid I wont be spending anymore time on this topic. I had hoped > that having the CMake build work under Linux would help developers > avoid breaking it. However I'm concerned that if developers do not > appreciate that there are differences between the Linux and Windows > builds it will actually create a false sense of security and be used > as an excuse not to properly test under Windows[1]. Recent events have > confirmed my view that changes like this need the attention of someone > with experience of Windows development and given that yesterday was > the first time I'd used MSVC since about 1994 I do not fit that > description. > [...] > [1] While our CI helps the MSVC job runs CMake manually, performs an > in-tree build and does not use ctest. In contrast a user running the > MSVC GUI does not run CMake themselves, ends up with an out-of-tree > build and runs the tests with ctest. I don't run Windows by choice, and I'm not interested in running a propriterary IDE (VS) either. The main reason I'm working on this series is that while we as a project are happy to support proprietary OS's, it hasn't been a requirement for participation that you need to buy a copy of Windows, OSX, AIX, HP/UX or whatever to submit patches. Of course we have platform-specific code. but this CMake component is unique in how invasive it is. It's easy to e.g. stay away from the OSX-specific code in compat/fsmonitor/*darwin*.[ch], or generally speaking the Windows-specific C code. But for CMake it's become a hard requirenment for many changes, even though it's a contrib/ component. Now, I'm not looking to get rid of it, it's clearly useful, particularly with MSVC (or so I've gathered). But I'd also like some future where any time I and others who don't use Windows need to patch certain parts of the Makefile that we don't need to spend a long time bouncing thigs against the Windows CI, but can run this component other platforms.. E.g. I think for cfe853e66be (hook-list.h: add a generated list of hooks, like config-list.h, 2021-09-26) I spent at least 3-4 hours on what should have been a 5-10 minute task of making the relatively minor change to generate hook-list.h. It was a push, wait 30-60 minutes, find some minor (e.g. syntax) issue, rinse & repeat. Now, clearly the outstanding issues with (if any) need to be fixed, and thanks for sticking with testing it for so long. But hopefully the above gives you some background. > In addition to the breakage I reported yesterday 623fde1438 (cmake: > chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4, 2022-11-03) > causes CMake older that 3.19 to error out when run from MSVC because > chmod does not exist on Windows. Also when running 'ctest' on "next" I > see tests failing because they cannot find 'test-tool' (I haven't > tried running the failing tests manually) That's odd, do you happen to have some output from "ctest" for that? Our "cmake" build already invokes shell scripts which rely on "grep", "sed" etc, and the test suite itself invokes "chmod", I just understand that it's a noop wrapper on Windows. So I can also re-roll and just ifdef that part awa on win32, but I wonder what's going on there. I wonder if it's because we'd need to spawn it via "/bin/sh" (but just skippin it seems better). One of the things I didn't change with this series is how "test-tool" is handled. I.e. it's there before in the generated "t/helper" directory, and in the generated "bin-wrappers/". We also find it when running the tests with Windows in GitHub CI. Did you try on v2.38.0 and/or "master" as well?
