Closed
Bug 757397
Opened 13 years ago
Closed 12 years ago
Make tests packaging less verbose
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla15
People
(Reporter: gps, Assigned: gps)
References
()
Details
Attachments
(1 file)
7.66 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
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•13 years ago
|
||
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.
Assignee | ||
Comment 2•12 years ago
|
||
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
Assignee | ||
Comment 3•12 years ago
|
||
Try run looks good!
Comment 4•12 years ago
|
||
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?
Attachment #626193 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 5•12 years ago
|
||
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.
Target Milestone: --- → mozilla15
Assignee | ||
Updated•12 years ago
|
Comment 6•12 years ago
|
||
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.
Assignee | ||
Comment 7•12 years ago
|
||
(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•12 years ago
|
||
I wholeheartedly endorse this patch. Scrolling through that useless output is sometimes the bane of my existence :)
Comment 9•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•