cc'ing the devel list.
Hi Matt,
I'll have to reply to this email properly tomorrow.
In the meantime, there are 3 spawning variants: I'm confident that the
custom process spawning code is definitely seriously buggy. I really do
need the system() code. I tried, but couldn't fix it. I know that last
time I tried it, the g_spawn didn't capture enough output. At least one
user - the one that contributed it - also had problems with g_spawn
capturing, and wrote the custom process spawning.
I suggest picking whichever one you and others think is best as the
default, but have a hidden pref to enable at least the system() version.
If you don't want to do that, I'll try to find some time, but I'm not
sure when yet.
On 03/10/2013 15:43, Matthew Brush wrote:
On 13-10-03 05:31 AM, Nick Treleaven wrote:
Moving to devel list.
On 02/10/2013 22:58, Matthew Brush wrote:
On 13-10-02 04:40 AM, Nick Treleaven wrote:
On 01/10/2013 22:28, Matthew Brush wrote:
On 13-10-01 05:34 AM, Nick Treleaven wrote:
Just checked my email, was away last week. I'll have a look at it,
but I
doubt the GLib spawning has been fixed. Either way it should be
definitely be tested before reverting to the broken GLib spawn. I
know
GLib spawn doesn't work with my version of GLib/gtk, which isn't that
old (1yr?).
Locally on my Win7 VM I have reverted the patch and removed all the
existing SYNC_SPAWN-guarded stuff and rid of win32-specific and to
just
use GLib spawning same code on all platforms and it runs easily as
good
as it does before the changes (just did some basic testing though).
It's
still not nice and non-blocking like it is on Linux, but seems to
be no
worse-off after removing like 200 lines of code :)
If you push your branch I'll try it. I'll need to use it for a few
weeks
to be convinced though. I remember the problem only occurred for
certain
things, maybe running make or non-gnu binaries or something.
For testing purposes it should be as simple as disabling the SYNC_SPAWN
workaround[1].
I tried that a while ago, it didn't work. That was when I added the
SYNC_SPAWN macro.
"Didn't work" how? The only time it didn't work for me was with old GTK+
2.16 binaries the gspawn-win32-helper program would crash instantly
causing a Microsoft dialog to come up asking to debug the executable. It
would be interesting to investigate the original original problem and
maybe we can find a solution that gets rid of the other work-arounds all
together.
In the meantime, I suggest reverting the "fix" until the actual problem
is identified and a fix is properly implemented. There's only 1
extremely minor conflict when reverting.
IMO changing/breaking path quoting is far less important than having a
Maybe to you, but it's very common on Windows to have spaces in the path
names, it's even the default on Windows XP (Documents and Settings). To
most people (ex. the bug reporters) it seems outright broken now,
whereas before it worked.
spawn that doesn't block Geany or outright hang. This definitely
It would be nice to figure out *why* it was hanging and fix *that*
problem though, not just drop the existing workaround code and write 3rd
workaround that's arguably worse[1] than the original issue.
happened very frequently when using dmd from dlang.org. See:
https://github.com/geany/geany/commit/a3664fae9ece396952d732cc937e63192d8c6f76
I don't remember seeing any bug reports for this, so it's possible it's
limited to you or the specific application you were using, maybe a bug
in a certain version of DMD? The commit messages doesn't explain what
the actual root of the problem was or why it could not be fixed
properly, or what was tried to fix it before abandoning the existing code.
BTW the blocking and hanging was with the custom windows process
spawning code, not with GLib's spawning. GLib IME just doesn't report
the output properly, or at least not both stdout and stderr for all
binaries.
What did you try to fix it before dropping asynchronous spawning? How
did it improperly report its output? Did it cause the handlers for the
IO channels not to always fire or something? What programs didn't it
work with? What versions were tested? Can it be reproduced in a clean
sample program outside of Geany?
BTW, do you have a link to the upstream bug report for this issue? I
googled a lot, and besides this old fixed issue[2], I don't seem to find
much besides a few Geany-related things that just say "don't work"
without additional details. Did anyone from Geany report the bug
upstream or make note of which upstream bug is causing the original
original issue?
Cheers,
Matthew Brush
[1]: Causes a command prompt to popup, doesn't support filenames with
spaces, uses a temp file, uses system(), leaves a bunch of complicated
dead code, now 3rd way to spawn process, etc.
[2]: https://bugzilla.gnome.org/show_bug.cgi?id=510664
_______________________________________________
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel
