[webkit-reviews] review denied: [Bug 210548] [Flatpak SDK] Not fully hooked in BuildSlaveSupport : [Attachment 396536] Patch

[bugzilla-daemon at webkit.org]

Carlos Alberto Lopez Perez <clopez at igalia.com> has denied Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 210548: [Flatpak SDK] Not fully hooked in BuildSlaveSupport
https://bugs.webkit.org/show_bug.cgi?id=210548

Attachment 396536: Patch

https://bugs.webkit.org/attachment.cgi?id=396536&action=review




--- Comment #4 from Carlos Alberto Lopez Perez <clopez at igalia.com> ---
Comment on attachment 396536
  --> https://bugs.webkit.org/attachment.cgi?id=396536
Patch

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

>> Tools/BuildSlaveSupport/built-product-archive:295
>> +	    flatpakutils.run_in_sandbox_if_available(sys.argv)
> 
> I think this its not going to work for our use case.
> Because the bots have a modified PATH and instead of the real "zip" command
they run a wrapper script that does some magic to instead of uploading the real
build-products only uploading a fake zip with a README with a pointer to the
built-product on a local server (so they avoid uploading/downloading hundreds
of MBs on each build to internet).
> And if we make this script run inside the flatpak environment, then it will
use the real "zip" command instead of the zipwrapper script.
> Let me test this, but i suspect it won't work

Tested it, it doesn't work.

Instead of running this script inside flatpak I think we need to change the
path it uses for the build-directory from WebKitBuild/$Configuration to
WebKitBuild/$PORT/$Configuration

See determineWebKitBuildDirectories() in
Tools/BuildSlaveSupport/built-product-archive

And only do that when the third-party userFlatpak directory its present and
WEBKIT_JHBUILD env variable its not defined