[Gtk] build.sh needs a -- before make options when the build command is cmake --build
Created attachment 237364 [details] Patch
This is needed on Arch Linux.
Can you take a look at this patch too (or suggest someone else if you don't usually review build system patches)?
Comment on attachment 237364 [details] Patch Why is this patch needed? Do you need to pass specific options to make?
Comment on attachment 237364 [details] Patch This looks like a fix that should be submitted upstream to CMake instead of to WebKit. Why does this only fail for Arch?
(In reply to comment #4) > (From update of attachment 237364 [details]) > Why is this patch needed? Do you need to pass specific options to make? webkit-build automatically passes -j8 (or something similar) to the build command. Specifically, on Arch Linux, build.sh looks like this: #!/bin/sh /usr/bin/cmake --build . --config "Release" $@ And $@ is -j8. "cmake --build . --config "Release" -j8" is invalid and gives this error: Unknown argument -j8 Usage: cmake --build <dir> [options] [-- [native-options]] (In reply to comment #5) > (From update of attachment 237364 [details]) > This looks like a fix that should be submitted upstream to CMake instead of to WebKit. Why does this only fail for Arch? The difference between Fedora 20 and Arch seems to be that Arch is using CMake 3.0 and Fedora is using CMake 2.8. http://www.cmake.org/cmake/help/v3.0/release/3.0.0.html#other-changes > The build_command() command now returns a cmake(1) --build command line instead of a direct invocation of the native build tool. This is a WebKit issue because we're calling cmake --build incorrectly. Expecting this to work correctly on two different major versions may be unreasonable, but this was the only change I needed to make to make it work. For reading the documentation, it looks like sometimes the build command will already include -- though, so I'll update my patch to check for that: http://www.cmake.org/cmake/help/v3.0/command/build_command.html#command:build_command
Created attachment 237492 [details] Patch
Comment on attachment 237492 [details] Patch I feel like we tried pretty hard to get the actual build command into the build.sh script. If CMake 3.0 no longer lets us do that, we may need to find another work-around. I cannot recall exactly why it was important that we invoked the build system directly, instead of using "cmake --build."
Fedora 21 is going to ship with CMake 3.0: https://apps.fedoraproject.org/packages/cmake The next version of Ubuntu doesn't have it yet, so we might be stuck with both for a little while: http://packages.ubuntu.com/search?keywords=cmake https://packages.debian.org/search?keywords=cmake
(In reply to comment #8) > (From update of attachment 237492 [details]) > I feel like we tried pretty hard to get the actual build command into the build.sh script. If CMake 3.0 no longer lets us do that, we may need to find another work-around. I cannot recall exactly why it was important that we invoked the build system directly, instead of using "cmake --build." I'm not sure if it's possible to get the real build command anymore..
(In reply to comment #10) > I'm not sure if it's possible to get the real build command anymore.. That's really unfortunate. We might have to resort to just creating the command-line manually. :/
(In reply to comment #8) > (From update of attachment 237492 [details]) > I feel like we tried pretty hard to get the actual build command into the build.sh script. If CMake 3.0 no longer lets us do that, we may need to find another work-around. I cannot recall exactly why it was important that we invoked the build system directly, instead of using "cmake --build." I think you meant bug130076
(In reply to comment #12) > (In reply to comment #8) > > (From update of attachment 237492 [details] [details]) > > I feel like we tried pretty hard to get the actual build command into the build.sh script. If CMake 3.0 no longer lets us do that, we may need to find another work-around. I cannot recall exactly why it was important that we invoked the build system directly, instead of using "cmake --build." > > I think you meant bug130076 Perhaps the original bug is now fixed in the later versions of CMake?
I think the answer here is that if build.sh is simply calling cmake --build, then we should just not use build.sh for CMake 3.
(In reply to comment #14) > I think the answer here is that if build.sh is simply calling cmake --build, then we should just not use build.sh for CMake 3. I'm kind of busy with other things at the moment, so if someone else wants to write a patch to do that it would be fine with me.
Created attachment 238648 [details] Another approach Martin is right, we don't need the build.sh hack at all with CMake 3.
Comment on attachment 238648 [details] Another approach Nice, thanks!
A clean build will be needed, or at least the build.sh script should be removed from the build dir.
Comment on attachment 238648 [details] Another approach Clearing flags on attachment: 238648 Committed r173964: <http://trac.webkit.org/changeset/173964>
Thanks for handling this!