Closed Bug 757397 Opened 13 years ago Closed 12 years ago

Make tests packaging less verbose

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla15

People

(Reporter: gps, Assigned: gps)

References

()

Details

Attachments

(1 file)

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.
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.
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: nobody → gps
Status: NEW → ASSIGNED
Attachment #626193 - Flags: review?(ted.mielczarek)
Try run looks good!
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+
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
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.
(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>
I wholeheartedly endorse this patch. Scrolling through that useless output is sometimes the bane of my existence :)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: