Page MenuHomePhabricator

[GraphWriter] Don't wait on xdg-open when not on Apple
ClosedPublic

Authored by chatur01 on Jul 1 2015, 10:34 AM.

Details

Summary

Hi,

I could be way off with this patch, but it seems like the waiting behaviour introduced in 255cd51fbd77 (2012) and its use extended by D6534 won’t work on systems using xdg-open, since there’s no option to have xdg-open wait on the application it launches AFAICT, and if there is, then GraphWriter is not making use of it.

What happens on my computer is that xdg-open launches a dot file viewer, then immediately returns, and then the GraphWriter quickly deletes all the temporary dot files, and then the dot viewers get upset, if they even got a chance to launch.

This patch moves the “wait” behaviour to only Apple platforms, where the “open” command actually has a wait option.

Diff Detail

Event Timeline

chatur01 retitled this revision from to [GraphWriter] Don't wait on xdg-open when not on Apple.
chatur01 updated this object.
chatur01 edited the test plan for this revision. (Show Details)
chatur01 added reviewers: MatzeB, atrick, chandlerc.
chatur01 set the repository for this revision to rL LLVM.
chatur01 added a subscriber: Unknown Object (MLST).
MatzeB accepted this revision.Jul 1 2015, 11:05 AM
MatzeB edited edge metadata.

The patch itself LGTM.

As we are on the topic: I think the current approach of having llvm spawn the viewer and wait/not wait for it should be changed: I'd really prefer llvm just writing a bunch of .dot files to the current directory (or /tmp) giving them some sensible names+numbers and maybe print those names.

This would avoid llvm having to find and launch the correct viewer. In my experience I also often have cases where n graphs are generated but I am only interested in one of them which forces me to close n-1 graph viewers.

This revision is now accepted and ready to land.Jul 1 2015, 11:05 AM
chatur01 closed this revision.Jul 2 2015, 2:39 AM

As we are on the topic: I think the current approach of having llvm
spawn the viewer and wait/not wait for it should be changed: I'd
really prefer llvm just writing a bunch of .dot files to the current
directory (or /tmp) giving them some sensible names+numbers and maybe
print those names.

This would avoid llvm having to find and launch the correct viewer. In
my experience I also often have cases where n graphs are generated but
I am only interested in one of them which forces me to close n-1 graph
viewers.

I suppose you mean the same thing as the -dot-* options to the opt
tool? I agree that would be handy extension to llc for the
visualisation tools too.

Landed r241250 in meantime. Thank you for reviewing my patch!

Kind regards, Charlie.

In my experience I also often have cases where n graphs are generated but I am only interested in one of them which forces me to close n-1 graph viewers.

I've just noticed there's a -filter-view-dags=name_of_BB_you_want_to_see that solves this problem. Still get random temp names for the dot file though.