Chromium buildbot: Avoid "zip" command dependency

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Kent Tamura]

Chromium buildbot: Avoid "zip" command dependency

[Kent Tamura]

Created attachment 63423 [details]
Patch

[Tony Chang]

Comment on attachment 63423 [details]
Patch

Why do we only do this for Chromium?  Why not use the same code for all platforms?

> diff --git a/WebKitTools/BuildSlaveSupport/test-result-archive b/WebKitTools/BuildSlaveSupport/test-result-archive
> +    for file in files:
> +        zipFileOrDirectory(zipper, file)

I think you can use os.walk instead of adding this helper method.

[Kent Tamura]

Created attachment 63558 [details]
Patch 2

[Kent Tamura]

Thank you for reviewing.

(In reply to comment #2)
> Why do we only do this for Chromium?  Why not use the same code for all platforms?

I'm not sure if other platforms want to avoid zip command, and I'd like to test this change only on Chromium bots for now :-)

> > +    for file in files:
> > +        zipFileOrDirectory(zipper, file)
> 
> I think you can use os.walk instead of adding this helper method.

ok, I updated the code with os.walk.

[Tony Chang]

Comment on attachment 63558 [details]
Patch 2

> diff --git a/WebKitTools/BuildSlaveSupport/test-result-archive b/WebKitTools/BuildSlaveSupport/test-result-archive
> +            for dirPath, dirNames, fileNames in os.walk(file):
> +                for fileName in fileNames:
> +                    relativePath = os.path.join(dirPath, fileName)
> +                    print "Adding ", relativePath

Nit: I think print automatically puts a space between each argument.  Not a big deal, but I think this puts 2 spaces between "Adding" and the path.  Same below.

[Tony Chang]

Mark: Do you think it would be ok to use the built in python module zipfile on all platforms?

[Mark Rowe (bdash)]

(In reply to comment #6)
> Mark: Do you think it would be ok to use the built in python module zipfile on all platforms?

Assuming that it works on all of the relevant platforms and generates zip files that are consistent with what the existing approach is generating, I see no reason not to.

[Kent Tamura]

(In reply to comment #5)
> > +                    print "Adding ", relativePath
> 
> Nit: I think print automatically puts a space between each argument.  Not a big deal, but I think this puts 2 spaces between "Adding" and the path.  Same below.

Thanks.  Fixed.


Landed as r64959.

[WebKit Review Bot]

http://trac.webkit.org/changeset/64959 might have broken Leopard Intel Release (Tests)