Bug 170105 – FreeBSD ar command doesn't support response files

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Ting-Wei Lan]

When cmake is set to generate build files for ninja, Source/cmake/OptionsCommon.cmake automatically enables the use of response files by setting CMAKE_NINJA_FORCE_RESPONSE_FILE to 1. This causes linking failure on FreeBSD because its ar command doesn't understand the response file syntax, and all object files listed in response files are not added.

FreeBSD ar is implemented using libarchive and it is part of elftoolchain project (https://sourceforge.net/projects/elftoolchain/). We can detect it by checking the output of 'ar -V' command and disable the use of response files for it.

[Ting-Wei Lan]

Created attachment 305419 [details]
Patch

[Build Bot]

Comment on attachment 305419 [details]
Patch

Attachment 305419 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3416117

New failing tests:
compositing/absolute-inside-out-of-view-fixed.html
fast/scrolling/ios/scroll-events-back-forward.html

[Build Bot]

Created attachment 305454 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Michael Catanzaro]

Comment on attachment 305419 [details]
Patch

FYI: you're going to have a (trivial) merge conflict here.

[Ting-Wei Lan]

Created attachment 306503 [details]
Patch

[Michael Catanzaro]

Comment on attachment 306503 [details]
Patch

OK

Then again, if the build works without response files, and response files are broken on so many platforms, maybe we should just get rid of them?

[Ting-Wei Lan]

(In reply to Michael Catanzaro from comment #6)
> Then again, if the build works without response files, and response files
> are broken on so many platforms, maybe we should just get rid of them?

Response files were added in bug 129771 to avoid 'Argument list too long' error, but we don't know which platform really requires them. If there are only a few platforms requiring response files, maybe we can disable them by default and only enable them for these specific platforms?

[Michael Catanzaro]

I'm suspect Martin was using Linux.

Let's just try removing it and see if something breaks! If it doesn't work, then we can go with the patch we have here.

[Ting-Wei Lan]

Created attachment 306570 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 306570 [details]
Patch

Clearing flags on attachment: 306570

Committed r215150: <http://trac.webkit.org/changeset/215150>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

(In reply to WebKit Commit Bot from comment #11)
> All reviewed patches have been landed.  Closing bug.

Broke GTK bot?

https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Build%29/builds/640/steps/compile-webkit/logs/stdio

[Michael Catanzaro]

Reverted r215150 for reason:

Broke

Committed r215155: <http://trac.webkit.org/changeset/215155>

[WebKit Commit Bot]

Comment on attachment 306503 [details]
Patch

Clearing flags on attachment: 306503

Committed r215156: <http://trac.webkit.org/changeset/215156>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

Committed r215203: <http://trac.webkit.org/changeset/215203>

[Loïc Yhuel]

(In reply to Ting-Wei Lan from comment #7)
> (In reply to Michael Catanzaro from comment #6)
> > Then again, if the build works without response files, and response files
> > are broken on so many platforms, maybe we should just get rid of them?
> 
> Response files were added in bug 129771 to avoid 'Argument list too long'
> error, but we don't know which platform really requires them. If there are
> only a few platforms requiring response files, maybe we can disable them by
> default and only enable them for these specific platforms?


Actually the "argument list too long" issue was probably specific to Linux, and fixed in CMake 3.2 (https://cmake.org/Bug/view.php?id=14892). CMake already used response files when needed, but the limit was wrong on Linux.
So so we don't need to force response files any more.

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

FreeBSD probably wasn't affected by the original issue, but even without CMAKE_NINJA_FORCE_RESPONSE_FILE, CMake will use a response file if the command line is larger than sysconf(_SC_ARG_MAX) on FreeBSD.
So if the linker (or ar) command line becomes too long, there would be an issue.

Btw, this commit disabled the response files with FreeBSD ar, but only for CMake >= 3.6, so the issue would remain for older versions.

[Ting-Wei Lan]

(In reply to Loïc Yhuel from comment #17)
> CMake 3.6 added response file support for compile rules (which created
> https://bugs.webkit.org/show_bug.cgi?id=168770).
> 
> FreeBSD probably wasn't affected by the original issue, but even without
> CMAKE_NINJA_FORCE_RESPONSE_FILE, CMake will use a response file if the
> command line is larger than sysconf(_SC_ARG_MAX) on FreeBSD.
> So if the linker (or ar) command line becomes too long, there would be an
> issue.

Yes, I know the problem can still happen if the command line is really too long, but I never see it on my machines. WebKitGTK+ in FreeBSD ports was only switched to ninja recently, so it may take some time to see bug reports on FreeBSD Bugzilla if it really happens on users' machines.

> 
> Btw, this commit disabled the response files with FreeBSD ar, but only for
> CMake >= 3.6, so the issue would remain for older versions.

The 'latest' repository of FreeBSD uses CMake 3.8.0 and the 'quarterly' repository uses CMake 3.7.2, so I think it is unlikely to happen on FreeBSD.

[Michael Catanzaro]

Loïc, what you're saying is that we can stop forcing response files now, right? Then we won't need to maintain platform-specific exceptions anymore. If so, it would be helpful to file a new bug for that.

[Loïc Yhuel]

(In reply to Michael Catanzaro from comment #19)
> Loïc, what you're saying is that we can stop forcing response files now,
> right? Then we won't need to maintain platform-specific exceptions anymore.
> If so, it would be helpful to file a new bug for that.

We only need to force response files on Linux hosts with CMake older than 3.2.
It's still platform-specific, but it's simpler that we do currently.

I'm checking WebKitGTK build correctly, I will create a bug after.