[Support] Always wait for GraphViz before opening the viewer

[Meinersbur]

Summary

When calling DisplayGraph and a PS viewer is chosen, two programs are executed: The GraphViz generator and the PostScript viewer. Always for the generator to finish to ensure that the .ps file is written before opening the viewer for that file. DisplayGraph's wait parameter refers to whether to wait until the user closes the viewer.

This happened on Windows and if none of the options to open the .dot file directly applies, also on Linux.

[Meinersbur]

ping

[aaron.ballman]

Will this cause incorrect behavior on non-Windows platforms where wait was false, but now causes the wait to happen. Specifically, I'm wondering about callers of ViewGraph().

~Aaron

[dwiberg]

This looks good to me. I do however not have commit rights which means I can't give an official approval. I did encounter the issue this patch fixes when testing the changes in D11877.

A suggestion for a follow-up patch is to rename "ExecGraphViewer" since it's used both for generation and viewing of the graph which can be a bit confusing.

[Meinersbur]

In D11876#226031, @aaron.ballman wrote:

Will this cause incorrect behavior on non-Windows platforms where wait was false, but now causes the wait to happen. Specifically, I'm wondering about callers of ViewGraph().

I'd say the old behavior is incorrect on all platforms. If GraphViz takes longer to execute than it takes for the viewer to launch, it won't work on any system. I don't know why this is much more likely to happen on Windows.

I am working on Linux as well and it works nicely there. I don't know about MacOS as I don't have an Apple.

[aaron.ballman]

I would guess that testing this is prohibitively problematic, but the logic seems sound. LGTM!

[Meinersbur]

I guess nobody notices this yet because Linux systems have xdg-open which will handle the .dot file directly ("open" on OSX). Forcing using a ps viewer also opens the viewer before the file has been generated on Linux.

lib/Support/GraphWriter.cpp

if (PSViewer &&
     args.push_back("-Gsize=7.5,10");
     args.push_back(Filename.c_str());
     args.push_back("-o");
     args.push_back(PSFilename.c_str());
     args.push_back(nullptr);
 
     errs() << "Running '" << GeneratorPath << "' program... ";
 
-    if (ExecGraphViewer(GeneratorPath, args, Filename, wait, ErrMsg))
+    if (ExecGraphViewer(GeneratorPath, args, Filename, true, ErrMsg))
       return true;
 
     args.clear();
     args.push_back(ViewerPath.c_str());
     switch (PSViewer) {
     case PSV_OSXOpen:
       args.push_back("-W");
       args.push_back(PSFilename.c_str());