[FYI] How I compile on IRIX 6.5 with the MIPSpro compiler and ksh

[FYI] How I compile on IRIX 6.5 with the MIPSpro compiler and ksh

[Brandon Casey]

This email describes how I have successfully compiled git on IRIX6.5 with the
native MIPSpro compiler version 7.4.4m and successfully run nearly all of the
tests using the Korn shell.

A series of 7 patches will follow this email. The patches work around a couple
of flaws in the IRIX c99 compiler, and the not-quite-recent build environment.

A build script is provided at the end of this email.

The GIT_SKIP_TESTS environment variable was used to skip tests which still fail.
The tests which still fail do so because of an ancient iconv, with one exception:
the t5000 tests which fail are due to the installed gnu tar being too old.

Two lingering concerns:
  1) Building with -Ofast produced an executable which segfaulted and could not
     pass the tests.
  2) Building without THREADED_DELTA_SEARCH=1 produced an executable which
     segfaulted and could not pass the tests.

I have no debugger.

Otherwise, I've been using this executable somewhat and have not experienced any
problems. Maybe it will be useful to others.

 Makefile                      |   12 ++++++++++++
 git-compat-util.h             |   12 +++++++++++-
 t/lib-git-svn.sh              |    3 +++
 t/t1002-read-tree-m-u-2way.sh |   10 ++++++----
 t/t9301-fast-export.sh        |    1 -
 t/test-lib.sh                 |    2 +-
 templates/Makefile            |    2 +-
 unpack-trees.c                |    3 ++-
 8 files changed, 36 insertions(+), 9 deletions(-)

-brandon


#!/bin/sh

GIT_SKIP_TESTS='
   t3900.1[0-289] t3900.2[023]
   t3901.*
   t5000.1[0-24-689] t5000.2[01]
   t5100.[56] t5100.1[02]
   t9301.4
'

export GIT_SKIP_TESTS

# NO_C99_FORMAT is required since this c99 compiler does not understand %td,
# and probably not %zu.

exec gmake \
    CC=c99 \
    CFLAGS='-n32 -O2' \
    SHELL_PATH='/bin/ksh' \
    TAR=gtar \
    NO_C99_FORMAT=1 \
    NO_CURL=1 \
    NO_OPENSSL=1 \
    NO_TCLTK=1 \
    NO_PERL_MAKEMAKER=1 \
    THREADED_DELTA_SEARCH=1

[PATCH] Makefile: add section for SGI IRIX

[Brandon Casey]

---
 Makefile |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 52c67c1..890694c 100644
--- a/Makefile
+++ b/Makefile
@@ -705,6 +705,18 @@ ifeq ($(uname_S),GNU)
 	# GNU/Hurd
 	NO_STRLCPY=YesPlease
 endif
+ifeq ($(uname_S),IRIX)
+	NO_SETENV = YesPlease
+	NO_UNSETENV = YesPlease
+	NO_STRCASESTR = YesPlease
+	NO_MEMMEM = YesPlease
+	NO_MKDTEMP = YesPlease
+	NO_EXTERNAL_GREP = UnfortunatelyYes
+	SHELL_PATH = /usr/gnu/bin/bash
+	ifdef NO_C99_FORMAT
+		BASIC_CFLAGS += -Dinline=__inline
+	endif
+endif
 ifeq ($(uname_S),IRIX64)
 	NO_IPV6=YesPlease
 	NO_SETENV=YesPlease
-- 
1.6.0.13.ge1c8

[PATCH] git-compat-util.h: adjust for SGI IRIX 6.5

[Brandon Casey]

Don't define _XOPEN_SOURCE
Do    define _SGI_SOURCE
Declare the _xpg5 versions of *snprintf() along with wrapper macros

Defining _XOPEN_SOURCE prevents many of the common functions and macros
from being defined. _Not_ setting _XOPEN_SOURCE, and instead setting
_SGI_SOURCE, provides all of the XPG4, XPG5, BSD, POSIX functions and
declarations, _BUT_ provides a horribly broken snprintf(). The provided
snprintf() can not be worked around using git's compat workaround, since
SGI's vsnprintf() always returns the number of characters written into
the string, instead of -1 which is what git's compat version expects.
SGI does have a working snprintf(), but it is only provided when
_NO_XOPEN5 evaluates to zero, and this only happens if _XOPEN_SOURCE is
defined which, as mentioned above, prevents many other common functions
and defines. The working *snprintf() functions are named _xpg5_vsnprintf()
and _xpg5_snprintf(), so declarations for these two functions were added
to git-compat-util.h and macros were added for vsnprintf and snprintf to
call these versions.
---
 git-compat-util.h |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index cf89cdf..f22707c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -39,13 +39,14 @@
 /* Approximation of the length of the decimal representation of this type. */
 #define decimal_length(x)	((int)(sizeof(x) * 2.56 + 0.5) + 1)
 
-#if !defined(__APPLE__) && !defined(__FreeBSD__)  && !defined(__USLC__) && !defined(_M_UNIX)
+#if !defined(__APPLE__) && !defined(__FreeBSD__)  && !defined(__USLC__) && !defined(_M_UNIX) && !defined(sgi)
 #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
 #define _ALL_SOURCE 1
 #define _GNU_SOURCE 1
 #define _BSD_SOURCE 1
