All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Joachim Schmitz" <jojo@schmitz-digital.de>
To: "'Junio C Hamano'" <gitster@pobox.com>
Cc: "'Shawn Pearce'" <spearce@spearce.org>, <git@vger.kernel.org>,
	<rsbecker@nexbridge.com>
Subject: RE: Porting git to HP NonStop
Date: Wed, 22 Aug 2012 18:30:15 +0200	[thread overview]
Message-ID: <002a01cd8083$69fb9960$3df2cc20$@schmitz-digital.de> (raw)
In-Reply-To: <7v4nnxld24.fsf@alter.siamese.dyndns.org>

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

  reply	other threads:[~2012-08-22 16:31 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-10 15:04 Porting git to HP NonStop Joachim Schmitz
2012-08-10 16:27 ` Shawn Pearce
2012-08-10 17:32   ` Joachim Schmitz
2012-08-10 17:38     ` Shawn Pearce
2012-08-19  8:57       ` Joachim Schmitz
2012-08-19 17:23         ` Junio C Hamano
2012-08-20 10:22           ` Joachim Schmitz
2012-08-20 14:41             ` Junio C Hamano
2012-08-20 16:09               ` Joachim Schmitz
2012-08-20 16:53                 ` Junio C Hamano
2012-08-22 16:30                   ` Joachim Schmitz [this message]
2012-08-22 17:00                     ` Brandon Casey
2012-08-22 17:13                       ` Brandon Casey
2012-08-22 17:18                       ` Joachim Schmitz
2012-08-22 17:23                         ` Brandon Casey
2012-08-22 17:30                           ` Joachim Schmitz
2012-08-22 17:30                       ` Junio C Hamano
2012-08-22 18:01                         ` Joachim Schmitz
2012-08-22 18:24                           ` Junio C Hamano
2012-08-22 18:52                             ` Joachim Schmitz
2012-08-22 17:41                       ` Johannes Sixt
2012-08-22 18:02                         ` Joachim Schmitz
2012-08-22 18:09                           ` Johannes Sixt
2012-08-22 18:18                             ` Joachim Schmitz
2012-08-22 18:09                         ` Brandon Casey
2012-08-22 18:24                           ` Brandon Casey
2012-08-22 18:33                             ` Junio C Hamano
2012-08-22 18:38                               ` Brandon Casey
2012-08-22 20:18                                 ` Joachim Schmitz
2012-08-22 20:49                                   ` Junio C Hamano
2012-08-22 21:05                                     ` Joachim Schmitz
2012-08-22 21:12                                       ` Junio C Hamano
2012-08-22 21:22                                         ` Joachim Schmitz
2012-08-22 21:54                                           ` Junio C Hamano
2012-08-22 18:26                           ` Junio C Hamano
2012-08-10 20:08   ` Joachim Schmitz
2012-08-11  8:20   ` Johannes Sixt
2012-08-14  7:05   ` Joachim Schmitz
2012-08-14 14:56     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='002a01cd8083$69fb9960$3df2cc20$@schmitz-digital.de' \
    --to=jojo@schmitz-digital.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=rsbecker@nexbridge.com \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.