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

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

[Alex Riesen]

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]

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

[-- Attachment #2: 0001-fix-compilation-of-sockaddr_storage.txt --]
[-- Type: text/plain, Size: 1355 bytes --]

>From nobody Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Wed Jan 18 14:02:54 2006 +0100
Subject: fix compilation of sockaddr_storage

---

 Makefile |    2 +-
 daemon.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

59379c380a6c2829c5614aadd4a5492abb8d14c8
diff --git a/Makefile b/Makefile
index f6d9e0a..5782e2a 100644
--- a/Makefile
+++ b/Makefile
@@ -342,7 +342,7 @@ ifdef NO_MMAP
 	COMPAT_OBJS += compat/mmap.o
 endif
 ifdef NO_IPV6
-	ALL_CFLAGS += -DNO_IPV6 -Dsockaddr_storage=sockaddr_in
+	ALL_CFLAGS += -DNO_IPV6 -Dsockaddr_stg_git=sockaddr_in
 endif
 
 ifdef PPC_SHA1
diff --git a/daemon.c b/daemon.c
index bb014fa..b424e5e 100644
--- a/daemon.c
+++ b/daemon.c
@@ -277,7 +277,7 @@ static unsigned int children_deleted = 0
 static struct child {
 	pid_t pid;
 	int addrlen;
-	struct sockaddr_storage address;
+	struct sockaddr_stg_git address;
 } live_child[MAX_CHILDREN];
 
 static void add_child(int idx, pid_t pid, struct sockaddr *addr, int addrlen)
@@ -578,7 +578,7 @@ static int service_loop(int socknum, int
 
 		for (i = 0; i < socknum; i++) {
 			if (pfd[i].revents & POLLIN) {
-				struct sockaddr_storage ss;
+				struct sockaddr_stg_git ss;
 				unsigned int sslen = sizeof(ss);
 				int incoming = accept(pfd[i].fd, (struct sockaddr *)&ss, &sslen);
 				if (incoming < 0) {
-- 
1.1.2-ge577




[-- Attachment #3: 0002-fix-compilation-of-dirent-d_type.txt --]
[-- Type: text/plain, Size: 1137 bytes --]

>From nobody Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Wed Jan 18 14:04:34 2006 +0100
Subject: fix compilation of dirent->d_type

---

 Makefile |    3 +++
 cache.h  |   10 +++++++++-
 2 files changed, 12 insertions(+), 1 deletions(-)

5232f9128ad9d10368525fe709954acc2d6f49b3
diff --git a/Makefile b/Makefile
index 5782e2a..c3bae0f 100644
--- a/Makefile
+++ b/Makefile
@@ -329,6 +329,9 @@ ifdef NEEDS_NSL
 	LIBS += -lnsl
 	SIMPLE_LIB += -lnsl
 endif
+ifdef NO_DTYPE
+	COMPAT_CFLAGS += -DNO_DTYPE
+endif
 ifdef NO_STRCASESTR
 	COMPAT_CFLAGS += -DNO_STRCASESTR
 	COMPAT_OBJS += compat/strcasestr.o
diff --git a/cache.h b/cache.h
index 29c9e81..3846fb9 100644
--- a/cache.h
+++ b/cache.h
@@ -10,13 +10,21 @@
 #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)
 #endif
 
-#ifdef DT_UNKNOWN
+#ifndef NO_DTYPE
 #define DTYPE(de)	((de)->d_type)
 #else
+#ifndef DT_UNKNOWN
 #define DT_UNKNOWN	0
+#endif
+#ifndef DT_DIR
 #define DT_DIR		1
+#endif
+#ifndef DT_REG
 #define DT_REG		2
+#endif
+#ifndef DT_LNK
 #define DT_LNK		3
+#endif
 #define DTYPE(de)	DT_UNKNOWN
 #endif
 
-- 
1.1.2-ge577

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

[Christopher Faylor]

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)

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

[Junio C Hamano]

Christopher Faylor <me@cgf.cx> writes:

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

First of all, thanks for joining our discussion.  Being able to
hear from somebody from other project firsthand (not just listen
to somebody talking in his own changelogs and code comments, but
in actual e-mail exchange discussion) lets us put faces and
names to the entity "so far just one of the external projects to
us".

>>For reasons unknown, cygwin decided to use our sockaddr_storage.

I haven't looked at the proposed patch by Alex, so would not
comment on this part, but I'd appreciate your input.

>>For the other, probably unrelated, reasons, they decided to leave
>>declarations of DT_* macros in dirent.h without providing dirent->d_type.

I was wondering what the justification for keeping DT_* without
d_type myself.  What is the preferred resolution on this one
from your point of view?  I suspect removing d_type while
leaving DT_* was just a transient error and you would want to
remove DT_* as well, in which case the patch on this issue by
Alex would become unnecessary.

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

I think renaming __invalid_* makes sense.  I'll see how we would
work this around on the git side to make things more portable.

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

[Alex Riesen]

On 1/19/06, Christopher Faylor <me@cgf.cx> 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.

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

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

[Petr Baudis]

Dear diary, on Wed, Jan 18, 2006 at 02:47:00PM CET, I got a letter
where Alex Riesen <raa.lkml@gmail.com> 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

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

[Petr Baudis]

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 <raa.lkml@gmail.com> said that...
>  Makefile |    2 +-
>  daemon.c |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> 59379c380a6c2829c5614aadd4a5492abb8d14c8
> diff --git a/Makefile b/Makefile
> index f6d9e0a..5782e2a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -342,7 +342,7 @@ ifdef NO_MMAP
>  	COMPAT_OBJS += compat/mmap.o
>  endif
>  ifdef NO_IPV6
> -	ALL_CFLAGS += -DNO_IPV6 -Dsockaddr_storage=sockaddr_in
> +	ALL_CFLAGS += -DNO_IPV6 -Dsockaddr_stg_git=sockaddr_in

  +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

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

[Alex Riesen]

On 1/19/06, Petr Baudis <pasky@suse.cz> wrote:
> Dear diary, on Wed, Jan 18, 2006 at 02:47:00PM CET, I got a letter
> where Alex Riesen <raa.lkml@gmail.com> 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. ;-)

That will remind me to consult the specs first.

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

[Alex Riesen]

On 1/19/06, Petr Baudis <pasky@suse.cz> 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.

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

[Christopher Faylor]

On Thu, Jan 19, 2006 at 12:59:57AM -0800, Junio C Hamano wrote:
>Christopher Faylor <me@cgf.cx> writes:
>>"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.
>
>First of all, thanks for joining our discussion.

You're welcome.  I use git on linux and cygwin so I'm happy to try to help.

>Being able to hear from somebody from other project firsthand (not just
>listen to somebody talking in his own changelogs and code comments, but
>in actual e-mail exchange discussion) lets us put faces and names to
>the entity "so far just one of the external projects to us".

I only read this list sporadically but I have chimed in from time to time
when people were talking about cygwin.  For the most part, it seems like
my input hasn't been needed all that much.

>>>For reasons unknown, cygwin decided to use our sockaddr_storage.
>
>I haven't looked at the proposed patch by Alex, so would not comment on
>this part, but I'd appreciate your input.
>
>>>For the other, probably unrelated, reasons, they decided to leave
>>>declarations of DT_* macros in dirent.h without providing
>>>dirent->d_type.
>
>I was wondering what the justification for keeping DT_* without d_type
>myself.  What is the preferred resolution on this one from your point
>of view?  I suspect removing d_type while leaving DT_* was just a
>transient error and you would want to remove DT_* as well, in which
>case the patch on this issue by Alex would become unnecessary.

Actually, I started adding DT_* macros at one point, in preparation for
adding d_type, and then got sidetracked.  Their existence is a real bug
so, I have ifdef'ed out the DT macros in current CVS.

>>>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.
>
>I think renaming __invalid_* makes sense.  I'll see how we would
>work this around on the git side to make things more portable.

There were two fields in the dirent struct.  One was the "real" d_ino
which was a 64-bit ino_t, __ino32 was a legacy field from a time when
inodes were 32 bits.  I had already renamed the d_ino to __invalid_d_ino
but I didn't think I had to rename __ino32, too, since it wasn't a
standard field and didn't think that anyone would be using it.  However,
it is now __invalid_ino32 (and will probably disappear entirely) in CVS.

I knew that there would be fallout from getting rid of d_ino but this
change has been a long time coming.  Previously, the inodes reported in
d_ino were different from the (correct) ones in st_ino and some
applications were understandbly confused by that fact.  Making d_ino
accurate would have meant that we'd have to open every file in readdir
to get the windows equivalent of inode information and, since we get
enough "cygwin is slow" complaints, that wasn't a cost we were willing
to pay.

cgf

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

[Christopher Faylor]

On Thu, Jan 19, 2006 at 11:42:46AM +0100, Alex Riesen wrote:
>On 1/19/06, Christopher Faylor <me@cgf.cx> 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.

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.

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

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

[Christopher Faylor]

On Thu, Jan 19, 2006 at 11:10:00AM -0500, Christopher Faylor wrote:
>On Thu, Jan 19, 2006 at 12:59:57AM -0800, Junio C Hamano wrote:
>>Christopher Faylor <me@cgf.cx> writes:
>>>"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.
>>
>>First of all, thanks for joining our discussion.
>
>You're welcome.  I use git on linux and cygwin so I'm happy to try to help.

Btw, we're looking to roll out a new release of cygwin which fixes the
embarrassing typo in sockaddr_storage.  It is fixed in cygwin snapshots:

http://cygwin.com/snapshots/

cgf

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

[Linus Torvalds]

On Thu, 19 Jan 2006, Christopher Faylor wrote:
> 
> Btw, we're looking to roll out a new release of cygwin which fixes the
> embarrassing typo in sockaddr_storage.  It is fixed in cygwin snapshots:
> 
> http://cygwin.com/snapshots/

Quick question for cygwin people (I asked this at an earlier point, but I 
don't think there was any reply): would cygwin prefer using "vfork()" over 
"fork()", or is there no advantage? With vfork(), I could imagine that you 
might avoid a lot of strange VM games.. 

I think almost all of the git fork usage is of the type where "vfork()" 
would work fine (git-daemon in non-inetd form is an exception, perhaps the 
only one).

So if using vfork() is preferable, we probably should do that. It tends to 
be a small performance improvement on Linux too, although on Linux it's 
really pretty much in the noise.

Alternatively, is there anything else we can do that makes things easier?

			Linus

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

[Christopher Faylor]

On Thu, Jan 19, 2006 at 01:16:17PM -0800, Linus Torvalds wrote:
>On Thu, 19 Jan 2006, Christopher Faylor wrote:
>>Btw, we're looking to roll out a new release of cygwin which fixes the
>>embarrassing typo in sockaddr_storage.  It is fixed in cygwin
>>snapshots:
>>
>>http://cygwin.com/snapshots/
>
>Quick question for cygwin people (I asked this at an earlier point, but
>I don't think there was any reply): would cygwin prefer using "vfork()"
>over "fork()", or is there no advantage?  With vfork(), I could imagine
>that you might avoid a lot of strange VM games..

Sorry, I missed the earlier question.  I have to get in the habit of
scanning this list more regularly for this type of thing.

Cygwin's vfork implementation currently defaults to fork so it doesn't
matter which is used.

We used to have a vfork which tried to cut down on some of the
substantial overhead that comes with cygwin's fork() but the vfork
implementation eventually grew so complicated that there was eventually
no performance gain and I decided to just yank it and revisit it at
a later point.

So, for now, there is no difference, but, eventually, there might be if
someone masters courage to revisit vfork-on-cygwin.

So, I guess that means that it would be a good idea to switch to vfork
if you were planning for the nebulous future when this made a difference
to cygwin.  Otherwise, I wouldn't bother.

>Alternatively, is there anything else we can do that makes things easier?

I'm really committed to making cygwin as much like linux as possible so
that you won't have to make things easier.  The last release added some
stuff which should make building linux easier, in fact.  Also, the
mmap() implementation should be a littler closer to linux.  It is, of
course, a work in progress, though.

The only thing that would speed up process creation in cygwin now is
the use of the windows spawn* family of function calls.  Those could be
used instead of fork/exec but I have a personal aversion to using them
since they are so non-UNIX.  If performance is an issue, however, that
could be something to investigate.

cgf

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

[Linus Torvalds]

On Thu, 19 Jan 2006, Christopher Faylor wrote:
> 
> The only thing that would speed up process creation in cygwin now is
> the use of the windows spawn* family of function calls.  Those could be
> used instead of fork/exec but I have a personal aversion to using them
> since they are so non-UNIX.  If performance is an issue, however, that
> could be something to investigate.

What about posix_spawn()? I don't know the windows spawn things, but I 
assume posix_spawn() is basically trying to be them.. Does it work with 
cygwin if we try to convert some of the more obvious users?

Some of the git users should be easy to convert to posix_spawn()..

		Linus

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

[Christopher Faylor]

On Thu, Jan 19, 2006 at 01:44:58PM -0800, Linus Torvalds wrote:
>
>On Thu, 19 Jan 2006, Christopher Faylor wrote:
>> 
>> The only thing that would speed up process creation in cygwin now is
>> the use of the windows spawn* family of function calls.  Those could be
>> used instead of fork/exec but I have a personal aversion to using them
>> since they are so non-UNIX.  If performance is an issue, however, that
>> could be something to investigate.
>
>What about posix_spawn()? I don't know the windows spawn things, but I 
>assume posix_spawn() is basically trying to be them.. Does it work with 
>cygwin if we try to convert some of the more obvious users?

You know, I couldn't remember what the name of the spawn that was
introduced for posix.  I was going to mention it in my email and I
forgot.  Obviously it was because the name so obscure that no one could
possibly remember it.  :-)

We don't emulate posix_spawn currently but we could and should.  I'll
put it on the todo list.

cgf

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

[Alex Riesen]

Christopher Faylor, Thu, Jan 19, 2006 19:31:43 +0100:
> >>"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.

> >>>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?

> >"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?

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

[Christopher Faylor]

On Thu, Jan 19, 2006 at 11:08:43PM +0100, Alex Riesen wrote:
>Christopher Faylor, Thu, Jan 19, 2006 19:31:43 +0100:
>>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

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

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> 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 <junkio@cox.net>

---

 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

[PATCH] fsck-objects: support platforms without d_ino in struct dirent.

[Junio C Hamano]

The d_ino field is only used for performance reasons in
fsck-objects.  On a typical filesystem, i-number tends to have a
strong correlation with where the actual bits sit on the disk
platter, and we sort the entries to allow us scan things that
ought to be close together together.

If the platform lacks support for it, it is not a big deal.
Just do not use d_ino for sorting, and scan them unsorted.

---

    Junio C Hamano <junkio@cox.net> writes:

    > ...  I'll see how we would
    > work this around on the git side to make things more portable.

 Makefile       |    6 ++++++
 fsck-objects.c |   13 +++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

4bec2123116a92fbeae12afc70317dfc85f6087b
diff --git a/Makefile b/Makefile
index e8f4b24..31898f7 100644
--- a/Makefile
+++ b/Makefile
@@ -19,6 +19,8 @@ all:
 # Define NO_EXPAT if you do not have expat installed.  git-http-push is
 # not built, and you cannot push using http:// and https:// transports.
 #
+# Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
+#
 # Define NO_STRCASESTR if you don't have strcasestr.
 #
 # Define NO_SETENV if you don't have setenv in the C library.
@@ -231,6 +233,7 @@ ifeq ($(uname_S),SunOS)
 	ALL_CFLAGS += -D__EXTENSIONS__
 endif
 ifeq ($(uname_O),Cygwin)
+	NO_D_INO_IN_DIRENT = YesPlease
 	NO_STRCASESTR = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	# There are conflicting reports about this.
@@ -332,6 +335,9 @@ ifdef NEEDS_NSL
 	LIBS += -lnsl
 	SIMPLE_LIB += -lnsl
 endif
+ifdef NO_D_INO_IN_DIRENT
+	ALL_CFLAGS += -DNO_D_INO_IN_DIRENT
+endif
 ifdef NO_STRCASESTR
 	COMPAT_CFLAGS += -DNO_STRCASESTR
 	COMPAT_OBJS += compat/strcasestr.o
diff --git a/fsck-objects.c b/fsck-objects.c
index 90e638e..9950be2 100644
--- a/fsck-objects.c
+++ b/fsck-objects.c
@@ -20,6 +20,13 @@ static int check_strict = 0;
 static int keep_cache_objects = 0; 
 static unsigned char head_sha1[20];
 
+#if NO_D_INO_IN_DIRENT
+#define SORT_DIRENT 0
+#define DIRENT_SORT_HINT(de) 0
+#else
+#define SORT_DIRENT 1
+#define DIRENT_SORT_HINT(de) ((de)->d_ino)
+#endif
 
 static void objreport(struct object *obj, const char *severity,
                       const char *err, va_list params)
@@ -307,7 +314,9 @@ static void fsck_sha1_list(void)
 {
 	int i, nr = sha1_list.nr;
 
-	qsort(sha1_list.entry, nr, sizeof(struct sha1_entry *), ino_compare);
+	if (SORT_DIRENT)
+		qsort(sha1_list.entry, nr,
+		      sizeof(struct sha1_entry *), ino_compare);
 	for (i = 0; i < nr; i++) {
 		struct sha1_entry *entry = sha1_list.entry[i];
 		unsigned char *sha1 = entry->sha1;
@@ -361,7 +370,7 @@ static int fsck_dir(int i, char *path)
 			memcpy(name+2, de->d_name, len+1);
 			if (get_sha1_hex(name, sha1) < 0)
 				break;
-			add_sha1_list(sha1, de->d_ino);
+			add_sha1_list(sha1, DIRENT_SORT_HINT(de));
 			continue;
 		}
 		fprintf(stderr, "bad sha1 file: %s/%s\n", path, de->d_name);
-- 
1.1.3-gacdd

[PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

[Junio C Hamano]

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 <junkio@cox.net>

---

 * 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

Re: [PATCH] fsck-objects: support platforms without d_ino in struct dirent.

[Christopher Faylor]

On Thu, Jan 19, 2006 at 05:13:51PM -0800, Junio C Hamano wrote:
>The d_ino field is only used for performance reasons in
>fsck-objects.  On a typical filesystem, i-number tends to have a
>strong correlation with where the actual bits sit on the disk
>platter, and we sort the entries to allow us scan things that
>ought to be close together together.
>
>If the platform lacks support for it, it is not a big deal.
>Just do not use d_ino for sorting, and scan them unsorted.

Ah, interesting.  I don't know if the file id objects would have
that sort of correlation on NT but obviously the file hashes don't.

cgf

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

[Alex Riesen]

On 1/20/06, Junio C Hamano <junkio@cox.net> 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...

Re: [PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

[Alex Riesen]

On 1/20/06, Junio C Hamano <junkio@cox.net> 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).

Re: [PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> On 1/20/06, Junio C Hamano <junkio@cox.net> 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).
> -

Ah, you mean something like this?

-- >8 --
diff --git a/cache.h b/cache.h
index 8339931..6f13434 100644
--- a/cache.h
+++ b/cache.h
@@ -13,6 +13,10 @@
 #if defined(DT_UNKNOWN) && !NO_D_TYPE_IN_DIRENT
 #define DTYPE(de)	((de)->d_type)
 #else
+#undef DT_UNKNOWN
+#undef DT_DIR
+#undef DT_REG
+#undef DT_LNK
 #define DT_UNKNOWN	0
 #define DT_DIR		1
 #define DT_REG		2

Re: [PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

[Alex Riesen]

Junio C Hamano, Fri, Jan 20, 2006 20:10:59 +0100:
> >> 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).
> 
> Ah, you mean something like this?
> 
> +#undef DT_UNKNOWN
> +#undef DT_DIR
> +#undef DT_REG
> +#undef DT_LNK

yes, of course

Re: [PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

>> +#undef DT_UNKNOWN
>> +#undef DT_DIR
>> +#undef DT_REG
>> +#undef DT_LNK
>
> yes, of course

That is technically correct but I suspect it would not matter in
practice.  The only thing you are avoiding is four compiler
warnings when compiling for Cygwin of this week (I heard this is
already fixed in Cygwin CVS).  On older Cygwin you simply would
not use NO_D_TYPE_IN_DIRENT, since d_type worked before.  No?

Re: [PATCH] DT_UNKNOWN: do not fully trust existence of DT_UNKNOWN

[Alex Riesen]

On 1/21/06, Junio C Hamano <junkio@cox.net> wrote:
> Alex Riesen <raa.lkml@gmail.com> writes:
>
> >> +#undef DT_UNKNOWN
> >> +#undef DT_DIR
> >> +#undef DT_REG
> >> +#undef DT_LNK
> >
> > yes, of course
>
> That is technically correct but I suspect it would not matter in
> practice.  The only thing you are avoiding is four compiler
> warnings when compiling for Cygwin of this week (I heard this is
> already fixed in Cygwin CVS).  On older Cygwin you simply would
> not use NO_D_TYPE_IN_DIRENT, since d_type worked before.  No?

Yes :)