+#define _SGI_SOURCE 1
 
 #include <unistd.h>
 #include <stdio.h>
@@ -262,6 +263,15 @@ extern int git_snprintf(char *str, size_t maxsize,
 #define vsnprintf git_vsnprintf
 extern int git_vsnprintf(char *str, size_t maxsize,
 			 const char *format, va_list ap);
+#elif defined(sgi)
+extern int      _xpg5_vsnprintf(char * __restrict,
+                        __SGI_LIBC_NAMESPACE_QUALIFIER size_t,
+                        const char * __restrict, /* va_list */ char *);
+#define vsnprintf _xpg5_vsnprintf
+extern int      _xpg5_snprintf(char * __restrict,
+                        __SGI_LIBC_NAMESPACE_QUALIFIER size_t,
+                        const char * __restrict, ...);
+#define snprintf _xpg5_snprintf
 #endif
 
 #ifdef __GLIBC_PREREQ
-- 
1.6.0.13.ge1c8

[PATCH] unpack-trees.c: work around run-time array initialization flaw on IRIX 6.5

[Brandon Casey]

The c99 MIPSpro Compiler version 7.4.4m on IRIX 6.5 does not properly
initialize run-time initialized arrays. An array which is initialized with
fewer elements than the length of the array should have the unitialized
elements initialized to zero. This compiler does perform proper
initialization when static initialization parameters are used. So, work
around this by initializing with static elements, followed by a simple
assignment.
---
 unpack-trees.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index cba0aca..de7cb0b 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -143,7 +143,8 @@ static inline int call_unpack_fn(struct cache_entry **src, struct unpack_trees_o
 
 static int unpack_index_entry(struct cache_entry *ce, struct unpack_trees_options *o)
 {
-	struct cache_entry *src[5] = { ce, };
+	struct cache_entry *src[5] = { NULL, };
+	src[0] = ce;
 
 	o->pos++;
 	if (ce_stage(ce)) {
-- 
1.6.0.13.ge1c8

[PATCH] templates/Makefile: work around SGI install which assumes / if ROOT not defined

[Brandon Casey]

---

Hmm, I'm thinking that this is probably only an issue when path is
not absolute, since I didn't have any problems installing the
executables. Possibly, a more general fix is to provide the
absolute path here.

-brandon

 templates/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/templates/Makefile b/templates/Makefile
index 9f3f1fc..3ea63e1 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -29,7 +29,7 @@ boilerplates.made : $(bpsrc)
 		case "$$boilerplate" in *~) continue ;; esac && \
 		dst=`echo "$$boilerplate" | sed -e 's|^this|.|;s|--|/|g'` && \
 		dir=`expr "$$dst" : '\(.*\)/'` && \
-		$(INSTALL) -d -m 755 blt/$$dir && \
+		ROOT=./ $(INSTALL) -d -m 755 blt/$$dir && \
 		case "$$boilerplate" in \
 		*--) ;; \
 		*) cp -p $$boilerplate blt/$$dst ;; \
-- 
1.6.0.13.ge1c8

Re: [PATCH] templates/Makefile: work around SGI install which assumes / if ROOT not defined

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> diff --git a/templates/Makefile b/templates/Makefile
> index 9f3f1fc..3ea63e1 100644
> --- a/templates/Makefile
> +++ b/templates/Makefile
> @@ -29,7 +29,7 @@ boilerplates.made : $(bpsrc)
>  		case "$$boilerplate" in *~) continue ;; esac && \
>  		dst=`echo "$$boilerplate" | sed -e 's|^this|.|;s|--|/|g'` && \
>  		dir=`expr "$$dst" : '\(.*\)/'` && \
> -		$(INSTALL) -d -m 755 blt/$$dir && \
> +		ROOT=./ $(INSTALL) -d -m 755 blt/$$dir && \

I do not see absolutely any reason to use install there.

I have to wonder why 9907721 (templates/Makefile: don't depend on local
umask setting, 2008-02-28) did not do this instead:

	$(QUIET)umask 022 && ls *--* 2>/dev/null | \
	while read boilerplate; \
	do \
		case "$$boilerplate" in *~) continue ;; esac && \
		dst=`echo "$$boilerplate" | sed -e 's|^this|.|;s|--|/|g'` && \
		dir=`expr "$$dst" : '\(.*\)/'` && \
		mkdir -p blt/$$dir && \
		case "$$boilerplate" in \
		*--) ;; \
		*) cp $$boilerplate blt/$$dst ;; \
		esac || exit; \
	done && \
	date >$@

Re: [PATCH] templates/Makefile: work around SGI install which assumes / if ROOT not defined

[Brandon Casey]

