All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Interix catch-ups for recent changes/releases.
@ 2011-05-25 14:15 mduft
  2011-05-25 14:15 ` [PATCH 1/2] Add additional build options for Interix, and remove obsolete ones mduft
  2011-05-25 14:15 ` [PATCH 2/2] Include unistd.h mduft
  0 siblings, 2 replies; 18+ messages in thread
From: mduft @ 2011-05-25 14:15 UTC (permalink / raw)
  To: git


Hey!

Since it has been a while since i last built git on interix, it didn't work out of the box for older interixen.
There are a few small changes required, which i hope should be not a big thing to get included in the git repo.

The removal of two previously required options from the Makefile is because of a special library (libsuacomp: [1]).
This library is required all over the place on interix, and provides those previously missing things. There are
others depending on this (gnulib, findutils, coreutils, ...) for interix support, and thus i think it should not 
be a problem to depend on it, right?

May i ask for a review?

Thanks,
markus

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] Add additional build options for Interix, and remove obsolete ones.
  2011-05-25 14:15 [PATCH] Interix catch-ups for recent changes/releases mduft
@ 2011-05-25 14:15 ` mduft
  2011-05-25 17:51   ` Junio C Hamano
  2011-05-26  6:29   ` Markus Duft
  2011-05-25 14:15 ` [PATCH 2/2] Include unistd.h mduft
  1 sibling, 2 replies; 18+ messages in thread
From: mduft @ 2011-05-25 14:15 UTC (permalink / raw)
  To: git; +Cc: Markus Duft

Interix versions older than 6.0 (so 3.5 and 5.2) both lack
struct sockaddr_storage and the FNM_CASEFOLD GNU extension,
so disable them both.

The removed options are obsolete, because interix support now
depends on libsuacomp.

Signed-off-by: Markus Duft <mduft@gentoo.org>
---
 Makefile |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index db72c45..c47cd4e 100644
--- a/Makefile
+++ b/Makefile
@@ -1113,8 +1113,6 @@ endif
 	X = .exe
 endif
 ifeq ($(uname_S),Interix)
-	NO_SYS_POLL_H = YesPlease
-	NO_INTTYPES_H = YesPlease
 	NO_INITGROUPS = YesPlease
 	NO_IPV6 = YesPlease
 	NO_MEMMEM = YesPlease
@@ -1125,10 +1123,14 @@ ifeq ($(uname_S),Interix)
 	ifeq ($(uname_R),3.5)
 		NO_INET_NTOP = YesPlease
 		NO_INET_PTON = YesPlease
+		NO_SOCKADDR_STORAGE = YesPlease
+		NO_FNMATCH_CASEFOLD = YesPlease
 	endif
 	ifeq ($(uname_R),5.2)
 		NO_INET_NTOP = YesPlease
 		NO_INET_PTON = YesPlease
+		NO_SOCKADDR_STORAGE = YesPlease
+		NO_FNMATCH_CASEFOLD = YesPlease
 	endif
 endif
 ifneq (,$(findstring MINGW,$(uname_S)))
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] Include unistd.h.
  2011-05-25 14:15 [PATCH] Interix catch-ups for recent changes/releases mduft
  2011-05-25 14:15 ` [PATCH 1/2] Add additional build options for Interix, and remove obsolete ones mduft
@ 2011-05-25 14:15 ` mduft
  2011-05-25 18:30   ` Junio C Hamano
                     ` (2 more replies)
  1 sibling, 3 replies; 18+ messages in thread
From: mduft @ 2011-05-25 14:15 UTC (permalink / raw)
  To: git; +Cc: Markus Duft

At least on Interix, NULL is defined in unistd.h, and not including it
causes compilation failure.

Signed-off-by: Markus Duft <mduft@gentoo.org>
---
 compat/fnmatch/fnmatch.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
index 14feac7..0238cca 100644
--- a/compat/fnmatch/fnmatch.c
+++ b/compat/fnmatch/fnmatch.c
@@ -25,6 +25,7 @@
 # define _GNU_SOURCE	1
 #endif
 
