[PATCH] D114845: [llvm] [Debuginfod] DebuginfodCollection and DebuginfodServer for tracking local debuginfo.

[Frank Ch. Eigler via Phabricator via llvm-commits]

fche2 added inline comments.


================
Comment at: llvm/lib/Debuginfod/Debuginfod.cpp:217-218
+    return Err;
+  outs() << "finished first update\n";
+  outs().flush();
+  std::this_thread::sleep_for(Interval);
----------------
noajshu wrote:
> mysterymath wrote:
> > This seems a bit odd to unconditionally put to stdout; is this necessary?
> > 
> > Otherwise, the whole routine just becomes
> > ```
> > while (true) {
> >   if (Error Err = update())
> >     return Err;
> >   std::this_thread::sleep_for(Interval);
> > }
> > return Error::success();
> > ```
> Thanks, I agree this is awkward. The first update is logged to stdout is so the lit tests in D114846 can tell when it's safe to query the server for debuginfo. If the test client pinged the server before it had found the binary it will return a 404 and the test will flake.
> 
> An alternative is to add a time delay before the client pings the server. This is discouraged as there is timing variability across systems.
> 
> We could hide this output except for when "verbose" logging is enabled, or similar. Would this be good?
(For reference, the elfutils debuginfod exports prometheus metrics about the progress of its operations, so that an external testsuite process can synchronize.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114845/new/

https://reviews.llvm.org/D114845