Bug 66361 - Pass --makeargs of build-webkit to chromium linux builder
Summary: Pass --makeargs of build-webkit to chromium linux builder
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-16 21:26 PDT by Xianzhu Wang
Modified: 2011-08-22 10:49 PDT (History)
5 users (show)

See Also:


Attachments
patch (2.65 KB, patch)
2011-08-16 21:29 PDT, Xianzhu Wang
tony: review-
Details | Formatted Diff | Diff
new patch (2.61 KB, patch)
2011-08-19 21:17 PDT, Xianzhu Wang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xianzhu Wang 2011-08-16 21:26:28 PDT
Sometimes we need to override the default -j$numCpus with --makeargs, for example, in a distributed-building environment.
Comment 1 Xianzhu Wang 2011-08-16 21:29:38 PDT
Created attachment 104145 [details]
patch
Comment 2 Eric Seidel (no email) 2011-08-17 11:06:45 PDT
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
Comment 3 Xianzhu Wang 2011-08-17 18:50:13 PDT
(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'.
Comment 4 Tony Chang 2011-08-18 09:47:07 PDT
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.
Comment 5 Xianzhu Wang 2011-08-18 18:06:30 PDT
(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.
Comment 6 Tony Chang 2011-08-19 09:47:32 PDT
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=(.*)/;
}
Comment 7 Xianzhu Wang 2011-08-19 21:17:41 PDT
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.
Comment 8 WebKit Review Bot 2011-08-22 10:49:37 PDT
Comment on attachment 104614 [details]
new patch

Clearing flags on attachment: 104614

Committed r93515: <http://trac.webkit.org/changeset/93515>
Comment 9 WebKit Review Bot 2011-08-22 10:49:43 PDT
All reviewed patches have been landed.  Closing bug.