RE: git on HP NonStop

From: Junio C Hamano [mailto:gitster@pobox.com]
Sent: Tuesday, August 14, 2012 4:44 PM
To: Joachim Schmitz
Subject: Re: git on HP NonStop

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

quoted

Interesting, I never mentioned Tandem did I, But still you recognized
HP NonStop as that.

No, *you* did in your patch "#ifdef".

Ah, I see.

quoted

Well, I do care about that platform,  but if you don't, there's not
much point in me trying to get Tandem specific patches applied, is it?

As long as the change is isolated (i.e. compilation without "#define

TANDEM"

__TANDEM actually

for other people will produce byte-for-byte identical result as before),

and

cleanly made (i.e. the resulting source code is not littered with "#ifdef
TANDEM" in many places), I do not think there is a reason not to have such

a

patchset.

It isn't in many places, only 2 places in git-compat-util.h so far:
/usr/local/bin/diff -EBbu ./git-compat-util.h.orig ./git-compat-util.h

--- ./git-compat-util.h.orig    2012-07-30 15:50:38 -0500
+++ ./git-compat-util.h 2012-08-12 11:26:46 -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 int intptr_t; /* not "int *" ?!? */
+  typedef unsigned int uintptr_t; /* not "unsigned int *" ?!? */
+#endif
 #include <errno.h>
 #include <limits.h>
 #include <sys/param.h>

Too much? The 2nd part is not necessary NonStop specific, any idea for a
better way?

There there's Makefile:
/usr/local/bin/diff -EBbu ./Makefile.orig ./Makefile

--- ./Makefile.orig     2012-07-30 15:50:38 -0500
+++ ./Makefile  2012-08-14 06:07:16 -0500

@@ -1297,6 +1297,45 @@
        NO_CURL =
        NO_EXPAT =
 endif
+ifeq ($(uname_S),NONSTOP_KERNEL)
+       CC = cc -c99 # needs some C99 features, "inline" is just one of them
+       CFLAGS= -g -O
+       prefix = /usr/local
+
+       # as detected by './configure'
+       #NO_CURL = YesPlease # missdetected, disabled, see below
+       NEEDS_SSL_WITH_CURL = YesPlease # added manually, see above
+       HAVE_LIBCHARSET_H=YesPlease
+       NEEDS_LIBICONV = YesPlease # needs libiconv first, changed further

down
+       NO_SYS_SELECT_H=UnfortunatelyYes
+       NO_D_TYPE_IN_DIRENT = YesPlease
+       NO_HSTRERROR=YesPlease
+       NO_STRCASESTR=YesPlease
+       NO_FNMATCH_CASEFOLD = YesPlease
+       NO_MEMMEM = YesPlease
+       NO_STRLCPY = YesPlease
+       NO_SETENV = YesPlease
+       NO_UNSETENV = YesPlease
+       NO_MKDTEMP = YesPlease
+       NO_MKSTEMPS = YesPlease
+       OLD_ICONV=UnfortunatelyYes # currently libiconv-1.9.1
+       NO_REGEX=YesPlease # Why? ToDo?
+       NO_PTHREADS=UnfortunatelyYes # ToDo? Using PUT, maybe?
+
+       # our's are in ${prefix}/bin
+       PERL_PATH = ${prefix}/bin/perl
+       PYTHON_PATH = ${prefix}/bin/python
+
+       # not detected (nor checked for) by './configure'
+       COMPAT_CFLAGS += -DSA_RESTART=0 # we don't have SA_RESTART on
NonStop
+       COMPAT_CFLAGS += -DHAVE_STRING_H=1 # needed in
compat/fnmatch/fnmatch.c
+       NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease # we don't have that on
NonStop
+       NO_NSEC = YesPlease # we don't have that on NonStop
+       NO_PREAD = YesPlease # we could use floss_pread though?
+       NO_MMAP = YesPlease # we could use floss_mmap though?
+       # newly implemented further down
+       NO_POLL = YesPlease # we could use floss_poll though?
+endif
 ifneq (,$(findstring MINGW,$(uname_S)))
        pathsep = ;
        NO_PREAD = YesPlease

@@ -1526,6 +1565,11 @@
        LIB_4_CRYPTO = $(OPENSSL_LINK) -lcrypto
 endif
 endif
+ifndef NO_GETTEXT
+ifndef LIBC_CONTAINS_LIBINTL
+       EXTLIBS += -lintl
+endif
+endif
 ifdef NEEDS_LIBICONV
        ifdef ICONVDIR
                BASIC_CFLAGS += -I$(ICONVDIR)/include

@@ -1538,11 +1582,6 @@
 ifdef NEEDS_LIBGEN
        EXTLIBS += -lgen
 Endif
-ifndef NO_GETTEXT
-ifndef LIBC_CONTAINS_LIBINTL
-       EXTLIBS += -lintl
-endif
-endif
 ifdef NEEDS_SOCKET
        EXTLIBS += -lsocket
 endif

@@ -1591,6 +1630,11 @@
        BASIC_CFLAGS += -DNO_GETTEXT
        USE_GETTEXT_SCHEME ?= fallthrough
 endif
+ifdef NO_POLL
+       NO_SYS_POLL_H = YesPlease
+       COMPAT_CFLAGS += -DNO_POLL -Icompat/win32 # so it finds poll.h
+       COMPAT_OBJS += compat/win32/poll.o
+endif
 ifdef NO_STRCASESTR
        COMPAT_CFLAGS += -DNO_STRCASESTR
        COMPAT_OBJS += compat/strcasestr.o

How das that look to you?

quoted

quoted

Finally, I would prefer to see any message that is addressed to me as
the project maintainer to be Cc'ed to the list.  I feel 80% of my
time writing

this

quoted

message wasted, as the contents of it would have helped not just you
but 4 other people who want to get their toes in the development
community, but you robbed them the chance of seeing it.

Sorry for having wasted your time, but you could just as easily have
CCed the list on your reply, couldn't you?

No, responding to a private message in public without permission goes

against

generally accepted netiquette (I am OK if you respond to this or to the

previous

message from me in public).

OK, understood and accepted. Herewith you have my permission ;-)