+#include <unistd.h>
 #include <errno.h>
 #include <fnmatch.h>
 #include <ctype.h>
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] Add additional build options for Interix, and remove obsolete ones.
  2011-05-25 14:15 ` [PATCH 1/2] Add additional build options for Interix, and remove obsolete ones mduft
@ 2011-05-25 17:51   ` Junio C Hamano
  2011-05-26  6:29   ` Markus Duft
  1 sibling, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2011-05-25 17:51 UTC (permalink / raw)
  To: mduft; +Cc: git

mduft@gentoo.org writes:

> Interix versions older than 6.0 (so 3.5 and 5.2) both lack
> struct sockaddr_storage and the FNM_CASEFOLD GNU extension,
> so disable them both.
>
> The removed options are obsolete, because interix support now
> depends on libsuacomp.

and linkage with -lsuacomp happens automatically without any change in the
Makefile for anybody?  Just asking, as I do not have an access to (nor any
particular desire to get an access to) an Interix to figure it out myself,
and the only think I care about in this patch is if it helps only your
installation or it will help everybody who has Interix but not necessarily
with the same set of additional configuration as you have.

> Signed-off-by: Markus Duft <mduft@gentoo.org>
> ---
>  Makefile |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index db72c45..c47cd4e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1113,8 +1113,6 @@ endif
>  	X = .exe
>  endif
>  ifeq ($(uname_S),Interix)
> -	NO_SYS_POLL_H = YesPlease
> -	NO_INTTYPES_H = YesPlease
>  	NO_INITGROUPS = YesPlease
>  	NO_IPV6 = YesPlease
>  	NO_MEMMEM = YesPlease
> @@ -1125,10 +1123,14 @@ ifeq ($(uname_S),Interix)
>  	ifeq ($(uname_R),3.5)
>  		NO_INET_NTOP = YesPlease
>  		NO_INET_PTON = YesPlease
> +		NO_SOCKADDR_STORAGE = YesPlease
> +		NO_FNMATCH_CASEFOLD = YesPlease
>  	endif
>  	ifeq ($(uname_R),5.2)
>  		NO_INET_NTOP = YesPlease
>  		NO_INET_PTON = YesPlease
> +		NO_SOCKADDR_STORAGE = YesPlease
> +		NO_FNMATCH_CASEFOLD = YesPlease
>  	endif
>  endif
>  ifneq (,$(findstring MINGW,$(uname_S)))

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Include unistd.h.
  2011-05-25 14:15 ` [PATCH 2/2] Include unistd.h mduft
@ 2011-05-25 18:30   ` Junio C Hamano
  2011-05-25 20:37     ` Tor Arntsen
  2011-05-25 21:52     ` Erik Faye-Lund
  2011-05-26  2:20   ` Jonathan Nieder
  2011-05-27  6:27   ` [PATCH 2/2] Include unistd.h Markus Duft
  2 siblings, 2 replies; 18+ messages in thread
From: Junio C Hamano @ 2011-05-25 18:30 UTC (permalink / raw)
  To: mduft; +Cc: git

mduft@gentoo.org writes:

> At least on Interix, NULL is defined in unistd.h, and not including it
> causes compilation failure.
>
> Signed-off-by: Markus Duft <mduft@gentoo.org>
> ---
>  compat/fnmatch/fnmatch.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
> index 14feac7..0238cca 100644
> --- a/compat/fnmatch/fnmatch.c
> +++ b/compat/fnmatch/fnmatch.c
> @@ -25,6 +25,7 @@
>  # define _GNU_SOURCE	1
>  #endif
>  
> +#include <unistd.h>
>  #include <errno.h>
>  #include <fnmatch.h>
>  #include <ctype.h>

The header stddef.h is where NULL is supposed to be defined, and commonly
used headers are supposed to define NULL the same way as stddef.h does.
There is a conditional inclusion of stdlib.h in fnmatch.c and stdlib.h is
one of those files; probably that is how the upstream pulls in NULL when
compiling this.

But we don't define STDC_HEADERS nor _LIBC (and we shouldn't), so I don't
know how the existing users of compat/fnmatch/ gets the defintion of NULL
from. Output from "gcc -E -dD -DNO_FNMATCH compat/fnmatch/fnmatch.c" does
not seem to show any NULL defined.

Other platforms (e.g. SunOS, IRIX, HPUX, Windows) use NO_FNMATCH_CASEFOLD
and compile this file.  How are they getting their NULLs?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Include unistd.h.
  2011-05-25 18:30   ` Junio C Hamano
