bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Daniel Colascione]

In Cygwin 1.7, vfork is an alias for fork.  The vfork the #define in
cygwin.h was protecting against is gone.  It's better to use vfork now
so that when Cygwin gains a new, working vfork, we use it
automatically.
---
 src/s/cygwin.h |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/src/s/cygwin.h b/src/s/cygwin.h
index 8f5a0ab..e025e46 100644
--- a/src/s/cygwin.h
+++ b/src/s/cygwin.h
@@ -81,10 +81,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #define HAVE_SOCKETS
 
-/* vfork() interacts badly with setsid(), causing ptys to fail to
-   change their controlling terminal */
-#define vfork fork
-
 /* This should work (at least when compiling with gcc).  But I have no way
    or intention to verify or even test it.  If you encounter a problem with
    it, feel free to change this setting, but please add a comment here about
-- 
1.7.5.1

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Lars Magne Ingebrigtsen]

Daniel Colascione <dancol@dancol.org> writes:

> In Cygwin 1.7, vfork is an alias for fork.  The vfork the #define in
> cygwin.h was protecting against is gone.  It's better to use vfork now
> so that when Cygwin gains a new, working vfork, we use it
> automatically.

[...]

> -/* vfork() interacts badly with setsid(), causing ptys to fail to
> -   change their controlling terminal */
> -#define vfork fork

Sounds sensible to me, but I'm no exactly (ahem) a Cygwin expert.

Eli, what do you think?

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Chong Yidong]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Daniel Colascione <dancol@dancol.org> writes:
>
>> In Cygwin 1.7, vfork is an alias for fork.  The vfork the #define in
>> cygwin.h was protecting against is gone.  It's better to use vfork now
>> so that when Cygwin gains a new, working vfork, we use it
>> automatically.
>
> [...]
>
>> -/* vfork() interacts badly with setsid(), causing ptys to fail to
>> -   change their controlling terminal */
>> -#define vfork fork
>
> Sounds sensible to me, but I'm no exactly (ahem) a Cygwin expert.
>
> Eli, what do you think?

Ken Brown is the Emacs maintainer for Cygwin.

Ken, any objection to the above proposed change?

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Eli Zaretskii]

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Cc: 10398@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> Date: Wed, 11 Apr 2012 01:26:56 +0200
> 
> Daniel Colascione <dancol@dancol.org> writes:
> 
> > In Cygwin 1.7, vfork is an alias for fork.  The vfork the #define in
> > cygwin.h was protecting against is gone.  It's better to use vfork now
> > so that when Cygwin gains a new, working vfork, we use it
> > automatically.
> 
> [...]
> 
> > -/* vfork() interacts badly with setsid(), causing ptys to fail to
> > -   change their controlling terminal */
> > -#define vfork fork
> 
> Sounds sensible to me, but I'm no exactly (ahem) a Cygwin expert.
> 
> Eli, what do you think?

Not that _I_ am a Cygwin expert, but isn't it safer to do that only
for Cygwin 1.7 and later?  Ken, WDYT?

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Ken Brown]

On 4/11/2012 2:58 AM, Eli Zaretskii wrote:
>> From: Lars Magne Ingebrigtsen<larsi@gnus.org>
>> Cc: 10398@debbugs.gnu.org, Eli Zaretskii<eliz@gnu.org>
>> Date: Wed, 11 Apr 2012 01:26:56 +0200
>>
>> Daniel Colascione<dancol@dancol.org>  writes:
>>
>>> In Cygwin 1.7, vfork is an alias for fork.  The vfork the #define in
>>> cygwin.h was protecting against is gone.  It's better to use vfork now
>>> so that when Cygwin gains a new, working vfork, we use it
>>> automatically.
>>
>> [...]
>>
>>> -/* vfork() interacts badly with setsid(), causing ptys to fail to
>>> -   change their controlling terminal */
>>> -#define vfork fork
>>
>> Sounds sensible to me, but I'm no exactly (ahem) a Cygwin expert.
>>
>> Eli, what do you think?
>
> Not that _I_ am a Cygwin expert, but isn't it safer to do that only
> for Cygwin 1.7 and later?  Ken, WDYT?

I think it's a good change.  And I don't think we should make an effort 
to support Cygwin versions earlier than 1.7.  Cygwin 1.5 is no longer 
supported by Cygwin, so I don't see a reason for emacs to try to support 
it.  I suspect there have been many emacs changes that are not 
compatible with Cygwin 1.5.

Ken

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Eli Zaretskii]

