We can use --cmakeargs="-DFOO=bar -DCMAKE_PREFIX_PATH=/usr/local" as the example says in the help. It works as expected. But it doesn't work if we use BUILD_WEBKIT_ARGS or BUILD_JSC_ARGS environment variable to set --cmakeargs option, because they are splitted because of the whitespace. Additionally if we set --cmakeargs in BUILD_WEBKIT_ARGS and pass an other --cmakeargs to the command line, the cmdline one is omitted, because Perl's GetOptions uses only the last option. I think the proper fix would be to allow more --cmakeargs and simply concatenate them.
Created attachment 284864 [details] Patch
Comment on attachment 284864 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284864&action=review > Tools/Scripts/build-jsc:67 > - --cmakeargs=<arguments> Optional CMake flags (e.g. --cmakeargs="-DFOO=bar -DCMAKE_PREFIX_PATH=/usr/local") > + --cmakeargs=<arguments> Optional CMake flags (e.g. --cmakeargs="-DFOO=bar -DCMAKE_PREFIX_PATH=/usr/local") More --cmakeargs allowed. The "More --cmakeargs allowed." at the end will be hard to read/notice in the shell. How about this? --cmakeargs=<arguments> One or more optional CMake flags (e.g. --cmakeargs="-DFOO=bar -DCMAKE_PREFIX_PATH=/usr/local") > Tools/Scripts/build-jsc:79 > - 'cmakeargs=s' => \$cmakeArgs > + 'cmakeargs=s' => sub { $cmakeArgs .= " $_[1]" } Can this also be accomplished by using an array for cmakeArgs? 'cmakeargs=s' => \@cmakeArgs, Or will that destroy the whitespace? This is apparently the preferred way to do this via the "Options with multiple values" section in "man Getopt::Long". > Tools/Scripts/build-webkit:104 > - --cmakeargs=<arguments> Optional CMake flags (e.g. --cmakeargs="-DFOO=bar -DCMAKE_PREFIX_PATH=/usr/local") > + --cmakeargs=<arguments> Optional CMake flags (e.g. --cmakeargs="-DFOO=bar -DCMAKE_PREFIX_PATH=/usr/local") More --cmakeargs allowed. Ditto. > Tools/Scripts/build-webkit:119 > - 'cmakeargs=s' => \$cmakeArgs, > + 'cmakeargs=s' => sub { $cmakeArgs .= " $_[1]" }, Ditto.
(In reply to comment #2) > Comment on attachment 284864 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=284864&action=review > > > Tools/Scripts/build-jsc:67 > > - --cmakeargs=<arguments> Optional CMake flags (e.g. --cmakeargs="-DFOO=bar -DCMAKE_PREFIX_PATH=/usr/local") > > + --cmakeargs=<arguments> Optional CMake flags (e.g. --cmakeargs="-DFOO=bar -DCMAKE_PREFIX_PATH=/usr/local") More --cmakeargs allowed. > > The "More --cmakeargs allowed." at the end will be hard to read/notice in > the shell. How about this? > > --cmakeargs=<arguments> One or more optional CMake flags (e.g. > --cmakeargs="-DFOO=bar -DCMAKE_PREFIX_PATH=/usr/local") OK, will use the one you suggested. > > Tools/Scripts/build-jsc:79 > > - 'cmakeargs=s' => \$cmakeArgs > > + 'cmakeargs=s' => sub { $cmakeArgs .= " $_[1]" } > > Can this also be accomplished by using an array for cmakeArgs? > > 'cmakeargs=s' => \@cmakeArgs, > > Or will that destroy the whitespace? > > This is apparently the preferred way to do this via the "Options with > multiple values" section in "man Getopt::Long". Good point, I wasn't thorough enough to find this super easy way in the man. Actually it isn't necessary to use whitespace separated string, let's use array everywhere for cmake arguments.
Created attachment 285005 [details] Patch
ping?
Comment on attachment 285005 [details] Patch Clearing flags on attachment: 285005 Committed r204278: <http://trac.webkit.org/changeset/204278>
All reviewed patches have been landed. Closing bug.