Hacks for AIX

Hacks for AIX

[Chris Cowan]

I saw some earlier postings about this, so I thought I would share my
solution (I'm using xlc, BTW).   The following tests were broken for
me (using a pull from 2 days ago).

    * t0002-gitfile.sh
    * t1002-read-tree-m-u-2way.sh
    * t2201-add-update-typechange.sh
    * t4109-apply-multifrag.sh
    * t4110-apply-scan.sh
    * t7002-grep.sh


The problems all seem to be rooted in the default utilities shipped with AIX:

    * /usr/bin/grep - behaves badly in t7002.   I believe it is test
12 and related to the -n -w -e combination of options.
    * /usr/bin/diff - has problems with -u and -U.
                         I saw the $GIT_CMP_TEST env var, but this is
not used everywhere within the test scripts above.
    * /usr/bin/patch - really old version, doesn't do well with some
diff formats.   I avoid using it.
    * /usr/bin/install - doesn't behave the expected way either.

In some cases, the tests could have been made more portable by using a
plain "diff" rather than "diff -u", for example.

Fortunately, there are optional freeware versions that can be
installed for all of these (along with tar and wish too).   These
versions if installed, are all found in /usr/linux/bin (or
equivalently /opt/freeware/bin), I just wish they weren't optional.  I
have found that having these utilities installed and prepending
/usr/linux/bin to the PATH results in a clean make test and build.

I also saw one instance where the behavior of git-grep was affected by
the grep selected at build time.    I'm not sure if there's other
instances within the code base, but I'm wondering whether the
configure script can be changed to do the check for /usr/linux/bin and
use those versions?   I can imagine that similar problems may occur on
Solaris and HPUX.

Otherwise, I'm quite happy with git.
-- 
CC

Re: Hacks for AIX

[Junio C Hamano]

"Chris Cowan" <chris.o.cowan@gmail.com> writes:

>     * /usr/bin/grep - behaves badly in t7002.   I believe it is test
> 12 and related to the -n -w -e combination of options.

Perhaps your version of AIX needs to...

    $ make NO_EXTERNAL_GREP=UnfortunatelyYesOnThisAIX

>     * /usr/bin/diff - has problems with -u and -U.
>                          I saw the $GIT_CMP_TEST env var, but this is
> not used everywhere within the test scripts above.
> In some cases, the tests could have been made more portable by using a
> plain "diff" rather than "diff -u", for example.

A patch to the testsuite to replace use of "diff" and "diff -u" that test
the actual output matches expected output with "test_cmp" would be
appreciated.

>     * /usr/bin/patch - really old version, doesn't do well with some
> diff formats.   I avoid using it.

t4109 seems to use patch to produce expected output for the tests; we
should ship a precomputed expected results.  Do you know of any other
places "patch" is used?

Re: Hacks for AIX

[Brandon Casey]

Junio C Hamano wrote:
> "Chris Cowan" <chris.o.cowan@gmail.com> writes:

>>     * /usr/bin/patch - really old version, doesn't do well with some
>> diff formats.   I avoid using it.
> 
> t4109 seems to use patch to produce expected output for the tests; we
> should ship a precomputed expected results.  Do you know of any other
> places "patch" is used?

t4110 also uses patch in the same way.

-brandon

Re: Hacks for AIX

[Junio C Hamano]

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

>>     * /usr/bin/patch - really old version, doesn't do well with some
>> diff formats.   I avoid using it.
>
> t4109 seems to use patch to produce expected output for the tests; we
> should ship a precomputed expected results.  Do you know of any other
> places "patch" is used?

As usual, I won't commit this patch unless I hear from people who
potentially would benefit from it.  I do not need such a patch myself and
I really shouldn't be spending too much of my time on these.

-- >8 --
[PATCH] do not rely on external "patch" in tests

Some of our tests assumed a working "patch" command to produce expected
results when checking "git-apply", but some systems have broken "patch".

We can compare our output with expected output that is precomputed
instead to sidestep this issue.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4109-apply-multifrag.sh |  119 +++++++++++++++++++++++++++++++++++---------
 t/t4110-apply-scan.sh      |   35 ++++++++++---
 2 files changed, 123 insertions(+), 31 deletions(-)

diff --git a/t/t4109-apply-multifrag.sh b/t/t4109-apply-multifrag.sh
index bd40a21..4805863 100755
--- a/t/t4109-apply-multifrag.sh
+++ b/t/t4109-apply-multifrag.sh
@@ -138,39 +138,112 @@ diff --git a/main.c b/main.c
  
 EOF
 
-test_expect_success "S = git apply (1)" \
-    'git apply patch1.patch patch2.patch'
-mv main.c main.c.git
+cat >expect_1 <<\EOF
+#include <stdlib.h>
+#include <stdio.h>
 
-test_expect_success "S = patch (1)" \
-    'cat patch1.patch patch2.patch | patch -p1'
+int func(int num);
+void print_int(int num);
+void print_ln();
 
-test_expect_success "S = cmp (1)" \
-    'cmp main.c.git main.c'
+int main() {
+	int i;
 
-rm -f main.c main.c.git
+	for (i = 0; i < 10; i++) {
+		print_int(func(i));
+	}
 
-test_expect_success "S = git apply (2)" \
-    'git apply patch1.patch patch2.patch patch3.patch'
-mv main.c main.c.git
+	print_ln();
 
-test_expect_success "S = patch (2)" \
-    'cat patch1.patch patch2.patch patch3.patch | patch -p1'
+	return 0;
+}
 
-test_expect_success "S = cmp (2)" \
-    'cmp main.c.git main.c'
+int func(int num) {
+	return num * num;
+}
 
-rm -f main.c main.c.git
+void print_int(int num) {
+	printf("%d", num);
+}
 
-test_expect_success "S = git apply (3)" \
-    'git apply patch1.patch patch4.patch'
-mv main.c main.c.git
+void print_ln() {
+	printf("\n");
+}
 
-test_expect_success "S = patch (3)" \
-    'cat patch1.patch patch4.patch | patch -p1'
+EOF
+
+test_expect_success 'git apply (1)' '
+	git apply patch1.patch patch2.patch &&
+	mv main.c actual &&
+	test_cmp expect_1 actual
+'
+
+cat >expect_2 <<\EOF
+#include <stdio.h>
+
+int func(int num);
+void print_int(int num);
+
+int main() {
+	int i;
+
+	for (i = 0; i < 10; i++) {
+		print_int(func(i));
+	}
+
+	return 0;
+}
+
+int func(int num) {
+	return num * num;
+}
+
+void print_int(int num) {
+	printf("%d", num);
+}
+
+EOF
 
-test_expect_success "S = cmp (3)" \
-    'cmp main.c.git main.c'
+test_expect_success 'git apply (2)' '
+	rm -f main.c &&
+	git apply patch1.patch patch2.patch patch3.patch &&
+	mv main.c actual &&
+	test_cmp expect_2 actual
+'
+
+cat >expect_3 <<\EOF
+#include <stdio.h>
+
+int func(int num);
+int func2(int num);
+
+int main() {
+	int i;
+
+	for (i = 0; i < 10; i++) {
+		printf("%d", func(i));
+		printf("%d", func3(i));
+	}
+
+	return 0;
+}
+
+int func(int num) {
+	return num * num;
+}
+
+int func2(int num) {
+	return num * num * num;
+}
+
+EOF
+
+test_expect_success 'git apply (3)' '
+	rm -f main.c &&
+	git apply patch1.patch patch4.patch &&
+	mv main.c actual &&
+	test_cmp expect_3 actual
+'
 
 test_done
 
diff --git a/t/t4110-apply-scan.sh b/t/t4110-apply-scan.sh
index db60652..ae300ca 100755
--- a/t/t4110-apply-scan.sh
+++ b/t/t4110-apply-scan.sh
@@ -86,15 +86,34 @@ diff --git a/new.txt b/new.txt
 +c2222
 EOF
 
-test_expect_success "S = git apply scan" \
-    'git apply patch1.patch patch2.patch patch3.patch patch4.patch patch5.patch'
-mv new.txt apply.txt
+cat >expect <<\EOF
+a1
+a11
+a111
+a1111
+b1
+b11
+b111
+b1111
+b2
+b22
+b222
+b2222
+c1
+c11
+c111
+c1111
+c2
+c22
+c222
+c2222
+EOF
 
-test_expect_success "S = patch scan" \
-    'cat patch1.patch patch2.patch patch3.patch patch4.patch patch5.patch | patch'
-mv new.txt patch.txt
 
-test_expect_success "S = cmp" \
-    'cmp apply.txt patch.txt'
+test_expect_success 'apply series of patches' '
+	git apply patch1.patch patch2.patch patch3.patch patch4.patch patch5.patch &&
+	mv new.txt actual &&
+	test_cmp expect actual
+'
 
 test_done

Re: Hacks for AIX

[Brandon Casey]

Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>>>     * /usr/bin/patch - really old version, doesn't do well with some
>>> diff formats.   I avoid using it.
>> t4109 seems to use patch to produce expected output for the tests; we
>> should ship a precomputed expected results.  Do you know of any other
>> places "patch" is used?
> 
> As usual, I won't commit this patch unless I hear from people who
> potentially would benefit from it.  I do not need such a patch myself and
> I really shouldn't be spending too much of my time on these.


Unless I'm doing something wrong, this doesn't apply cleanly to master.


> -- >8 --
> [PATCH] do not rely on external "patch" in tests
> 
> Some of our tests assumed a working "patch" command to produce expected
> results when checking "git-apply", but some systems have broken "patch".
> 
> We can compare our output with expected output that is precomputed
> instead to sidestep this issue.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  t/t4109-apply-multifrag.sh |  119 +++++++++++++++++++++++++++++++++++---------
>  t/t4110-apply-scan.sh      |   35 ++++++++++---
>  2 files changed, 123 insertions(+), 31 deletions(-)
> 
> diff --git a/t/t4109-apply-multifrag.sh b/t/t4109-apply-multifrag.sh
> index bd40a21..4805863 100755
> --- a/t/t4109-apply-multifrag.sh
> +++ b/t/t4109-apply-multifrag.sh
> @@ -138,39 +138,112 @@ diff --git a/main.c b/main.c

The t4109-apply-multifrag.sh that I have only has 52 lines.

>  EOF

It does not have this line above. Perhaps you also moved the
../t4109/patch*.patch files into the script in another commit?

-brandon

Re: Hacks for AIX

[Brandon Casey]

Brandon Casey wrote:
> Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>>>     * /usr/bin/patch - really old version, doesn't do well with some
>>>> diff formats.   I avoid using it.
>>> t4109 seems to use patch to produce expected output for the tests; we
>>> should ship a precomputed expected results.  Do you know of any other
>>> places "patch" is used?
>> As usual, I won't commit this patch unless I hear from people who
>> potentially would benefit from it.  I do not need such a patch myself and
>> I really shouldn't be spending too much of my time on these.
> 
> 
> Unless I'm doing something wrong, this doesn't apply cleanly to master.

I figured out that your patch was against maint.

I see it is now applied to master and it helps out on solaris with old patch.

Now I need to get rid of 'diff -U0' in t1002-read-tree-m-u-2way.sh.

-brandon

Re: Hacks for AIX

[Linus Torvalds]

On Wed, 16 Jul 2008, Chris Cowan wrote:
> 
> I also saw one instance where the behavior of git-grep was affected by
> the grep selected at build time.    I'm not sure if there's other
> instances within the code base, but I'm wondering whether the
> configure script can be changed to do the check for /usr/linux/bin and
> use those versions?   I can imagine that similar problems may occur on
> Solaris and HPUX.

The grep selection at compile time is purely a choice between "no external 
grep at all" and "whatever external grep is in $PATH".

exec_grep() literally does

	..
	pid = fork();
	if (pid < 0)
		return pid;
	if (!pid) {
		execvp("grep", (char **) argv);
		exit(255);
	}
	..

so you can choose your version of external grep at run-time by just 
setting PATH appropriately.

Or you can just decide that you don't want to use any external grep binary 
at all, which is the compile-time choice of NO_EXTERNAL_GREP. In that 
case, git will do the grep implementation all internally. It can do so, 
but then it relies on the regex() library which is often less optimized 
than the external grep.

Note the "often". It's possible that the external grep is never worth it, 
in which case you should use NO_EXTERNAL_GREP. GNU grep happens to be very 
good.

Even with an external grep configured in, you'll end up using the internal 
one for the case where you ask for the index information ("--cached") or 
when you ask for a particular version of the tree rather than the 
checked-out tree. So regardless, you'll fall back to the internal version 
for some things.

			Linus