From: Jonathan Callen <hidden> Date: 2016-06-15 22:48:52
This series of patches adds support for building git on Interix.
Interix is an interesting system, as it lacks some functions that are
normally present on POSIX systems, such as poll(). As I did not have
time to implement poll() on top of select(), these patches simply
disable the three commands that require poll() to be present:
git-daemon, git-upload-archive, and git-upload-pack.
Jonathan Callen (3):
Support building on systems without poll(2)
Support building without inttypes.h
Add Interix support
Makefile | 40 +++++++++++++++++++++++++++++++++++-----
builtin.h | 2 ++
git-compat-util.h | 6 ++++++
git.c | 2 ++
4 files changed, 45 insertions(+), 5 deletions(-)
From: Jonathan Callen <hidden> Date: 2016-06-15 22:48:52
Some systems, such as Interix, do not have a inttypes.h header.
Attempt to use stdint.h instead.
Signed-off-by: Jonathan Callen <redacted>
---
Makefile | 6 ++++++
git-compat-util.h | 4 ++++
2 files changed, 10 insertions(+), 0 deletions(-)
@@ -70,6 +70,8 @@ all::## Define NO_SYS_SELECT_H if you don't have sys/select.h.#+# Define NO_INTTYPES_H if you don't have inttypes.h.+## Define NO_SYMLINK_HEAD if you never want .git/HEAD to be a symbolic link.# Enable it on Windows. By default, symrefs are still used.#
From: Jonathan Callen <hidden> Date: 2016-06-15 22:48:52
Add an Interix clause in the makefile. Interix's libc does not have
support for many of the functions that are generally taken for granted.
Signed-off-by: Jonathan Callen <redacted>
---
Makefile | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
From: Jonathan Callen <hidden> Date: 2016-06-15 22:48:52
Some systems do not have sys/poll.h or poll(2). Don't build
git-daemon, git-upload-archive, or git-upload-pack on such systems.
Signed-off-by: Jonathan Callen <redacted>
---
Makefile | 21 ++++++++++++++++-----
builtin.h | 2 ++
git-compat-util.h | 2 ++
git.c | 2 ++
4 files changed, 22 insertions(+), 5 deletions(-)
@@ -62,6 +62,8 @@ all::## Define NO_MKSTEMPS if you don't have mkstemps in the C library.#+# Define NO_POLL if you don't have poll in the C library, or it does not work.+## Define NO_LIBGEN_H if you don't have libgen.h.## Define NEEDS_LIBGEN if your libgen needs -lgen when linking
@@ -434,9 +435,7 @@ OTHER_PROGRAMS = git$X# what test wrappers are needed and 'install' will install, in bindirBINDIR_PROGRAMS_NEED_X+=git-BINDIR_PROGRAMS_NEED_X+=git-upload-packBINDIR_PROGRAMS_NEED_X+=git-receive-pack-BINDIR_PROGRAMS_NEED_X+=git-upload-archiveBINDIR_PROGRAMS_NEED_X+=git-shellBINDIR_PROGRAMS_NO_X+=git-cvsserver
From: Michael J Gruber <hidden> Date: 2016-06-15 22:48:52
Jonathan Callen venit, vidit, dixit 27.05.2010 10:19:
quoted hunk
Some systems do not have sys/poll.h or poll(2). Don't build
git-daemon, git-upload-archive, or git-upload-pack on such systems.
Signed-off-by: Jonathan Callen <redacted>
---
Makefile | 21 ++++++++++++++++-----
builtin.h | 2 ++
git-compat-util.h | 2 ++
git.c | 2 ++
4 files changed, 22 insertions(+), 5 deletions(-)
@@ -62,6 +62,8 @@ all::## Define NO_MKSTEMPS if you don't have mkstemps in the C library.#+# Define NO_POLL if you don't have poll in the C library, or it does not work.+## Define NO_LIBGEN_H if you don't have libgen.h.## Define NEEDS_LIBGEN if your libgen needs -lgen when linking
@@ -434,9 +435,7 @@ OTHER_PROGRAMS = git$X# what test wrappers are needed and 'install' will install, in bindirBINDIR_PROGRAMS_NEED_X+=git-BINDIR_PROGRAMS_NEED_X+=git-upload-packBINDIR_PROGRAMS_NEED_X+=git-receive-pack-BINDIR_PROGRAMS_NEED_X+=git-upload-archiveBINDIR_PROGRAMS_NEED_X+=git-shellBINDIR_PROGRAMS_NO_X+=git-cvsserver
Shouldn't this be "ifndef"? makes me wonder how the test compile worked...
Yeah, that's a mistake. It should be #ifndef. The test compile worked
because cmd_upload_archive is never called when NO_POLL is defined (and
I didn't test on a system where it wasn't).
- --
Jonathan Callen
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQIcBAEBCAAGBQJL/jfNAAoJELHSF2kinlg4avYP/3CxgTwg1Wcraf0JLxtg590T
r0VPk/FJfN70kCwch1107qrXFH2Lm5j57R2qcJRPbO4xLg3/7SVDo8uh5srRVBwh
jHV/lVHiDofzAwogv0T1dAd02/WrbpBKM8Wt6sT0fz9Y3yMGPfSuYCazQ0uiH6NA
I/Rk2CcTelNNSo4h8+XRTdMwiEVIehw1zUNX9HhH1SYWastMCgoh2PS+cpeGot+q
IvInTpcOJfOsREXTxjer9zf/GeZsTiEVLgDWZG6Uc0JOEvLTKOqk/BcSeOn4L2zv
lHz14vMT8NS68U/rfPvTjscgczyS2E47XHrvM0+1bswLuNVrilHdzgcnH2rhWFFP
tU/dA6qzzFPvpqLwhXOwxECUsnJ8km5i8oR1914Qwfxr88KVR8syYyDl4eSI3qMg
MbFRTAzRC+4KD1FcGaQ7f7f2J/2Um4wtTb5SrKUJVKk2ROI/2KfBFthI4SbIonOf
NYJp+2XEpc3vbwE6YbiFzXBkP2z/F4fFvorYi8epa1neX7YKF7Hp+U5UJIFBb8de
KMKCoX1ClXjsTQ3B72M29D/o52L67/xcyoPDIfmnjuxeBcrJBO82z8pqMCyF4nh+
xa7mU4rrk6D8ZHxv1uULf8Bqz29Eq6dzzeuNkoV/5plLC2XjWJ2qsMxvZY9dED4h
6SUO3TH9eJJW6eAZG9bB
=RkSh
-----END PGP SIGNATURE-----
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:48:52
Implement the subset of poll() semantics needed by git in terms of
select(), for use by the Interix port. Inspired by commit 6ed807f
(Windows: A rudimentary poll() emulation, 2007-12-01).
Cc: Johannes Sixt <redacted>
Cc: Jonathan Callen <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Jonathan Callen wrote:
Some systems do not have sys/poll.h or poll(2).
Maybe this could help. Thanks to DrNick on IRC for the suggestion.
Warning: untested.
Makefile | 8 ++++++++
compat/poll.c | 33 +++++++++++++++++++++++++++++++++
compat/poll.h | 13 +++++++++++++
git-compat-util.h | 6 +++++-
4 files changed, 59 insertions(+), 1 deletions(-)
create mode 100644 compat/poll.c
create mode 100644 compat/poll.h
@@ -111,6 +111,9 @@ all::## Define NO_PTHREADS if you do not have or do not want to use Pthreads.#+# Define NO_POLL if you have a problem with the poll() system call (e.g.+# Interix).+## Define NO_PREAD if you have a problem with pread() system call (e.g.# cygwin1.dll before v1.5.22).#
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:48:52
On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
Implement the subset of poll() semantics needed by git in terms of
select(), for use by the Interix port. Inspired by commit 6ed807f
(Windows: A rudimentary poll() emulation, 2007-12-01).
A possible problem with this approach is that the maximum number of
file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
maximum number of file descriptors select can handle is limited by
FD_SETSIZE.
I don't think this is a big problem in reality, though - both values
seem to be pretty high in most implementations. And IIRC git-daemon is
the only one who needs more than 2, and it doesn't even check
RLIMIT_NOFILE.
If we decide to go this route, perhaps it'd make sense to change to
this code for Windows also? Our Windows-implementation of poll() has
some annoying limitations...
--
Erik "kusma" Faye-Lund
From: Marko Kreen <hidden> Date: 2016-06-15 22:48:52
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
> Implement the subset of poll() semantics needed by git in terms of
> select(), for use by the Interix port. Inspired by commit 6ed807f
> (Windows: A rudimentary poll() emulation, 2007-12-01).
>
A possible problem with this approach is that the maximum number of
file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
maximum number of file descriptors select can handle is limited by
FD_SETSIZE.
I don't think this is a big problem in reality, though - both values
seem to be pretty high in most implementations. And IIRC git-daemon is
the only one who needs more than 2, and it doesn't even check
RLIMIT_NOFILE.
If we decide to go this route, perhaps it'd make sense to change to
this code for Windows also? Our Windows-implementation of poll() has
some annoying limitations...
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:48:52
On Thu, May 27, 2010 at 1:39 PM, Marko Kreen [off-list ref] wrote:
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
quoted
On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
> Implement the subset of poll() semantics needed by git in terms of
> select(), for use by the Interix port. Inspired by commit 6ed807f
> (Windows: A rudimentary poll() emulation, 2007-12-01).
>
A possible problem with this approach is that the maximum number of
file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
maximum number of file descriptors select can handle is limited by
FD_SETSIZE.
I don't think this is a big problem in reality, though - both values
seem to be pretty high in most implementations. And IIRC git-daemon is
the only one who needs more than 2, and it doesn't even check
RLIMIT_NOFILE.
If we decide to go this route, perhaps it'd make sense to change to
this code for Windows also? Our Windows-implementation of poll() has
some annoying limitations...
How does this code convince FD_SET() that the buffer has increased? It
looks to me like it depends on a specific FD_SET() implementation...
For instance, Windows' FD_SET() implementation is like this:
#define FD_SET(fd, set) do { \
if (((fd_set FAR *)(set))->fd_count < FD_SETSIZE) \
((fd_set FAR *)(set))->fd_array[((fd_set FAR
*)(set))->fd_count++]=(fd);\
} while(0)
...so unless another set is passed in, it won't add any more fds once
fd_count reaches FD_SETSIZE.
Also, FD_SETSIZE is 64 on Windows. IIRC it's 1024 on Linux, so it is
much more likely that we encounter this issue on Windows than on
Linux, at least ;)
--
Erik "kusma" Faye-Lund
From: Marko Kreen <hidden> Date: 2016-06-15 22:48:52
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
On Thu, May 27, 2010 at 1:39 PM, Marko Kreen [off-list ref] wrote:
> On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
>> > Implement the subset of poll() semantics needed by git in terms of
>> > select(), for use by the Interix port. Inspired by commit 6ed807f
>> > (Windows: A rudimentary poll() emulation, 2007-12-01).
>> >
>>
>>
>> A possible problem with this approach is that the maximum number of
>> file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
>> maximum number of file descriptors select can handle is limited by
>> FD_SETSIZE.
>>
>> I don't think this is a big problem in reality, though - both values
>> seem to be pretty high in most implementations. And IIRC git-daemon is
>> the only one who needs more than 2, and it doesn't even check
>> RLIMIT_NOFILE.
>>
>> If we decide to go this route, perhaps it'd make sense to change to
>> this code for Windows also? Our Windows-implementation of poll() has
>> some annoying limitations...
>
> Example of poll() compat without FD_SETSIZE limit:
>
> http://github.com/markokr/plproxy-dev/blob/master/src/poll_compat.c
>
How does this code convince FD_SET() that the buffer has increased? It
looks to me like it depends on a specific FD_SET() implementation...
For instance, Windows' FD_SET() implementation is like this:
#define FD_SET(fd, set) do { \
if (((fd_set FAR *)(set))->fd_count < FD_SETSIZE) \
((fd_set FAR *)(set))->fd_array[((fd_set FAR
*)(set))->fd_count++]=(fd);\
} while(0)
...so unless another set is passed in, it won't add any more fds once
fd_count reaches FD_SETSIZE.
Also, FD_SETSIZE is 64 on Windows. IIRC it's 1024 on Linux, so it is
much more likely that we encounter this issue on Windows than on
Linux, at least ;)
Hm, good catch. Seems such compat poll() cannot be done without
OS-specific hacks.
Do you know perhaps what other OS-es have non-bitmap fd_set?
--
marko
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:48:52
On Thu, May 27, 2010 at 2:36 PM, Marko Kreen [off-list ref] wrote:
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
quoted
On Thu, May 27, 2010 at 1:39 PM, Marko Kreen [off-list ref] wrote:
> On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
>> > Implement the subset of poll() semantics needed by git in terms of
>> > select(), for use by the Interix port. Inspired by commit 6ed807f
>> > (Windows: A rudimentary poll() emulation, 2007-12-01).
>> >
>>
>>
>> A possible problem with this approach is that the maximum number of
>> file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
>> maximum number of file descriptors select can handle is limited by
>> FD_SETSIZE.
>>
>> I don't think this is a big problem in reality, though - both values
>> seem to be pretty high in most implementations. And IIRC git-daemon is
>> the only one who needs more than 2, and it doesn't even check
>> RLIMIT_NOFILE.
>>
>> If we decide to go this route, perhaps it'd make sense to change to
>> this code for Windows also? Our Windows-implementation of poll() has
>> some annoying limitations...
>
> Example of poll() compat without FD_SETSIZE limit:
>
> http://github.com/markokr/plproxy-dev/blob/master/src/poll_compat.c
>
How does this code convince FD_SET() that the buffer has increased? It
looks to me like it depends on a specific FD_SET() implementation...
For instance, Windows' FD_SET() implementation is like this:
#define FD_SET(fd, set) do { \
if (((fd_set FAR *)(set))->fd_count < FD_SETSIZE) \
((fd_set FAR *)(set))->fd_array[((fd_set FAR
*)(set))->fd_count++]=(fd);\
} while(0)
...so unless another set is passed in, it won't add any more fds once
fd_count reaches FD_SETSIZE.
Also, FD_SETSIZE is 64 on Windows. IIRC it's 1024 on Linux, so it is
much more likely that we encounter this issue on Windows than on
Linux, at least ;)
Hm, good catch. Seems such compat poll() cannot be done without
OS-specific hacks.
Perhaps getrlimit() could overridden to return FD_SETSIZE for both the
soft and hard limit when asking about RLIMIT_NOFILE? In such cases,
anyone who passes nfds above FD_SETSIZE hasn't consulted
RLIMIT_NOFILE, and should be outside the standard. But your point
might have been about a limitless poll()-implementation, like the code
you linked tried to achieve. In that context, no. I doubt it's
possible to do in a robust fashion.
For git, I don't think this is necessary, though. As said, I think
git-daemon is the only call-site for poll where nfds can be above 2.
And git-daemon's default max-connection is 32, which shouldn't cause
much problems. There's the theoretical problem of someone setting
--max-connections above their platform's limit, but I don't think we
need to touch that ;)
Do you know perhaps what other OS-es have non-bitmap fd_set?
No, I don't have much low-level file-descriptor knowledge about other
OS'es than Windows, really.
--
Erik "kusma" Faye-Lund
From: Jeff King <hidden> Date: 2016-06-15 22:48:52
On Thu, May 27, 2010 at 10:43:35AM +0200, Sverre Rabbelier wrote:
On Thu, May 27, 2010 at 10:19, Jonathan Callen [off-list ref] wrote:
quoted
Some systems do not have sys/poll.h or poll(2). Don't build
git-daemon, git-upload-archive, or git-upload-pack on such systems.
I thought git-upload-pack is required for push support in git?
No, that's send-pack. Upload-pack is the server side of fetching. And
yes, these names are confusing. Before I worked a lot on the send-pack
code, I used to mix them up all the time. :)
So without upload-pack, one cannot be the server side of a fetch. Which
means you can still work as a client, but it does mean that even local
clones won't work (actually, the clone will work due to the local
hardlink optimization, but further fetches will fail).
-Peff
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:48:52
On Thu, May 27, 2010 at 1:00 PM, Erik Faye-Lund
[off-list ref] wrote:
On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
quoted
Implement the subset of poll() semantics needed by git in terms of
select(), for use by the Interix port. Inspired by commit 6ed807f
(Windows: A rudimentary poll() emulation, 2007-12-01).
A possible problem with this approach is that the maximum number of
file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
maximum number of file descriptors select can handle is limited by
FD_SETSIZE.
I don't think this is a big problem in reality, though - both values
seem to be pretty high in most implementations. And IIRC git-daemon is
the only one who needs more than 2, and it doesn't even check
RLIMIT_NOFILE.
To be clear: I think this strategy is the best option (at least for
non-Windows, where select() might be our only option).
But perhaps you should include a check along the lines of this:
if (nfds > FD_SETSIZE)
return errno = EINVAL, error("poll: nfds must be below %d", FD_SETSIZE);
Just so we can know when the code fails :)
--
Erik "kusma" Faye-Lund
From: Marko Kreen <hidden> Date: 2016-06-15 22:48:52
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
On Thu, May 27, 2010 at 1:00 PM, Erik Faye-Lund
[off-list ref] wrote:
> On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
>> Implement the subset of poll() semantics needed by git in terms of
>> select(), for use by the Interix port. Inspired by commit 6ed807f
>> (Windows: A rudimentary poll() emulation, 2007-12-01).
>>
>
> A possible problem with this approach is that the maximum number of
> file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
> maximum number of file descriptors select can handle is limited by
> FD_SETSIZE.
>
> I don't think this is a big problem in reality, though - both values
> seem to be pretty high in most implementations. And IIRC git-daemon is
> the only one who needs more than 2, and it doesn't even check
> RLIMIT_NOFILE.
>
To be clear: I think this strategy is the best option (at least for
non-Windows, where select() might be our only option).
But perhaps you should include a check along the lines of this:
if (nfds > FD_SETSIZE)
return errno = EINVAL, error("poll: nfds must be below %d", FD_SETSIZE);
Just so we can know when the code fails :)
Well, per your own FD_SET example, the FD_SETSIZE on windows
means different thing than FD_SETSIZE on old-style bitmap-based
select() implementation.
On Unix, it's max fd number + 1, on windows it's max count.
Thus on windows the nfds can safely be larger than FD_SETSIZE,
assuming the actual count is smaller.
RLIMIT_NOFILE - I don't see why it is relevant here, as you
get error on open() when you cross the limit. So how can you
pass more than that meny fds to select()/poll()?
--
marko
From: Albert Dvornik <hidden> Date: 2016-06-15 22:48:52
On Thu, May 27, 2010 at 8:57 AM, Erik Faye-Lund
[off-list ref] wrote:
On Thu, May 27, 2010 at 2:36 PM, Marko Kreen [off-list ref] wrote:
[...]
quoted
Hm, good catch. Seems such compat poll() cannot be done without
OS-specific hacks.
Perhaps getrlimit() could overridden to return FD_SETSIZE for both the
soft and hard limit when asking about RLIMIT_NOFILE? In such cases,
anyone who passes nfds above FD_SETSIZE hasn't consulted
RLIMIT_NOFILE, and should be outside the standard. But your point
might have been about a limitless poll()-implementation, like the code
you linked tried to achieve. In that context, no. I doubt it's
possible to do in a robust fashion.
This is getting far off-topic, but I do think it's possible to do in a
robust fashion if you are willing to sacrifice some performance [i.e.
we should not consider this for Git =) ], by creating multiple fd_set
structures and only shoving up to FD_SETSIZE descriptors into each.
Cycling between the structures would cause some extra latency and a
LOT more code complexity, but it should work.
--bert
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:48:52
On Thu, May 27, 2010 at 3:29 PM, Marko Kreen [off-list ref] wrote:
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
quoted
On Thu, May 27, 2010 at 1:00 PM, Erik Faye-Lund
[off-list ref] wrote:
> On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
>> Implement the subset of poll() semantics needed by git in terms of
>> select(), for use by the Interix port. Inspired by commit 6ed807f
>> (Windows: A rudimentary poll() emulation, 2007-12-01).
>>
>
> A possible problem with this approach is that the maximum number of
> file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
> maximum number of file descriptors select can handle is limited by
> FD_SETSIZE.
>
> I don't think this is a big problem in reality, though - both values
> seem to be pretty high in most implementations. And IIRC git-daemon is
> the only one who needs more than 2, and it doesn't even check
> RLIMIT_NOFILE.
>
To be clear: I think this strategy is the best option (at least for
non-Windows, where select() might be our only option).
But perhaps you should include a check along the lines of this:
if (nfds > FD_SETSIZE)
return errno = EINVAL, error("poll: nfds must be below %d", FD_SETSIZE);
Just so we can know when the code fails :)
Well, per your own FD_SET example, the FD_SETSIZE on windows
means different thing than FD_SETSIZE on old-style bitmap-based
select() implementation.
On Unix, it's max fd number + 1, on windows it's max count.
Are you sure this applies for all Unix, not just some given Unix-y system?
RLIMIT_NOFILE - I don't see why it is relevant here, as you
get error on open() when you cross the limit. So how can you
pass more than that meny fds to select()/poll()?
Good point, this was my bad.
--
Erik "kusma" Faye-Lund
From: Marko Kreen <hidden> Date: 2016-06-15 22:48:52
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
On Thu, May 27, 2010 at 3:29 PM, Marko Kreen [off-list ref] wrote:
> On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> On Thu, May 27, 2010 at 1:00 PM, Erik Faye-Lund
>> [off-list ref] wrote:
>> > On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
>> >> Implement the subset of poll() semantics needed by git in terms of
>> >> select(), for use by the Interix port. Inspired by commit 6ed807f
>> >> (Windows: A rudimentary poll() emulation, 2007-12-01).
>> >>
>> >
>> > A possible problem with this approach is that the maximum number of
>> > file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
>> > maximum number of file descriptors select can handle is limited by
>> > FD_SETSIZE.
>> >
>> > I don't think this is a big problem in reality, though - both values
>> > seem to be pretty high in most implementations. And IIRC git-daemon is
>> > the only one who needs more than 2, and it doesn't even check
>> > RLIMIT_NOFILE.
>> >
>>
>>
>> To be clear: I think this strategy is the best option (at least for
>> non-Windows, where select() might be our only option).
>>
>> But perhaps you should include a check along the lines of this:
>>
>> if (nfds > FD_SETSIZE)
>> return errno = EINVAL, error("poll: nfds must be below %d", FD_SETSIZE);
>>
>> Just so we can know when the code fails :)
>
> Well, per your own FD_SET example, the FD_SETSIZE on windows
> means different thing than FD_SETSIZE on old-style bitmap-based
> select() implementation.
>
> On Unix, it's max fd number + 1, on windows it's max count.
>
Are you sure this applies for all Unix, not just some given Unix-y system?
Not sure. Just pointing out that the above check is not
universal enough.
--
marko
From: Albert Dvornik <hidden> Date: 2016-06-15 22:48:52
On Thu, May 27, 2010 at 9:06 AM, Erik Faye-Lund
[off-list ref] wrote:
[...]
But perhaps you should include a check along the lines of this:
if (nfds > FD_SETSIZE)
return errno = EINVAL, error("poll: nfds must be below %d", FD_SETSIZE);
Just so we can know when the code fails :)
If you're checking against FD_SETSIZE (which is IMO a good idea), you
should consider that
(a) on the one system I'm aware of where fd_set doesn't use a bitmap
(Windows), FD_SETSIZE is a limit on the number of descriptors added to
the set, but
(b) on systems where fd_set uses a bitmap (i.e. Linux, perhaps all
UNIXes, etc), FD_SETSIZE is a limit on *each descriptor value*. This
is also what POSIX says.
So on the latter systems, we want something like this before each FD_SET:
if (ufds[i].fd >= FD_SETSIZE) {
errno = EINVAL;
return error("poll: each fd must be below %d", FD_SETSIZE);
}
(The reason to have it in the loop, rather than just check maxfd
afterwards, is that FD_SET with an argument that's too big can trash
the stack.)
Of course, on Windows this would impose a limitation that all
descriptors be < 64, which is probably crazy. Which means that you'd
have to actually distinguish the two types. Sigh.
--bert
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:48:52
On Thu, May 27, 2010 at 3:58 PM, Marko Kreen [off-list ref] wrote:
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
quoted
On Thu, May 27, 2010 at 3:29 PM, Marko Kreen [off-list ref] wrote:
> On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> On Thu, May 27, 2010 at 1:00 PM, Erik Faye-Lund
>> [off-list ref] wrote:
>> > On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
>> >> Implement the subset of poll() semantics needed by git in terms of
>> >> select(), for use by the Interix port. Inspired by commit 6ed807f
>> >> (Windows: A rudimentary poll() emulation, 2007-12-01).
>> >>
>> >
>> > A possible problem with this approach is that the maximum number of
>> > file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
>> > maximum number of file descriptors select can handle is limited by
>> > FD_SETSIZE.
>> >
>> > I don't think this is a big problem in reality, though - both values
>> > seem to be pretty high in most implementations. And IIRC git-daemon is
>> > the only one who needs more than 2, and it doesn't even check
>> > RLIMIT_NOFILE.
>> >
>>
>>
>> To be clear: I think this strategy is the best option (at least for
>> non-Windows, where select() might be our only option).
>>
>> But perhaps you should include a check along the lines of this:
>>
>> if (nfds > FD_SETSIZE)
>> return errno = EINVAL, error("poll: nfds must be below %d", FD_SETSIZE);
>>
>> Just so we can know when the code fails :)
>
> Well, per your own FD_SET example, the FD_SETSIZE on windows
> means different thing than FD_SETSIZE on old-style bitmap-based
> select() implementation.
>
> On Unix, it's max fd number + 1, on windows it's max count.
>
Are you sure this applies for all Unix, not just some given Unix-y system?
Not sure. Just pointing out that the above check is not
universal enough.
Isn't it? How could one possibly pass more than max fd number + 1 file
descriptors, since they start at 0? I guess one could specify a given
fd more than once, but that'd be kind of redundant... and also very
unlikely in our case ;)
--
Erik "kusma" Faye-Lund
From: Marko Kreen <hidden> Date: 2016-06-15 22:48:52
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
On Thu, May 27, 2010 at 3:58 PM, Marko Kreen [off-list ref] wrote:
> On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> On Thu, May 27, 2010 at 3:29 PM, Marko Kreen [off-list ref] wrote:
>> > On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> >> On Thu, May 27, 2010 at 1:00 PM, Erik Faye-Lund
>> >> [off-list ref] wrote:
>> >> > On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
>> >> >> Implement the subset of poll() semantics needed by git in terms of
>> >> >> select(), for use by the Interix port. Inspired by commit 6ed807f
>> >> >> (Windows: A rudimentary poll() emulation, 2007-12-01).
>> >> >>
>> >> >
>> >> > A possible problem with this approach is that the maximum number of
>> >> > file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
>> >> > maximum number of file descriptors select can handle is limited by
>> >> > FD_SETSIZE.
>> >> >
>> >> > I don't think this is a big problem in reality, though - both values
>> >> > seem to be pretty high in most implementations. And IIRC git-daemon is
>> >> > the only one who needs more than 2, and it doesn't even check
>> >> > RLIMIT_NOFILE.
>> >> >
>> >>
>> >>
>> >> To be clear: I think this strategy is the best option (at least for
>> >> non-Windows, where select() might be our only option).
>> >>
>> >> But perhaps you should include a check along the lines of this:
>> >>
>> >> if (nfds > FD_SETSIZE)
>> >> return errno = EINVAL, error("poll: nfds must be below %d", FD_SETSIZE);
>> >>
>> >> Just so we can know when the code fails :)
>> >
>> > Well, per your own FD_SET example, the FD_SETSIZE on windows
>> > means different thing than FD_SETSIZE on old-style bitmap-based
>> > select() implementation.
>> >
>> > On Unix, it's max fd number + 1, on windows it's max count.
>> >
>>
>>
>> Are you sure this applies for all Unix, not just some given Unix-y system?
>
> Not sure. Just pointing out that the above check is not
> universal enough.
>
Isn't it? How could one possibly pass more than max fd number + 1 file
descriptors, since they start at 0? I guess one could specify a given
fd more than once, but that'd be kind of redundant... and also very
unlikely in our case ;)
Pass one fd with value 70 it. Check returns error, although
everything would work.
--
marko
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:48:52
On Thu, May 27, 2010 at 4:11 PM, Marko Kreen [off-list ref] wrote:
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
quoted
On Thu, May 27, 2010 at 3:58 PM, Marko Kreen [off-list ref] wrote:
> On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> On Thu, May 27, 2010 at 3:29 PM, Marko Kreen [off-list ref] wrote:
>> > On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> >> On Thu, May 27, 2010 at 1:00 PM, Erik Faye-Lund
>> >> [off-list ref] wrote:
>> >> > On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
>> >> >> Implement the subset of poll() semantics needed by git in terms of
>> >> >> select(), for use by the Interix port. Inspired by commit 6ed807f
>> >> >> (Windows: A rudimentary poll() emulation, 2007-12-01).
>> >> >>
>> >> >
>> >> > A possible problem with this approach is that the maximum number of
>> >> > file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
>> >> > maximum number of file descriptors select can handle is limited by
>> >> > FD_SETSIZE.
>> >> >
>> >> > I don't think this is a big problem in reality, though - both values
>> >> > seem to be pretty high in most implementations. And IIRC git-daemon is
>> >> > the only one who needs more than 2, and it doesn't even check
>> >> > RLIMIT_NOFILE.
>> >> >
>> >>
>> >>
>> >> To be clear: I think this strategy is the best option (at least for
>> >> non-Windows, where select() might be our only option).
>> >>
>> >> But perhaps you should include a check along the lines of this:
>> >>
>> >> if (nfds > FD_SETSIZE)
>> >> return errno = EINVAL, error("poll: nfds must be below %d", FD_SETSIZE);
>> >>
>> >> Just so we can know when the code fails :)
>> >
>> > Well, per your own FD_SET example, the FD_SETSIZE on windows
>> > means different thing than FD_SETSIZE on old-style bitmap-based
>> > select() implementation.
>> >
>> > On Unix, it's max fd number + 1, on windows it's max count.
>> >
>>
>>
>> Are you sure this applies for all Unix, not just some given Unix-y system?
>
> Not sure. Just pointing out that the above check is not
> universal enough.
>
Isn't it? How could one possibly pass more than max fd number + 1 file
descriptors, since they start at 0? I guess one could specify a given
fd more than once, but that'd be kind of redundant... and also very
unlikely in our case ;)
Pass one fd with value 70 it. Check returns error, although
everything would work.
No, not with the check I posted. I checked nfds, not the value of the
fds themselves. nfds is clearly the size of the fds array -- if it
wasn't, it'd be impossible for the poll-implementation to know how big
the array is!
--
Erik "kusma" Faye-Lund
From: Marko Kreen <hidden> Date: 2016-06-15 22:48:52
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
On Thu, May 27, 2010 at 4:11 PM, Marko Kreen [off-list ref] wrote:
> On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> On Thu, May 27, 2010 at 3:58 PM, Marko Kreen [off-list ref] wrote:
>> > On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> >> On Thu, May 27, 2010 at 3:29 PM, Marko Kreen [off-list ref] wrote:
>> >> > On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> >> >> On Thu, May 27, 2010 at 1:00 PM, Erik Faye-Lund
>> >> >> [off-list ref] wrote:
>> >> >> > On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
>> >> >> >> Implement the subset of poll() semantics needed by git in terms of
>> >> >> >> select(), for use by the Interix port. Inspired by commit 6ed807f
>> >> >> >> (Windows: A rudimentary poll() emulation, 2007-12-01).
>> >> >> >>
>> >> >> >
>> >> >> > A possible problem with this approach is that the maximum number of
>> >> >> > file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
>> >> >> > maximum number of file descriptors select can handle is limited by
>> >> >> > FD_SETSIZE.
>> >> >> >
>> >> >> > I don't think this is a big problem in reality, though - both values
>> >> >> > seem to be pretty high in most implementations. And IIRC git-daemon is
>> >> >> > the only one who needs more than 2, and it doesn't even check
>> >> >> > RLIMIT_NOFILE.
>> >> >> >
>> >> >>
>> >> >>
>> >> >> To be clear: I think this strategy is the best option (at least for
>> >> >> non-Windows, where select() might be our only option).
>> >> >>
>> >> >> But perhaps you should include a check along the lines of this:
>> >> >>
>> >> >> if (nfds > FD_SETSIZE)
>> >> >> return errno = EINVAL, error("poll: nfds must be below %d", FD_SETSIZE);
>> >> >>
>> >> >> Just so we can know when the code fails :)
>> >> >
>> >> > Well, per your own FD_SET example, the FD_SETSIZE on windows
>> >> > means different thing than FD_SETSIZE on old-style bitmap-based
>> >> > select() implementation.
>> >> >
>> >> > On Unix, it's max fd number + 1, on windows it's max count.
>> >> >
>> >>
>> >>
>> >> Are you sure this applies for all Unix, not just some given Unix-y system?
>> >
>> > Not sure. Just pointing out that the above check is not
>> > universal enough.
>> >
>>
>>
>> Isn't it? How could one possibly pass more than max fd number + 1 file
>> descriptors, since they start at 0? I guess one could specify a given
>> fd more than once, but that'd be kind of redundant... and also very
>> unlikely in our case ;)
>
> Pass one fd with value 70 it. Check returns error, although
> everything would work.
>
No, not with the check I posted. I checked nfds, not the value of the
fds themselves. nfds is clearly the size of the fds array -- if it
wasn't, it'd be impossible for the poll-implementation to know how big
the array is!
Ah, ok. I though this is the nfds for select(). In this case it can
still fail, as nfds=1 for poll() can still go over FD_SETSIZE for
select() if the fd value is big enough. On non-windows, that is.
Dunno if this matters for git-daemon..
--
marko
From: Erik Faye-Lund <hidden> Date: 2016-06-15 22:48:52
On Thu, May 27, 2010 at 4:32 PM, Marko Kreen [off-list ref] wrote:
On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
quoted
On Thu, May 27, 2010 at 4:11 PM, Marko Kreen [off-list ref] wrote:
> On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> On Thu, May 27, 2010 at 3:58 PM, Marko Kreen [off-list ref] wrote:
>> > On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> >> On Thu, May 27, 2010 at 3:29 PM, Marko Kreen [off-list ref] wrote:
>> >> > On 5/27/10, Erik Faye-Lund [off-list ref] wrote:
>> >> >> On Thu, May 27, 2010 at 1:00 PM, Erik Faye-Lund
>> >> >> [off-list ref] wrote:
>> >> >> > On Thu, May 27, 2010 at 12:10 PM, Jonathan Nieder [off-list ref] wrote:
>> >> >> >> Implement the subset of poll() semantics needed by git in terms of
>> >> >> >> select(), for use by the Interix port. Inspired by commit 6ed807f
>> >> >> >> (Windows: A rudimentary poll() emulation, 2007-12-01).
>> >> >> >>
>> >> >> >
>> >> >> > A possible problem with this approach is that the maximum number of
>> >> >> > file descriptors poll can handle limited by RLIMIT_NOFILE, whereas the
>> >> >> > maximum number of file descriptors select can handle is limited by
>> >> >> > FD_SETSIZE.
>> >> >> >
>> >> >> > I don't think this is a big problem in reality, though - both values
>> >> >> > seem to be pretty high in most implementations. And IIRC git-daemon is
>> >> >> > the only one who needs more than 2, and it doesn't even check
>> >> >> > RLIMIT_NOFILE.
>> >> >> >
>> >> >>
>> >> >>
>> >> >> To be clear: I think this strategy is the best option (at least for
>> >> >> non-Windows, where select() might be our only option).
>> >> >>
>> >> >> But perhaps you should include a check along the lines of this:
>> >> >>
>> >> >> if (nfds > FD_SETSIZE)
>> >> >> return errno = EINVAL, error("poll: nfds must be below %d", FD_SETSIZE);
>> >> >>
>> >> >> Just so we can know when the code fails :)
>> >> >
>> >> > Well, per your own FD_SET example, the FD_SETSIZE on windows
>> >> > means different thing than FD_SETSIZE on old-style bitmap-based
>> >> > select() implementation.
>> >> >
>> >> > On Unix, it's max fd number + 1, on windows it's max count.
>> >> >
>> >>
>> >>
>> >> Are you sure this applies for all Unix, not just some given Unix-y system?
>> >
>> > Not sure. Just pointing out that the above check is not
>> > universal enough.
>> >
>>
>>
>> Isn't it? How could one possibly pass more than max fd number + 1 file
>> descriptors, since they start at 0? I guess one could specify a given
>> fd more than once, but that'd be kind of redundant... and also very
>> unlikely in our case ;)
>
> Pass one fd with value 70 it. Check returns error, although
> everything would work.
>
No, not with the check I posted. I checked nfds, not the value of the
fds themselves. nfds is clearly the size of the fds array -- if it
wasn't, it'd be impossible for the poll-implementation to know how big
the array is!
Ah, ok. I though this is the nfds for select(). In this case it can
still fail, as nfds=1 for poll() can still go over FD_SETSIZE for
select() if the fd value is big enough. On non-windows, that is.
I don't quite get where one would get a fd that's value is FD_SETSIZE
or above from. I mean, FD_SETSIZE would have to be the maximum value
of a file descriptor, or else you risk open() or socket() returning
file descriptors that cannot be select()'ed. Which would just be
insane, no?
--
Erik "kusma" Faye-Lund
From: Peter Kjellerstedt <hidden> Date: 2016-06-15 22:48:52
-----Original Message-----
From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On
Behalf Of Erik Faye-Lund
Sent: den 27 maj 2010 16:44
To: Marko Kreen
Cc: Jonathan Nieder; Jonathan Callen; git@vger.kernel.org;
mduft@gentoo.org; Sverre Rabbelier; Michael J Gruber; Johannes Sixt;
msysGit
Subject: Re: [PATCH] compat: Add another rudimentary poll() emulation
On Thu, May 27, 2010 at 4:32 PM, Marko Kreen [off-list ref] wrote:
[cut]
quoted
Ah, ok. I though this is the nfds for select(). In this case it can
still fail, as nfds=1 for poll() can still go over FD_SETSIZE for
select() if the fd value is big enough. On non-windows, that is.
I don't quite get where one would get a fd that's value is FD_SETSIZE
or above from. I mean, FD_SETSIZE would have to be the maximum value
of a file descriptor, or else you risk open() or socket() returning
file descriptors that cannot be select()'ed. Which would just be
insane, no?
--
Erik "kusma" Faye-Lund
That is exactly how e.g. Linux works. If your application can have more
than 1024 file descriptors open simultaneously, you are restricted to
use poll() as you risk having a file descriptor with a value >= 1024.
And using FD_SET() with a file descriptor >= 1024 will trash memory...
//Peter
From: Jakub Narebski <hidden> Date: 2016-06-15 22:48:52
Jonathan Callen [off-list ref] writes:
This series of patches adds support for building git on Interix.
Interix is an interesting system, as it lacks some functions that are
normally present on POSIX systems, such as poll(). As I did not have
time to implement poll() on top of select(), these patches simply
disable the three commands that require poll() to be present:
git-daemon, git-upload-archive, and git-upload-pack.
Jonathan Callen (3):
Support building on systems without poll(2)
Support building without inttypes.h
Add Interix support
Makefile | 40 +++++++++++++++++++++++++++++++++++-----
builtin.h | 2 ++
git-compat-util.h | 6 ++++++
git.c | 2 ++
4 files changed, 45 insertions(+), 5 deletions(-)
Could you please, if you can, add support for detecting existence of
poll(2) and existence of inttypes.h to configure.ac, to make it
automatically detected by
$ make configure
$ ./configure [options]
Thanks in advance.
--
Jakub Narebski
Poland
ShadeHawk on #git
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:48:52
Implement the subset of poll() semantics needed by git in terms of
select(), for use by the Interix port. Inspired by commit 6ed807f
(Windows: A rudimentary poll() emulation, 2007-12-01).
This will only poll on descriptors with value <= the maximum fd that
select() can portably handle, FD_SETSIZE - 1. On Windows (and not on
Unix), in principle git could permit higher fds as long as the number
of descriptors used in a single select() call was limited to
FD_SETSIZE; but such a facility would be rarely used, since the number
of sockets the ‘git daemon’ parent process keeps open tends to be very
small.
Helped-by: Erik Faye-Lund [off-list ref]
Cc: Johannes Sixt <redacted>
Cc: Jonathan Callen <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Erik Faye-Lund wrote:
But perhaps you should include a check along the lines of this:
if (nfds > FD_SETSIZE)
return errno = EINVAL, error("poll: nfds must be below %d", FD_SETSIZE);
Just so we can know when the code fails :)
Good catch, thanks. I opted to check each fd instead so the NO_POLL
code can be safely used on Unix for debugging.
Other changes from v1:
. use COMPAT_CFLAGS instead of BASIC_CFLAGS
. timeout is a number of milliseconds, not seconds
. lightly tested on Linux
I would be interested to hear whether this works on msysgit and Interix.
Thanks again for the help.
Makefile | 8 +++++++
compat/poll.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
compat/poll.h | 13 ++++++++++++
git-compat-util.h | 6 ++++-
4 files changed, 82 insertions(+), 1 deletions(-)
create mode 100644 compat/poll.c
create mode 100644 compat/poll.h
@@ -111,6 +111,9 @@ all::## Define NO_PTHREADS if you do not have or do not want to use Pthreads.#+# Define NO_POLL if you have a problem with the poll() system call (e.g.+# Interix).+## Define NO_PREAD if you have a problem with pread() system call (e.g.# cygwin1.dll before v1.5.22).#
@@ -0,0 +1,56 @@+#include"../git-compat-util.h"++staticintmsleep(inttimeout)+{+structtimevaltv;+tv.tv_sec=0;+tv.tv_usec=1000*timeout;+returnselect(0,NULL,NULL,NULL,&tv);+}++staticintvalidate_fd(conststructgit_pollfd*ufd)+{+constintfd=ufd->fd;+if(ufd->events!=POLLIN){+errno=EINVAL;+returnerror("poll: unsupported events");+}+if(fd>=FD_SETSIZE){+errno=EINVAL;+returnerror("poll: fd must be below %d",FD_SETSIZE);+}+returnfd;+}++intgit_poll(structgit_pollfd*ufds,intnfds,inttimeout)+{+inti,maxfd,result;+fd_setfds;++if(timeout>=0){+if(nfds!=0){+errno=EINVAL;+returnerror("poll timeout not supported");+}+returnmsleep(timeout);+}++FD_ZERO(&fds);+maxfd=-1;+for(i=0;i<nfds;i++){+constintfd=validate_fd(&ufds[i]);+if(fd<0)+returnfd;+maxfd=(fd>maxfd)?fd:maxfd;+FD_SET(fd,&fds);+}++result=select(maxfd+1,&fds,NULL,NULL,NULL);+if(result<0)+returnresult;+for(i=0;i<nfds;i++){+if(FD_ISSET(ufds[i].fd,&fds))+ufds[i].revents|=POLLIN;+}+returnresult;+}
From: Johannes Sixt <hidden> Date: 2016-06-15 22:48:52
On Sonntag, 30. Mai 2010, Jonathan Nieder wrote:
I would be interested to hear whether this works on msysgit and Interix.
It cannot work on msysgit because
- on Windows, select() works only for sockets, but we poll() on pipes, too;
- in our emulation layer, fds that are sockets must be unpacked with
_get_osfhandle() before they can be passed to FD_SET() because (as Paolo has
pointed out) FD_SET() must be populated with handles, not fds.
-- Hannes
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:48:52
Johannes Sixt wrote:
It cannot work on msysgit because
- on Windows, select() works only for sockets, but we poll() on pipes, too;
- in our emulation layer, fds that are sockets must be unpacked with
_get_osfhandle() before they can be passed to FD_SET() because (as Paolo has
pointed out) FD_SET() must be populated with handles, not fds.
impression that FD_SETSIZE on Interix might not be so small after all
(good). Apparently older versions of Interix (5.3?) do not obey
timeouts properly, so the poll() in compat/poll.c should probably use
usleep() instead (meh).
Anyway, I leave it to developers on that platform to take it from here
if they so wish.
Ciao,
Jonathan
On May 29, 2010, at 5:37 PM, Jonathan Nieder wrote:
Implement the subset of poll() semantics needed by git in terms of
select(), for use by the Interix port. Inspired by commit 6ed807f
(Windows: A rudimentary poll() emulation, 2007-12-01).
I would be interested to hear whether this works on msysgit and
Interix.
For what it's worth, after creating a poll_compat library for Lamp
(Lamp ain't Mac POSIX) from the original plproxy code and linking git
against it, I'm able to serve fetch requests from git daemon. I
haven't tried simultaneous connections, though.
http://github.com/jjuran/git/
Josh
I assume the lamp-git branch is the interesting one. Some of
these patches (e.g., a3b378781) look like they might be of general
interest, while others (e.g., 6e6b106d) reveal Metrowerks to be
a bit braindead.
Have you thought about getting git to compile in C++ mode, where
Metrowerks might be a little more sane[1]? Sure, this runs into
basically all the major incompatibilities between C and C++[2], but
that might not be insurmountable:
. No implicit conversion from void* to other: don’t use void *,
then. With type-safe interfaces like
#define typed_malloc(size, type) (type *)xmalloc(size)
#define malloc_many(nmemb, type) typed_malloc((nmemb) * sizeof(type), type)
one can take advantage of type checking without the annoyance of
casts at the call site.
Another place git uses void * is for low-level access to the
object database, because it is not obvious whether objects data
should use char * or unsigned char *. unsigned char * should
be fine.
. Use of C++ keywords:
#ifdef __cplusplus
#define template git_template
#define typename git_typename
#endif
It is not like this is an actual C++ program.
. Assignment of ints to enums is forbidden: okay, this one is
not worth working around. Does Metrowerks have an option to turn
off this piece of C++ insanity?
Curious,
Jonathan
[1] e.g., to solve the problem you described before:
http://thread.gmane.org/gmane.comp.version-control.git/115301
[2] http://www2.research.att.com/~bs/bs_faq.html#merge
I assume the lamp-git branch is the interesting one. Some of
The lamp-git branch is the one you'd checkout to build git. It
merges the interesting ones.
these patches (e.g., a3b378781) look like they might be of general
$ git show a3b378781
commit a3b378781228d59a13ee055d06c8f5465b543178
Author: Joshua Juran [off-list ref]
Date: Sun Nov 29 03:57:54 2009 -0800
Avoid pointless use of one-celled array (which also confounds
Metrowerks C).
Pass the address of fetch_refspec_str directly to
parse_refspec_internal()
instead of copying fetch_refspec_str to an array and passing that.
Metrowerks C won't initialize aggregates with values determined
at runtime, and
we can't use the C++ translator for parse_refspec_internal()
because it uses the
same identifier (refspec) for a variable as an existing struct
type.
That, and there's no purpose to the array in the first place.
@@ -132,8 +132,8 @@ static int read_pack_info_file(const char *infofile)staticintcompare_info(constvoid*a_,constvoid*b_){-structpack_info*const*a=a_;-structpack_info*const*b=b_;+conststructpack_info*const*a=a_;+conststructpack_info*const*b=b_;if(0<=(*a)->old_num&&0<=(*b)->old_num)/* Keep the order in the original */
I certainly won't defend Metrowerks C 2.4.1, but the patch is
harmless enough.
Have you thought about getting git to compile in C++ mode, where
If the Git maintainers are interested in this, I'm glad to do it. I
expect it to require generally less maintenance than my c++-
initializer-fix branch, which is rather ugly to boot.
Metrowerks might be a little more sane[1]? Sure, this runs into
Well, I wasn't going to suggest general C++ compatibility just for my
port, but if there's interest, then yes, that would obviate my (by
far) largest and nastiest patch.
basically all the major incompatibilities between C and C++[2], but
that might not be insurmountable:
Writing code which is both valid C and valid C++ is not difficult.
. No implicit conversion from void* to other: don’t use void *,
then. With type-safe interfaces like
#define typed_malloc(size, type) (type *)xmalloc(size)
#define malloc_many(nmemb, type) typed_malloc((nmemb) * sizeof
(type), type)
one can take advantage of type checking without the annoyance of
casts at the call site.
Replacing xmalloc(size) with typed_malloc(size, struct foo) instead
of (struct foo *)xmalloc(size) still feels like a cast to me. IMHO
it adds no safety and reduces readability.
Maybe you could have:
#define alloc_n(type, n) ((type*)xmalloc(sizeof (type) * n))
to factor the multiplication out of the calling code. But for a
plain data buffer, nothing is shorter or clearer than a simple
(char*) cast.
Another place git uses void * is for low-level access to the
object database, because it is not obvious whether objects data
should use char * or unsigned char *. unsigned char * should
be fine.
If there were a configurable typedef defining this, I'd use char*
since the Metrowerks debugger by default displays it as a C string
(as opposed to a Pascal string for unsigned char* -- it's a Mac OS
convention). But that's a minor point and not one I'd spend time
advocating.
. Use of C++ keywords:
#ifdef __cplusplus
#define template git_template
#define typename git_typename
#endif
It is not like this is an actual C++ program.
My c++initializer-fix patch does this locally in a few places. I
suppose it makes sense to nail them all at once in git-compat-util.h.
. Assignment of ints to enums is forbidden: okay, this one is
not worth working around. Does Metrowerks have an option to turn
off this piece of C++ insanity?
It's a feature. :-) I use enums for type-safety in my C++ POSIX
wrapper library. p7::wait() returns p7::wait_t, and if you try to
pass that to p7::exit() (which takes p7::exit_t) you get a compile
error (instead of a program that 'works' when the wait status is zero).
(No, I'm not aware of an option to disable this restriction.)
If you want the type-safety, you have to pay for it. Otherwise,
declare variables and parameters as integral types instead. I don't
care one way or the other -- I just want git to work on Lamp, and if
there's community interest in C++ compatibility then I can assist
with that.
Josh
P.S. Every patch in my git repository is Signed-off-by: Joshua Juran
[off-list ref] (or s/gmail/metamage/).
@@ -132,8 +132,8 @@ static int read_pack_info_file(const char *infofile)staticintcompare_info(constvoid*a_,constvoid*b_){-structpack_info*const*a=a_;-structpack_info*const*b=b_;+conststructpack_info*const*a=a_;+conststructpack_info*const*b=b_;if(0<=(*a)->old_num&&0<=(*b)->old_num)/* Keep the order in the original */
I certainly won't defend Metrowerks C 2.4.1, but the patch is
harmless enough.
Yes, it is even a good change.
It just emphasized for me that Metrowerks is a little confused
about type qualifiers.
#define alloc_n(type, n) ((type*)xmalloc(sizeof (type) * n))
to factor the multiplication out of the calling code.
Maybe, imitating the signature of calloc:
#define malloc_n(n, type) ((type *)xmalloc((n) * sizeof(type)))
For structs with variable-length arrays at the end:
#define malloc_plus(type, extra) ((type *)xmalloc(sizeof(type) + (extra)))
That should cover most of the calls in git.
I just want git to work on Lamp, and if
there's community interest in C++ compatibility then I can assist
with that.
There probably is not much direct interest. But if in the process we
gain a little type-safety and portability, that would be nice. :)
Thanks for your hard work,
Jonathan
This code will do the right thing only when timeout is < 1000. (This
is probably true for us in practice, so this is likely just
nitpicking.) For the general case, you'd want
tv.tv_sec = timeout / 1000;
tv.tv_usec = 1000 * (timeout % 1000);
--bert
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:48:53
Albert Dvornik wrote:
For the general case, you'd want
tv.tv_sec = timeout / 1000;
tv.tv_usec = 1000 * (timeout % 1000);
Thanks for the catch. Actually, it is not so unlikely that someone
would ask the autocorrect to wait longer than a second.
On Linux this is not an issue, but maybe Interix cares. Posix is
vague and only says "the timeout period is given in seconds and
microseconds", staying silent on what the range of valid values is.
Jonathan
From: Albert Dvornik <hidden> Date: 2016-06-15 22:48:53
On Mon, May 31, 2010 at 8:46 AM, Jonathan Nieder [off-list ref] wrote:
Albert Dvornik wrote:
quoted
For the general case, you'd want
tv.tv_sec = timeout / 1000;
tv.tv_usec = 1000 * (timeout % 1000);
Thanks for the catch. Actually, it is not so unlikely that someone
would ask the autocorrect to wait longer than a second.
Good point, I take back what I said. =)
On Linux this is not an issue, but maybe Interix cares. Posix is
vague and only says "the timeout period is given in seconds and
microseconds", staying silent on what the range of valid values is.
Some traditional UNIXes are actually picky about the usec range, or at
least used to be (I seem to recall BSD and/or Solaris, but I really
don't remember for sure, this was a long time ago).
--bert