patch, 2 partsAdd additional build options for Interix, and remove obsolete ones.

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

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

mduft@gentoo.org writes:

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.

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

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

Markus Duft <mduft@gentoo.org> writes:

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

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

mduft@gentoo.org writes:

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?

On Wed, May 25, 2011 at 20:30, Junio C Hamano <gitster@pobox.com> wrote:

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
"

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

Tor Arntsen <tor@spacetec.no> writes:

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

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

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.

On Wed, May 25, 2011 at 8:30 PM, Junio C Hamano <gitster@pobox.com> wrote:

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.

Hi,

mduft@gentoo.org wrote:

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

Jonathan Nieder <jrnieder@gmail.com> writes:

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

Junio C Hamano wrote:

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.

On 05/26/11 18:39, Jonathan Nieder wrote:

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

Thanks Markus

On 05/30/11 08:51, Markus Duft wrote:

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

Hi Markus,

Markus Duft wrote:

[...]

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

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

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

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

[snip]

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.

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.

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

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.

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