RE: Porting git to HP NonStop
From: Junio C Hamano [mailto:gits...@pobox.com]
Sent: Monday, August 20, 2012 6:54 PM
To: Joachim Schmitz
Cc: 'Shawn Pearce'; git@vger.kernel.org; rsbec...@nexbridge.com
Subject: Re: Porting git to HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes:
I haven't found any other to be needed. Well, poll, maybe, but with
only minor tweaks for the win32 one works for me (and those tweaks are
compatible with win32
A separate file, compat/tandem/mkdir.c, is fine, though.
If you wouldn't have dozens of them, so compat/tandem/mkdir.c is not
suitable;
compat/tandem.c would be good, then.
I'll go for git_mkdir(), similar to other git wrappers, (like for
mmap, pread, fopen, snprintf, vsnprintf, qsort).
Again, no. Your breakage is that having underlying system mkdir that
does
not
understand trailing slash, which may not be specific to __TANDEM, but
still is
_not_ the only possible mode of breakage.
True.
Well, it is the only one GNUlib's mkdir caters for and I'd regard that
an authoritative source...
I suspect that you may be misunderstanding what compat/ is about
I don't think so, it server the same purpose for git as gnulib does for
others.
, so let's try again.
Platform difference in mkdir may not be limited to on this platform, the
underlying one supplied by the system does not like path ending with a
slash.
What I am saying is that it is unacceptable to call something that caters
to that
specific kind of difference from what the codebase expects with a generic
name such as git_mkdir(). Look at mingw's replacement. The platform
difference over there is that the one from the system does not take mode
parameter. Imagine that one was already called git_mkdir(). Now we
have
two different kind of differences, and one has more officially-looking
git_mkdir() name; yours cannot take it---what would you do in that case?
Neither kind of difference is more officially sanctioned difference; don't
call
yours any more official/generic than necessary.
Gnulib's rpl_mkdir caters for 3 possible problems, the WIN32 one which mkdir
taking only one argument, the trailing slash one discussed here (victims
being at least NetBSD 1.5.2 and current HP NonStop) and a trailing dot one
(that allegedly Cygwin 1.5 suffered from).
As far as I can see git will not suffer from the latter, but even if, at
that time a git_mkdir() could be expanded to cater for this to, just like
gnulib's one does, there it is an additional section inside their
rpl_mkdir().
And the WIN32 one is already being taken care of in compat/mingw.h. However,
this could as easily get integrated into
a git_mkdir(), just like in gnulib.
Your wrapper is not limited to tandem, but is applicable to ancient BSDs,
so it is
fine to call it as compat_mkdir_wo_trailing_slash(), so that it can be
shared
among platforms whose mkdir do not want to see trailing slashes. If you
are
going that route, the function should live in its own file (without any
other
wrapper), and not be named after specific platform (should be named after
the
specific difference from what we expect, instead). I am perfectly fine
with that
approach as well.
Squatting on a generic git_mkdir() name makes it harder for other
people
to
name their compat mkdir functions to tweak for the breakage on their
platforms. The examples you listed are all the platform does not
offer
it, so
we implement the whole thing kind, so it is in a different genre.
Nope, git_fopen() definitly is a wrapper for fopen(), as is
git_vsnprintf() for vsnprintf().
I was talking more about mmap() and pread().
For the two you mentioned, ideally they should have been named after the
specific breakages they cover (fopen that does not error out on
directories is
primarily AIX thing IIRC, and snprintf returns bogus result are shared
between
HPUX and Windows), but over these years we haven't seen any other kind of
differences from various platforms, so the need to rename them away is
very
low.
On the other hand, we already know there are at least two kinds of
platform
mkdir() that need different compat/ layer support, so calling one
git_mkdir()
to cover one particular kind of difference does not make any sense.
Besides, an earlier mistake is not a valid excuse to add new mistakes.
OK, so how about this:
/usr/local/bin/diff -EBbu ./compat/mkdir.c.orig ./compat/mkdir.c
--- ./compat/mkdir.c.orig 2012-08-21 05:02:11 -0500
+++ ./compat/mkdir.c2012-08-21 05:02:11 -0500
@@ -0,0 +1,24 @@
+#include ../git-compat-util.h
+#undef mkdir
+
+/* for platforms that can't deal with a trailing '/' */
+int compat_mkdir_wo_trailing_slash(const char *dir, mode_t mode)
+{
+ int retval;
+ char *tmp_dir = NULL;
+ size_t len = strlen(dir);
+
+ if (len dir[len-1] == '/') {
+ if ((tmp_dir = strdup(dir)) == NULL)
+ return -1;
+ tmp_dir[len-1] = '\0
Re: Porting git to HP NonStop
On Wed, Aug 22, 2012 at 9:30 AM, Joachim Schmitz
j...@schmitz-digital.de wrote:
OK, so how about this:
/usr/local/bin/diff -EBbu ./compat/mkdir.c.orig ./compat/mkdir.c
--- ./compat/mkdir.c.orig 2012-08-21 05:02:11 -0500
+++ ./compat/mkdir.c2012-08-21 05:02:11 -0500
@@ -0,0 +1,24 @@
+#include ../git-compat-util.h
+#undef mkdir
+
+/* for platforms that can't deal with a trailing '/' */
+int compat_mkdir_wo_trailing_slash(const char *dir, mode_t mode)
+{
+ int retval;
+ char *tmp_dir = NULL;
+ size_t len = strlen(dir);
+
+ if (len dir[len-1] == '/') {
+ if ((tmp_dir = strdup(dir)) == NULL)
+ return -1;
+ tmp_dir[len-1] = '\0';
+ }
+ else
+ tmp_dir = (char *)dir;
+
+ retval = mkdir(tmp_dir, mode);
+ if (tmp_dir != dir)
+ free(tmp_dir);
+
+ return retval;
+}
Why not rearrange this so that you assign to dir the value of tmp_dir
and then just pass dir to mkdir. Then you can avoid the recast of dir
to (char*) in the else branch. Later, just call free(tmp_dir). Also,
we have xstrndup. So I think the body of your function can become
something like:
if (len dir[len-1] == '/')
dir = tmp_dir = xstrndup(dir, len-1);
retval = mkdir(dir, mode);
free(tmp_dir);
-Brandon
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
On Wed, Aug 22, 2012 at 10:00 AM, Brandon Casey draf...@gmail.com wrote: Also, we have xstrndup. So I think the body of your function can become something like: if (len dir[len-1] == '/') dir = tmp_dir = xstrndup(dir, len-1); retval = mkdir(dir, mode); free(tmp_dir); Actually, xmemdupz could be used in place of xstrndup since we've already called strlen. -Brandon -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
From: Brandon Casey [mailto:draf...@gmail.com]
Sent: Wednesday, August 22, 2012 7:01 PM
To: Joachim Schmitz
Cc: Junio C Hamano; Shawn Pearce; git@vger.kernel.org;
rsbec...@nexbridge.com
Subject: Re: Porting git to HP NonStop
On Wed, Aug 22, 2012 at 9:30 AM, Joachim Schmitz j...@schmitz-digital.de
wrote:
OK, so how about this:
/usr/local/bin/diff -EBbu ./compat/mkdir.c.orig ./compat/mkdir.c
--- ./compat/mkdir.c.orig 2012-08-21 05:02:11 -0500
+++ ./compat/mkdir.c2012-08-21 05:02:11 -0500
@@ -0,0 +1,24 @@
+#include ../git-compat-util.h
+#undef mkdir
+
+/* for platforms that can't deal with a trailing '/' */ int
+compat_mkdir_wo_trailing_slash(const char *dir, mode_t mode) {
+ int retval;
+ char *tmp_dir = NULL;
+ size_t len = strlen(dir);
+
+ if (len dir[len-1] == '/') {
+ if ((tmp_dir = strdup(dir)) == NULL)
+ return -1;
+ tmp_dir[len-1] = '\0';
+ }
+ else
+ tmp_dir = (char *)dir;
+
+ retval = mkdir(tmp_dir, mode);
+ if (tmp_dir != dir)
+ free(tmp_dir);
+
+ return retval;
+}
Why not rearrange this so that you assign to dir the value of tmp_dir and then
just pass dir to mkdir. Then you can avoid the recast of dir to (char*) in
the
else branch. Later, just call free(tmp_dir). Also, we have xstrndup. So I
think
the body of your function can become something like:
if (len dir[len-1] == '/')
dir = tmp_dir = xstrndup(dir, len-1);
xstndup() can't fail?
retval = mkdir(dir, mode);
free(tmp_dir);
-Brandon
Bye, Jojo
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
On Wed, Aug 22, 2012 at 10:18 AM, Joachim Schmitz
j...@schmitz-digital.de wrote:
From: Brandon Casey [mailto:draf...@gmail.com]
Sent: Wednesday, August 22, 2012 7:01 PM
To: Joachim Schmitz
Cc: Junio C Hamano; Shawn Pearce; git@vger.kernel.org;
rsbec...@nexbridge.com
Subject: Re: Porting git to HP NonStop
On Wed, Aug 22, 2012 at 9:30 AM, Joachim Schmitz j...@schmitz-digital.de
wrote:
OK, so how about this:
/usr/local/bin/diff -EBbu ./compat/mkdir.c.orig ./compat/mkdir.c
--- ./compat/mkdir.c.orig 2012-08-21 05:02:11 -0500
+++ ./compat/mkdir.c2012-08-21 05:02:11 -0500
@@ -0,0 +1,24 @@
+#include ../git-compat-util.h
+#undef mkdir
+
+/* for platforms that can't deal with a trailing '/' */ int
+compat_mkdir_wo_trailing_slash(const char *dir, mode_t mode) {
+ int retval;
+ char *tmp_dir = NULL;
+ size_t len = strlen(dir);
+
+ if (len dir[len-1] == '/') {
+ if ((tmp_dir = strdup(dir)) == NULL)
+ return -1;
+ tmp_dir[len-1] = '\0';
+ }
+ else
+ tmp_dir = (char *)dir;
+
+ retval = mkdir(tmp_dir, mode);
+ if (tmp_dir != dir)
+ free(tmp_dir);
+
+ return retval;
+}
Why not rearrange this so that you assign to dir the value of tmp_dir and
then
just pass dir to mkdir. Then you can avoid the recast of dir to (char*) in
the
else branch. Later, just call free(tmp_dir). Also, we have xstrndup. So I
think
the body of your function can become something like:
if (len dir[len-1] == '/')
dir = tmp_dir = xstrndup(dir, len-1);
xstndup() can't fail?
Correct. It will either succeed or die. It will also try to free up
some memory used by git if possible.
-Brandon
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Brandon Casey draf...@gmail.com writes:
On Wed, Aug 22, 2012 at 9:30 AM, Joachim Schmitz
j...@schmitz-digital.de wrote:
OK, so how about this:
/usr/local/bin/diff -EBbu ./compat/mkdir.c.orig ./compat/mkdir.c
--- ./compat/mkdir.c.orig 2012-08-21 05:02:11 -0500
+++ ./compat/mkdir.c2012-08-21 05:02:11 -0500
@@ -0,0 +1,24 @@
+#include ../git-compat-util.h
+#undef mkdir
+
+/* for platforms that can't deal with a trailing '/' */
+int compat_mkdir_wo_trailing_slash(const char *dir, mode_t mode)
+{
+ int retval;
+ char *tmp_dir = NULL;
+ size_t len = strlen(dir);
+ ...
Why not rearrange this so that you assign to dir the value of tmp_dir
and then just pass dir to mkdir. Then you can avoid the recast of dir
to (char*) in the else branch. Later, just call free(tmp_dir). Also,
we have xstrndup. So I think the body of your function can become
something like:
if (len dir[len-1] == '/')
dir = tmp_dir = xstrndup(dir, len-1);
retval = mkdir(dir, mode);
free(tmp_dir);
Nice. And we have xmemdupz() would be even better as you
followed-up.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
From: Brandon Casey [mailto:draf...@gmail.com]
Sent: Wednesday, August 22, 2012 7:23 PM
To: Joachim Schmitz
Cc: Junio C Hamano; Shawn Pearce; git@vger.kernel.org;
rsbec...@nexbridge.com
Subject: Re: Porting git to HP NonStop
On Wed, Aug 22, 2012 at 10:18 AM, Joachim Schmitz j...@schmitz-digital.de
wrote:
From: Brandon Casey [mailto:draf...@gmail.com]
Sent: Wednesday, August 22, 2012 7:01 PM
To: Joachim Schmitz
Cc: Junio C Hamano; Shawn Pearce; git@vger.kernel.org;
rsbec...@nexbridge.com
Subject: Re: Porting git to HP NonStop
On Wed, Aug 22, 2012 at 9:30 AM, Joachim Schmitz
j...@schmitz-digital.de
wrote:
OK, so how about this:
/usr/local/bin/diff -EBbu ./compat/mkdir.c.orig ./compat/mkdir.c
--- ./compat/mkdir.c.orig 2012-08-21 05:02:11 -0500
+++ ./compat/mkdir.c2012-08-21 05:02:11 -0500
@@ -0,0 +1,24 @@
+#include ../git-compat-util.h
+#undef mkdir
+
+/* for platforms that can't deal with a trailing '/' */ int
+compat_mkdir_wo_trailing_slash(const char *dir, mode_t mode) {
+ int retval;
+ char *tmp_dir = NULL;
+ size_t len = strlen(dir);
+
+ if (len dir[len-1] == '/') {
+ if ((tmp_dir = strdup(dir)) == NULL)
+ return -1;
+ tmp_dir[len-1] = '\0';
+ }
+ else
+ tmp_dir = (char *)dir;
+
+ retval = mkdir(tmp_dir, mode);
+ if (tmp_dir != dir)
+ free(tmp_dir);
+
+ return retval;
+}
Why not rearrange this so that you assign to dir the value of tmp_dir
and then just pass dir to mkdir. Then you can avoid the recast of
dir to (char*) in the else branch. Later, just call free(tmp_dir).
Also, we have xstrndup. So I think the body of your function can become
something like:
if (len dir[len-1] == '/')
dir = tmp_dir = xstrndup(dir, len-1);
xstndup() can't fail?
Correct. It will either succeed or die. It will also try to free up some
memory
used by git if possible.
OK. So let's use that then.
Bye, Jojo
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Am 22.08.2012 19:00, schrieb Brandon Casey: So I think the body of [compat_mkdir] can become something like: if (len dir[len-1] == '/') dir = tmp_dir = xstrndup(dir, len-1); Don't use x* wrappers in the compat layer, at least not those that allocate memory: They behave unpredictably due to try_to_free_routine and may lead to recursive invocations. retval = mkdir(dir, mode); free(tmp_dir); -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Am 22.08.2012 20:02, schrieb Joachim Schmitz: From: Johannes Sixt [mailto:j...@kdbg.org] Don't use x* wrappers in the compat layer, at least not those that allocate memory: They behave unpredictably due to try_to_free_routine and may lead to recursive invocations. I was just following orders ;-) What about the other proposal, xmemdupz? Same story I guess? xmemdupz calls xmalloc, so, yes, same story. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
On Wed, Aug 22, 2012 at 10:41 AM, Johannes Sixt j...@kdbg.org wrote: Am 22.08.2012 19:00, schrieb Brandon Casey: So I think the body of [compat_mkdir] can become something like: if (len dir[len-1] == '/') dir = tmp_dir = xstrndup(dir, len-1); Don't use x* wrappers in the compat layer, at least not those that allocate memory: They behave unpredictably due to try_to_free_routine and may lead to recursive invocations. I thought that rule only applied to die handlers. i.e. don't use the x* wrappers to allocate memory in a die handler like compat/win32/syslog.c. At least that's what I wrote in 040a6551 when you pointed out this issue back then. Admittedly, it could get pretty sticky trying to trace the die handlers to ensure they don't invoke your new compat/ function. So, yeah, adopting this rule of not using x* wrappers that allocate memory in compat/ generally seems like a good idea. Should we also try to detect recursive invocation of die and friends? In theory recursion could be triggered by any die handler that makes use of a code path that calls an x* wrapper that allocates memory, couldn't it? -Brandon -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
-Original Message- From: Johannes Sixt [mailto:j...@kdbg.org] Sent: Wednesday, August 22, 2012 8:09 PM To: Joachim Schmitz Cc: 'Brandon Casey'; 'Junio C Hamano'; 'Shawn Pearce'; git@vger.kernel.org; rsbec...@nexbridge.com Subject: Re: Porting git to HP NonStop Am 22.08.2012 20:02, schrieb Joachim Schmitz: From: Johannes Sixt [mailto:j...@kdbg.org] Don't use x* wrappers in the compat layer, at least not those that allocate memory: They behave unpredictably due to try_to_free_routine and may lead to recursive invocations. I was just following orders ;-) What about the other proposal, xmemdupz? Same story I guess? xmemdupz calls xmalloc, so, yes, same story. So back to my original patch, using strdup, check the return value, etc. Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
On Wed, Aug 22, 2012 at 11:09 AM, Brandon Casey draf...@gmail.com wrote:
On Wed, Aug 22, 2012 at 10:41 AM, Johannes Sixt j...@kdbg.org wrote:
Don't use x* wrappers in the compat layer, at least not those that
allocate memory: They behave unpredictably due to try_to_free_routine
and may lead to recursive invocations.
I thought that rule only applied to die handlers. i.e. don't use the
x* wrappers to allocate memory in a die handler like
compat/win32/syslog.c. At least that's what I wrote in 040a6551 when
you pointed out this issue back then.
Admittedly, it could get pretty sticky trying to trace the die
handlers to ensure they don't invoke your new compat/ function. So,
yeah, adopting this rule of not using x* wrappers that allocate memory
in compat/ generally seems like a good idea.
Should we also try to detect recursive invocation of die and friends?
In theory recursion could be triggered by any die handler that makes
use of a code path that calls an x* wrapper that allocates memory,
couldn't it?
Perhaps something like:
diff --git a/usage.c b/usage.c
index a2a6678..2d0ff35 100644
--- a/usage.c
+++ b/usage.c
@@ -80,8 +80,15 @@ void NORETURN usage(const char *err)
void NORETURN die(const char *err, ...)
{
+ static int dying;
va_list params;
+ if (dying) {
+ fputs(fatal: recursion detected in die handler\n, stderr);
+ exit(128);
+ }
+ dying = 1;
+
va_start(params, err);
die_routine(err, params);
va_end(params);
@@ -89,11 +96,18 @@ void NORETURN die(const char *err, ...)
void NORETURN die_errno(const char *fmt, ...)
{
+ static int dying;
va_list params;
char fmt_with_err[1024];
char str_error[256], *err;
int i, j;
+ if (dying) {
+ fputs(fatal: recursion detected in die handler\n, stderr);
+ exit(128);
+ }
+ dying = 1;
+
err = strerror(errno);
for (i = j = 0; err[i] j sizeof(str_error) - 1; ) {
if ((str_error[j++] = err[i++]) != '%')
-Brandon
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes:
Nice. And we have xmemdupz() would be even better as you followed-up.
How's that one used?
I forgot that we frown upon use of any xallocate() wrapper in the
compat/ layer as J6t mentioned.
So probably something along these lines...
int retval;
char *dir_to_free = NULL;
size_t len = strlen(dir);
if (len dir[len - 1] == '/') {
dir_to_free = malloc(len);
if (!dir_to_free) {
fprintf(stderr, malloc failed!\n);
exit(1);
}
memcpy(dir_to_free, dir, len - 1);
dir_to_free[len - 1] = '\0';
dir = dir_to_free;
}
retval = mkdir(dir, mode);
free(dir_to_free);
return retval;
It might be possible to for the error path to get away with
something like:
if (!dir_to_free)
return -1;
if we know the callers are prepared to see mkdir() failing with
ENOMEM, but that is not very likely.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Brandon Casey draf...@gmail.com writes: On Wed, Aug 22, 2012 at 10:41 AM, Johannes Sixt j...@kdbg.org wrote: Am 22.08.2012 19:00, schrieb Brandon Casey: So I think the body of [compat_mkdir] can become something like: if (len dir[len-1] == '/') dir = tmp_dir = xstrndup(dir, len-1); Don't use x* wrappers in the compat layer, at least not those that allocate memory: They behave unpredictably due to try_to_free_routine and may lead to recursive invocations. I thought that rule only applied to die handlers. i.e. don't use the x* wrappers to allocate memory in a die handler like compat/win32/syslog.c. At least that's what I wrote in 040a6551 when you pointed out this issue back then. Admittedly, it could get pretty sticky trying to trace the die handlers to ensure they don't invoke your new compat/ function. So, yeah, adopting this rule of not using x* wrappers that allocate memory in compat/ generally seems like a good idea. Should we also try to detect recursive invocation of die and friends? In theory recursion could be triggered by any die handler that makes use of a code path that calls an x* wrapper that allocates memory, couldn't it? Correct, but at that point we will end up dying anyway, so... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Brandon Casey draf...@gmail.com writes:
Perhaps something like:
diff --git a/usage.c b/usage.c
index a2a6678..2d0ff35 100644
--- a/usage.c
+++ b/usage.c
@@ -80,8 +80,15 @@ void NORETURN usage(const char *err)
void NORETURN die(const char *err, ...)
{
+ static int dying;
va_list params;
+ if (dying) {
+ fputs(fatal: recursion detected in die handler\n, stderr);
+ exit(128);
+ }
+ dying = 1;
+
va_start(params, err);
die_routine(err, params);
va_end(params);
@@ -89,11 +96,18 @@ void NORETURN die(const char *err, ...)
void NORETURN die_errno(const char *fmt, ...)
{
+ static int dying;
va_list params;
char fmt_with_err[1024];
char str_error[256], *err;
int i, j;
+ if (dying) {
+ fputs(fatal: recursion detected in die handler\n, stderr);
+ exit(128);
+ }
+ dying = 1;
+
err = strerror(errno);
for (i = j = 0; err[i] j sizeof(str_error) - 1; ) {
if ((str_error[j++] = err[i++]) != '%')
With two function-scope static, you can go like this:
die()
- die_routine()
- xsomething()
- die_errno()
- die_routine()
- xsomethingelse()
- die() or die_errno()
Not that we probably care too deeply about, as at least we won't
infinitely recurse and die out of stack space.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
On Wed, Aug 22, 2012 at 11:33 AM, Junio C Hamano gits...@pobox.com wrote:
Brandon Casey draf...@gmail.com writes:
Perhaps something like:
diff --git a/usage.c b/usage.c
index a2a6678..2d0ff35 100644
--- a/usage.c
+++ b/usage.c
@@ -80,8 +80,15 @@ void NORETURN usage(const char *err)
void NORETURN die(const char *err, ...)
{
+ static int dying;
va_list params;
+ if (dying) {
+ fputs(fatal: recursion detected in die handler\n, stderr);
+ exit(128);
+ }
+ dying = 1;
+
va_start(params, err);
die_routine(err, params);
va_end(params);
@@ -89,11 +96,18 @@ void NORETURN die(const char *err, ...)
void NORETURN die_errno(const char *fmt, ...)
{
+ static int dying;
va_list params;
char fmt_with_err[1024];
char str_error[256], *err;
int i, j;
+ if (dying) {
+ fputs(fatal: recursion detected in die handler\n, stderr);
+ exit(128);
+ }
+ dying = 1;
+
err = strerror(errno);
for (i = j = 0; err[i] j sizeof(str_error) - 1; ) {
if ((str_error[j++] = err[i++]) != '%')
With two function-scope static, you can go like this:
die()
- die_routine()
- xsomething()
- die_errno()
- die_routine()
- xsomethingelse()
- die() or die_errno()
Not that we probably care too deeply about, as at least we won't
infinitely recurse and die out of stack space.
Yeah, I noticed that, but didn't think it was important or likely.
But there's no reason not to make dying a global.
-Brandon
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
From: Junio C Hamano [mailto:gits...@pobox.com]
Sent: Wednesday, August 22, 2012 8:25 PM
To: Joachim Schmitz
Cc: 'Brandon Casey'; 'Shawn Pearce'; git@vger.kernel.org;
rsbec...@nexbridge.com
Subject: Re: Porting git to HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes:
Nice. And we have xmemdupz() would be even better as you followed-up.
How's that one used?
I forgot that we frown upon use of any xallocate() wrapper in the
compat/
layer as J6t mentioned.
So probably something along these lines...
int retval;
char *dir_to_free = NULL;
size_t len = strlen(dir);
if (len dir[len - 1] == '/') {
dir_to_free = malloc(len);
if (!dir_to_free) {
fprintf(stderr, malloc failed!\n);
exit(1);
}
memcpy(dir_to_free, dir, len - 1);
dir_to_free[len - 1] = '\0';
dir = dir_to_free;
}
retval = mkdir(dir, mode);
free(dir_to_free);
return retval;
So why not just strdup? I stole the idea from gnulib...
int
rpl_mkdir (char const *dir, mode_t mode maybe_unused)
{
int ret_val;
char *tmp_dir;
size_t len = strlen (dir);
if (len dir[len - 1] == '/')
{
tmp_dir = strdup (dir);
if (!tmp_dir)
{
/* Rather than rely on strdup-posix, we set errno ourselves. */
errno = ENOMEM;
return -1;
}
strip_trailing_slashes (tmp_dir);
}
else
{
tmp_dir = (char *) dir;
}
They strip more than one trailing slash, but for git's purpose I believed
this to be too much overhead. Also the errno stuff doesn't seem to be really
needed IMHO. Same for the following code
#if FUNC_MKDIR_DOT_BUG
/* Additionally, cygwin 1.5 mistakenly creates a directory d/./. */
{
char *last = last_component (tmp_dir);
if (*last == '.' (last[1] == '\0'
|| (last[1] == '.' last[2] == '\0')))
{
struct stat st;
if (stat (tmp_dir, st) == 0)
errno = EEXIST;
return -1;
}
}
#endif /* FUNC_MKDIR_DOT_BUG */
Then it goes on like mine:
ret_val = mkdir (tmp_dir, mode);
if (tmp_dir != dir)
free (tmp_dir);
return ret_val;
}
Compare:
$ cat compat/mkdir.c
#include ../git-compat-util.h
#undef mkdir
/* for platforms that can't deal with a trailing '/' */
int compat_mkdir_wo_trailing_slash(const char *dir, mode_t mode)
{
int retval;
char *tmp_dir = NULL;
size_t len = strlen(dir);
if (len dir[len-1] == '/') {
if ((tmp_dir = strdup(dir)) == NULL)
return -1;
tmp_dir[len-1] = '\0';
}
else
tmp_dir = (char *)dir;
retval = mkdir(tmp_dir, mode);
if (tmp_dir != dir)
free(tmp_dir);
return retval;
}
Bye, Jojo
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
Hi folks
There another API missing on HP NonStop and that is setitimer(), used in
progress.c and build/log.c
I do have a homebrewed implementation, on top of alarm(), it goes like this:
#include ../git-compat-util.h
#undef getitimer
#undef setitimer
int
git_getitimer(int which, struct itimerval *value)
{
int ret = 0;
switch (which) {
case ITIMER_REAL:
value-it_value.tv_usec = 0;
value-it_value.tv_sec = alarm(0);
ret = 0; /* if alarm() fails we get a SIGLIMIT */
break;
case ITIMER_VIRTUAL: /* FALLTHRU */
case ITIMER_PROF: errno = ENOTSUP; ret = -1; break;
default: errno = EINVAL; ret = -1;
}
return ret;
}
int
git_setitimer(int which, const struct itimerval *value,
struct itimerval *ovalue)
{
int ret = 0;
if (!value
|| value-it_value.tv_usec 0
|| value-it_value.tv_usec 100
|| value-it_value.tv_sec 0) {
errno = EINVAL;
return -1;
}
else if (ovalue)
if (!git_getitimer(which, ovalue))
return -1; /* errno set in git_getitimer() */
else
switch (which) {
case ITIMER_REAL:
alarm(value-it_value.tv_sec +
(value-it_value.tv_usec 0) ? 1 : 0);
ret = 0; /* if alarm() fails we get a SIGLIMIT */
break;
case ITIMER_VIRTUAL: /* FALLTHRU */
case ITIMER_PROF: errno = ENOTSUP; ret = -1; break;
default: errno = EINVAL; ret = -1;
}
return ret;
}
Worth being added to compat/, e.g. as setitimer.c, or, as itimer.c (as a
by-product, it has getitimer() too)?
Bye, Jojo
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes:
Hi folks
There another API missing on HP NonStop and that is setitimer(), used in
progress.c and build/log.c
I do have a homebrewed implementation, on top of alarm(), it goes like this:
#include ../git-compat-util.h
#undef getitimer
#undef setitimer
int
git_getitimer(int which, struct itimerval *value)
See Documentation/CodingGuidelines for style nits.
{
int ret = 0;
switch (which) {
case ITIMER_REAL:
value-it_value.tv_usec = 0;
value-it_value.tv_sec = alarm(0);
ret = 0; /* if alarm() fails we get a SIGLIMIT */
break;
case ITIMER_VIRTUAL: /* FALLTHRU */
case ITIMER_PROF: errno = ENOTSUP; ret = -1; break;
default: errno = EINVAL; ret = -1;
}
return ret;
}
int
git_setitimer(int which, const struct itimerval *value,
struct itimerval *ovalue)
{
int ret = 0;
if (!value
|| value-it_value.tv_usec 0
|| value-it_value.tv_usec 100
|| value-it_value.tv_sec 0) {
errno = EINVAL;
return -1;
}
else if (ovalue)
if (!git_getitimer(which, ovalue))
return -1; /* errno set in git_getitimer() */
else
switch (which) {
case ITIMER_REAL:
alarm(value-it_value.tv_sec +
(value-it_value.tv_usec 0) ? 1 : 0);
ret = 0; /* if alarm() fails we get a SIGLIMIT */
break;
case ITIMER_VIRTUAL: /* FALLTHRU */
case ITIMER_PROF: errno = ENOTSUP; ret = -1; break;
default: errno = EINVAL; ret = -1;
}
return ret;
}
Worth being added to compat/, e.g. as setitimer.c, or, as itimer.c
(as a by-product, it has getitimer() too)?
If it helps your port, compat/itimer.c sounds like a good place.
Doesn't it need a new header file to introduce structures and
constants, too?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Wednesday, August 22, 2012 10:50 PM To: Joachim Schmitz Cc: git@vger.kernel.org; rsbec...@nexbridge.com Subject: Re: Porting git to HP NonStop Joachim Schmitz j...@schmitz-digital.de writes: Hi folks There another API missing on HP NonStop and that is setitimer(), used in progress.c and build/log.c I do have a homebrewed implementation, on top of alarm(), it goes like this: #include ../git-compat-util.h #undef getitimer #undef setitimer int git_getitimer(int which, struct itimerval *value) See Documentation/CodingGuidelines for style nits. Will do and adjust code accordingly. Here I was more concerned about content though ;-) ... Worth being added to compat/, e.g. as setitimer.c, or, as itimer.c (as a by-product, it has getitimer() too)? If it helps your port, compat/itimer.c sounds like a good place. Doesn't it need a new header file to introduce structures and constants, too? You mean the ITIMER_* and struct itimerval, right? On NonStop these are available in sys/time.h, so here's no need to add them. Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes: If it helps your port, compat/itimer.c sounds like a good place. Doesn't it need a new header file to introduce structures and constants, too? You mean the ITIMER_* and struct itimerval, right? On NonStop these are available in sys/time.h, so here's no need to add them. At least you would need a header to declare these two functions and make them visible so that the remainder of the codebase will not have to know about git_setitimer(), no? Or does your header files on NonStop declare setitimer() but does not implement it? As your proposed name is not compat/tandem.c but more generic sounding compat/itimer.c, we would have to plan for systems other than NonStop, so we may later have to introduce makefile variables to ask that header file to declare the structure and define the constants that are missing from such a system. While you are porting to NonStop, you may not have to define/declare them, but knowing that these files are the place to later do so is part of the planning. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
-Original Message- From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Wednesday, August 22, 2012 11:12 PM To: Joachim Schmitz Cc: git@vger.kernel.org; rsbec...@nexbridge.com Subject: Re: Porting git to HP NonStop Joachim Schmitz j...@schmitz-digital.de writes: If it helps your port, compat/itimer.c sounds like a good place. Doesn't it need a new header file to introduce structures and constants, too? You mean the ITIMER_* and struct itimerval, right? On NonStop these are available in sys/time.h, so here's no need to add them. At least you would need a header to declare these two functions and make them visible so that the remainder of the codebase will not have to know about git_setitimer(), no? Or does your header files on NonStop declare setitimer() but does not implement it? No it doesn't, at least not if a form visible to a compiler... As your proposed name is not compat/tandem.c but more generic sounding compat/itimer.c, we would have to plan for systems other than NonStop, so we may later have to introduce makefile variables to ask that header file to declare the structure and define the constants that are missing from such a system. While you are porting to NonStop, you may not have to define/declare them, but knowing that these files are the place to later do so is part of the planning. I thought of having the function decclaration in git-compat-util.h, just like for eg. setenv, gitmkdtemp, etc. Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes: I thought of having the function decclaration in git-compat-util.h, just like for eg. setenv, gitmkdtemp, etc. Yeah, that's also fine, especially if you do not have to declare structures and constants. Once you start having to declare other things in order to declare the function missing on the system, it won't be like setenv where a pair of #ifdef NO_SETENV/#endif just surrounds a single line. At that point, a separate header file to hold them together would become easier to read. It's a judgement call; we'll see how it turns out (we do not have to get everything right in our first attempt). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
From: Junio C Hamano [mailto:gits...@pobox.com]
Sent: Sunday, August 19, 2012 7:23 PM
To: Joachim Schmitz
Cc: 'Shawn Pearce'; git@vger.kernel.org; rsbec...@nexbridge.com
Subject: Re: Porting git to HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes:
Found the problem: our mkdir(dir,flags) fails with ENOENT when dir
ends with a '/'.
Not sure whether this us a bug on out platform or just allowed by
POSIX and as such a wrong assumption in git though?
[shortly after]
A bit of googleing revealed that there is a GNUlib solution for this,
which claims that at least NetBSD 1.5.2 has the same problem.
(http://www.opensource.apple.com/source/gpatch/gpatch-2/patch/mkdir.c)
And apparently this has been discussed on the git mailing list too, 2
years
ago:
http://lists-archives.com/git/728359-git-s-use-of-mkdir-2.html,
there's a patch too.
Given that newer BSDs have fixed libc to accept directory name with a
trailing
slash, and that we use mkdir(2) in many places, I think the right way to
do so is
still what I suggested in that old thread in the last paragraph of my
message
http://thread.gmane.org/gmane.comp.version-
control.git/155812/focus=155876
That is, have compat/tandem.c and define a replacement mkdir(2) in a way
similar to how MinGW does so.
OK, I'll go for a compat/mkdir.c though.
We shouldn't call it tandem.c as Tandem, the Company, doesn't exist anymore
and since more than a decade (bough by Compaq, then HP), only the __TANDEM
survived in our compiler and headers/libraries. Could call it NonStop.c, but
I don't really like that idea either, I'd rather keep it more generic, just
in case someone else might need it too, or that issue someday gets fixed for
NonStop.
For now I've fixed it like this:
/usr/local/bin/diff -EBbu ./builtin/init-db.c.orig ./builtin/init-db.c
--- ./builtin/init-db.c.orig2012-08-19 03:55:50 -0500
+++ ./builtin/init-db.c 2012-08-19 03:39:57 -0500
@@ -25,7 +25,16 @@
static void safe_create_dir(const char *dir, int share) {
+#ifdef __TANDEM /* our mkdir() can't cope with a trailing '/' */
+ char mydir[PATH_MAX];
+
+ strcpy(mydir,dir);
+ if (dir[strlen(dir)-1] == '/')
+ mydir[strlen(dir)-1] = '\0';
+ if (mkdir(mydir, 0777) 0) {
+#else
if (mkdir(dir, 0777) 0) {
+#endif
Move that part inside #ifdef __TANDEM to define
int tandem_mkdir(const char *dir, mode_t mode)
{
...
}
I'll go for git_mkdir(), similar to other git wrappers, (like for mmap,
pread, fopen, snprintf, vsnprintf, qsort). Could call it gitmkdir() too
(like for basename, setenv, mkdtemp, mkstemps, unsetenv, strcasestr,
strlcpy, strtoumax, strtoimax, strtok_r, hstrerror, memmem, strchrnul,
memcpy), Opinions?
It seems the ones without the _ are for missing APIs and the ones with _
to wrap existing APIs (not sure about mmap and pread)?
Here it's current state:
$ cat compat/mkdir.c
#include ../git-compat-util.h
#undef mkdir
/* for platforms that can't deal with a trailing '/' */
int git_mkdir(const char *dir, mode_t mode)
{
int retval;
char *tmp_dir = NULL;
size_t len = strlen(dir);
if (len dir[len-1] == '/') {
if ((tmp_dir = strdup(dir)) == NULL)
return -1;
tmp_dir[len-1] = '\0';
}
else
tmp_dir = (char *)dir;
retval = mkdir(tmp_dir, mode);
if (tmp_dir != dir)
free(tmp_dir);
return retval;
}
$
There is room for improvement though: it only removes one trailing slash. By
far not as advanced and generic as GNUlib's mkdir wrapper, but should be
good enough for git's usage.
in your new file compat/tandem.c, add
#ifdef __TANDEM
#define mkdir(a,b) tandem_mkdir((a), (b))
#endif
to git-compat-util.h
Again, git_mkdir, see above
and then add compat/tandem.o to COMPAT_OBJS in the
top-level Makefile.
For now I've added it to the (new) NOSTOP_KERNEL section.
We may want it to go along with some
MKDIR_DISLIKES_TRAILING_SLASH or MKDIR_BOGUS_TRAILING_SLASH some such.
Opinions, Ideas?
That way we do not have to keep an ugly platform specific ifdef in the
very
generic codepath.
Agreed, it was my quick and dirty fix for it.
Bye, Jojo
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes: OK, I'll go for a compat/mkdir.c though. No. See below. We shouldn't call it tandem.c as Tandem, the Company, doesn't exist anymore and since more than a decade (bough by Compaq, then HP), only the __TANDEM survived in our compiler and headers/libraries. Could call it NonStop.c, but I don't really like that idea either, I'd rather keep it more generic, just in case someone else might need it too, or that issue someday gets fixed for NonStop. compat/hp_nonstop.c is also fine, but I think matching #ifdef __TANDEM is the most sensible. And I wouldn't call it just mkdir, as it is more likely than not that we will find other incompatibilities that needs to be absorbed in the compat/ layer, and we can add it to compat/tandem.c, but not to compat/mkdir.c, as that will be another nonstop specific tweak. A separate file, compat/tandem/mkdir.c, is fine, though. I'll go for git_mkdir(), similar to other git wrappers, (like for mmap, pread, fopen, snprintf, vsnprintf, qsort). Again, no. Your breakage is that having underlying system mkdir that does not understand trailing slash, which may not be specific to __TANDEM, but still is _not_ the only possible mode of breakage. Squatting on a generic git_mkdir() name makes it harder for other people to name their compat mkdir functions to tweak for the breakage on their platforms. The examples you listed are all the platform does not offer it, so we implement the whole thing kind, so it is in a different genre. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes: I haven't found any other to be needed. Well, poll, maybe, but with only minor tweaks for the win32 one works for me (and those tweaks are compatible with win32 A separate file, compat/tandem/mkdir.c, is fine, though. If you wouldn't have dozens of them, so compat/tandem/mkdir.c is not suitable; compat/tandem.c would be good, then. I'll go for git_mkdir(), similar to other git wrappers, (like for mmap, pread, fopen, snprintf, vsnprintf, qsort). Again, no. Your breakage is that having underlying system mkdir that does not understand trailing slash, which may not be specific to __TANDEM, but still is _not_ the only possible mode of breakage. Well, it is the only one GNUlib's mkdir caters for and I'd regard that an authoritative source... I suspect that you may be misunderstanding what compat/ is about, so let's try again. Platform difference in mkdir may not be limited to on this platform, the underlying one supplied by the system does not like path ending with a slash. What I am saying is that it is unacceptable to call something that caters to that specific kind of difference from what the codebase expects with a generic name such as git_mkdir(). Look at mingw's replacement. The platform difference over there is that the one from the system does not take mode parameter. Imagine that one was already called git_mkdir(). Now we have two different kind of differences, and one has more officially-looking git_mkdir() name; yours cannot take it---what would you do in that case? Neither kind of difference is more officially sanctioned difference; don't call yours any more official/generic than necessary. Your wrapper is not limited to tandem, but is applicable to ancient BSDs, so it is fine to call it as compat_mkdir_wo_trailing_slash(), so that it can be shared among platforms whose mkdir do not want to see trailing slashes. If you are going that route, the function should live in its own file (without any other wrapper), and not be named after specific platform (should be named after the specific difference from what we expect, instead). I am perfectly fine with that approach as well. Squatting on a generic git_mkdir() name makes it harder for other people to name their compat mkdir functions to tweak for the breakage on their platforms. The examples you listed are all the platform does not offer it, so we implement the whole thing kind, so it is in a different genre. Nope, git_fopen() definitly is a wrapper for fopen(), as is git_vsnprintf() for vsnprintf(). I was talking more about mmap() and pread(). For the two you mentioned, ideally they should have been named after the specific breakages they cover (fopen that does not error out on directories is primarily AIX thing IIRC, and snprintf returns bogus result are shared between HPUX and Windows), but over these years we haven't seen any other kind of differences from various platforms, so the need to rename them away is very low. On the other hand, we already know there are at least two kinds of platform mkdir() that need different compat/ layer support, so calling one git_mkdir() to cover one particular kind of difference does not make any sense. Besides, an earlier mistake is not a valid excuse to add new mistakes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
From: Shawn Pearce [mailto:spea...@spearce.org]
Sent: Friday, August 10, 2012 7:38 PM
To: Joachim Schmitz
Cc: git@vger.kernel.org; rsbec...@nexbridge.com
Subject: Re: Porting git to HP NonStop
On Fri, Aug 10, 2012 at 10:32 AM, Joachim Schmitz
j...@schmitz-digital.de
wrote:
then use `git init --bare` in a new directory to copy in the
templates,
and see if
its the template copying code that is making an incorrect copy.
git init --bare gives the same error. It isn't copying any of the
subdirectories, only the file 'description'
Time to start debugging copy_templates_1 in builtin/init-db.c. :-(
Found the problem: our mkdir(dir,flags) fails with ENOENT when dir ends with
a '/'.
Not sure whether this us a bug on out platform or just allowed by POSIX and
as such a wrong assumption in git though?
[shortly after]
A bit of googleing revealed that there is a GNUlib solution for this, which
claims that at least NetBSD 1.5.2 has the same problem.
(http://www.opensource.apple.com/source/gpatch/gpatch-2/patch/mkdir.c)
And apparently this has been discussed on the git mailing list too, 2 years
ago:
http://lists-archives.com/git/728359-git-s-use-of-mkdir-2.html, there's a
patch too.
For now I've fixed it like this:
/usr/local/bin/diff -EBbu ./builtin/init-db.c.orig ./builtin/init-db.c
--- ./builtin/init-db.c.orig2012-08-19 03:55:50 -0500
+++ ./builtin/init-db.c 2012-08-19 03:39:57 -0500
@@ -25,7 +25,16 @@
static void safe_create_dir(const char *dir, int share)
{
+#ifdef __TANDEM /* our mkdir() can't cope with a trailing '/' */
+ char mydir[PATH_MAX];
+
+ strcpy(mydir,dir);
+ if (dir[strlen(dir)-1] == '/')
+ mydir[strlen(dir)-1] = '\0';
+ if (mkdir(mydir, 0777) 0) {
+#else
if (mkdir(dir, 0777) 0) {
+#endif
if (errno != EEXIST) {
perror(dir);
exit(1);
Bye, Jojo
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes:
Found the problem: our mkdir(dir,flags) fails with ENOENT when dir ends with
a '/'.
Not sure whether this us a bug on out platform or just allowed by POSIX and
as such a wrong assumption in git though?
[shortly after]
A bit of googleing revealed that there is a GNUlib solution for this, which
claims that at least NetBSD 1.5.2 has the same problem.
(http://www.opensource.apple.com/source/gpatch/gpatch-2/patch/mkdir.c)
And apparently this has been discussed on the git mailing list too, 2 years
ago:
http://lists-archives.com/git/728359-git-s-use-of-mkdir-2.html, there's a
patch too.
Given that newer BSDs have fixed libc to accept directory name with
a trailing slash, and that we use mkdir(2) in many places, I think
the right way to do so is still what I suggested in that old thread
in the last paragraph of my message
http://thread.gmane.org/gmane.comp.version-control.git/155812/focus=155876
That is, have compat/tandem.c and define a replacement mkdir(2) in a
way similar to how MinGW does so.
For now I've fixed it like this:
/usr/local/bin/diff -EBbu ./builtin/init-db.c.orig ./builtin/init-db.c
--- ./builtin/init-db.c.orig2012-08-19 03:55:50 -0500
+++ ./builtin/init-db.c 2012-08-19 03:39:57 -0500
@@ -25,7 +25,16 @@
static void safe_create_dir(const char *dir, int share)
{
+#ifdef __TANDEM /* our mkdir() can't cope with a trailing '/' */
+ char mydir[PATH_MAX];
+
+ strcpy(mydir,dir);
+ if (dir[strlen(dir)-1] == '/')
+ mydir[strlen(dir)-1] = '\0';
+ if (mkdir(mydir, 0777) 0) {
+#else
if (mkdir(dir, 0777) 0) {
+#endif
Move that part inside #ifdef __TANDEM to define
int tandem_mkdir(const char *dir, mode_t mode)
{
...
}
in your new file compat/tandem.c, add
#ifdef __TANDEM
#define mkdir(a,b) tandem_mkdir((a), (b))
#endif
to git-compat-util.h and then add compat/tandem.o to COMPAT_OBJS in
the top-level Makefile.
That way we do not have to keep an ugly platform specific ifdef in
the very generic codepath.
if (errno != EEXIST) {
perror(dir);
exit(1);
Bye, Jojo
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
From: Joachim Schmitz [mailto:j...@schmitz-digital.de] Sent: Friday, August 10, 2012 10:09 PM To: 'Shawn Pearce' Cc: 'git@vger.kernel.org'; 'rsbec...@nexbridge.com' Subject: RE: Porting git to HP NonStop From: Joachim Schmitz [mailto:j...@schmitz-digital.de] Sent: Friday, August 10, 2012 7:33 PM To: 'Shawn Pearce' Cc: 'git@vger.kernel.org'; 'rsbec...@nexbridge.com' Subject: RE: Porting git to HP NonStop From: Shawn Pearce [mailto:spea...@spearce.org] Sent: Friday, August 10, 2012 6:28 PM To: Joachim Schmitz Cc: git@vger.kernel.org; rsbec...@nexbridge.com Subject: Re: Porting git to HP NonStop On Fri, Aug 10, 2012 at 8:04 AM, Joachim Schmitz j...@schmitz-digital.de wrote: snip - HP NonStop doesn't have stat.st_?time.nsec, there are several places what an #ifdef USE_NSEC is missing, I can provide a diff if needed (offending files: builtin/fetch-pack.c and read-cache.c). I think this would be appreciated by anyone else that has a similar problem where the platform lacks nsec. Will do. OK, here we go: /usr/local/bin/diff -EBbu ./builtin/fetch-pack.c.orig ./builtin/fetch-pack.c snip Sorry, this is not needed if I just set NO_NSEC, so just forget about it (and thanks to Junio for telling be) /usr/local/bin/diff -EBbu ./git-compat-util.h.orig ./git-compat-util.h --- ./git-compat-util.h.orig2012-07-30 15:50:38 -0500 +++ ./git-compat-util.h 2012-08-10 09:59:56 -0500 @@ -74,7 +74,8 @@ # define _XOPEN_SOURCE 500 # endif #elif !defined(__APPLE__) !defined(__FreeBSD__) !defined(__USLC__) \ - !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__) + !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__) \ + !defined(__TANDEM) #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */ #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */ #endif @@ -98,6 +99,11 @@ #include stdlib.h #include stdarg.h #include string.h +#ifdef __TANDEM +# include strings.h /* for strcasecmp() */ + typedef long int intptr_t; + typedef unsigned long int uintptr_t; +#endif #include errno.h #include limits.h #include sys/param.h This one still stands though, unless someone can come up with a better idea? Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Joachim Schmitz j...@schmitz-digital.de writes: /usr/local/bin/diff -EBbu ./git-compat-util.h.orig ./git-compat-util.h --- ./git-compat-util.h.orig2012-07-30 15:50:38 -0500 +++ ./git-compat-util.h 2012-08-10 09:59:56 -0500 @@ -74,7 +74,8 @@ # define _XOPEN_SOURCE 500 # endif #elif !defined(__APPLE__) !defined(__FreeBSD__) !defined(__USLC__) \ - !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__) + !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__) \ + !defined(__TANDEM) #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */ #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */ #endif @@ -98,6 +99,11 @@ #include stdlib.h #include stdarg.h #include string.h +#ifdef __TANDEM +# include strings.h /* for strcasecmp() */ + typedef long int intptr_t; + typedef unsigned long int uintptr_t; +#endif #include errno.h #include limits.h #include sys/param.h This one still stands though, unless someone can come up with a better idea? This hunk looks unobtrusive and obviously will not impact other platforms, which is good. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
Am 10.08.2012 18:27, schrieb Shawn Pearce: There is no need to define your own mmap(). Define NO_MMAP=1 in the Makefile. Git already has its own fake mmap and knows how to write it back to disk when making changes. Or better to say: the fake mmap has functionality that is sufficient for git. In particular, it does *not* write back changes to disk (it supports only MAP_PRIVATE), and the mapped area does not change if the file is changed by a third party. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On Behalf Of Joachim Schmitz Sent: Friday, August 10, 2012 5:01 PM To: git@vger.kernel.org Cc: rsbec...@nexbridge.com Subject: RE: [PATCH v2] add tests for 'git rebase --keep-empty' Hi folks I'm a brand new subscriper of this mailing list, so please forgive if I violate some protocol or talk about things that had been discussed to death earlier. Ahrgl, 1st mistake: wrong subject, sorry I'm currently in the process of porting git (1.7.11.4 for now) to the HP NonStop platform and found several issues: - HP NonStop is lacking poll(), git is making quite some use of it. My Solution: I 'stole' the implementation from GNUlib, which implements poll() using select(). Git should either provide its own poll(), not use it at all or resort to using GNUlib, what do you think?. - HP NonStop is lacking getrlimit(), fsync(), setitimer() and memory mapped IO. For now I've commented out the part that used getrlimit() and use a home brewed implementation for fsync(), setitimer() and mmap(). - git makes use of some C99 features or at least feature that are not availabe in C89, like 'inline' C89 is the default compiler on HP NonStop, but we also habe a c99 compiler, so telling configure to search for c99 should help here. - libintl and libiconv sem to get linked in the wrong order, resulting in unresolved symbols. I've just moved the ifndef NO_GETTEXT section of Makefile to above the ifdef NEEDS_LIBICONF section. - HP NonStop doesn't have stat.st_blocks, this is used in builtin/count-objects.c around line 45, not sure yet how to fix that. - HP NonStop doesn't have stat.st_?time.nsec, there are several places what an #ifdef USE_NSEC is missing, I can provide a diff if needed (offending files: builtin/fetch-pack.c and read-cache.c). - HP NonStop doesn't know SA_RESTART I fixed that with a #define SA_RESTART 0 in the 3 files affected (builtin/log.c, fast-import.c and progress.c) - using C99 but not using #include strings.h results in compiler errors due to a missing prototype for strcasecmp() I fixed it by adding that to git-compat- util.h - HP NonStop doesn't have intptr_t and uintpr_t (in its stdint.h) I added them to git-compat-util.h - HP NonStop doesn't need the #define _XOPEN_SOURCE 600, just like __APPLE__, __FreeBSD__ etc, so I added a !defined(__TANDEM) in git- compat-util.h - there seems to be an issue with compat/fnmatch/fnmatch.c not including string.h, seems that HAVE_STRING_H is not #define'd anywhere. - Once compiled and installed, a simple jojo@\hpitug:/home/jojo/GitHub $ git clone git://github.com/git/git.git fails with: /home/jojo/GitHub/git/.git/branches/: No such file or directory After creating those manually it fails because the directory isn't empty, catch-22 After some trial'n'error I found that the culprit seems to be the subdirectories branches, hook and info in /usr/local/share/git-core/templates/, if I remove/rename those, the above command works fine. I have no idea why that is nor how to properly fix it, anyone out there? Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
On Fri, Aug 10, 2012 at 8:04 AM, Joachim Schmitz j...@schmitz-digital.de wrote: - HP NonStop is lacking poll(), git is making quite some use of it. My Solution: I 'stole' the implementation from GNUlib, which implements poll() using select(). Git should either provide its own poll(), not use it at all or resort to using GNUlib, what do you think?. poll() is usually better than select() because you don't need to worry about FD_SETSIZE. That said, the compat/ directory contains implementations of some functions. You could contribute a fake poll that uses select if it was under the GPLv2. - HP NonStop is lacking getrlimit(), fsync(), setitimer() and memory mapped IO. For now I've commented out the part that used getrlimit() and use a home brewed implementation for fsync(), setitimer() and mmap(). There is no need to define your own mmap(). Define NO_MMAP=1 in the Makefile. Git already has its own fake mmap and knows how to write it back to disk when making changes. - git makes use of some C99 features or at least feature that are not availabe in C89, like 'inline' C89 is the default compiler on HP NonStop, but we also habe a c99 compiler, so telling configure to search for c99 should help here. You could also disable inline by #define inline /**/, but this will probably result in a slower binary. - HP NonStop doesn't have stat.st_blocks, this is used in builtin/count-objects.c around line 45, not sure yet how to fix that. IIRC the block count is only used to give the user some notion of how much disk was wasted by the repository. You could hack a macro that redefines this as st_size. - HP NonStop doesn't have stat.st_?time.nsec, there are several places what an #ifdef USE_NSEC is missing, I can provide a diff if needed (offending files: builtin/fetch-pack.c and read-cache.c). I think this would be appreciated by anyone else that has a similar problem where the platform lacks nsec. - Once compiled and installed, a simple jojo@\hpitug:/home/jojo/GitHub $ git clone git://github.com/git/git.git fails with: /home/jojo/GitHub/git/.git/branches/: No such file or directory After creating those manually it fails because the directory isn't empty, catch-22 After some trial'n'error I found that the culprit seems to be the subdirectories branches, hook and info in /usr/local/share/git-core/templates/, if I remove/rename those, the above command works fine. I have no idea why that is nor how to properly fix it, anyone out there? This sounds like the templates directory was not created correctly during installation, or is being copied incorrectly during the git init process. I would start by comparing the structure and permissions of the templates directory on your HP NonStop system to one on a Linux system and see if there was a mistake made during the installation process. If the directory matches, I would then use `git init --bare` in a new directory to copy in the templates, and see if its the template copying code that is making an incorrect copy. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Porting git to HP NonStop
On Fri, Aug 10, 2012 at 10:32 AM, Joachim Schmitz j...@schmitz-digital.de wrote: then use `git init --bare` in a new directory to copy in the templates, and see if its the template copying code that is making an incorrect copy. git init --bare gives the same error. It isn't copying any of the subdirectories, only the file 'description' Time to start debugging copy_templates_1 in builtin/init-db.c. :-( -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Porting git to HP NonStop
From: Joachim Schmitz [mailto:j...@schmitz-digital.de]
Sent: Friday, August 10, 2012 7:33 PM
To: 'Shawn Pearce'
Cc: 'git@vger.kernel.org'; 'rsbec...@nexbridge.com'
Subject: RE: Porting git to HP NonStop
From: Shawn Pearce [mailto:spea...@spearce.org]
Sent: Friday, August 10, 2012 6:28 PM
To: Joachim Schmitz
Cc: git@vger.kernel.org; rsbec...@nexbridge.com
Subject: Re: Porting git to HP NonStop
On Fri, Aug 10, 2012 at 8:04 AM, Joachim Schmitz
j...@schmitz-digital.de
wrote:
snip
- HP NonStop doesn't have stat.st_blocks, this is used in
builtin/count-objects.c
around line 45, not sure yet how to fix that.
IIRC the block count is only used to give the user some notion of how
much disk was wasted by the repository. You could hack a macro that
redefines this as st_size.
OK, thanks, will try that.
Setting NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease in Makefile helps, no need
for further hacking ;-).
- HP NonStop doesn't have stat.st_?time.nsec, there are several
places
what an
#ifdef USE_NSEC is missing, I can provide a diff if needed
(offending
files: builtin/fetch-pack.c and read-cache.c).
I think this would be appreciated by anyone else that has a similar
problem where the platform lacks nsec.
Will do.
OK, here we go:
/usr/local/bin/diff -EBbu ./builtin/fetch-pack.c.orig ./builtin/fetch-pack.c
--- ./builtin/fetch-pack.c.orig 2012-07-30 15:50:38 -0500
+++ ./builtin/fetch-pack.c 2012-08-10 01:50:28 -0500
@@ -1096,7 +1096,9 @@
int fd;
mtime.sec = st.st_mtime;
+#ifdef USE_NSEC
mtime.nsec = ST_MTIME_NSEC(st);
+#endif
if (stat(shallow, st)) {
if (mtime.sec)
die(shallow file was removed during
fetch);
/usr/local/bin/diff -EBbu ./read-cache.c.orig ./read-cache.c
--- ./read-cache.c.orig 2012-07-30 15:50:38 -0500
+++ ./read-cache.c 2012-08-09 10:57:57 -0500
@@ -72,8 +72,10 @@
{
ce-ce_ctime.sec = (unsigned int)st-st_ctime;
ce-ce_mtime.sec = (unsigned int)st-st_mtime;
+#ifdef USE_NSEC
ce-ce_ctime.nsec = ST_CTIME_NSEC(*st);
ce-ce_mtime.nsec = ST_MTIME_NSEC(*st);
+#endif
ce-ce_dev = st-st_dev;
ce-ce_ino = st-st_ino;
ce-ce_uid = st-st_uid;
@@ -1465,7 +1467,9 @@
}
strbuf_release(previous_name_buf);
istate-timestamp.sec = st.st_mtime;
+#ifdef USE_NSEC
istate-timestamp.nsec = ST_MTIME_NSEC(st);
+#endif
while (src_offset = mmap_size - 20 - 8) {
/* After an array of active_nr index entries,
@@ -1821,7 +1825,9 @@
if (ce_flush(c, newfd) || fstat(newfd, st))
return -1;
istate-timestamp.sec = (unsigned int)st.st_mtime;
+#ifdef USE_NSEC
istate-timestamp.nsec = ST_MTIME_NSEC(st);
+#endif
return 0;
}
Hope this helps?
Could you also consider adding the following:
/usr/local/bin/diff -EBbu ./git-compat-util.h.orig ./git-compat-util.h
--- ./git-compat-util.h.orig2012-07-30 15:50:38 -0500
+++ ./git-compat-util.h 2012-08-10 09:59:56 -0500
@@ -74,7 +74,8 @@
# define _XOPEN_SOURCE 500
# endif
#elif !defined(__APPLE__) !defined(__FreeBSD__) !defined(__USLC__)
\
- !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__)
+ !defined(_M_UNIX) !defined(__sgi) !defined(__DragonFly__) \
+ !defined(__TANDEM)
#define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs
600 for S_ISLNK() */
#define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
#endif
@@ -98,6 +99,11 @@
#include stdlib.h
#include stdarg.h
#include string.h
+#ifdef __TANDEM
+# include strings.h /* for strcasecmp() */
+ typedef long int intptr_t;
+ typedef unsigned long int uintptr_t;
+#endif
#include errno.h
#include limits.h
#include sys/param.h
Bye, Jojo
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