Junio C Hamano wrote:
> I do not see absolutely any reason to use install there.
> 
> I have to wonder why 9907721 (templates/Makefile: don't depend on local
> umask setting, 2008-02-28) did not do this instead:
> 
> 	$(QUIET)umask 022 && ls *--* 2>/dev/null | \
> 	while read boilerplate; \
> 	do \
> 		case "$$boilerplate" in *~) continue ;; esac && \
> 		dst=`echo "$$boilerplate" | sed -e 's|^this|.|;s|--|/|g'` && \
> 		dir=`expr "$$dst" : '\(.*\)/'` && \
> 		mkdir -p blt/$$dir && \
> 		case "$$boilerplate" in \
> 		*--) ;; \
> 		*) cp $$boilerplate blt/$$dst ;; \
> 		esac || exit; \
> 	done && \
> 	date >$@
> 

That works just fine for me.

-brandon

[PATCH] templates/Makefile: install is unnecessary, just use mkdir -p

[Brandon Casey]

From: Junio C Hamano <gitster@pobox.com>

The native install on some platforms (namely IRIX 6.5) treats non-absolute
paths as being relative to the root directory rather than relative to
the current directory. Work around this by avoiding install in this case
since it is unnecessary, and instead depend on the local umask setting
and use mkdir.

Tested-by: Brandon Casey <casey@nrlssc.navy.mil>
---


Junio C Hamano wrote:

> I do not see absolutely any reason to use install there.
> 
> I have to wonder why 9907721 (templates/Makefile: don't depend on local
> umask setting, 2008-02-28) did not do this instead:

works for me.

-brandon


 templates/Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/templates/Makefile b/templates/Makefile
index cc3fc30..0722a92 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -23,13 +23,13 @@ all: boilerplates.made custom
 
 bpsrc = $(filter-out %~,$(wildcard *--*))
 boilerplates.made : $(bpsrc)
-	$(QUIET)ls *--* 2>/dev/null | \
+	$(QUIET)umask 022 && ls *--* 2>/dev/null | \
 	while read boilerplate; \
 	do \
 		case "$$boilerplate" in *~) continue ;; esac && \
 		dst=`echo "$$boilerplate" | sed -e 's|^this|.|;s|--|/|g'` && \
 		dir=`expr "$$dst" : '\(.*\)/'` && \
-		$(INSTALL) -d -m 755 blt/$$dir && \
+		mkdir -p blt/$$dir && \
 		case "$$boilerplate" in \
 		*--) ;; \
 		*) cp -p $$boilerplate blt/$$dst ;; \
-- 
1.6.0.21.g35a2e

[PATCH] test-lib.sh: work around ksh's trap shortcomings

[Brandon Casey]

In ksh, if trap is called within a function with 0 or EXIT as its signal,
then the trap will be executed at the time the function returns. This
causes a problem in the test functions since 'trap - exit' is called
within the test_done function in order to remove the trap which calls
die() on exit. This means trap has to be called from the scripts top-level.
Do so using an alias.

Additionally, there is some strangeness with respect to aliases and
sourced script files; the alias hack doesn't work. So call 'trap - 0'
directly in lib-git-svn.sh before calling the test_done function.
---
 t/lib-git-svn.sh |    3 +++
 t/test-lib.sh    |    2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index a841df2..e2e8cf3 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -3,6 +3,7 @@
 if test -n "$NO_SVN_TESTS"
 then
 	test_expect_success 'skipping git-svn tests, NO_SVN_TESTS defined' :
+	trap - exit
 	test_done
 	exit
 fi
@@ -15,6 +16,7 @@ svn >/dev/null 2>&1
 if test $? -ne 1
 then
     test_expect_success 'skipping git-svn tests, svn not found' :
+    trap - exit
     test_done
     exit
 fi
@@ -39,6 +41,7 @@ then
 		err='Perl SVN libraries not found or unusable, skipping test'
 	fi
 	test_expect_success "$err" :
