cygwin-latest: compile errors related to sockaddr_storage, dirent->d_type and dirent->d_ino

For reasons unknown, cygwin decided to use our sockaddr_storage.
As it is redefined to sockaddr_in it'd cause compilation errors in
cygwin headers. Fixed by first patch, which uses a more git-related
name (can we claim rights for the symbol, being first to use it? :-)

For the other, probably unrelated, reasons, they decided to leave
declarations of DT_* macros in dirent.h without providing dirent->d_type.
This is what NO_DTYPE in the 0002-patch is all about.

And on top of that, they removed dirent->d_ino (or probably replaced
it by __ino32, if at all).
BTW, can we somehow avoid using d_ino? It is referenced only in fsck-objects.c
Anyway, to workaround this I put

  COMPAT_CFLAGS += -Dd_ino=__ino32

It helps, but surely is not the solution.

P.S. For all you poor souls on Win2k, my config.mak contains:

  NO_MMAP = YesPlease
  NO_DTYPE = YesPlease
  COMPAT_CFLAGS += -Dd_ino=__ino32

Just to make it work (at least it's enough for me, even the tests run...)

On Wed, Jan 18, 2006 at 02:47:00PM +0100, Alex Riesen wrote:

For reasons unknown, cygwin decided to use our sockaddr_storage.
As it is redefined to sockaddr_in it'd cause compilation errors in
cygwin headers. Fixed by first patch, which uses a more git-related
name (can we claim rights for the symbol, being first to use it? :-)

For the other, probably unrelated, reasons, they decided to leave
declarations of DT_* macros in dirent.h without providing dirent->d_type.
This is what NO_DTYPE in the 0002-patch is all about.

"They" probably would like to hear about any irregularities that are found.
"They" probably don't like it when people treat an open source project as
if it was some unresponsive proprietary enterprise which does not listen
to or accept patches.

And on top of that, they removed dirent->d_ino (or probably replaced it
by __ino32, if at all).  BTW, can we somehow avoid using d_ino?  It is
referenced only in fsck-objects.c Anyway, to workaround this I put

COMPAT_CFLAGS += -Dd_ino=__ino32

It helps, but surely is not the solution.

I don't see how it could help since __ino32 is not actually filled in
with anything.  In fact, I'll rename the field to __invalid_ino32 to
make that clear.

cgf
(cygwin project lead)

On 1/19/06, Christopher Faylor [off-list ref] wrote:

"They" probably don't like it when people treat an open source project as
if it was some unresponsive proprietary enterprise which does not listen
to or accept patches.

Please, accept my appologies for the sarcasm in the original post.
Sometimes I get an impression of cygwin being not maintained at
all, and that, if not justifies my behavior, but at least is an attempt
to explain it.

quoted

And on top of that, they removed dirent->d_ino (or probably replaced it
by __ino32, if at all).  BTW, can we somehow avoid using d_ino?  It is
referenced only in fsck-objects.c Anyway, to workaround this I put

COMPAT_CFLAGS += -Dd_ino=__ino32

It helps, but surely is not the solution.

I don't see how it could help since __ino32 is not actually filled in
with anything.  In fact, I'll rename the field to __invalid_ino32 to
make that clear.

But why keep the DT_-macros?! And why there is two fields
hinting at d_ino, and why there is 3 (!) "struct dirent"
definitions in dirent.h (sys/dirent.h)?
Some with different names (d_reserved?).
And if cygwin is aiming for posix, what would d_fd or d_version
be (Open Group Specs v6[1] mention only d_ino and d_name)?

[1] http://www.opengroup.org/onlinepubs/009695399/basedefs/dirent.h.html

Dear diary, on Wed, Jan 18, 2006 at 02:47:00PM CET, I got a letter
where Alex Riesen [off-list ref] said that...

For reasons unknown, cygwin decided to use our sockaddr_storage.
As it is redefined to sockaddr_in it'd cause compilation errors in
cygwin headers. Fixed by first patch, which uses a more git-related
name (can we claim rights for the symbol, being first to use it? :-)

Huh? "Our"? See RFC 2553 and e.g.:

	http://www.opengroup.org/onlinepubs/009695399/basedefs/sys/socket.h.html

You have no business meddling with this identifier except working around
platforms which do not support it, but then do not complain that things
break when the platforms actually start supporting it. ;-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Of the 3 great composers Mozart tells us what it's like to be human,
Beethoven tells us what it's like to be Beethoven and Bach tells us
what it's like to be the universe.  -- Douglas Adams

I actually forgot to look at the patch.

Dear diary, on Wed, Jan 18, 2006 at 02:47:00PM CET, I got a letter
where Alex Riesen [off-list ref] said that...

  +else
  +	ALL_CFLAGS += -Dsockaddr_stg_git=sockaddr_storage

 endif
 
 ifdef PPC_SHA1

  But of course when this goes on, soon the gcc commandline will get
really awfully ugly. This is why something _like_ autoconf is a good
thing - you can just detect if the system headers provide the type and
#define it only when they don't.

  Of course, inserting the #define to some .h file might work; I'm not
sure if any of the relevant socket routines is permitted to be a macro
which could expand to something - but even then, it's probably not very
likely that it would break stuff in practice.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Of the 3 great composers Mozart tells us what it's like to be human,
Beethoven tells us what it's like to be Beethoven and Bach tells us
what it's like to be the universe.  -- Douglas Adams

On 1/19/06, Petr Baudis [off-list ref] wrote:

Dear diary, on Wed, Jan 18, 2006 at 02:47:00PM CET, I got a letter
where Alex Riesen [off-list ref] said that...

quoted

For reasons unknown, cygwin decided to use our sockaddr_storage.
As it is redefined to sockaddr_in it'd cause compilation errors in
cygwin headers. Fixed by first patch, which uses a more git-related
name (can we claim rights for the symbol, being first to use it? :-)

Huh? "Our"? See RFC 2553 and e.g.:

        http://www.opengroup.org/onlinepubs/009695399/basedefs/sys/socket.h.html

You have no business meddling with this identifier except working around
platforms which do not support it, but then do not complain that things
break when the platforms actually start supporting it. ;-)

