Bug 173066

Summary: [CMake] Only force response files for Ninja with CMake < 3.2 on Linux
Product: WebKit Reporter: Loïc Yhuel <loic.yhuel>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, clopez, commit-queue, mcatanzaro, tpopela
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=170105
Attachments:
Description Flags
Patch none

Description Loïc Yhuel 2017-06-07 10:35:10 PDT
[CMake] Only force response files for Ninja with CMake < 3.2 on Linux
Comment 1 Loïc Yhuel 2017-06-07 10:36:41 PDT
Created attachment 312195 [details]
Patch
Comment 2 Loïc Yhuel 2017-06-07 10:48:52 PDT
CMake uses response files when needed, but the limit was wrong on Linux (https://cmake.org/Bug/view.php?id=14892, fixed in CMake 3.2).
It created the original https://bugs.webkit.org/show_bug.cgi?id=129771, with "argument list too long" errors. 

CMake 3.6 added response file support for compile rules (which created https://bugs.webkit.org/show_bug.cgi?id=168770 since we forced the response files).

On Linux, CMake >= 3.2 uses response files as needed (ie only for libWebCore.a).
Hopefully platforms which don't support response files (FreeBSD, Apple) have a big enough command line length limit (sysconf(_SC_ARG_MAX)) to avoid them.
Comment 3 Michael Catanzaro 2017-06-07 10:51:26 PDT
Comment on attachment 312195 [details]
Patch

Thanks, let's try it! I only wonder if we still need the conditionals to force disable response files when using icecc. Carlos Lopez, Tom, what do you think?
Comment 4 Loïc Yhuel 2017-06-07 11:09:55 PDT
(In reply to Michael Catanzaro from comment #3)
> Comment on attachment 312195 [details]
> Patch
> 
> Thanks, let's try it! I only wonder if we still need the conditionals to
> force disable response files when using icecc. Carlos Lopez, Tom, what do
> you think?

The icecc test was only disabling CMAKE_NINJA_FORCE_RESPONSE_FILE for CMake > 3.5, older versions only support response files to link or create archives.
The archives are done locally with ar, and for the link icecc probably forwards all arguments to the local CXX.
Comment 5 Michael Catanzaro 2017-06-07 11:13:13 PDT
Comment on attachment 312195 [details]
Patch

I'm not sure I understand, but let's try it!
Comment 6 Michael Catanzaro 2017-06-07 11:14:08 PDT
Ah well yeah, I must have misread the conditionals... clearly this shouldn't affect icecc at all (unless using old CMake).
Comment 7 WebKit Commit Bot 2017-06-07 11:41:12 PDT
Comment on attachment 312195 [details]
Patch

Clearing flags on attachment: 312195

Committed r217894: <http://trac.webkit.org/changeset/217894>
Comment 8 WebKit Commit Bot 2017-06-07 11:41:14 PDT
All reviewed patches have been landed.  Closing bug.