Bug 252548

Summary: build-webkit: Don't remove CMakeFiles directory
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: CMakeAssignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: clopez, don.olmstead, ews-watchlist, jbedard, mcatanzaro, philn, stephan.szabo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
[fast-cq] Patch none

Description Fujii Hironori 2023-02-19 14:21:48 PST
[CMake][Win] ninja: error: build.ninja:35: loading 'CMakeFiles\rules.ninja': The system cannot find the file specified.

WinCairo and PlayStation builders are rarely observing the following error.

https://build.webkit.org/#/builders/731/builds/1945

> ninja: error: build.ninja:35: loading 'CMakeFiles\rules.ninja': The system cannot find the file specified.
> include CMakeFiles\rules.ninja
>                               ^ near here
Comment 1 Fujii Hironori 2023-02-19 14:28:34 PST
I don't know how this problem appears. But, it's easy to reproduce the error by removing WebKitBuild/{Debug,Release}/CMakeFiles directory manually.
However, this problem doesn't happen if I remove both CMakeFiles directory and CMakeCache.txt. It executes cmake to regenerate in such a case.
removeCMakeCache actually removes both. Weird.

https://github.com/WebKit/WebKit/blob/5a049809955a360f7ec43aa244acc1e65b4b70e6/Tools/Scripts/webkitdirs.pm#L2683-L2684

> unlink($cmakeCache) if -e $cmakeCache;
> rmtree($cmakeFiles) if -d $cmakeFiles;
Comment 2 Fujii Hironori 2023-02-19 16:04:27 PST
Steps to reproduce:

1. Invoke .\Tools\Scripts\build-webkit
   CMakeLists.txt and CMakeFiles directory are created.
2. Add "message(FATAL_ERROR)" to Source/cmake/OptionsCommon.cmake
3. Invoke .\Tools\Scripts\build-webkit
   CMakeLists.txt and CMakeFiles directory are removed.
   Only CMakeLists.txt is created.
4. Invoke .\Tools\Scripts\build-webkit
   Report the ninja error
Comment 3 Fujii Hironori 2023-02-19 16:24:30 PST
Bug#165008 introduced this bug.
And, CMake 3.8 fixed the CMake problem. 

https://gitlab.kitware.com/cmake/cmake/-/issues/13934#note_200624
https://gitlab.kitware.com/cmake/cmake/-/commit/25b6e7b710d7739cca44ed19bf45a190e72a6b82

I'm going to revert the WebKit change.
Comment 4 Fujii Hironori 2023-02-19 16:43:12 PST
Created attachment 465079 [details]
Patch
Comment 5 Carlos Alberto Lopez Perez 2023-02-19 18:07:01 PST
(In reply to Fujii Hironori from comment #4)
> Created attachment 465079 [details]
> Patch

I don't understand why or how this patch would work.

As far as I can see what CMake developers did on 3.8 was to also purge the CMakeFiles directory if the CMakeCache.txt file was removed, which is also basically what the current code in the script 'build-webkit' does.

So why patching the script 'build-webkit' to not remove the CMakeFiles directory fixes the problem? CMake is also supposed to remove this directory in any case, doesn't it?

I'm confused, sorry
Comment 6 Fujii Hironori 2023-02-19 18:34:41 PST
IIUC, it removes ${CMAKE_BINARY_DIR}/CMakeFiles/${CMAKE_VERSION} directory.
https://gitlab.kitware.com/cmake/cmake/-/blob/master/Source/cmGlobalGenerator.cxx#L584

rules.ninja is not removed becaused it is in ${CMAKE_BINARY_DIR}/CMakeFiles directory.
Comment 7 Michael Catanzaro 2023-02-20 05:55:10 PST
Comment on attachment 465079 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=465079&action=review

Fujii, I think you might be the only developer still attaching patches to Bugzilla. :P

> COMMIT_MESSAGE:11
> +And, the original CMake proglem was fixed by CMake 3.8.

problem
Comment 8 Fujii Hironori 2023-02-20 11:51:48 PST
Created attachment 465090 [details]
Patch
Comment 9 Fujii Hironori 2023-02-20 12:42:05 PST
Created attachment 465093 [details]
[fast-cq] Patch
Comment 10 EWS 2023-02-20 13:18:18 PST
Committed 260563@main (4e08fdf96101): <https://commits.webkit.org/260563@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 465093 [details].