@ 2011-05-25 20:37     ` Tor Arntsen
  2011-05-25 21:00       ` Junio C Hamano
  2011-05-25 21:52     ` Erik Faye-Lund
  1 sibling, 1 reply; 18+ messages in thread
From: Tor Arntsen @ 2011-05-25 20:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: mduft, git

On Wed, May 25, 2011 at 20:30, Junio C Hamano <gitster@pobox.com> wrote:
> mduft@gentoo.org writes:
>
>> At least on Interix, NULL is defined in unistd.h, and not including it
>> causes compilation failure.
>>
>> Signed-off-by: Markus Duft <mduft@gentoo.org>
>> ---
>>  compat/fnmatch/fnmatch.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
>> index 14feac7..0238cca 100644
>> --- a/compat/fnmatch/fnmatch.c
>> +++ b/compat/fnmatch/fnmatch.c
>> @@ -25,6 +25,7 @@
>>  # define _GNU_SOURCE 1
>>  #endif
>>
>> +#include <unistd.h>
>>  #include <errno.h>
>>  #include <fnmatch.h>
>>  #include <ctype.h>
>
> The header stddef.h is where NULL is supposed to be defined, and commonly
> used headers are supposed to define NULL the same way as stddef.h does.
> There is a conditional inclusion of stdlib.h in fnmatch.c and stdlib.h is
> one of those files; probably that is how the upstream pulls in NULL when
> compiling this.

According to POSIX (well, IEEE Std 1003.1, 2004 Edition at least)
unistd.h must define NULL:

"
The following symbolic constant shall be defined:

NULL
    Null pointer
"

> But we don't define STDC_HEADERS nor _LIBC (and we shouldn't), so I don't
> know how the existing users of compat/fnmatch/ gets the defintion of NULL
> from. Output from "gcc -E -dD -DNO_FNMATCH compat/fnmatch/fnmatch.c" does
> not seem to show any NULL defined.
>
> Other platforms (e.g. SunOS, IRIX, HPUX, Windows) use NO_FNMATCH_CASEFOLD
> and compile this file.  How are they getting their NULLs?

IRIX defines NULL in <unistd.h> and <stddef.h> (the latter via
<internal/stddef_core.h>).
Solaris 10 defines NULL in <unistd.h>, but also in <stddef.h> via
<iso/stddef_iso.h>
Tru64 v5.1 in <unistd.h> and <stddef.h> and a bunch of other places
(string.h, stdio,h, stdlib.h, everywhere)
AIX 5.1 in <unistd.h> and <stddef.h> and just about everywhere.
Linux also gets NULL via <unistd.h> because it includes <stddef.h>,
but then stddef.h isn't in /usr/include, at least on my system -
presumably it's in some compiler-provided place.

For all those non-Linu systems it's always a straight-forward no-dependency
#ifndef NULL
#define NULL 0 /* or sometimes 0L */
#endif

-Tor

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Include unistd.h.
  2011-05-25 20:37     ` Tor Arntsen
@ 2011-05-25 21:00       ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2011-05-25 21:00 UTC (permalink / raw)
  To: Tor Arntsen; +Cc: mduft, git

Tor Arntsen <tor@spacetec.no> writes:

> On Wed, May 25, 2011 at 20:30, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> The header stddef.h is where NULL is supposed to be defined, and commonly
>> used headers are supposed to define NULL the same way as stddef.h does.
>> There is a conditional inclusion of stdlib.h in fnmatch.c and stdlib.h is
>> one of those files; probably that is how the upstream pulls in NULL when
>> compiling this.
>
> According to POSIX (well, IEEE Std 1003.1, 2004 Edition at least)
> unistd.h must define NULL:

Yes, unistd.h is also one of the common header files just like stdlib.h and
stdio.h that are required to define NULL the same way as stddef.h does.

Cf.
    http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/unistd.h.html

>> Other platforms (e.g. SunOS, IRIX, HPUX, Windows) use NO_FNMATCH_CASEFOLD
>> and compile this file.  How are they getting their NULLs?
>
> IRIX defines NULL in ...
> ... presumably it's in some compiler-provided place.

Yes, but that still does not explain where compat/fnmatch/fnmatch.c gets
its NULL from.

> #ifndef NULL
> #define NULL 0 /* or sometimes 0L */
> #endif

You probably would want to say "(void *) 0" if you quote POSIX in the same
message ;-).

Cf.
    http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stddef.h.html

To recap my original question that is not unanswered, the current codebase
does not include unistd.h nor stddef.h when compiling the compatibility
fnmatch.c source we borrowed from glibc, but nobody seems to have
complained that it does not compile due to the lack of NULL, so they must
be getting their NULL from somewhere already, but I couldn't find where.
That is what I was wondering.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Include unistd.h.
  2011-05-25 18:30   ` Junio C Hamano
  2011-05-25 20:37     ` Tor Arntsen
