Bug 176965 – teach build.webkit.org to include run-webkit-archive in the root folder of uploaded macOS archives

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

[Lucas Forschler]

Once run-webkit-archive is checked in, we'll need to teach our build system to include it in archives.
This could be done as an additional step, or possibly inside the existing archive step.

The script should be added to the root folder of the archive, so it sits beside the 'Release' or 'Debug' frameworks directory.
This can be done with a command like:
zip -j filename.zip /path/to/OpenSource/Tools/Scripts/run-webkit-archive

[Radar WebKit Bug Importer]

<rdar://problem/34445866>

[Lucas Forschler]

Created attachment 320920 [details]
v1 patch for review

[Lucas Forschler]

Created attachment 320926 [details]
v2 patch, this one changes the working folder to the script folder, and will allow double click execution

[Lucas Forschler]

Created attachment 320927 [details]
v2 patch, this one changes the working folder to the script folder, and will allow double click execution

upload the correct patch this time.

[Alexey Proskuryakov]

Comment on attachment 320927 [details]
v2 patch, this one changes the working folder to the script folder, and will allow double click execution

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

> Tools/ChangeLog:8
> +        * BuildSlaveSupport/built-product-archive:

This is not new to the patch, but the name of the script doesn't seem to parse. Is it "build product archive", or "archive built product"?

> Tools/BuildSlaveSupport/built-product-archive:141
> +    command = ['zip', '-j', archiveFile, PATH_TO_LAUNCHER]

FWIW I like to use full paths in scripts, /usr/bin/zip.

> Tools/BuildSlaveSupport/built-product-archive:161
> +        return subprocess.call(command) or addLauncherToArchive(archiveFile)   

trailing spaces in this line

> Tools/Scripts/run-webkit-archive:56
>      os.environ['DYLD_LIBRARY_PATH'] = dyld_path

Looking at this, I'm wondering if we also need __XPC_ variants. Did you confirm that the downloaded frameworks are actually used in WebContent?

> Tools/Scripts/run-webkit-archive:60
> +    script_path = os.path.abspath(__file__)

These changes are not mentioned in ChangeLog.

Not new to the patch either, but this file should probably be hidden somewhere (maybe in Tools/BuildSlaveSupport). It's not great to add a new script in a directory that's in every WebKit developer's path when we are not going to ever run it from this location.

[Lucas Forschler]

Comment on attachment 320927 [details]
v2 patch, this one changes the working folder to the script folder, and will allow double click execution

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

>> Tools/ChangeLog:8
>> +        * BuildSlaveSupport/built-product-archive:
> 
> This is not new to the patch, but the name of the script doesn't seem to parse. Is it "build product archive", or "archive built product"?

./Tools/BuildSlaveSupport/built-product-archive
I don't care for this name either, but it's been around a decade.

>> Tools/BuildSlaveSupport/built-product-archive:141
>> +    command = ['zip', '-j', archiveFile, PATH_TO_LAUNCHER]
> 
> FWIW I like to use full paths in scripts, /usr/bin/zip.

I can update this to use full path, however there are many other instances in this file which do not use the full path. I'll leave them as-is, and update this one.

>> Tools/BuildSlaveSupport/built-product-archive:161
>> +        return subprocess.call(command) or addLauncherToArchive(archiveFile)   
> 
> trailing spaces in this line

fixed.

>> Tools/Scripts/run-webkit-archive:56
>>      os.environ['DYLD_LIBRARY_PATH'] = dyld_path
> 
> Looking at this, I'm wondering if we also need __XPC_ variants. Did you confirm that the downloaded frameworks are actually used in WebContent?

I did check, I think we are good.

>> Tools/Scripts/run-webkit-archive:60
>> +    script_path = os.path.abspath(__file__)
> 
> These changes are not mentioned in ChangeLog.
> 
> Not new to the patch either, but this file should probably be hidden somewhere (maybe in Tools/BuildSlaveSupport). It's not great to add a new script in a directory that's in every WebKit developer's path when we are not going to ever run it from this location.

These changes were not meant for this patch. I'll move them to their own change.

[Lucas Forschler]

Committed revision 222121.
Note: moved the run-webkit-archive to it's new home in Tools/BuildSlaveSupport.

[Lucas Forschler]

Committed revision 222122. (remove script from old location)