Bug 43470 - Chromium buildbot: Avoid "zip" command dependency
Summary: Chromium buildbot: Avoid "zip" command dependency
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other All
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-04 00:28 PDT by Kent Tamura
Modified: 2010-08-08 19:32 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.99 KB, patch)
2010-08-04 00:31 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 2 (3.11 KB, patch)
2010-08-04 23:43 PDT, Kent Tamura
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2010-08-04 00:28:11 PDT
Chromium buildbot: Avoid "zip" command dependency
Comment 1 Kent Tamura 2010-08-04 00:31:37 PDT
Created attachment 63423 [details]
Patch
Comment 2 Tony Chang 2010-08-04 09:12:36 PDT
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.
Comment 3 Kent Tamura 2010-08-04 23:43:22 PDT
Created attachment 63558 [details]
Patch 2
Comment 4 Kent Tamura 2010-08-04 23:48:29 PDT
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.
Comment 5 Tony Chang 2010-08-05 09:47:06 PDT
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.
Comment 6 Tony Chang 2010-08-05 09:49:42 PDT
Mark: Do you think it would be ok to use the built in python module zipfile on all platforms?
Comment 7 Mark Rowe (bdash) 2010-08-05 11:45:52 PDT
(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.
Comment 8 Kent Tamura 2010-08-08 18:34:08 PDT
(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.
Comment 9 WebKit Review Bot 2010-08-08 19:32:05 PDT
http://trac.webkit.org/changeset/64959 might have broken Leopard Intel Release (Tests)