Created attachment 226356 [details] [review]
run-dialog: Avoid double forking; it breaks "pkexec"

Hi Colin,

I quickly tested the patch and it does seem to work fine for the ALT-F2 launcher, thanks.

It still an incomplete fix for gnome-panel though, as you might have .desktop files using "Exec=pkexec foo" and starting such an application via the menu still fails.

(In reply to comment #2)
> Hi Colin,
> 
> I quickly tested the patch and it does seem to work fine for the ALT-F2
> launcher, thanks.

Ok since I know you speak C, mind marking the patch as "accepted-commit-now"?  The point is to have some peer review of changes, not to block all patches until one or two people look at it.  If it was an invasive change, we'd want someone from MAINTAINERS to sign off.

> It still an incomplete fix for gnome-panel though, as you might have .desktop
> files using "Exec=pkexec foo" and starting such an application via the menu
> still fails.

Oh...right.  Ugh, this eggdesktopfile stuff is going to take a bit to unwind.  But we can at least do the Alt-F2 fix first.

Created attachment 226376 [details] [review]
Avoid double forking when launching apps; it breaks "pkexec"

Now also fixes the main panel launcher

Actually eggdesktopfile wasn't used; this patch should work, but I didn't test it.

(In reply to comment #3)
> (In reply to comment #2)
> > Hi Colin,
> > 
> > I quickly tested the patch and it does seem to work fine for the ALT-F2
> > launcher, thanks.
> 
> Ok since I know you speak C, mind marking the patch as "accepted-commit-now"? 

Stupid question: How can I do that?

(In reply to comment #6)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Hi Colin,
> > > 
> > > I quickly tested the patch and it does seem to work fine for the ALT-F2
> > > launcher, thanks.
> > 
> > Ok since I know you speak C, mind marking the patch as "accepted-commit-now"? 
> 
> Stupid question: How can I do that?

In the "Attachments" list, click "Review".  You can comment there, and there's a patch status option in the bottom right.

Review of attachment 226376 [details] [review]:

Looks good to me aside from some minor nitpicks (dummy_child_watch and gather_pid_callback in gnome-panel/libpanel-util/panel-launch.c use spaces whereas everywhere else you use tabs).
It also refers to https://bugzilla.gnome.org/show_bug.cgi?id=675789 in the code comments while it probaby should refer to https://bugzilla.gnome.org/show_bug.cgi?id=686059 . The latter is filed against gnome-panel and linked to Bug#675789.

Forgot to add: Tested patch in GNOME fallback mode and I could successfully start applications via pkexec through both the ALT+F2 launcher and the menu.

Colin, are you going to push this ?

Attachment 226376 [details] pushed as 76acc5b - Avoid double forking when launching apps; it breaks "pkexec"