+	trap - exit
 	test_done
 	exit
 fi
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 11c0275..6a3fc93 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -415,7 +415,6 @@ test_create_repo () {
 }
 
 test_done () {
-	trap - exit
 	test_results_dir="$TEST_DIRECTORY/test-results"
 	mkdir -p "$test_results_dir"
 	test_results_path="$test_results_dir/${0%-*}-$$"
@@ -457,6 +456,7 @@ test_done () {
 
 	esac
 }
+alias test_done='trap - exit && test_done'
 
 # Test the binaries we have just built.  The tests are kept in
 # t/ subdirectory and are run in 'trash directory' subdirectory.
-- 
1.6.0.13.ge1c8

Re: [PATCH] test-lib.sh: work around ksh's trap shortcomings

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> In ksh, if trap is called within a function with 0 or EXIT as its signal,
> then the trap will be executed at the time the function returns. This
> causes a problem in the test functions since 'trap - exit' is called
> within the test_done function

Your alias test_done that calls function test_done look ugly and confusing
beyond words.  Perhaps test_done() can instead set a global variable and
die() can notice it instead, like this?  I haven't bothered to change the
other "trap - exit" but I think you got the idea...

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 11c0275..010bfda 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -156,10 +156,16 @@ test_count=0
 test_fixed=0
 test_broken=0
 test_success=0
+test_exit_ok=0
 
 die () {
-	echo >&5 "FATAL: Unexpected exit with code $?"
-	exit 1
+	status=$?
+	if test 1 != $test_exit_ok
+	then
+		echo >&5 "FATAL: Unexpected exit with code $status"
+		exit 1
+	fi
+	exit $status
 }
 
 trap 'die' exit
@@ -415,6 +421,7 @@ test_create_repo () {
 }
 
 test_done () {
+	test_exit_ok=1
 	trap - exit
 	test_results_dir="$TEST_DIRECTORY/test-results"
 	mkdir -p "$test_results_dir"

Re: [PATCH] test-lib.sh: work around ksh's trap shortcomings

[Brandon Casey]

I will try your suggestions.

I also hope you noticed that these patches followed an FYI message
and were not meant for direct inclusion. I hope you noticed that
since I don't want you to waste your time. Maybe I should have
put FYI in the [PATCH] string.

That being said, I (as always) appreciate the comments.

-brandon


Junio C Hamano wrote:
> Brandon Casey <casey@nrlssc.navy.mil> writes:
> 
>> In ksh, if trap is called within a function with 0 or EXIT as its signal,
>> then the trap will be executed at the time the function returns. This
>> causes a problem in the test functions since 'trap - exit' is called
>> within the test_done function
> 
> Your alias test_done that calls function test_done look ugly and confusing
> beyond words.  Perhaps test_done() can instead set a global variable and
> die() can notice it instead, like this?  I haven't bothered to change the
> other "trap - exit" but I think you got the idea...
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 11c0275..010bfda 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -156,10 +156,16 @@ test_count=0
>  test_fixed=0
>  test_broken=0
>  test_success=0
> +test_exit_ok=0
>  
>  die () {
> -	echo >&5 "FATAL: Unexpected exit with code $?"
> -	exit 1
> +	status=$?
> +	if test 1 != $test_exit_ok
> +	then
> +		echo >&5 "FATAL: Unexpected exit with code $status"
> +		exit 1
> +	fi
> +	exit $status
>  }
>  
>  trap 'die' exit
> @@ -415,6 +421,7 @@ test_create_repo () {
>  }
>  
>  test_done () {
> +	test_exit_ok=1
>  	trap - exit
>  	test_results_dir="$TEST_DIRECTORY/test-results"
>  	mkdir -p "$test_results_dir"

Re: [PATCH] test-lib.sh: work around ksh's trap shortcomings

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> Junio C Hamano wrote:
>> Brandon Casey <casey@nrlssc.navy.mil> writes:
>> 
>>> In ksh, if trap is called within a function with 0 or EXIT as its signal,
>>> then the trap will be executed at the time the function returns. This
>>> causes a problem in the test functions since 'trap - exit' is called
>>> within the test_done function
>> 
>> Your alias test_done that calls function test_done look ugly and confusing
>> beyond words.
> ...
> I will try your suggestions.
>
> I also hope you noticed that these patches followed an FYI message
> and were not meant for direct inclusion. I hope you noticed that
> since I don't want you to waste your time. Maybe I should have
> put FYI in the [PATCH] string.

Yeah, I think I got the subtle distinction between [PATCH FYI], [PATCH]
and ones with and without sign-offs.

I've read "trap" section of 1003.1 2004 three times but I do not see the
exact definition of "EXIT" condition mentioned.  Is the behaviour to
consider "function return" the EXIT condition considered a "shortcoming"
(aka bug)?

Re: [PATCH] test-lib.sh: work around ksh's trap shortcomings

[Brandon Casey]

Junio C Hamano wrote:

> Yeah, I think I got the subtle distinction between [PATCH FYI], [PATCH]
> and ones with and without sign-offs.

Heh, so subtle as to be unintentional. I think I mucked up that posting.
I should have labeled them all FYI and numbered them. I did hope there
would be some discussion that would result in a followup patch for
submission.

> I've read "trap" section of 1003.1 2004 three times but I do not see the
> exact definition of "EXIT" condition mentioned.  Is the behaviour to
> consider "function return" the EXIT condition considered a "shortcoming"
> (aka bug)?

No, I think it is the defined behavior for the Korn shell.

The SunOS 5.7 man page states:

     If sig  is  0 or EXIT and the trap  statement is exe-
     cuted inside the body of a function, then the com-
     mand arg is executed after the function completes.
     If sig is 0 or EXIT for a trap   set  outside  any
     function, the command arg is executed on exit from
     the shell.

The IRIX 6.5 man page has the exact same wording.

On linux, the man page for the public domain Korn shell states

    Functions  defined with the function reserved word are treated differ-
    ently in the following ways from functions defined with the  ()  nota-
    tion:

        ...

      - The EXIT trap, if set in a function, will be executed after the
        function returns.

But then later in the trap section:

    The original Korn shell’s DEBUG trap and the  handling  of  ERR
    and EXIT traps in functions are not yet implemented.


If Korn shell is to be supported, it seems that trap on EXIT must be
avoided within functions. I think the only one is in git-bisect.

-brandon

Re: [PATCH] test-lib.sh: work around ksh's trap shortcomings

[Brandon Casey]

Junio C Hamano wrote:
> Your alias test_done that calls function test_done look ugly and confusing
> beyond words.  Perhaps test_done() can instead set a global variable and
> die() can notice it instead, like this?  I haven't bothered to change the
> other "trap - exit" but I think you got the idea...

Yes that works and is much clearer. Tested on solaris and irix.

-brandon

Re: [PATCH] test-lib.sh: work around ksh's trap shortcomings

[Brandon Casey]

Brandon Casey wrote:
> Junio C Hamano wrote:
>> Your alias test_done that calls function test_done look ugly and confusing
>> beyond words.  Perhaps test_done() can instead set a global variable and
>> die() can notice it instead, like this?  I haven't bothered to change the
>> other "trap - exit" but I think you got the idea...
> 
> Yes that works and is much clearer. Tested on solaris and irix.

I spoke too soon. Failing tests do not terminate the testing.

ksh does not place the exit status of the shell in $? or provide it as an argument
to the trap function. Possibly the status of the 'exit' command is provided instead?

$ cat test.sh
#!/bin/sh

die () {
    status=$?
    echo >&2 "die status: $status"
    exit $status
}

trap 'die' 0

echo "I am dying"
exit 1

###### END TEST SCRIPT ######

linux $ /bin/sh test.sh
I am dying
die status: 1
linux $ echo $?
1

linux $ /usr/bin/ksh test.sh
I am dying
die status: 1
linux $ echo $?
1

IRIX $ /bin/ksh test.sh
I am dying
die status: 0
IRIX $ echo $?
0

SunOS $ /usr/xpg4/bin/sh test.sh
I am dying
die status: 0
SunOS $ echo $?
0

Ah, no, it looks like the status of the command executed immediately before
exit is what the trap has access to in $?. Adding a call to false before exit
in the above script causes the korn scripts to exit with status 1 on IRIX and
SunOS.

#!/bin/sh

die () {
    status=$?
    echo >&2 "die status: $status"
    exit $status
}

trap 'die' 0

echo "I am dying"
false
exit 1

-brandon

Re: [PATCH] test-lib.sh: work around ksh's trap shortcomings

[Mike Ralphson]

2008/8/20 Brandon Casey <casey@nrlssc.navy.mil>
>
> Brandon Casey wrote:
> > Junio C Hamano wrote:
> >> Your alias test_done that calls function test_done look ugly and confusing
> >> beyond words.  Perhaps test_done() can instead set a global variable and
> >> die() can notice it instead, like this?  I haven't bothered to change the
> >> other "trap - exit" but I think you got the idea...
> >
> > Yes that works and is much clearer. Tested on solaris and irix.
>
> I spoke too soon. Failing tests do not terminate the testing.
>
> ksh does not place the exit status of the shell in $? or provide it as an argument
> to the trap function. ...  it looks like the status of the command executed immediately before
> exit is what the trap has access to in $?. Adding a call to false before exit
> in the above script causes the korn scripts to exit with status 1 on IRIX and
> SunOS.

Just to report the same findings with AIX 5.3's /bin/sh (which is
/bin/ksh), /bin/ksh93 and even /bin/bsh

Mike

[PATCH] t1002-read-tree-m-u-2way.sh: use 'git diff -U0' rather than 'diff -U0'

[Brandon Casey]

Some old platforms have an old diff which doesn't have the -U option.
'git diff' can be used in its place. Adjust the comparison function to
strip git's additional header lines to make this possible.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 t/t1002-read-tree-m-u-2way.sh |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh
index aa9dd58..5e40cec 100755
--- a/t/t1002-read-tree-m-u-2way.sh
+++ b/t/t1002-read-tree-m-u-2way.sh
@@ -14,6 +14,8 @@ _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
 compare_change () {
 	sed >current \
+	    -e '1{/^diff --git /d;}' \
+	    -e '2{/^index /d;}' \
 	    -e '/^--- /d; /^+++ /d; /^@@ /d;' \
 	    -e 's/^\(.[0-7][0-7][0-7][0-7][0-7][0-7]\) '"$_x40"' /\1 X /' "$1"
 	test_cmp expected current
@@ -75,7 +77,7 @@ test_expect_success \
      git update-index --add yomin &&
      git read-tree -m -u $treeH $treeM &&
      git ls-files --stage >4.out || return 1
-     diff -U0 M.out 4.out >4diff.out
+     git diff -U0 --no-index M.out 4.out >4diff.out
      compare_change 4diff.out expected &&
      check_cache_at yomin clean &&
      sum bozbar frotz nitfol >actual4.sum &&
@@ -94,7 +96,7 @@ test_expect_success \
      echo yomin yomin >yomin &&
      git read-tree -m -u $treeH $treeM &&
      git ls-files --stage >5.out || return 1
-     diff -U0 M.out 5.out >5diff.out
+     git diff -U0 --no-index M.out 5.out >5diff.out
      compare_change 5diff.out expected &&
      check_cache_at yomin dirty &&
      sum bozbar frotz nitfol >actual5.sum &&
@@ -206,7 +208,7 @@ test_expect_success \
      git update-index --add nitfol &&
      git read-tree -m -u $treeH $treeM &&
      git ls-files --stage >14.out || return 1
-     diff -U0 M.out 14.out >14diff.out
+     git diff -U0 --no-index M.out 14.out >14diff.out
      compare_change 14diff.out expected &&
      sum bozbar frotz >actual14.sum &&
      grep -v nitfol M.sum > expected14.sum &&
@@ -227,7 +229,7 @@ test_expect_success \
      echo nitfol nitfol nitfol >nitfol &&
      git read-tree -m -u $treeH $treeM &&
      git ls-files --stage >15.out || return 1
-     diff -U0 M.out 15.out >15diff.out
+     git diff -U0 --no-index M.out 15.out >15diff.out
      compare_change 15diff.out expected &&
      check_cache_at nitfol dirty &&
      sum bozbar frotz >actual15.sum &&
-- 
1.6.0.13.ge1c8

[PATCH] t9301-fast-export.sh: don't unset config variable while we're skipping test 4

[Brandon Casey]

---


This is necessary if t9301.4 is included in the GIT_SKIP_TESTS
environment variable.

-brandon


 t/t9301-fast-export.sh |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index c19b4a2..475aadd 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh
@@ -190,7 +190,6 @@ export GIT_COMMITTER_NAME='C O Mitter'
 
 test_expect_success 'setup copies' '
 
-	git config --unset i18n.commitencoding &&
 	git checkout -b copy rein &&
 	git mv file file3 &&
 	git commit -m move1 &&
-- 
1.6.0.13.ge1c8

Re: [PATCH] t9301-fast-export.sh: don't unset config variable while we're skipping test 4

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> ---
>
>
> This is necessary if t9301.4 is included in the GIT_SKIP_TESTS
> environment variable.

If it is not skipped what happens?  Does your change break this test?

How about attempting to unset always (because the test does not want to
have the configuration) but ignore the result from the command (because
the config may not have had anything to unset to begin with depending on
what happened in the previous tests)?

>  t/t9301-fast-export.sh |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
> index c19b4a2..475aadd 100755
> --- a/t/t9301-fast-export.sh
> +++ b/t/t9301-fast-export.sh
> @@ -190,7 +190,6 @@ export GIT_COMMITTER_NAME='C O Mitter'
>  
>  test_expect_success 'setup copies' '
>  
> -	git config --unset i18n.commitencoding &&
>  	git checkout -b copy rein &&
>  	git mv file file3 &&
>  	git commit -m move1 &&
> -- 
> 1.6.0.13.ge1c8

Re: [PATCH] t9301-fast-export.sh: don't unset config variable while we're skipping test 4

[Brandon Casey]

Junio C Hamano wrote:
> Brandon Casey <casey@nrlssc.navy.mil> writes:
> 
>> ---
>>
>>
>> This is necessary if t9301.4 is included in the GIT_SKIP_TESTS
>> environment variable.
> 
> If it is not skipped what happens?  Does your change break this test?

Yes, it was only meant as an example to be used along with the build
script I supplied.

> How about attempting to unset always (because the test does not want to
> have the configuration) but ignore the result from the command (because
> the config may not have had anything to unset to begin with depending on
> what happened in the previous tests)?

Yes, unsetting always and ignoring the result would allow this test to
pass. It could also be moved into its own test so that it could be skipped
using GIT_SKIP_TESTS, but still signal failure when it is not skipped.

-brandon

[PATCH] t9301-fast-export: move unset of config variable into its own test function

[Brandon Casey]

On platforms with a broken iconv it may be necessary to skip the fourth
test in this script. Test four sets the i18n.commitencoding config variable.
If test four is skipped, then a later unset of the i18n.commitencoding
config variable will return a non-zero exit status and cause a test to fail.
So move the 'config --unset' into its own test function which allows it to
be skipped independently.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 t/t9301-fast-export.sh |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index 3cb9f80..41d444d 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh
@@ -185,12 +185,17 @@ test_expect_success 'submodule fast-export | fast-import' '
 
 '
 
+test_expect_success 'unset i18n.commitencoding' '
+
+	git config --unset i18n.commitencoding
+
+'
+
 export GIT_AUTHOR_NAME='A U Thor'
 export GIT_COMMITTER_NAME='C O Mitter'
 
 test_expect_success 'setup copies' '
 
-	git config --unset i18n.commitencoding &&
 	git checkout -b copy rein &&
 	git mv file file3 &&
 	git commit -m move1 &&
-- 
1.6.0.21.g35a2e

Re: [PATCH] t9301-fast-export: move unset of config variable into its own test function

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> On platforms with a broken iconv it may be necessary to skip the fourth
> test in this script. Test four sets the i18n.commitencoding config variable.
> If test four is skipped, then a later unset of the i18n.commitencoding
> config variable will return a non-zero exit status and cause a test to fail.
> So move the 'config --unset' into its own test function which allows it to
> be skipped independently.

I do not know if this is worth it, and I am reasonably sure this is not an
optimal solution for this particular case.

Many existing pieces in the test scripts, I even suspect majority of them,
do depend on earlier piece in the sequence to succeed and try to build on
top of the state they have left.  In the ideal world, especially when the
software is young and flaky, perhaps that should not be the case and
having test pieces that are as independent as each other would be easier
to fix many tests that do not pass, but these days we expect all tests to
pass, so running tests has become like reading the error output from
compilers --- very often, the first error is the only one that counts.

Even though test-lib.sh does support skipping individual test pieces by
adding t9301.12 to GIT_TEST_SKIP, it is not very useful in practice with
the current set of tests, it would involve huge effort to make it reliably
usable, and individual test pieces in the end result from such an effort
will have to perform their own set-up from scratch, which would mean the
test will take even longer to run, to catch occasional breakages.

For this particular case, what we are interested in testing is not that
"config --unset" exits with 0 status.  We are however interested in making
sure that i18n.commitencoding is not set when the body of #12 runs.

So I think a more appropriate change would be something like this for this
particular case.

 t/t9301-fast-export.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git c/t/t9301-fast-export.sh i/t/t9301-fast-export.sh
index 3cb9f80..2ce2aff 100755
--- c/t/t9301-fast-export.sh
+++ i/t/t9301-fast-export.sh
@@ -190,7 +190,9 @@ export GIT_COMMITTER_NAME='C O Mitter'
 
 test_expect_success 'setup copies' '
 
-	git config --unset i18n.commitencoding &&
+	git config --unset i18n.commitencoding
+
+	test -z $(git config i18n.commitencoding) &&
 	git checkout -b copy rein &&
 	git mv file file3 &&
 	git commit -m move1 &&

Re: [PATCH] t9301-fast-export: move unset of config variable into its own test function

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> For this particular case, what we are interested in testing is not that
> "config --unset" exits with 0 status.  We are however interested in making
> sure that i18n.commitencoding is not set when the body of #12 runs.
>
> So I think a more appropriate change would be something like this for this
> particular case.

Having said that, we may want to have an easier way to exclude certain
classes of pieces, and also encourage test writers to group pieces that
are related to these classes together.

For example, this introduces a new environment you can set,
GIT_SKIP_TEST_CLASS, which is a space separated list of classes of
features that you would want to exclude from the test.
test_expect_success/failure can now take an optional "class token" as the
first parameter (they traditionally took only two parameters, but with
class token, they take three).

This example defines I18N class, and lets you exclude the one you were
manually excluding with "GIT_SKIP_TESTS=t9301.4"


 t/t9301-fast-export.sh |    2 +-
 t/test-lib.sh          |   50 ++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git i/t/t9301-fast-export.sh w/t/t9301-fast-export.sh
index 2ce2aff..361e8dc 100755
--- i/t/t9301-fast-export.sh
+++ w/t/t9301-fast-export.sh
@@ -63,7 +63,7 @@ test_expect_success 'fast-export master~2..master' '
 
 '
 
-test_expect_success 'iso-8859-1' '
+test_expect_success I18N 'iso-8859-1' '
 
 	git config i18n.commitencoding ISO-8859-1 &&
 	# use author and committer name in ISO-8859-1 to match it.
diff --git i/t/test-lib.sh w/t/test-lib.sh
index e2b106c..88d6d50 100644
--- i/t/test-lib.sh
+++ w/t/test-lib.sh
@@ -232,22 +232,44 @@ test_run_ () {
 	return 0
 }
 
+# space sparated list of skippable test classes
+GIT_SKIPPABLE_TEST_CLASSES='I18N'
+
 test_skip () {
 	this_test=$(expr "./$0" : '.*/\(t[0-9]*\)-[^/]*$')
 	this_test="$this_test.$(expr "$test_count" + 1)"
+
 	to_skip=
-	for skp in $GIT_SKIP_TESTS
-	do
-		case "$this_test" in
-		$skp)
+	if test -n "$test_class"
+	then
+		case " $GIT_SKIPPABLE_TEST_CLASSES " in
+		*" $test_class "*) ;; # ok
+		*)
+			say_color error "'$test_class' is not a skippable test class"
+			error "Skippable are $GIT_SKIPPABLE_TEST_CLASSES"
+		esac
+		case " $GIT_SKIP_TEST_CLASS " in
+		*" $test_class "*)
 			to_skip=t
