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

[Nick Treleaven]

Now I'll answer the rest of your mail Matt.

On 03/10/2013 15:43, Matthew Brush wrote:
> On 13-10-03 05:31 AM, Nick Treleaven wrote:
>> 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.

Didn't capture the output properly - stdout or stderr were missing.

>>> 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.

See my other mail, I made a pull request. I tend to put my code in 
C:\git, but obviously others use paths with spaces in. I didn't realize 
my system() change was the cause of that breakage.

>> 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.

I did try.

>> 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.

It was last year, but I think it wasn't just with dmd, although that 
triggered it the most, and it only happened when dmd wrote more than a 
certain amount to stderr (cascading errors). It was very suspicious that 
the custom spawn code works until there are too many errors, at least 
with dmd.

>> 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

I didn't drop it, Enrico/contributor did some years ago.

> 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?

I don't have any links, but there are many serious sounding bugs in 
Glib's bug tracker for g_spawn on Windows.

>
> 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
>