quoted

The use of getrlimit(RLIMIT_NOFILE, ...) vs. sysconf(_SC_OPEN_MAX) in
sha1_file.c, I believe the later to be more portable, also it is used
in other places already (namely compat/win32/poll.c)

Careful.  Getting rid of the use of getrlimit() from shared code by

introducing

our own abstraction to be implemented in compat/* for each platform is

fine,

but that does not mean sysconf() is acceptable by all platforms (your

finding

use of sysconf() in compat/win32 merely means Win is OK with it).

Well. NonStop is OK with it too. 
And I've brackets with #ifdef _SC_OPEN_MAX, just like in compat/win23:
/usr/local/bin/diff -EBbu ./sha1_file.c.orig ./sha1_file.c

--- ./sha1_file.c.orig  2012-07-30 15:50:38 -0500
+++ ./sha1_file.c       2012-08-14 02:42:03 -0500

@@ -747,6 +747,9 @@
                return error("packfile %s index unavailable", p->pack_name);

        if (!pack_max_fds) {
+#ifdef _SC_OPEN_MAX
+               unsigned int max_fds = sysconf(_SC_OPEN_MAX);
+#else
                struct rlimit lim;
                unsigned int max_fds;

@@ -754,6 +757,7 @@
                        die_errno("cannot get RLIMIT_NOFILE");

                max_fds = lim.rlim_cur;
+#endif

                /* Save 3 for stdin/stdout/stderr, 22 for work */
                if (25 < max_fds)
 

Can it get any better?

quoted

The use of curl_easy_strerror() in help.c without checking for a
proper curl version (>=7.12.0)

I do not recall the minimum version of curl we support offhand, but if

there is a

clean workaround that would be good.

Well, I really have none but to disable that part:
/usr/local/bin/diff -EBbu ./http.c.orig ./http.c