That will remind me to consult the specs first.

On 1/19/06, Petr Baudis [off-list ref] wrote:

  But of course when this goes on, soon the gcc commandline will get
really awfully ugly. This is why something _like_ autoconf is a good
thing - you can just detect if the system headers provide the type and
#define it only when they don't.

not you. Someone has done the job of detecting things for you.
It's just a fair amount of hard work badly done.

On Thu, Jan 19, 2006 at 11:42:46AM +0100, Alex Riesen wrote:

On 1/19/06, Christopher Faylor [off-list ref] wrote:

quoted

"They" probably don't like it when people treat an open source project
as if it was some unresponsive proprietary enterprise which does not
listen to or accept patches.

Please, accept my appologies for the sarcasm in the original post.
Sometimes I get an impression of cygwin being not maintained at all,
and that, if not justifies my behavior, but at least is an attempt to
explain it.

Hmm.  I thought we'd already dispelled the myth that cygwin is
unsupported in this very mailing list.  That is an odd impression given
the fact that you were complaining about behavior in a version of cygwin
which was released on Monday but, apology accepted.

If you want to see evidence of continual cygwin development, you can always
visit this page:  http://cygwin.com/snapshots/ .  This page has snapshots
of cygwin built from cvs.  We make these available so that people will check
things out prior to an actual release.

quoted

quoted

And on top of that, they removed dirent->d_ino (or probably replaced it
by __ino32, if at all).  BTW, can we somehow avoid using d_ino?  It is
referenced only in fsck-objects.c Anyway, to workaround this I put

COMPAT_CFLAGS += -Dd_ino=__ino32

It helps, but surely is not the solution.

