Bug 213727 – [webkitperl] nativeArchitecture() needs to support remote devices

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

[Jonathan Bedard]

When targeting a remote platform, nativeArchitecture() needs to support return the architecture of the remote device.

[Radar WebKit Bug Importer]

<rdar://problem/64892021>

[Jonathan Bedard]

Created attachment 403066 [details]
Patch

[Jonathan Bedard]

Created attachment 403073 [details]
Patch

[Guillaume Emont]

Comment on attachment 403066 [details]
Patch

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

The changes look sensible at first sight. I'll try to run a couple tests locally, though the arm EWS should pick it up soon.

> Tools/Scripts/webkitdirs.pm:131
> +my %nativeArchitectureMap = ();

Do we need this to be a map? It looks like we would always have exactly one element in it, or are there some nativeArchitecture() calls without arguments that are left there on purpose?

[Jonathan Bedard]

(In reply to Guillaume Emont from comment #4)
> Comment on attachment 403066 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=403066&action=review
> 
> The changes look sensible at first sight. I'll try to run a couple tests
> locally, though the arm EWS should pick it up soon.
> 
> > Tools/Scripts/webkitdirs.pm:131
> > +my %nativeArchitectureMap = ();
> 
> Do we need this to be a map? It looks like we would always have exactly one
> element in it, or are there some nativeArchitecture() calls without
> arguments that are left there on purpose?

It's hard to get rid of all the nativeArchitecture() calls without arguments since other places in webkitdirs may call it. Seemed better to just cache the results on a per-host basis.

[Jonathan Bedard]

Comment on attachment 403073 [details]
Patch

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

> Tools/Scripts/webkitdirs.pm:362
> +    if ($target eq 'localhost') {

Just realized this won't work, embedded devices will target localhost on a different port.

[Jonathan Bedard]

Created attachment 403080 [details]
Patch

[Jonathan Bedard]

Created attachment 403109 [details]
Patch

[Jonathan Bedard]

Created attachment 403115 [details]
Patch

[Jonathan Bedard]

Created attachment 403127 [details]
Patch

[Jonathan Bedard]

Created attachment 403132 [details]
Patch

[Jonathan Bedard]

Created attachment 403141 [details]
Patch

[Jonathan Bedard]

Created attachment 403160 [details]
Patch

[Guillaume Emont]

The patch makes sense to me, I could locally check that it works with --remote, and the jsc-armv7 ews confirms that it fixes the issues introduced in r263569. I am not a reviewer, but I think that this patch should be landed ASAP as it will unbreak the armv7 and mips bots (EWS and post-commit).

[Aakash Jain]

Comment on attachment 403160 [details]
Patch

rs=me

[EWS]

Committed r263742: <https://trac.webkit.org/changeset/263742>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403160 [details].