1.5.3-rc5 regression on OS X?

1.5.3-rc5 regression on OS X?

[Perrin Meyer]

Hi,

When I try to build and test 1.5.3-rc5 on my OS X box, I get a failure on "make test"

*   ok 82: 15 (fail) - must match A in O && A && B && O==A && O==B case
* FAIL 83: 16 - A matches in one and B matches in another.
        rm -f .git/index F16 &&
            echo F16 >F16 &&
            git update-index --add F16 &&
            tree0=`git write-tree` &&
            echo E16 >F16 &&
            git update-index F16 &&
            tree1=`git write-tree` &&
            git read-tree -m $tree0 $tree1 $tree1 $tree0 &&
            git ls-files --stage
* failed 1 among 83 test(s)
make[1]: *** [t1000-read-tree-m-3way.sh] Error 1
make: *** [test] Error 2


I was able to build and test git 1.5.2.4 fine, with no errors in the test suite. 

I also just build 1.5.3-rc5 on a redhat box, with no errors in the testsuite. 

My OS X box is a quad G5 running OS X 10.4.10, with gcc version: 

perrin-meyers-power-mac-g5:~/GIT/git perrin$ gcc -v
Using built-in specs.
Target: powerpc-apple-darwin8
Configured with: /private/var/tmp/gcc/gcc-5026.obj~19/src/configure --disable-checking --prefix=/usr --mandir=/share/man --enable-languages=c,objc,c++,obj-c++ --program-transform-name=/^[cg][^+.-]*$/s/$/-4.0/ --with-gxx-include-dir=/include/gcc/darwin/4.0/c++ --build=powerpc-apple-darwin8 --host=powerpc-apple-darwin8 --target=powerpc-apple-darwin8
Thread model: posix
gcc version 4.0.0 (Apple Computer, Inc. build 5026)

I define "NO_EXPAT" when I build on OS X. 

Am I doing something wrong?

Thanks,

Perrin

Re: 1.5.3-rc5 regression on OS X?

[Junio C Hamano]

Perrin Meyer <perrinmeyer@yahoo.com> writes:

> I was able to build and test git 1.5.2.4 fine, with no errors in the test suite. 
>
> I also just build 1.5.3-rc5 on a redhat box, with no errors in the testsuite. 
The above sounds as if you built 1.5.2.4 on your Mac in not so
recent past, and 1.5.3-rc5 on your Mac and RH recently.

Has anything changed on your Mac between the time you built
1.5.2.4 and failed to pass the test for 1.5.3-rc5?

Re: 1.5.3-rc5 regression on OS X?

[Brian Gernhardt]

On Aug 15, 2007, at 5:19 PM, Perrin Meyer wrote:

> My OS X box is a quad G5 running OS X 10.4.10, with gcc version:
>
> perrin-meyers-power-mac-g5:~/GIT/git perrin$ gcc -v
> Using built-in specs.
> Target: powerpc-apple-darwin8
> Configured with: /private/var/tmp/gcc/gcc-5026.obj~19/src/configure  
> --disable-checking --prefix=/usr --mandir=/share/man --enable- 
> languages=c,objc,c++,obj-c++ --program-transform-name=/^[cg][^+.-]* 
> $/s/$/-4.0/ --with-gxx-include-dir=/include/gcc/darwin/4.0/c++ -- 
> build=powerpc-apple-darwin8 --host=powerpc-apple-darwin8 -- 
> target=powerpc-apple-darwin8
> Thread model: posix
> gcc version 4.0.0 (Apple Computer, Inc. build 5026)
>
> I define "NO_EXPAT" when I build on OS X.
>
> Am I doing something wrong?

I believe you have an old version of the Developer Tools.  I have no  
issues with that test on my MacBook Pro (2.16 GHz Intel Core Duo).

$ gcc -v
Using built-in specs.
Target: i686-apple-darwin8
Configured with: /private/var/tmp/gcc/gcc-5367.obj~1/src/configure -- 
disable-checking -enable-werror --prefix=/usr --mandir=/share/man -- 
enable-languages=c,objc,c++,obj-c++ --program-transform-name=/^[cg] 
[^.-]*$/s/$/-4.0/ --with-gxx-include-dir=/include/c++/4.0.0 --with- 
slibdir=/usr/lib --build=powerpc-apple-darwin8 --with-arch=nocona -- 
with-tune=generic --program-prefix= --host=i686-apple-darwin8 -- 
target=i686-apple-darwin8
Thread model: posix
gcc version 4.0.1 (Apple Computer, Inc. build 5367)

