Bug 66361 – Pass --makeargs of build-webkit to chromium linux builder

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Xianzhu Wang]

Sometimes we need to override the default -j$numCpus with --makeargs, for example, in a distributed-building environment.

[Xianzhu Wang]

Created attachment 104145 [details]
patch

[Eric Seidel (no email)]

Comment on attachment 104145 [details]
patch

Why?  Normally all args not understood by build-webkit are passed along to the build tool.

For example

build-webkit --various-xcode-arguments --another-xcode-arg WHATEVER_XCODE_ARG

is very common

[Xianzhu Wang]

(In reply to comment #2)
> (From update of attachment 104145 [details])
> Why?  Normally all args not understood by build-webkit are passed along to the build tool.
> 
> For example
> 
> build-webkit --various-xcode-arguments --another-xcode-arg WHATEVER_XCODE_ARG
> 
> is very common

Currently there are different ways to pass additional builder parameters for the buildXXX functions in webkitdirs.pm:

1. buildAutoToolsProject, buildQMakeProject, buildCMakeProjectOrExit use --makeargs to accept additional parameters to make, while pass other unknown parameters (and also --qmakeargs for buildQMakeProject) to configuration tools like autotools, qmake or cmake.

2. buildXCodeProject, buildWafProject directly pass unknown parameters to the build tool.

3. Other buildXXXs just ignore --makeargs or unknown parameters, like buildChromiumMakefile, buildChromiumVisualStudioProject, buildVisualStudioProject.

It seems reasonable to me to follow the convention of the 'make' family (above 1) for buildChromiumMakefile, because it also uses 'make'.

[Tony Chang]

Comment on attachment 104145 [details]
patch

Whenever I need to pass args to make, I just run make directly.  make -fMakefile.chromium <other args go here> doesn't seem like that much work to me.

[Xianzhu Wang]

(In reply to comment #4)
> (From update of attachment 104145 [details])
> Whenever I need to pass args to make, I just run make directly.  make -fMakefile.chromium <other args go here> doesn't seem like that much work to me.

Yes, I'm also doing this.

However, 'build-webkit --help' shows
  --makeargs=<arguments>            Optional Makefile flags

buildChromiumMakefile is the only buildXXX that uses Makefile but doesn't support 
the option, so I suggest to fix it.

[Tony Chang]

Comment on attachment 104145 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=104145&action=review

It seems fine to take this patch since many other ports use --makeargs.

> Tools/Scripts/webkitdirs.pm:1834
> +    my $makeArgs = "-j$numCpus";

Nit: We end up add -j$numCpus below so you don't need to set the value here.

> Tools/Scripts/webkitdirs.pm:1839
> +    for my $i (0 .. $#options) {
> +        if ($options[$i] =~ /^--makeargs=(.*)/i ) {
> +            $makeArgs = $1;
> +        }
> +    }

This could be:
for (@options) {
   $makeArgs = $1 if /^--makeargs=(.*)/;
}

[Xianzhu Wang]

Created attachment 104614 [details]
new patch

View in context: https://bugs.webkit.org/attachment.cgi?id=104145&action=review

>> Tools/Scripts/webkitdirs.pm:1834
>> +    my $makeArgs = "-j$numCpus";
> 
> Nit: We end up add -j$numCpus below so you don't need to set the value here.

Sorry this was my mistake that I forgot to remove the latter assignment.

>> Tools/Scripts/webkitdirs.pm:1839
>> +    }
> 
> This could be:
> for (@options) {
>    $makeArgs = $1 if /^--makeargs=(.*)/;
> }

Thanks. Done.

[WebKit Review Bot]

Comment on attachment 104614 [details]
new patch

Clearing flags on attachment: 104614

Committed r93515: <http://trac.webkit.org/changeset/93515>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.