Chromium buildbot: Avoid "zip" command dependency
Created attachment 63423 [details] Patch
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.
Created attachment 63558 [details] Patch 2
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 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.
Mark: Do you think it would be ok to use the built in python module zipfile on all platforms?
(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.
(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.
http://trac.webkit.org/changeset/64959 might have broken Leopard Intel Release (Tests)