@ 2011-05-25 21:52     ` Erik Faye-Lund
  1 sibling, 0 replies; 18+ messages in thread
From: Erik Faye-Lund @ 2011-05-25 21:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: mduft, git

On Wed, May 25, 2011 at 8:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> mduft@gentoo.org writes:
>
>> At least on Interix, NULL is defined in unistd.h, and not including it
>> causes compilation failure.
>>
>> Signed-off-by: Markus Duft <mduft@gentoo.org>
>> ---
>>  compat/fnmatch/fnmatch.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
>> index 14feac7..0238cca 100644
>> --- a/compat/fnmatch/fnmatch.c
>> +++ b/compat/fnmatch/fnmatch.c
>> @@ -25,6 +25,7 @@
>>  # define _GNU_SOURCE 1
>>  #endif
>>
>> +#include <unistd.h>
>>  #include <errno.h>
>>  #include <fnmatch.h>
>>  #include <ctype.h>
>
> The header stddef.h is where NULL is supposed to be defined, and commonly
> used headers are supposed to define NULL the same way as stddef.h does.
> There is a conditional inclusion of stdlib.h in fnmatch.c and stdlib.h is
> one of those files; probably that is how the upstream pulls in NULL when
> compiling this.
>
> But we don't define STDC_HEADERS nor _LIBC (and we shouldn't), so I don't
> know how the existing users of compat/fnmatch/ gets the defintion of NULL
> from. Output from "gcc -E -dD -DNO_FNMATCH compat/fnmatch/fnmatch.c" does
> not seem to show any NULL defined.
>
> Other platforms (e.g. SunOS, IRIX, HPUX, Windows) use NO_FNMATCH_CASEFOLD
> and compile this file.  How are they getting their NULLs?

Windows/MinGW:

$ gcc -E -dD -DNO_FNMATCH compat/fnmatch/fnmatch.c | grep NULL
compat/fnmatch/fnmatch.c:29:21: error: fnmatch.h: No such file or directory
#define __MINGW_ATTRIB_NONNULL(arg) __attribute__ ((__nonnull__ (arg)))
#undef __need_NULL
#define __need_NULL
#undef NULL
#define NULL ((void *)0)
#undef __need_NULL

The "#define NULL ((void *)0)"-line comes from stddef.h, which is
included from string.h, which in turn is included by fnmatch.c.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Include unistd.h.
  2011-05-25 14:15 ` [PATCH 2/2] Include unistd.h mduft
  2011-05-25 18:30   ` Junio C Hamano
@ 2011-05-26  2:20   ` Jonathan Nieder
  2011-05-26 15:48     ` Junio C Hamano
  2011-05-27  6:27   ` [PATCH 2/2] Include unistd.h Markus Duft
  2 siblings, 1 reply; 18+ messages in thread
From: Jonathan Nieder @ 2011-05-26  2:20 UTC (permalink / raw)
  To: mduft; +Cc: git, Tor Arntsen, Junio C Hamano, Erik Faye-Lund

Hi,

mduft@gentoo.org wrote:

> --- a/compat/fnmatch/fnmatch.c
> +++ b/compat/fnmatch/fnmatch.c
> @@ -25,6 +25,7 @@
>  # define _GNU_SOURCE	1
>  #endif
>  
> +#include <unistd.h>
>  #include <errno.h>

Given that we are touching this file anyway, how about relying on
git-compat-util for this?

That way, there is no need to debug feature test macros, order of
#includes, etc.  Untested.

-- >8 --
Subject: compat/fnmatch: use git-compat-util.h for system headers

Finding the right feature test macros and ordering of #includes to
get the desired functionality from all operating systems can be a big
pain.  Take advantage of the debugging already done and avoid future
churn by using git's usual git-compat-util for this.

In particular, the current fnmatch.c doesn't #include anything that
ought to provide NULL unless HAVE_STRING_H is defined, and it fails to
compile on Interix because of this.  Other platforms must have been
getting the macro through another header.

To make this code easier to reuse and to simplify future automated
merges from upstream, still keep the old #includes, just disabled with
"#if 0".

Reported-by: Markus Duft <mduft@gentoo.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Maybe the old #includes after #include-ing git-compat-util should be
left uncommented because harmless.

 compat/fnmatch/fnmatch.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
index 14feac7..4bf3b5c 100644
--- a/compat/fnmatch/fnmatch.c
+++ b/compat/fnmatch/fnmatch.c
@@ -16,6 +16,9 @@
    write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    Boston, MA 02111-1307, USA.  */
 
+#include "git-compat-util.h"
+
+#if 0
 #if HAVE_CONFIG_H
 # include <config.h>
 #endif
@@ -46,6 +49,7 @@
 # include <wchar.h>
 # include <wctype.h>
 #endif
