Bug 71370 – webkitdirs.pm: add getArchitecture implementation for autotools ports

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Andy Wingo]

If I run "Tools/Scripts/run-javascriptcore-tests --gtk" on my Linux box, it will run build-jsc with the --32-bit argument, which is not what I want, given that I have already built JSC for x86-64.

The patch to be attached fixes this issue.

(If I omit --gtk, even having already built JSC, I get worse results, but that's another bug.)

[Andy Wingo]

Created attachment 113320 [details]
Patch

[Philippe Normand]

Comment on attachment 113320 [details]
Patch

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

> Tools/Scripts/webkitdirs.pm:250
> +    if (isGtk() or isEfl()) {
> +        determineConfigurationProductDir();
> +        my $host_triple = `grep -E '^host = ' $configurationProductDir/GNUmakefile`;

I think EFL uses CMake so this check on the GNUMakefile will likely always fail for them. Maybe remove the isEfl() test?

[Andy Wingo]

Created attachment 114080 [details]
Patch

[Andy Wingo]

Updated patch.  R? :)

[Martin Robinson]

Comment on attachment 114080 [details]
Patch

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

> Tools/Scripts/webkitdirs.pm:250
> +        determineConfigurationProductDir();
> +        my $host_triple = `grep -E '^host = ' $configurationProductDir/GNUmakefile`;
> +        $architecture = $1 if $host_triple =~ m/^host = ([^-]+)-/;

In the Mac case they first check the architecture set for the build and then if it's not there, they fall back to the system architecture. I think you should do the same here.

[Andy Wingo]

Created attachment 114228 [details]
Patch

[Xan Lopez]

Comment on attachment 114228 [details]
Patch

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

> Tools/Scripts/webkitdirs.pm:254
> +            # Fall back to output of `arch', if it is present.

Ultra nitpick, you don't really have further fallbacks, so I guess the "if it is present" is a bit misleading. You'll use that or fail in some way I suppose. Anyway, this looks OK to me.

[WebKit Review Bot]

Comment on attachment 114228 [details]
Patch

Clearing flags on attachment: 114228

Committed r99729: <http://trac.webkit.org/changeset/99729>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.