+			test_class="($test_class) "
 		esac
-	done
+	fi
+	if test -z "$to_skip"
+	then
+		for skp in $GIT_SKIP_TESTS
+		do
+			case "$this_test" in
+			$skp)
+				to_skip=t
+				break
+			esac
+		done
+	fi
 	case "$to_skip" in
 	t)
 		say_color skip >&3 "skipping test: $@"
 		test_count=$(expr "$test_count" + 1)
-		say_color skip "skip $test_count: $1"
+		say_color skip "skip $test_count: $test_class$1"
 		: true
 		;;
 	*)
@@ -257,6 +279,10 @@ test_skip () {
 }
 
 test_expect_failure () {
+	case $# in
+	2)	test_class= ;;
+	3)	test_class=$1; shift ;;
+	esac
 	test "$#" = 2 ||
 	error "bug in the test script: not 2 parameters to test-expect-failure"
 	if ! test_skip "$@"
@@ -274,6 +300,10 @@ test_expect_failure () {
 }
 
 test_expect_success () {
+	case $# in
+	2)	test_class= ;;
+	3)	test_class=$1; shift ;;
+	esac
 	test "$#" = 2 ||
 	error "bug in the test script: not 2 parameters to test-expect-success"
 	if ! test_skip "$@"
@@ -291,6 +321,10 @@ test_expect_success () {
 }
 
 test_expect_code () {
+	case $# in
+	3)	test_class= ;;
+	4)	test_class=$1; shift ;;
+	esac
 	test "$#" = 3 ||
 	error "bug in the test script: not 3 parameters to test-expect-code"
 	if ! test_skip "$@"
