Last Comment Bug 757397 - Make tests packaging less verbose
: Make tests packaging less verbose
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Gregory Szorc [:gps]
:
: Gregory Szorc [:gps]
Mentors:
http://gregoryszorc.com/blog/2012/05/...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-22 04:25 PDT by Gregory Szorc [:gps]
Modified: 2012-05-24 10:57 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Don't print test files when packaging, v1 (7.66 KB, patch)
2012-05-22 14:44 PDT, Gregory Szorc [:gps]
ted: review+
Details | Diff | Splinter Review

Description Gregory Szorc [:gps] 2012-05-22 04:25:12 PDT
Tests packaging is really verbose (and possibly inefficient). It frequently copies files to the tests stage directory by piping files between two tar processes. Along the way the name of every file being added to the tar archives is being printed to stdout.

Using https://tbpl.mozilla.org/php/getParsedLog.php?id=11927127&tree=Firefox&full=1 as an example, packaging output starts at line 42049 and goes until line 126425. The sample log is 174903 lines long. So, ~48% of the build log lines are output from this packaging step.

I'm having a hard time justifying the existence of nearly half the log lines (which add overhead to RelEng processes) for something that can be discerned simply by listing the contents of an archive. So, I'm requesting that it be removed. If it is justified, I guess WONTFIX is the proper solution. But, think of all those extra bytes and cycles that could be saved!

While this code is being updated, it might be worthwhile to change the file copying to all use the same methodology. e.g. mochitests are using cp -RL and xpcshell tests are using tar | tar. I think cp is cleaner. Perhaps we could even switch to rsync with one of its --delete flags and nuke the initial rm -rf command.

This bug could be expanded to cover suppression of other output during packaging. But, I figured starting small would be best.
Comment 1 Ted Mielczarek [:ted.mielczarek] 2012-05-22 06:00:56 PDT
I'm definitely fine with being less verbose. We have to be careful about other commands, there are a few requirements:
1. Portable (that rules out rsync)
2. Must dereference symlinks

There are probably other things that I've forgotten.
Comment 2 Gregory Szorc [:gps] 2012-05-22 14:44:12 PDT
Created attachment 626193 [details] [diff] [review]
Don't print test files when packaging, v1

Keeping the scope small, this patch preserves the existing copy semantics but essentially removes "-v" from tar and adds "-q" to zip for the test lists. There are other places I could quiet. The testing ones showing up in the diff context are purposefully skipped because I feel those might have slightly higher value for being displayed.

Try at https://tbpl.mozilla.org/?tree=Try&rev=80f20673d1e8
Comment 3 Gregory Szorc [:gps] 2012-05-23 01:32:01 PDT
Try run looks good!
Comment 4 Ted Mielczarek [:ted.mielczarek] 2012-05-23 04:53:01 PDT
Comment on attachment 626193 [details] [diff] [review]
Don't print test files when packaging, v1

Review of attachment 626193 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mochitest/Makefile.in
@@ +204,1 @@
>  	@(cd $(DIST_BIN)/plugins && tar $(TAR_CREATE_FLAGS) - $(TEST_HARNESS_PLUGINS)) | (cd $(PKG_STAGE)/bin/plugins && tar -xf -)

Seems a little confusing to me to leave some of these as quiet and some not, but I don't care too much either way. Want to remove the leading @ from the lines that are quiet so we get the command in the logs, now that it won't be lost in the noise?
Comment 5 Gregory Szorc [:gps] 2012-05-23 07:57:17 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/85e40de1d54c

Removed leading @ from now-quiet lines.

I agree things are kinda inconsistent/confusing now. If someone wants to tackle this, I suggest a follow-up bug: I just want to fry the big fish.
Comment 6 Aki Sasaki [:aki] 2012-05-23 20:21:00 PDT
One of the reasons we have verbose output is to avoid hitting buildbot timeouts (x number of minutes without output).  As long as the zip/unzip steps always take less than 20 minutes, this shouldn't be a problem.
Comment 7 Gregory Szorc [:gps] 2012-05-24 01:48:32 PDT
(In reply to Aki Sasaki [:aki] from comment #6)
> One of the reasons we have verbose output is to avoid hitting buildbot
> timeouts (x number of minutes without output).  As long as the zip/unzip
> steps always take less than 20 minutes, this shouldn't be a problem.

If these test archive operations take more than 20 minutes, the infra is probably burning. </famous_last_words>
Comment 8 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-24 04:49:02 PDT
I wholeheartedly endorse this patch. Scrolling through that useless output is sometimes the bane of my existence :)
Comment 9 Ed Morley [:emorley] 2012-05-24 10:57:31 PDT
https://hg.mozilla.org/mozilla-central/rev/85e40de1d54c

Note You need to log in before you can comment on or make changes to this bug.