RE: Porting git to HP NonStop

From: Joachim Schmitz <jojo@schmitz-digital.de>
Date: Aug 22, 2012, 16:30 UTC
Message-ID: <002a01cd8083$69fb9960$3df2cc20$@schmitz-digital.de>
In-Reply-To: <7v4nnxld24.fsf@alter.siamese.dyndns.org>

Show 15 quoted lines

> 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" <jojo@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;

Show 11 quoted lines

> 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

Show 18 quoted lines

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