@@ -316,6 +350,10 @@ test_expect_code () {
 # Usage: test_external description command arguments...
 # Example: test_external 'Perl API' perl ../path/to/test.pl
 test_external () {
+	case $# in
+	3)	test_class= ;;
+	4)	test_class=$1; shift ;;
+	esac
 	test "$#" -eq 3 ||
 	error >&5 "bug in the test script: not 3 parameters to test_external"
 	descr="$1"

Re: [PATCH] t9301-fast-export: move unset of config variable into its own test function

[Johannes Sixt]

Junio C Hamano schrieb:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> For this particular case, what we are interested in testing is not that
>> "config --unset" exits with 0 status.  We are however interested in making
>> sure that i18n.commitencoding is not set when the body of #12 runs.
>>
>> So I think a more appropriate change would be something like this for this
>> particular case.
> 
> Having said that, we may want to have an easier way to exclude certain
> classes of pieces, and also encourage test writers to group pieces that
> are related to these classes together.
> 
> For example, this introduces a new environment you can set,
> GIT_SKIP_TEST_CLASS, which is a space separated list of classes of
> features that you would want to exclude from the test.
> test_expect_success/failure can now take an optional "class token" as the
> first parameter (they traditionally took only two parameters, but with
> class token, they take three).

Nice idea. Another class would be the tests that depend on that the
filesystem supports symbolic links.