I don't use NO_EXPAT, but that shouldn't affect read-tree at all.

~~ Brian

Re: 1.5.3-rc5 regression on OS X?

[Randal L. Schwartz]

>>>>> "Perrin" == Perrin Meyer <perrinmeyer@yahoo.com> writes:

Perrin> Hi,

Perrin> When I try to build and test 1.5.3-rc5 on my OS X box, I get a failure on "make test"

My t1000 test succeeded on modern OSX.  But "t1301-shared-repo" failed:

*** t1301-shared-repo.sh ***
* FAIL 1: shared=all

                mkdir sub &&
                cd sub &&
                git init --shared=all &&
                test 2 = $(git config core.sharedrepository)

* FAIL 2: update-server-info honors core.sharedRepository

                : > a1 &&
                git add a1 &&
                test_tick &&
                git commit -m a1 &&
                umask 0277 &&
                git update-server-info &&
                test 444 = $(stat -c %a .git/info/refs)

* failed 2 among 2 test(s)
make[1]: *** [t1301-shared-repo.sh] Error 1

This is on 1467b5fec376b6c43a5fa54bffd356e458a8d57c.

Oh... I notice "stat -c" there.  My stat doesn't know -c:

    localhost.local:~/MIRROR/git-GIT % stat -c %a /
    stat: illegal option -- c
    usage: stat [-FlLnqrsx] [-f format] [-t timefmt] [file ...]

Gotta stop using those non-portable options, I guess. :)

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: 1.5.3-rc5 regression on OS X?

[Junio C Hamano]

Does this help?

---
 builtin-read-tree.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 1967d10..5b07e9a 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -96,7 +96,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 {
 	int i, newfd, stage = 0;
 	unsigned char sha1[20];
-	struct tree_desc t[3];
+	struct tree_desc t[20];
 	struct unpack_trees_options opts;
 
 	memset(&opts, 0, sizeof(opts));
@@ -263,6 +263,9 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 			opts.head_idx = 1;
 	}
 