+#endif
 
 /* Comment out all this code if we are using the GNU C Library, and are not
    actually compiling the library itself.  This code is part of the GNU C
-- 
1.7.5.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] Add additional build options for Interix, and remove obsolete ones.
  2011-05-25 14:15 ` [PATCH 1/2] Add additional build options for Interix, and remove obsolete ones mduft
  2011-05-25 17:51   ` Junio C Hamano
@ 2011-05-26  6:29   ` Markus Duft
  2011-05-26 18:06     ` Junio C Hamano
  1 sibling, 1 reply; 18+ messages in thread
From: Markus Duft @ 2011-05-26  6:29 UTC (permalink / raw)
  To: git, Junio C Hamano

On 05/25/11 16:15, mduft@gentoo.org wrote:

(sorry for actually replying to the wrong mail - wasn't subscribed to the list (but am now.))

>> The removed options are obsolete, because interix support now
>> depends on libsuacomp.
>
> and linkage with -lsuacomp happens automatically without any change in the
> Makefile for anybody?  Just asking, as I do not have an access to (nor any
> particular desire to get an access to) an Interix to figure it out myself,
> and the only think I care about in this patch is if it helps only your
> installation or it will help everybody who has Interix but not necessarily
> with the same set of additional configuration as you have.

Yes. suacomp installs itself as libc.{a,so}. Of course the path to the suacomp prefix needs to be told to the compiler. Without it, interix (at least the newer versions) are near unusable, because of a whole lot of bugs M$ won't fix (as usual...). Suacomp can be installed manually by whoever likes to, and is included in the Gentoo Prefix project automatically (which is my target). I guess without Gentoo Prefix there'd be another whole lot of things missing to build git anyway (haven't tried). Gentoo Prefix also automatically get's all paths right, etc.

Currently, suacomp is already required for things like coreutils, perl, python, openssh, findutils, etc., etc. to work correctly, if not build at all...

Hope that explains some, sorry for not doing so at first :)

Regards, Markus

>
>> Signed-off-by: Markus Duft <mduft <at> gentoo.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Include unistd.h.
  2011-05-26  2:20   ` Jonathan Nieder
@ 2011-05-26 15:48     ` Junio C Hamano
  2011-05-26 16:39       ` Jonathan Nieder
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2011-05-26 15:48 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: mduft, git, Tor Arntsen, Erik Faye-Lund

Jonathan Nieder <jrnieder@gmail.com> writes:

> Given that we are touching this file anyway, how about relying on
> git-compat-util for this?

Many files in compat/ implementation already includes this header, so it
logically feels like a sane thing to do. I vaguely recall there was one
corner case where we didn't want to do this, but I do not remember the
details.

But I am tempted to do the following, as Tor Arntsen suggested, which I
think is the least risky solution. I deliberately spelled "0" without the
(void *) pointer cast, as this code borrowed from upstream is in old K&R
style and nobody talks about "void" elsewhere in the code.

 compat/fnmatch/fnmatch.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
index 14feac7..9473aed 100644
--- a/compat/fnmatch/fnmatch.c
+++ b/compat/fnmatch/fnmatch.c
@@ -127,6 +127,10 @@ extern char *getenv ();
 extern int errno;
 # endif
 
+# ifndef NULL
+#  define NULL 0
+# endif
+
 /* This function doesn't exist on most systems.  */
 
 # if !defined HAVE___STRCHRNUL && !defined _LIBC

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Include unistd.h.
  2011-05-26 15:48     ` Junio C Hamano
@ 2011-05-26 16:39       ` Jonathan Nieder
  2011-05-30  6:51         ` Markus Duft
  0 siblings, 1 reply; 18+ messages in thread
From: Jonathan Nieder @ 2011-05-26 16:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: mduft, git, Tor Arntsen, Erik Faye-Lund

Junio C Hamano wrote:

> --- a/compat/fnmatch/fnmatch.c
> +++ b/compat/fnmatch/fnmatch.c
> @@ -127,6 +127,10 @@ extern char *getenv ();
>  extern int errno;
>  # endif
> +
> +# ifndef NULL
> +#  define NULL 0
> +# endif

Makes a lot of sense.  This fits well with the style of the rest of
the file and other projects using glibc fnmatch could reuse the fix
even if targeting ancient platforms.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] Add additional build options for Interix, and remove obsolete ones.
  2011-05-26  6:29   ` Markus Duft
@ 2011-05-26 18:06     ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2011-05-26 18:06 UTC (permalink / raw)
  To: Markus Duft; +Cc: git

Markus Duft <mduft@gentoo.org> writes:

> On 05/25/11 16:15, mduft@gentoo.org wrote:
>
> (sorry for actually replying to the wrong mail - wasn't subscribed to the list (but am now.))
>
>>> The removed options are obsolete, because interix support now
>>> depends on libsuacomp.
>>
>> and linkage with -lsuacomp happens automatically without any change in the
>> Makefile for anybody?  Just asking, as I do not have an access to (nor any
>> particular desire to get an access to) an Interix to figure it out myself,
>> and the only think I care about in this patch is if it helps only your
>> installation or it will help everybody who has Interix but not necessarily
>> with the same set of additional configuration as you have.
>
> Yes. suacomp installs itself as libc.{a,so}. Of course the path to the
> suacomp prefix needs to be told to the compiler. Without it, interix (at
> least the newer versions) are near unusable,...

You are much more familiar with Interix than I am, and if you were the
only person who uses Interix with git, I would buy that argument
unconditionally.

How has one built and used git before suacomp days? Are these users
extinct? Are there users who do not still use suacomp and for whatever
reason do not want to use it, but still want to use git?

What I am getting at is that I have to come up with a description in the
release notes, and I cannot decide what the entry for this change should
say, and if I can stand behind that statement.

Here is one version, based on my reading of what you said so far:

 * The build procedure for Interix now requires use of suacomp. Older
   versions of Interix that are incompatible with suacomp are no longer
   supported. The Makefile does not automatically tell "the path to the
   suacomp prefix" to the compiler, so you would need to do that yourself.

Doesn't sound pretty, and I hesitate to stand behind such a statement.
Abandoning obsoleted versions of obscure platform nobody cares about is
fine, but at least we should make it clear who are being abandoned by
saying which version. Also "needs to be told to the compiler" part needs
some end-user explanation in Makefile ("set HAVE_SUACOMP=/usr/lib/suacomp
when building on/for Interix", or something).

Here is another possible version (you would need to update your patch to
support both):

 * On Interix, it is preferrable to use suacomp to build git, but the old
   way is still supported. Give HAVE_SUACOMP=/path/to/suacomp to Make (or
   override it in your config.mak) when building git.

That is more easily justifiable, but I cannot tell from this exchange with
you how important to keep supporting the old way is (or if it is even
possible).

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Include unistd.h.
  2011-05-25 14:15 ` [PATCH 2/2] Include unistd.h mduft
  2011-05-25 18:30   ` Junio C Hamano
  2011-05-26  2:20   ` Jonathan Nieder
@ 2011-05-27  6:27   ` Markus Duft
  2 siblings, 0 replies; 18+ messages in thread
From: Markus Duft @ 2011-05-27  6:27 UTC (permalink / raw)
  To: git, Junio C Hamano

(still not getting mail from the list, so copied from the archive...).

> Markus Duft <mduft <at> gentoo.org> writes:
> 
[snip]
>>
>> Yes. suacomp installs itself as libc.{a,so}. Of course the path to the
>> suacomp prefix needs to be told to the compiler. Without it, interix (at
>> least the newer versions) are near unusable,...
> 
> You are much more familiar with Interix than I am, and if you were the
> only person who uses Interix with git, I would buy that argument
> unconditionally.
> 
> How has one built and used git before suacomp days? Are these users
> extinct? Are there users who do not still use suacomp and for whatever
> reason do not want to use it, but still want to use git?

I did start the git porting when i ported Gentoo Prefix [1] to interix. I only ever tried to build it in there, and i actually don't think that there is any other build-case for interix (ATM). Maybe someday someone has the great idea of building/using git without Gentoo Prefix (good luck .. :)), but currently this is (AFAICT) not the case. In Gentoo Prefix in turn, suacomp is always there, on interix, and all versions are supported right now (interix 3.5 was not, but i did the work). Also, the compiler and linker are set up accordingly there.

so, in order:
 1) yes, i did in gentoo prefix, which now uses suacomp
 2) yes, with the appearence of suacomp in gentoo prefix
 3) if a user of gentoo prefix does not yet have suacomp,
    he/she will get upgraded automatically. i do not know
    of other users, and believe they don't exist.

> 
> What I am getting at is that I have to come up with a description in the
> release notes, and I cannot decide what the entry for this change should
> say, and if I can stand behind that statement.
> 
> Here is one version, based on my reading of what you said so far:
> 
>  * The build procedure for Interix now requires use of suacomp. Older
>    versions of Interix that are incompatible with suacomp are no longer
>    supported. The Makefile does not automatically tell "the path to the
>    suacomp prefix" to the compiler, so you would need to do that yourself.

The part with incompatible is no longer true. i tried building git on all interix versions, and it works fine. I feel, that if a user really installs plain interix, wan't git and isntalls suacomp himself, he/she will be power-user enough to manage and tell the build about some include/lib paths.