--- ./http.c.orig       2012-07-30 15:50:38 -0500
+++ ./http.c    2012-08-14 02:49:43 -0500

@@ -805,10 +805,12 @@
                                ret = HTTP_REAUTH;
                        }
                } else {
+#if LIBCURL_VERSION_NUM >= 0x070c00 /* no curl_easy_strerror() before that

*/
                        if (!curl_errorstr[0])
                                strlcpy(curl_errorstr,
 
curl_easy_strerror(results.curl_result),
                                        sizeof(curl_errorstr));
+#endif
                        ret = HTTP_ERROR;
                }
        } else {


It certainly is a workaround, whether it is clean or clean enough for now is
to you to decide ;-)

Bye, Jojo

On Tuesday 2012-08-14 17:52, Joachim Schmitz wrote:

Of course not. intptr_t is an integral value capable of holding
a pointer; it is not a pointer to int (because that would really
be redundant to int*.)

From: Jan Engelhardt [mailto:jengelh@inai.de]
Sent: Sunday, August 19, 2012 6:26 PM
To: Joachim Schmitz
Cc: 'Junio C Hamano'; git@vger.kernel.org
Subject: RE: git on HP NonStop


On Tuesday 2012-08-14 17:52, Joachim Schmitz wrote:

quoted

@@ -98,6 +99,11 @@

#include <stdlib.h>
#include <stdarg.h>
#include <string.h>
+#ifdef __TANDEM
+# include <strings.h> /* for strcasecmp() */
+  typedef int intptr_t; /* not "int *" ?!? */
+  typedef unsigned int uintptr_t; /* not "unsigned int *" ?!? */

Of course not. intptr_t is an integral value capable of holding a pointer;

it is not

a pointer to int (because that would really be redundant to int*.)

OK, thanks for the clarification.

Another issue I stumbled across:
There are numerous places (well, some 10) were something like the following
is done

int var = var;
char *othervar = othervar;

Here this leads to Compiler warnings  'variable "var" is used before its
value is set' on NonStop. This self-initialization seems to be a GCC
extension (?), but even gcc has a -Winit-self option to warn about this.
Shouldn't that better be like the following?

int var = 0;
char *othervar = NULL;

What is the reason for using that self-init stuff? I don't think it is
really portable, is it?

Bye, Jojo

Am 8/20/2012 12:36, schrieb Joachim Schmitz:

int var = var;
char *othervar = othervar;

... 

What is the reason for using that self-init stuff? I don't think it is
really portable, is it?

It is used to avoid "var may be used uninitialized" warnings for some
compilers. Officially (according to the C standard), it is undefined
behavior. But I've observed considerable resistance by Junio to fix this
properly. Therefore, unless you can show that your compiler generates
unusable code you better live with the "self-initialization" warnings.

-- Hannes

From: Johannes Sixt [mailto:j.sixt@viscovery.net]
Sent: Monday, August 20, 2012 12:57 PM
To: Joachim Schmitz
Cc: 'Jan Engelhardt'; 'Junio C Hamano'; git@vger.kernel.org
Subject: Re: git on HP NonStop

Am 8/20/2012 12:36, schrieb Joachim Schmitz:

quoted

int var = var;
char *othervar = othervar;

...

What is the reason for using that self-init stuff? I don't think it is
really portable, is it?

It is used to avoid "var may be used uninitialized" warnings for some

compilers.

Well, it results in a similar warning on NonStop. "var is used before it is
set" and I think this is equally bad.
In either case we don't know what the content of that var is.

E.g. in wt_status.c the variable 'status' is set at only one place, but
later it is switched on. If lucky we get to the default case and die.
So why not just 
int status = 0;

Officially (according to the C standard), it is undefined behavior. 

Yes, I had that suspicion. Not good to rely in this...

But I've observed considerable resistance by Junio to fix this properly. 

What's the  reason behind that?

Therefore, unless
you can show that your compiler generates unusable code you better live

with

the "self-initialization" warnings.

So far I can't, so I guess I'll have to live with the warnings, but don't
quite like it.
 

-- Hannes

Bye, Jojo