> Date: Wed, 11 Apr 2012 07:15:46 -0400
> From: Ken Brown <kbrown@cornell.edu>
> CC: Lars Magne Ingebrigtsen <larsi@gnus.org>, dancol@dancol.org,
>         10398@debbugs.gnu.org, Chong Yidong <cyd@stupidchicken.com>
> 
> I don't think we should make an effort to support Cygwin versions
> earlier than 1.7.  Cygwin 1.5 is no longer supported by Cygwin, so I
> don't see a reason for emacs to try to support it.  I suspect there
> have been many emacs changes that are not compatible with Cygwin
> 1.5.

Fair enough, but then perhaps the configure script should at least
warn, or maybe even refuse to continue, if it sees an older version?

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Ken Brown]

On 4/11/2012 7:39 AM, Eli Zaretskii wrote:
>> Date: Wed, 11 Apr 2012 07:15:46 -0400
>> From: Ken Brown<kbrown@cornell.edu>
>> CC: Lars Magne Ingebrigtsen<larsi@gnus.org>, dancol@dancol.org,
>>          10398@debbugs.gnu.org, Chong Yidong<cyd@stupidchicken.com>
>>
>> I don't think we should make an effort to support Cygwin versions
>> earlier than 1.7.  Cygwin 1.5 is no longer supported by Cygwin, so I
>> don't see a reason for emacs to try to support it.  I suspect there
>> have been many emacs changes that are not compatible with Cygwin
>> 1.5.
>
> Fair enough, but then perhaps the configure script should at least
> warn, or maybe even refuse to continue, if it sees an older version?

A warning would be fine, but I would suggest waiting until someone 
reports an actual problem.  My guess is that no one running Cygwin 1.5 
will ever try to build the emacs trunk.  But if someone does try and 
runs into problems, we can take the appropriate action.  If you 
disagree, I have no objection to inserting a warning now.

Ken

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Eli Zaretskii]

> Date: Wed, 11 Apr 2012 08:35:02 -0400
> From: Ken Brown <kbrown@cornell.edu>
> CC: larsi@gnus.org, dancol@dancol.org, 10398@debbugs.gnu.org,
>         cyd@stupidchicken.com
> 
> >> I don't think we should make an effort to support Cygwin versions
> >> earlier than 1.7.  Cygwin 1.5 is no longer supported by Cygwin, so I
> >> don't see a reason for emacs to try to support it.  I suspect there
> >> have been many emacs changes that are not compatible with Cygwin
> >> 1.5.
> >
> > Fair enough, but then perhaps the configure script should at least
> > warn, or maybe even refuse to continue, if it sees an older version?
> 
> A warning would be fine, but I would suggest waiting until someone 
> reports an actual problem.  My guess is that no one running Cygwin 1.5 
> will ever try to build the emacs trunk.  But if someone does try and 
> runs into problems, we can take the appropriate action.  If you 
> disagree, I have no objection to inserting a warning now.

I think we should put our money where our mouth is.  If we consider
Cygwin 1.5 unsupported, we should at least warn the prospective users
that they are building Emacs with unsupported tools.

So yes, I think we should insert a warning now.  Could you please make
such a change?  TIA.

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Ken Brown]

On 4/11/2012 3:26 PM, Eli Zaretskii wrote:
> I think we should put our money where our mouth is.  If we consider
> Cygwin 1.5 unsupported, we should at least warn the prospective users
> that they are building Emacs with unsupported tools.
> 
> So yes, I think we should insert a warning now.  Could you please make
> such a change?  TIA.

How's this:

=== modified file 'configure.in'
--- configure.in        2012-04-02 04:38:31 +0000
+++ configure.in        2012-04-12 02:38:52 +0000
@@ -3805,6 +3805,14 @@
    echo
 fi

+if test "${opsys}" = "cygwin"; then
+   cygwin_rel=`uname -r|sed -e 's/\(^1\.[[579]]\)\..*/\1/'`
+   if test "${cygwin_rel}" = "1.5"; then
+      echo
+      echo "*** Warning: Building Emacs on Cygwin 1.5 is not supported."
+      echo
+   fi
+fi

 # Remove any trailing slashes in these variables.
 [test "${prefix}" != NONE &&

FYI, here's what the output of `uname -r' looks like on Cygwin:

$ uname -r
1.7.14s(0.260/5/3)

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Glenn Morris]

Ken Brown wrote:

> +      echo
> +      echo "*** Warning: Building Emacs on Cygwin 1.5 is not supported."
> +      echo

I think AC_MSG_WARN is the standard way to do that kind of thing.
See eg the one for Gtk+ already in configure.in.

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Eli Zaretskii]