+	if (ARRAY_SIZE(t) < nr_trees)
+		die("I cannot read more than %d trees", ARRAY_SIZE(t));
+
 	for (i = 0; i < nr_trees; i++) {
 		struct tree *tree = trees[i];
 		parse_tree(tree);

[PATCH] Fix read-tree merging more than 3 trees using 3-way merge

[Junio C Hamano]

For multi-base merges, we allowed read-tree -m to take more than
three trees (the last two are our and their branches, and all the
earlier ones, typically one but potentially more, are used as the
merge base).  Unfortunately, the conversion done by commit 933bf40
broke this.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-read-tree.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 1967d10..5b07e9a 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -96,7 +96,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 {
 	int i, newfd, stage = 0;
 	unsigned char sha1[20];
-	struct tree_desc t[3];
+	struct tree_desc t[20];
 	struct unpack_trees_options opts;
 
 	memset(&opts, 0, sizeof(opts));
@@ -263,6 +263,9 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 			opts.head_idx = 1;
 	}
 
+	if (ARRAY_SIZE(t) < nr_trees)
+		die("I cannot read more than %d trees", ARRAY_SIZE(t));
+
 	for (i = 0; i < nr_trees; i++) {
 		struct tree *tree = trees[i];
 		parse_tree(tree);
-- 
1.5.3.rc5

Re: [PATCH] Fix read-tree merging more than 3 trees using 3-way merge

[Linus Torvalds]

On Wed, 15 Aug 2007, Junio C Hamano wrote:
>
> For multi-base merges, we allowed read-tree -m to take more than
> three trees (the last two are our and their branches, and all the
> earlier ones, typically one but potentially more, are used as the
> merge base).  Unfortunately, the conversion done by commit 933bf40
> broke this.

Sorry. I didn't even realize people did this. Oops.

I note that the thign you committed limited the number of trees to 4. 
May I suggest this patch in addition to your patch? Also, is 4 actually 
the real maximum? Maybe it should be raised?

What *is* the behaviour for four trees? Reading "unpack_trees()" source 
implies that all other trees go into stage 1, where the first matching 
entry is used?

		Linus

---
 builtin-read-tree.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index f6764b9..d5f849b 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -21,7 +21,7 @@ static int list_tree(unsigned char *sha1)
 {
 	struct tree *tree;
 
-	if (nr_trees >= 4)
+	if (nr_trees >= MAX_TREES)
 		return -1;
 	tree = parse_tree_indirect(sha1);
 	if (!tree)

Re: [PATCH] Fix read-tree merging more than 3 trees using 3-way merge

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Sorry. I didn't even realize people did this. Oops.

This is no big deal.  It would have triggered if you are using
resolve strategy _and_ did a multi merge-base merge, the former
of which is quite unusual these days.

> What *is* the behaviour for four trees? Reading "unpack_trees()" source 
> implies that all other trees go into stage 1, where the first matching 
> entry is used?

Documentation/technical/trivial-merge.txt describes this under
three-way case.  Essentially this was an attempt to extend the
original merge-resolve so that it would catch problematic
criss-cross cases (case #16 in the description).

Re: [PATCH] Fix read-tree merging more than 3 trees using 3-way merge

[Linus Torvalds]

On Thu, 16 Aug 2007, Junio C Hamano wrote:
>
> Linus Torvalds <torvalds@linux-foundation.org> writes:
> > 
> > Sorry. I didn't even realize people did this. Oops.
> 
> This is no big deal.  It would have triggered if you are using
> resolve strategy _and_ did a multi merge-base merge, the former
> of which is quite unusual these days.

Yeah, ok. I see what's up. 

Anyway, it sounds like we should just increase MAX_TREES to get the old 
behaviour (and that requires applying my trivial one-liner patch too, 
otherwise "git read-tree" will refuse to do more than 4 trees anyway).

Even if nothing apparently actually does it.

The alternative, I think, would be to just say that the three-way merge is 
really just three ways, and remove the test that tests four different 
trees. That would allow simplifying the logic in unpack_trees(). But since 
the logic already exists, there's no real upside.

		Linus

Re: [PATCH] Fix read-tree merging more than 3 trees using 3-way merge

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Thu, 16 Aug 2007, Junio C Hamano wrote:
>>
>> Linus Torvalds <torvalds@linux-foundation.org> writes:
>> > 
>> > Sorry. I didn't even realize people did this. Oops.
>> 
>> This is no big deal.  It would have triggered if you are using
>> resolve strategy _and_ did a multi merge-base merge, the former
>> of which is quite unusual these days.
>
> Yeah, ok. I see what's up. 

So no worries.  It was triggered by an error in t1000 test.

-- >8 --
[PATCH] Clean-up read-tree error condition.

This is a follow-up to f34f2b0b; list_tree() function is where it
first notices that the command line fed too many trees for us to
handle, so move the error exit message to there, and raise the
MAX_TREES to 8 (not that it matters very much in practice).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-read-tree.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index f6764b9..43cd56a 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -13,7 +13,7 @@
 #include "dir.h"
 #include "builtin.h"
 
-#define MAX_TREES 4
+#define MAX_TREES 8
 static int nr_trees;
 static struct tree *trees[MAX_TREES];
 
@@ -21,8 +21,8 @@ static int list_tree(unsigned char *sha1)
 {
 	struct tree *tree;
 
-	if (nr_trees >= 4)
-		return -1;
+	if (nr_trees >= MAX_TREES)
+		die("I cannot read more than %d trees", MAX_TREES);
 	tree = parse_tree_indirect(sha1);
 	if (!tree)
 		return -1;
@@ -264,9 +264,6 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 			opts.head_idx = 1;
 	}
 
-	if (MAX_TREES < nr_trees)
-		die("I cannot read more than %d trees", MAX_TREES);
-
 	for (i = 0; i < nr_trees; i++) {
 		struct tree *tree = trees[i];
 		parse_tree(tree);
-- 
1.5.3.rc5

Re: [PATCH] Fix read-tree merging more than 3 trees using 3-way merge

[Linus Torvalds]

On Thu, 16 Aug 2007, Junio C Hamano wrote:
>
> [PATCH] Clean-up read-tree error condition.
> 
> This is a follow-up to f34f2b0b; list_tree() function is where it
> first notices that the command line fed too many trees for us to
> handle, so move the error exit message to there, and raise the
> MAX_TREES to 8 (not that it matters very much in practice).
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

ACK. Obviously.

		Linus