> -test_expect_success 'iso-8859-1' '
> +test_expect_success I18N 'iso-8859-1' '

How do the tests look like if this token is the *last* argument?

To continue the idea, please look into t5302-pack-index.sh: We skip some
tests if we don't have support for 64bit file offsets. Making these tests
a "static" class would not be appropriate because the condition whether
64bit support is present is derived dynamically by the testsuite. What if
we could write tests like this:

test_expect_success \
    'index v2: verify a pack with some 64-bit offsets' \
    'git verify-pack -v "test-3-${pack3}.pack"' \
    'test "$have_64bits"'

i.e. the 3rd argument is a condition that tells whether the test should be
run. And in other cases the 3rd argument is the token that you propose:

test_expect_success 'iso-8859-1' '

     ...test goes here...

' I18N

Hm?

-- Hannes

Re: [PATCH] t9301-fast-export: move unset of config variable into its own test function

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Nice idea. Another class would be the tests that depend on that the
> filesystem supports symbolic links.
>
>> -test_expect_success 'iso-8859-1' '
>> +test_expect_success I18N 'iso-8859-1' '
>
> How do the tests look like if this token is the *last* argument?

I thought about it but rejected it because it is much easier to spot class
tokens if it comes immediately after test_expect_xyzzy.

I suspect that certain classes of tests that need to be skipped can be
autodetected inside test-lib.sh; it would be an independent topic to build
on top of this.  Your example of 64-bit may be one of them.