> Date: Wed, 11 Apr 2012 22:51:45 -0400
> From: Ken Brown <kbrown@cornell.edu>
> CC: larsi@gnus.org, dancol@dancol.org, 10398@debbugs.gnu.org,
>         cyd@stupidchicken.com
> 
> --- configure.in        2012-04-02 04:38:31 +0000
> +++ configure.in        2012-04-12 02:38:52 +0000
> @@ -3805,6 +3805,14 @@
>     echo
>  fi
> 
> +if test "${opsys}" = "cygwin"; then
> +   cygwin_rel=`uname -r|sed -e 's/\(^1\.[[579]]\)\..*/\1/'`
> +   if test "${cygwin_rel}" = "1.5"; then
> +      echo
> +      echo "*** Warning: Building Emacs on Cygwin 1.5 is not supported."
> +      echo
> +   fi
> +fi

The text of the warning looks good to me, thanks.

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Andreas Schwab]

Ken Brown <kbrown@cornell.edu> writes:

> +if test "${opsys}" = "cygwin"; then
> +   cygwin_rel=`uname -r|sed -e 's/\(^1\.[[579]]\)\..*/\1/'`

Why do you need the filter?

      case `uname -r` in 1.5.*) ...

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Ken Brown]

On 4/12/2012 3:40 AM, Andreas Schwab wrote:
> Ken Brown<kbrown@cornell.edu>  writes:
>
>> +if test "${opsys}" = "cygwin"; then
>> +   cygwin_rel=`uname -r|sed -e 's/\(^1\.[[579]]\)\..*/\1/'`
>
> Why do you need the filter?
>
>        case `uname -r` in 1.5.*) ...

Thanks.  And thanks to Glenn for telling me about AC_MSG_WARN.  I've 
committed the change to the trunk (bzr revision 107877), and I'm closing 
the bug.

Ken

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Drew Adams]

> I think it's a good change.  And I don't think we should make 
> an effort to support Cygwin versions earlier than 1.7.
> Cygwin 1.5 is no longer supported by Cygwin, so I don't see
> a reason for emacs to try to support it.  I suspect there
> have been many emacs changes that are not compatible with
> Cygwin 1.5.

Dunno what is meant here by "support Cygwin", but FWIW (probably not much) I
still use Cygwin 1.5.  I do so because it seems to work OK for my use cases and
because I have come across lots of problems reported here and there wrt later
versions (and Emacs).

(Yes, I know, you can find whatever problems you want on the Internet, real or
wrong.  Still, why upgrade if I don't need to?)

I do not run Emacs inside Cygwin, but I use Cygwin's `grep' and other commands
from Emacs, and I use Cygwin's bash shell interactively.

I probably won't be upgrading Cygwin anytime soon unless I'm forced to.  But I'm
no doubt not your typical Cygwin user.  Just one data point to consider.  Or
not.

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Ken Brown]

On 4/11/2012 1:48 PM, Drew Adams wrote:
>> I think it's a good change.  And I don't think we should make
>> an effort to support Cygwin versions earlier than 1.7.
>> Cygwin 1.5 is no longer supported by Cygwin, so I don't see
>> a reason for emacs to try to support it.  I suspect there
>> have been many emacs changes that are not compatible with
>> Cygwin 1.5.
>
> Dunno what is meant here by "support Cygwin", but FWIW (probably not much) I
> still use Cygwin 1.5.  I do so because it seems to work OK for my use cases and
> because I have come across lots of problems reported here and there wrt later
> versions (and Emacs).
>
> (Yes, I know, you can find whatever problems you want on the Internet, real or
> wrong.  Still, why upgrade if I don't need to?)
>
> I do not run Emacs inside Cygwin, but I use Cygwin's `grep' and other commands
> from Emacs, and I use Cygwin's bash shell interactively.

The change we're talking about here won't affect you.  It's only 
relevant to people working in Cygwin and building the Emacs trunk (for 
use within Cygwin).

Ken

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Drew Adams]

> The change we're talking about here won't affect you.  It's only 
> relevant to people working in Cygwin and building the Emacs 
> trunk (for use within Cygwin).

Got it.  Sorry for the noise.  Thx.

bug#10398: [PATCH] Use real vfork under Cygwin: it's fine these days

[Lars Magne Ingebrigtsen]

Daniel Colascione <dancol@dancol.org> writes:

> In Cygwin 1.7, vfork is an alias for fork.  The vfork the #define in
> cygwin.h was protecting against is gone.  It's better to use vfork now
> so that when Cygwin gains a new, working vfork, we use it
> automatically.

I've now applied your patch to the Emacs trunk.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/