RE: Porting git to HP NonStop

From: Junio C Hamano [mailto:gitster@pobox.com]
Sent: Monday, August 20, 2012 6:54 PM
To: Joachim Schmitz
Cc: 'Shawn Pearce'; git@vger.kernel.org; rsbecker@nexbridge.com
Subject: Re: Porting git to HP NonStop

"Joachim Schmitz" [off-list ref] writes:

quoted

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

quoted

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.

quoted

quoted

quoted

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

quoted

understand trailing slash, which may not be specific to __TANDEM, but

still is

quoted

_not_ the only possible mode of breakage.

True.

quoted

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.

quoted

quoted

Squatting on a generic "git_mkdir()" name makes it harder for other
people

to

quoted

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

quoted

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.c    2012-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;
+}

BTW: I've just today reported that mkdir bug to HP NonStop development.
Let's see how fast they fix it and whether at all, but I'd be pretty
surprised if that happened earlier than 6 months from now.

Bye, Jojo