> 
> Doesn't sound pretty, and I hesitate to stand behind such a statement.
> Abandoning obsoleted versions of obscure platform nobody cares about is
> fine, but at least we should make it clear who are being abandoned by
> saying which version. Also "needs to be told to the compiler" part needs
> some end-user explanation in Makefile ("set HAVE_SUACOMP=/usr/lib/suacomp
> when building on/for Interix", or something).

Hm. suacomp is designed to be completely transparent to (nearly) everything involved in building a package. It should be part of the suacomp setup process to get it into the compiler/linker (by, for example, creating shell script wrappers around compiler/linker, much like colorgcc et. al.). If at all, i'd only set a HAVE_SUACOMP=YesPlease, and based on that, default some symbols to different values (exactly those that the 1/2 patch modifies).

> 
> Here is another possible version (you would need to update your patch to
> support both):
> 
>  * On Interix, it is preferrable to use suacomp to build git, but the old
>    way is still supported. Give HAVE_SUACOMP=/path/to/suacomp to Make (or
>    override it in your config.mak) when building git.

The "old way" was only supported on interix 6.0, as i found out. both interix 5.2 and 3.5 needed at least some additional workarounds, which are now gone with suacomp. I don't think that it would be worth the work (although i'd happily do it, if you want), to support an "old way" which has not a single advantage, and doesn't event work everywhere.

> 
> That is more easily justifiable, but I cannot tell from this exchange with
> you how important to keep supporting the old way is (or if it is even
> possible).

It is possible, but as i said, i don't think it is worth it.

However we proceed: thanks for contemplating about such an exotic platform. i really appreciate it!
Markus

> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Include unistd.h.
  2011-05-26 16:39       ` Jonathan Nieder
@ 2011-05-30  6:51         ` Markus Duft
  2011-06-15  9:31           ` Markus Duft
  0 siblings, 1 reply; 18+ messages in thread
From: Markus Duft @ 2011-05-30  6:51 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git, Tor Arntsen, Erik Faye-Lund

On 05/26/11 18:39, Jonathan Nieder wrote:
> Junio C Hamano wrote:
> 
>> --- a/compat/fnmatch/fnmatch.c
>> +++ b/compat/fnmatch/fnmatch.c
>> @@ -127,6 +127,10 @@ extern char *getenv ();
>>  extern int errno;
>>  # endif
>> +
>> +# ifndef NULL
>> +#  define NULL 0
>> +# endif
> 
> Makes a lot of sense.  This fits well with the style of the rest of
> the file and other projects using glibc fnmatch could reuse the fix
> even if targeting ancient platforms.

I tested, and it works well on interix. Will you commit, or should i prepare a patch?

Thanks
Markus

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Include unistd.h.
  2011-05-30  6:51         ` Markus Duft
@ 2011-06-15  9:31           ` Markus Duft
  2011-06-15  9:48             ` Jonathan Nieder
  0 siblings, 1 reply; 18+ messages in thread
From: Markus Duft @ 2011-06-15  9:31 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git, Tor Arntsen, Erik Faye-Lund

On 05/30/11 08:51, Markus Duft wrote:
> On 05/26/11 18:39, Jonathan Nieder wrote:
>> Junio C Hamano wrote:
>>
>>> --- a/compat/fnmatch/fnmatch.c
>>> +++ b/compat/fnmatch/fnmatch.c
>>> @@ -127,6 +127,10 @@ extern char *getenv ();
>>>  extern int errno;
>>>  # endif
>>> +
>>> +# ifndef NULL
>>> +#  define NULL 0
>>> +# endif
>>
>> Makes a lot of sense.  This fits well with the style of the rest of
>> the file and other projects using glibc fnmatch could reuse the fix
>> even if targeting ancient platforms.
> 
> I tested, and it works well on interix. Will you commit, or should i prepare a patch?

ping... is this one ok now?

about the other patch (1/2), i'm sorry that i messed up the thread a little... my last response was at [1]. could you take a look at it and tell me whether the patch is ok as is?

[1] http://news.gmane.org/find-root.php?message_id=%3c4DDF4440.4040405%40gentoo.org%3e

thanks,
markus

