Re: [PATCH] Do not rely on the exit status of "unset" for unset variables

From: H.Merijn Brand <h.m.brand@xs4all.nl>
Date: Dec 5, 2007, 08:01 UTC
Message-ID: <20071205080157.5121bfee@pc09.procura.nl>
In-Reply-To: <Pine.LNX.4.64.0712042242310.27959@racer.site>

On Tue, 4 Dec 2007 22:45:16 +0000 (GMT), Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 7 quoted lines

> From: H.Merijn Brand <h.m.brand@xs4all.nl>
> 
> POSIX says that exit status "0" means that "unset" successfully unset
> the variable.  However, it is kind of ambiguous if an environment
> variable which was not set could be successfully unset.
> 
> At least the default shell on HP-UX insists on reporting an error in

please, for now make that HP-UX 11.11 and older. I'll check if it also fails in 11.23/IPF.

On 11.11 HP C-ANSI-C cannot be used either.

And I have to remove "#include <sys/select.h>" from pager.c on HP-UX, I forgot to tell. With the Makefile change in place, building with 64bit gcc,

--8<--- skip this part if you're not interested in 64bit builds on HP-UX On 64bit gcc on HP-UX, there is no strtoull (). Nowhere! strtoul () is the same there. But this is only in the 64bit world, so NO_STRTOULL can not be set to YesPlease unconditionally. When I also set NO_STRTOUMAX, I get shiploads of warnings like:

    CC commit.o
In file included from cache.h:4,
                 from commit.c:1:
git-compat-util.h:166:1: warning: "strtoumax" redefined
In file included from git-compat-util.h:62,
                 from cache.h:4,
                 from commit.c:1:
/usr/include/inttypes.h:527:1: warning: this is the location of the previous
definition

And that is NOT your fault, as this include file has defines like

** When compiling in ILP32 mode long long will be used for the 64-bit data ** types. This will cause compilation errors if 64-bit data types are ** requested and the compiler in use does not support them.

#define strtoimax(__a, __b, __c) __strtoll(__a, __b, __c) #define strtoumax(__a, __b, __c) __strtoull(__a, __b, __c)

and that is not guarded with something like

/* LP64 takes precedence */ #if (defined(__STDC_EXT__) || defined(_INCLUDE_LONGLONG)) && !defined(__LP64__)

so I ended up replacing all strtoumax () to strtoul () in git-fast-import.c

Then I end up with the same error as on 11.00. -->8---

Show 31 quoted lines

> such a case, so just ignore the exit status of "unset".
> 
> [Dscho: extended the patch to git-submodule.sh, as Junio realized that
>  this is the only other place where we check the exit status of "unset".]
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
> 	On Tue, 4 Dec 2007, Junio C Hamano wrote:
> 
> 	> "H.Merijn Brand" <h.m.brand@xs4all.nl> writes:
> 	> 
> 	> > On Tue, 4 Dec 2007 15:39:47 +0000 (GMT), Johannes Schindelin
> 	> > ...
> 	> > I found it! unset returns false
> 	> > ...
> 	> > I must leave now.
> 	> 
> 	> Thanks, you two.
> 	> 
> 	> I do not see "unset VAR... &&" outside t0001 test, but there are
> 	> instances of "unset VAR... &&" in git-submodule implementations 
> 	> as well.
> 	> 
> 	> In short, not too many places to fix.
> 
> 	You're right.  I grepped for "unset" in t/*.sh, but that catches 
> 	only false positives other than t0001.
> 
> 	Merijn, maybe you want to have your sign-off in the commit 
> 	message?

Feel free to do so, I don't really care.

Will you also be looking into the install issue?

-- 
H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)
using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,
& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org
http://mirrors.develooper.com/hpux/            http://www.test-smoke.org
                        http://www.goldmark.org/jeff/stupid-disclaimers/