[Geany-Devel] Ping on Bug #943 - windows build command

[Matthew Brush]

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