Bug 139829 – copy-webkitlibraries-to-product-directory uses wrong SDK when called from build-webkit

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

[Daniel Bates]

The script build-webkit calls copy-webkitlibraries-to-product-directory using --sdk to specify the Xcode SDK to use, but copy-webkitlibraries-to-product-directory only recognizes --sdk-name. The script copy-webkitlibraries-to-product-directory is the only tool that recognizes --sdk-name instead of --sdk. We should make copy-webkitlibraries-to-product-directory consistent with our other tools, remove --sdk-name, and teach it to recognize --sdk.

[Daniel Bates]

Created attachment 243565 [details]
Patch

[Daniel Bates]

Created attachment 243567 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 243567 [details]
Patch

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

> Tools/Scripts/copy-webkitlibraries-to-product-directory:-76
> -    'sdk-name=s' => \$sdkName,

I think that you should also remove this line above:

my $sdkName = ""; # Ideally we only use this for build commands, rather than deciding policies about what needs to get copied or built and where it needs to be placed.

> Tools/Scripts/copy-webkitlibraries-to-product-directory:103
> +sub ranLib($)

This function name lacks a verb. Maybe executeRanlib? Or if there is a more helpful explanation of what this function does, that should be in the name.

[Daniel Bates]

(In reply to comment #3)
> > Tools/Scripts/copy-webkitlibraries-to-product-directory:-76
> > -    'sdk-name=s' => \$sdkName,
> 
> I think that you should also remove this line above:
> 
> my $sdkName = ""; # Ideally we only use this for build commands, rather than
> deciding policies about what needs to get copied or built and where it needs
> to be placed.

Will remove.

> 
> > Tools/Scripts/copy-webkitlibraries-to-product-directory:103
> > +sub ranLib($)
> 
> This function name lacks a verb. Maybe executeRanlib? Or if there is a more
> helpful explanation of what this function does, that should be in the name.

Will rename to updateTableOfContentsForLibrary.

[Daniel Bates]

(In reply to comment #4)
> > 
> > > Tools/Scripts/copy-webkitlibraries-to-product-directory:103
> > > +sub ranLib($)
> > 
> > This function name lacks a verb. Maybe executeRanlib? Or if there is a more
> > helpful explanation of what this function does, that should be in the name.
> 
> Will rename to updateTableOfContentsForLibrary.

Actually, there are many references through out this script to ranlib. For now, I'm going to go with your suggestion and rename the function to executeRanlib() for consistency. I suggest we look to update the terminology used in this script in another patch.

[Daniel Bates]

Committed r177602: <http://trac.webkit.org/changeset/177602>