Re: [FYI] How I compile on IRIX 6.5 with the MIPSpro compiler and ksh

[Brandon Casey]

I hope these messages threaded correctly for everyone.

They show up correctly on gmane, but not for me in Thunderbird.

For me, the Message-Id of the first message is:
  giNXZFTxzY3B65dQob7CwvwwfSKlZpw_60oz81RxU5UN3PsTT_3dMQ@cipher.nrlssc.navy.mil
and this is what I provide to git-send-email.

But when I receive a message sent in such a way back from vger (i.e. using
git-send-email to set In-Reply-To), the In-Reply-To field looks like:

  48A9FDCE.3070906@nrlssc.navy.mil

so thunderbird does not nest the replies beneath the original message.

-brandon

Re: [FYI] How I compile on IRIX 6.5 with the MIPSpro compiler and ksh

[Boyd Lynn Gerber]

I would have liked to see 

Subject : patch [0/7] How to compile IRIX 6.5 w/ MIPSpro compiler/ksh
...
Subject : patch [7/7] How to compile IRIX 6.5 w/ MIPSpro compiler/ksh

It would be easier to follow all your patches with all the other email on 
the lists.

Thanks,

--
Boyd Gerber <gerberb@zenez.com>
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047

Re: [FYI] How I compile on IRIX 6.5 with the MIPSpro compiler and ksh

[Brandon Casey]

Boyd Lynn Gerber wrote:
> I would have liked to see 
> 
> Subject : patch [0/7] How to compile IRIX 6.5 w/ MIPSpro compiler/ksh
> ...
> Subject : patch [7/7] How to compile IRIX 6.5 w/ MIPSpro compiler/ksh
> 
> It would be easier to follow all your patches with all the other email on 
> the lists.

whoops, yeah, I should have done that. Sorry.

-brandon