Summary: | FreeBSD ar command doesn't support response files | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ting-Wei Lan <lantw44> | ||||||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | buildbot, cdumez, commit-queue, loic.yhuel, mcatanzaro, mrobinson, rniwa | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Local Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=173066 | ||||||||||||
Attachments: |
|
Description
Ting-Wei Lan
2017-03-26 05:36:14 PDT
Created attachment 305419 [details]
Patch
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 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
Comment on attachment 305419 [details]
Patch
FYI: you're going to have a (trivial) merge conflict here.
Created attachment 306503 [details]
Patch
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?
(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? 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. Created attachment 306570 [details]
Patch
Comment on attachment 306570 [details] Patch Clearing flags on attachment: 306570 Committed r215150: <http://trac.webkit.org/changeset/215150> All reviewed patches have been landed. Closing bug. (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 Reverted r215150 for reason: Broke Committed r215155: <http://trac.webkit.org/changeset/215155> Comment on attachment 306503 [details] Patch Clearing flags on attachment: 306503 Committed r215156: <http://trac.webkit.org/changeset/215156> All reviewed patches have been landed. Closing bug. Committed r215203: <http://trac.webkit.org/changeset/215203> (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. (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. 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. (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. |