I don't see how it could help since __ino32 is not actually filled in
with anything.  In fact, I'll rename the field to __invalid_ino32 to
make that clear.

But why keep the DT_-macros?! And why there is two fields hinting at
d_ino, and why there is 3 (!)

The default entry (i.e., the one you get without defining
__INSIDE_CYGWIN__ or __CYGWIN_USE_BIG_TYPES__) in dirent.h is the
correct one.

"struct dirent" definitions in dirent.h (sys/dirent.h)?  Some with
different names (d_reserved?).  And if cygwin is aiming for posix, what
would d_fd or d_version be (Open Group Specs v6[1] mention only d_ino
and d_name)?

[1]
http://www.opengroup.org/onlinepubs/009695399/basedefs/dirent.h.html

Hmm.  On linux, my /usr/include/bits/dirent.h has a d_reclen field in
dirent.  I know what that is and what it is used for but it's not
mentioned, that I can see, in SUSv3.  But, since I don't see anything in
the description of dirent in SUSv3 which says that the must have only
the fields mentiond, that's ok.

In any event, we don't claim to be POSIX compatible.  We actually are
working for linux compatibility but this is one regrettable place where
Windows doesn't allow that.

I explained about the DT macros and why we dropped d_ino support in
another message.

Anyway, I understand why the DT macros would cause problems and I have
removed them from the current CVS.  I don't see why the existence of
extra fields in dirent or why other non-default definitions would
cause any problems other than the "Doctor, doctor, it hurts when I
do this" variety.

cgf

Christopher Faylor, Thu, Jan 19, 2006 19:31:43 +0100:

quoted

quoted

"They" probably don't like it when people treat an open source project
as if it was some unresponsive proprietary enterprise which does not
listen to or accept patches.

Please, accept my appologies for the sarcasm in the original post.
Sometimes I get an impression of cygwin being not maintained at all,
and that, if not justifies my behavior, but at least is an attempt to
explain it.

Hmm.  I thought we'd already dispelled the myth that cygwin is
unsupported in this very mailing list.  That is an odd impression given
the fact that you were complaining about behavior in a version of cygwin
which was released on Monday but, apology accepted.

It was my first update since a long time (which BTW broke some
programs like cp: they missed symbols in cygwin1.dll).

If you want to see evidence of continual cygwin development, you can always
visit this page:  http://cygwin.com/snapshots/ .  This page has snapshots
of cygwin built from cvs.  We make these available so that people will check
things out prior to an actual release.

Thanks.

quoted

quoted

quoted

And on top of that, they removed dirent->d_ino (or probably replaced it
by __ino32, if at all).  BTW, can we somehow avoid using d_ino?  It is
referenced only in fsck-objects.c Anyway, to workaround this I put

COMPAT_CFLAGS += -Dd_ino=__ino32

It helps, but surely is not the solution.

I don't see how it could help since __ino32 is not actually filled in
with anything.  In fact, I'll rename the field to __invalid_ino32 to
make that clear.

But why keep the DT_-macros?! And why there is two fields hinting at
d_ino, and why there is 3 (!)

The default entry (i.e., the one you get without defining
__INSIDE_CYGWIN__ or __CYGWIN_USE_BIG_TYPES__) in dirent.h is the
correct one.

Maybe it'd be a good idea just to remove the definitions? Or, as
__INSIDE_CYGWIN__ implies, move them into cygwin internal sources.
Would be less confusion and no chance of someone defining one of the
macros and getting a binary-incompatible object?

quoted

"struct dirent" definitions in dirent.h (sys/dirent.h)?  Some with
different names (d_reserved?).  And if cygwin is aiming for posix, what
would d_fd or d_version be (Open Group Specs v6[1] mention only d_ino
and d_name)?

[1]
http://www.opengroup.org/onlinepubs/009695399/basedefs/dirent.h.html

Hmm.  On linux, my /usr/include/bits/dirent.h has a d_reclen field in
dirent.  I know what that is and what it is used for but it's not
mentioned, that I can see, in SUSv3.  But, since I don't see anything in
the description of dirent in SUSv3 which says that the must have only
the fields mentiond, that's ok.