> 
> Thanks
> Markus

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] Include unistd.h.
  2011-06-15  9:31           ` Markus Duft
@ 2011-06-15  9:48             ` Jonathan Nieder
  2011-06-15 11:34               ` [PATCH] Update the Interix default build configuration mduft
  0 siblings, 1 reply; 18+ messages in thread
From: Jonathan Nieder @ 2011-06-15  9:48 UTC (permalink / raw)
  To: Markus Duft; +Cc: Junio C Hamano, git, Tor Arntsen, Erik Faye-Lund

Hi Markus,

Markus Duft wrote:
>>> Junio C Hamano wrote:

>>>> +++ b/compat/fnmatch/fnmatch.c
>>>> @@ -127,6 +127,10 @@ extern char *getenv ();
>>>>  extern int errno;
>>>>  # endif
>>>> +
>>>> +# ifndef NULL
>>>> +#  define NULL 0
>>>> +# endif
[...]
> ping... is this one ok now?

Hopefully yes. :)  "git log --grep=Interix origin/pu | git name-rev
--stdin" tells me that Junio applied it as v1.7.6-rc0~30
(compat/fnmatch/fnmatch.c: give a fall-back definition for NULL,
2011-05-26).

> http://thread.gmane.org/gmane.comp.version-control.git/174407/focus=174600

That one I know less about, but I assume writing a hypothetical blurb
for the release notes (as part of the commit message) would help.

The main question I have is what the positive effect of removing the
NO_SYS_POLL_H and NO_INTTYPES_H settings from the Makefile is --- does
new Gentoo not work with those settings?  Is this a preparatory step
before giving people time to upgrade and then removing support for
those settings from elsewhere in the codebase?  Whatever the answer
is, the log message would be nice to document it.

Thanks and hope that helps,
Jonathan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] Update the Interix default build configuration.
  2011-06-15  9:48             ` Jonathan Nieder
@ 2011-06-15 11:34               ` mduft
  0 siblings, 0 replies; 18+ messages in thread
From: mduft @ 2011-06-15 11:34 UTC (permalink / raw)
  To: git; +Cc: gitster, Markus Duft

Currently, on Interix, libsuacomp is required for building, see [1].
Since suacomp provides poll() and inttypes.h for all interix versions,
the NO_*=YesPleas are removed.

Interix versions 3 and 5 miss struct sockaddr_storage, so make git
avoid using it.

Same for FNMATCH_CASEFOLD, which does not exist for Interix 3 and 5.

[1] http://news.gmane.org/find-root.php?message_id=%3c4DDF4440.4040405%40gentoo.org%3e

Signed-off-by: Markus Duft <mduft@gentoo.org>
---
 Makefile |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index e40ac0c..2939e8d 100644
--- a/Makefile
+++ b/Makefile
@@ -1124,8 +1124,6 @@ endif
 	X = .exe
 endif
 ifeq ($(uname_S),Interix)
-	NO_SYS_POLL_H = YesPlease
-	NO_INTTYPES_H = YesPlease
 	NO_INITGROUPS = YesPlease
 	NO_IPV6 = YesPlease
 	NO_MEMMEM = YesPlease
@@ -1136,10 +1134,14 @@ ifeq ($(uname_S),Interix)
 	ifeq ($(uname_R),3.5)
 		NO_INET_NTOP = YesPlease
 		NO_INET_PTON = YesPlease
+		NO_SOCKADDR_STORAGE = YesPlease
+		NO_FNMATCH_CASEFOLD = YesPlease
 	endif
 	ifeq ($(uname_R),5.2)
 		NO_INET_NTOP = YesPlease
 		NO_INET_PTON = YesPlease
+		NO_SOCKADDR_STORAGE = YesPlease
+		NO_FNMATCH_CASEFOLD = YesPlease
 	endif
 endif
 ifneq (,$(findstring MINGW,$(uname_S)))
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2011-06-15 11:31 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-25 14:15 [PATCH] Interix catch-ups for recent changes/releases mduft
2011-05-25 14:15 ` [PATCH 1/2] Add additional build options for Interix, and remove obsolete ones mduft
2011-05-25 17:51   ` Junio C Hamano
2011-05-26  6:29   ` Markus Duft
2011-05-26 18:06     ` Junio C Hamano
2011-05-25 14:15 ` [PATCH 2/2] Include unistd.h mduft
2011-05-25 18:30   ` Junio C Hamano
2011-05-25 20:37     ` Tor Arntsen
2011-05-25 21:00       ` Junio C Hamano
2011-05-25 21:52     ` Erik Faye-Lund
2011-05-26  2:20   ` Jonathan Nieder
2011-05-26 15:48     ` Junio C Hamano
2011-05-26 16:39       ` Jonathan Nieder
2011-05-30  6:51         ` Markus Duft
2011-06-15  9:31           ` Markus Duft
2011-06-15  9:48             ` Jonathan Nieder
2011-06-15 11:34               ` [PATCH] Update the Interix default build configuration mduft
2011-05-27  6:27   ` [PATCH 2/2] Include unistd.h Markus Duft

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.