Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1771143002:
DM: verbose and quiet logging (Closed)
DescriptionDM: verbose and quiet logging
If you pass no flags, output is pretty much unchanged from today: calls to
info() go to the console, calls to vlog() are ignored.
If you pass --verbose and -w dir, all calls to info(), vlog() and all of stderr
are redirected to dir/verbose.log. info() calls also go to the console,
unless you pass --quiet, in which case nothing goes to the console at all.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1771143002
Committed: https://skia.googlesource.com/skia/+/c41fd9251dad3e2e2797436a4ccfaf0490914fe0
Patch Set 1 #Patch Set 2 : require --verbose too #Patch Set 3 : more C++11 #
Total comments: 2
Patch Set 4 : boring #Messages
Total messages: 31 (12 generated)
Description was changed from ========== DM: verbose and quiet logging If you pass no flags, output is pretty much unchanged from today: calls to info() go to the console, calls to vlog() are ignored. If you pass -w dir, all calls to info(), vlog() and all of stderr are redirected to dir/verbose.log. info() calls also go to the console, unless you pass --quiet, in which case nothing goes to the console at all. BUG=skia: ========== to ========== DM: verbose and quiet logging If you pass no flags, output is pretty much unchanged from today: calls to info() go to the console, calls to vlog() are ignored. If you pass -w dir, all calls to info(), vlog() and all of stderr are redirected to dir/verbose.log. info() calls also go to the console, unless you pass --quiet, in which case nothing goes to the console at all. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by mtklein@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1771143002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1771143002/1
Description was changed from ========== DM: verbose and quiet logging If you pass no flags, output is pretty much unchanged from today: calls to info() go to the console, calls to vlog() are ignored. If you pass -w dir, all calls to info(), vlog() and all of stderr are redirected to dir/verbose.log. info() calls also go to the console, unless you pass --quiet, in which case nothing goes to the console at all. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== DM: verbose and quiet logging If you pass no flags, output is pretty much unchanged from today: calls to info() go to the console, calls to vlog() are ignored. If you pass --verbose and -w dir, all calls to info(), vlog() and all of stderr are redirected to dir/verbose.log. info() calls also go to the console, unless you pass --quiet, in which case nothing goes to the console at all. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by mtklein@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1771143002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1771143002/20001
mtklein@chromium.org changed reviewers: + borenet@google.com, bsalomon@google.com, halcanary@google.com
Brian, Hal: please see if this scratches your various logging itches? Eric: is it possible for us to get this new log also linked on the buildbot results, like stdio is today?
The CQ bit was checked by mtklein@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1771143002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1771143002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
On 2016/03/08 00:39:32, mtklein_C wrote: > Brian, Hal: please see if this scratches your various logging itches? > > Eric: is it possible for us to get this new log also linked on the buildbot > results, like stdio is today? Yes, it's possible, but it would disappear once we switch to swarming.
On 2016/03/08 14:30:02, borenet wrote: > On 2016/03/08 00:39:32, mtklein_C wrote: > > Brian, Hal: please see if this scratches your various logging itches? > > > > Eric: is it possible for us to get this new log also linked on the buildbot > > results, like stdio is today? > > Yes, it's possible, but it would disappear once we switch to swarming. Can we make it not disappear when we switch to swarming?
On 2016/03/08 14:45:03, mtklein wrote: > On 2016/03/08 14:30:02, borenet wrote: > > On 2016/03/08 00:39:32, mtklein_C wrote: > > > Brian, Hal: please see if this scratches your various logging itches? > > > > > > Eric: is it possible for us to get this new log also linked on the buildbot > > > results, like stdio is today? > > > > Yes, it's possible, but it would disappear once we switch to swarming. > > Can we make it not disappear when we switch to swarming? AFAIK the only way to do it is to include the log file in the output isolate, download it in the recipe and add a step which cats the file and attaches its contents to the DM step. That's assuming we *want* to download the output isolate, which at the moment is true for DM and nanobench but not for compiles.
On 2016/03/08 14:48:56, borenet wrote: > On 2016/03/08 14:45:03, mtklein wrote: > > On 2016/03/08 14:30:02, borenet wrote: > > > On 2016/03/08 00:39:32, mtklein_C wrote: > > > > Brian, Hal: please see if this scratches your various logging itches? > > > > > > > > Eric: is it possible for us to get this new log also linked on the > buildbot > > > > results, like stdio is today? > > > > > > Yes, it's possible, but it would disappear once we switch to swarming. > > > > Can we make it not disappear when we switch to swarming? > > AFAIK the only way to do it is to include the log file in the output isolate, > download it in the recipe and add a step which cats the file and attaches its > contents to the DM step. That's assuming we *want* to download the output > isolate, which at the moment is true for DM and nanobench but not for compiles. We must be downloading dm.json and all the images, right? Can we just do whatever we do for them? (It might be neat to have dm.json show up on the bot page too.) Does it help if we shove this all in one big-ass string in dm.json?
On 2016/03/08 14:59:52, mtklein wrote: > On 2016/03/08 14:48:56, borenet wrote: > > On 2016/03/08 14:45:03, mtklein wrote: > > > On 2016/03/08 14:30:02, borenet wrote: > > > > On 2016/03/08 00:39:32, mtklein_C wrote: > > > > > Brian, Hal: please see if this scratches your various logging itches? > > > > > > > > > > Eric: is it possible for us to get this new log also linked on the > > buildbot > > > > > results, like stdio is today? > > > > > > > > Yes, it's possible, but it would disappear once we switch to swarming. > > > > > > Can we make it not disappear when we switch to swarming? > > > > AFAIK the only way to do it is to include the log file in the output isolate, > > download it in the recipe and add a step which cats the file and attaches its > > contents to the DM step. That's assuming we *want* to download the output > > isolate, which at the moment is true for DM and nanobench but not for > compiles. > > We must be downloading dm.json and all the images, right? Can we just do > whatever we do for them? (It might be neat to have dm.json show up on the bot > page too.) > > Does it help if we shove this all in one big-ass string in dm.json? Yeah, at the moment the recipe downloads all of DM's outputs so that it can upload them to GS, so that should all be feasible.
no objections to the change. https://codereview.chromium.org/1771143002/diff/40001/dm/DM.cpp File dm/DM.cpp (right): https://codereview.chromium.org/1771143002/diff/40001/dm/DM.cpp#newcode92 dm/DM.cpp:92: FLAGS_quiet || printf(fmt, args...); nit: We never use this syntax. if (!FLAGS_quiet) { printf(fmt, args...); } is much more readable. (I don't mind one-line conditionals, as long as {} are used.)
The CQ bit was checked by mtklein@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1771143002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1771143002/60001
Hal, does --quiet do what you were asking for? https://codereview.chromium.org/1771143002/diff/40001/dm/DM.cpp File dm/DM.cpp (right): https://codereview.chromium.org/1771143002/diff/40001/dm/DM.cpp#newcode92 dm/DM.cpp:92: FLAGS_quiet || printf(fmt, args...); On 2016/03/08 15:09:34, Hal Canary wrote: > nit: We never use this syntax. > > if (!FLAGS_quiet) { printf(fmt, args...); } > > is much more readable. (I don't mind one-line conditionals, as long as {} are > used.) Done.
> Yeah, at the moment the recipe downloads all of DM's outputs so that it can > upload them to GS, so that should all be feasible. Cool. I'm gonna take "should all be feasible" as a green light here.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...) Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
On 2016/03/08 15:15:56, mtklein wrote: > Hal, does --quiet do what you were asking for? I think so. I have not tested it.
The CQ bit was checked by mtklein@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com Link to the patchset: https://codereview.chromium.org/1771143002/#ps60001 (title: "boring")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1771143002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1771143002/60001
Message was sent while issue was closed.
Description was changed from ========== DM: verbose and quiet logging If you pass no flags, output is pretty much unchanged from today: calls to info() go to the console, calls to vlog() are ignored. If you pass --verbose and -w dir, all calls to info(), vlog() and all of stderr are redirected to dir/verbose.log. info() calls also go to the console, unless you pass --quiet, in which case nothing goes to the console at all. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== DM: verbose and quiet logging If you pass no flags, output is pretty much unchanged from today: calls to info() go to the console, calls to vlog() are ignored. If you pass --verbose and -w dir, all calls to info(), vlog() and all of stderr are redirected to dir/verbose.log. info() calls also go to the console, unless you pass --quiet, in which case nothing goes to the console at all. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/c41fd9251dad3e2e2797436a4ccfaf0490914fe0 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://skia.googlesource.com/skia/+/c41fd9251dad3e2e2797436a4ccfaf0490914fe0 |