Of course, you don't have to. It all about making an impression

In any event, we don't claim to be POSIX compatible.  We actually are
working for linux compatibility but this is one regrettable place where
Windows doesn't allow that.

The word was "aiming"

Anyway, I understand why the DT macros would cause problems and I have
removed them from the current CVS.  I don't see why the existence of
extra fields in dirent or why other non-default definitions would
cause any problems other than the "Doctor, doctor, it hurts when I
do this" variety.

It is not the existance of the extra fields which cause problems. It
is an existance of fields, the names of which imply a functionality
they do not provide which causes problems. Why should I seeing __ino32
in an official header think: "it is never filled anyway, so I
shouldn't use it"?! Or what could "__invalid_d_ino" mean? If it is
invalid (as in "can't be used", why is it there at all?

On Thu, Jan 19, 2006 at 11:08:43PM +0100, Alex Riesen wrote:

Christopher Faylor, Thu, Jan 19, 2006 19:31:43 +0100:

quoted

Hmm.  I thought we'd already dispelled the myth that cygwin is
unsupported in this very mailing list.  That is an odd impression given
the fact that you were complaining about behavior in a version of
cygwin which was released on Monday but, apology accepted.

It was my first update since a long time (which BTW broke some programs
like cp: they missed symbols in cygwin1.dll).

Detailed bug reports are always welcome on the cygwin mailing list:
cygwin at cygwin dot com .  I should point out that the existence of
bugs in a product does not indicate a lack of support of the product,
however.

Maybe it'd be a good idea just to remove the definitions? Or, as
__INSIDE_CYGWIN__ implies, move them into cygwin internal sources.
Would be less confusion and no chance of someone defining one of the
macros and getting a binary-incompatible object?

I sincerely doubt that anyone in this mailing list wants to get into a
discussion of cygwin design.  If you really want to discuss this, the
cygwin mailing list is a much better place for that.

cgf

Alex Riesen [off-list ref] writes:

For reasons unknown, cygwin decided to use our sockaddr_storage.
As it is redefined to sockaddr_in it'd cause compilation errors in
cygwin headers.

Just removing "-Dsockaddr_storage=sockaddr_in" from ALL_CFLAGS
seems to solve the problem for new Cygwin.

I doubt that -Dsockaddr_storage=sockaddr_in should be tied to
NO_IPV6.  Maybe a better approach would be something like the
attached patch.

Although /usr/include/cygwin/in.h seems to have struct sockaddr_in6,
getaddrinfo() and friends still do not seem to be there, so
NO_IPV6 still remains in effect on Cygwin.

Comments?

-- >8 --
[PATCH] Makefile: do not assume lack of IPV6 means no sockaddr_storage.

Noticed first by Alex, that the latest Cygwin now properly has
sockaddr_storage.

Signed-off-by: Junio C Hamano <redacted>

---

 Makefile |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

0b61a10bb258ae0971bce3e01e40a5a9e5d9537f

diff --git a/Makefile b/Makefile
index f6d9e0a..e8f4b24 100644
--- a/Makefile
+++ b/Makefile

@@ -42,6 +42,9 @@ all:
 #
 # Define NO_IPV6 if you lack IPv6 support and getaddrinfo().
 #
+# Define NO_SOCKADDR_STORAGE if your platform does not have struct
+# sockaddr_storage.
+#
 # Define COLLISION_CHECK below if you believe that SHA1's
 # 1461501637330902918203684832716283019655932542976 hashes do not give you
 # sufficient guarantee that no collisions between objects will ever happen.

@@ -342,7 +345,14 @@ ifdef NO_MMAP
 	COMPAT_OBJS += compat/mmap.o
 endif
 ifdef NO_IPV6
-	ALL_CFLAGS += -DNO_IPV6 -Dsockaddr_storage=sockaddr_in
+	ALL_CFLAGS += -DNO_IPV6
+endif
+ifdef NO_SOCKADDR_STORAGE
+ifdef NO_IPV6
+	ALL_CFLAGS += -Dsockaddr_storage=sockaddr_in
+else
+	ALL_CFLAGS += -Dsockaddr_storage=sockaddr_in6
+endif
 endif
 
 ifdef PPC_SHA1

-- 
1.1.3-gacdd

The recent Cygwin defines DT_UNKNOWN although it does not have d_type
in struct dirent.  Give an option to tell us not to use d_type on such
platforms.  Hopefully this problem will be transient.

Signed-off-by: Junio C Hamano <redacted>

---

 * By sending this out, I am not claiming this is better than
   your version or anything like that.  This is just for
   completeness and convenience, because my other two patches
   touch overlapping areas in the same Makefile.  This patch
   comes after the other two.

 Makefile |    7 +++++++
 cache.h  |    2 +-
 2 files changed, 8 insertions(+), 1 deletions(-)

80a4322a9bfbf0389ba8cb50f674682349f40a3a

diff --git a/Makefile b/Makefile
index 31898f7..84f5d24 100644
--- a/Makefile
+++ b/Makefile

@@ -21,6 +21,9 @@ all:
 #
 # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
 #
+# Define NO_D_TYPE_IN_DIRENT if your platform defines DT_UNKNOWN but lacks
+# d_type in struct dirent (latest Cygwin -- will be fixed soonish).
+#
 # Define NO_STRCASESTR if you don't have strcasestr.
 #
 # Define NO_SETENV if you don't have setenv in the C library.

@@ -233,6 +236,7 @@ ifeq ($(uname_S),SunOS)
 	ALL_CFLAGS += -D__EXTENSIONS__
 endif
 ifeq ($(uname_O),Cygwin)
+	NO_D_TYPE_IN_DIRENT = YesPlease
 	NO_D_INO_IN_DIRENT = YesPlease
 	NO_STRCASESTR = YesPlease
 	NEEDS_LIBICONV = YesPlease

@@ -335,6 +339,9 @@ ifdef NEEDS_NSL
 	LIBS += -lnsl
 	SIMPLE_LIB += -lnsl
 endif
+ifdef NO_D_TYPE_IN_DIRENT
+	ALL_CFLAGS += -DNO_D_TYPE_IN_DIRENT
+endif
 ifdef NO_D_INO_IN_DIRENT
 	ALL_CFLAGS += -DNO_D_INO_IN_DIRENT
 endif

diff --git a/cache.h b/cache.h
index 29c9e81..3d79c55 100644
--- a/cache.h
+++ b/cache.h

@@ -10,7 +10,7 @@
 #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)
 #endif
 
-#ifdef DT_UNKNOWN
+#if defined(DT_UNKNOWN) && !NO_D_TYPE_IN_DIRENT
 #define DTYPE(de)	((de)->d_type)
 #else
 #define DT_UNKNOWN	0

-- 
1.1.3-gacdd

On 1/20/06, Junio C Hamano [off-list ref] wrote:

Just removing "-Dsockaddr_storage=sockaddr_in" from ALL_CFLAGS
seems to solve the problem for new Cygwin.

I doubt that -Dsockaddr_storage=sockaddr_in should be tied to
NO_IPV6.  Maybe a better approach would be something like the
attached patch.

Although /usr/include/cygwin/in.h seems to have struct sockaddr_in6,
getaddrinfo() and friends still do not seem to be there, so
NO_IPV6 still remains in effect on Cygwin.

Comments?

It compiles, I don't think I can try the daemon though...

On 1/20/06, Junio C Hamano [off-list ref] wrote:

The recent Cygwin defines DT_UNKNOWN although it does not have d_type
in struct dirent.  Give an option to tell us not to use d_type on such
platforms.  Hopefully this problem will be transient.

You still have to #undef all the DT_ macros if you have a somewhat old Cygwin
(before Christopher removed the macros).