RE: [PATCH 0/3 V3] Test mingw32 GDB in cygwin

["Pierre Muller" <pierre dot muller at ics-cnrs dot unistra dot fr>]

Hi all,


> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Yao Qi
> Envoyé : lundi 29 juillet 2013 10:46
> À : gdb-patches@sourceware.org
> Objet : [PATCH 0/3 V3] Test mingw32 GDB in cygwin
> 
> This patch series try to fix the problems we've seen on running
> mingw32 native for testing in cygwin.  Patch 2/3 unbuffer the
> stdout and stderr, so that dejagnu/expect can match the output in
> the right order.  Likewise, patch 3/3 sets stdin/stdout/stderr into
> binary mode, so that dejagnu/expects can match the eol correctly
> too.  In order to avoid the side effects of these changes to native
> win32 platform, we need some bits to detect whether GDB is running
> in cygwin.  This is what patch 1/3 tries to do, and most of
> discussions are on it.
> 
> In V2, I proposed a new GDB option '--cygwin-tty' to tell GDB that
> it is in cygwin.  People don't like it, and Corinna gave an example
> that we can detect whether GDB is in cygwin or not.

  I didn't like it because I explained that the changes you propose are
useful
to run the testsuite on Windows OS, but not only if running
under a cygwin terminal.
  I am using a msys port of dejagnu expect,
and this needs the same changes (remove buffering and switch to binary
mode),
but with the new version of your patch, 
nothing would happen for me and the testsuite would still fail.


 
> Thanks to Corinna's example, we can know whether GDB is in cygwin
> by checking the file name of handler of stdin (or stdout).  As a
> result, a new option '--cygwin-tty' is avoided.  Patch 1/3 is
> almost rewritten in V3.

  Yes, that's true, but the down side is that you transform
the terminal settings only when you are on a cygwin terminal...
 I think this is not correct:
 I suspect that is_in_cygwin will return true on every type of cygwin
terminal,
in particular also when you are in interactive mode... 
  I thought that your patch should only change settings if the pipes
from expect redirection are detected, but there are no pipe checks
anymore...
Note that I am not sure of the above, as 'maybe' the pty/tty name doesn't
match in the interactive shell case.
  Secondly, your patch will never trigger the no buffer/binary mode changes
on non-cygwin terminals, while I argued before that it should also be usable
in those conditions.
  
  This is why I would rather like to have the settings modification
grouped into a function (let's call it setup_handles_for_testsuite)
which would be an empty function for all but __MINGW32__ code.
This function could then be called by
(gdb) set maint testsuite-mode on
command or automatically when a cygwin shell and pipes are detected.

  I am sorry to bother you again, but I would really like to
get a patch that can be used in more general framework and
that avoids to change the standard handles when not necessary.

  And, to finish, I would like to reiterate my support for this
patch, even though I am still criticizing it.

Sorry to bother you more,


Pierre Muller