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?

[Perrin Meyer]

Nothing has changed,, I just did a git checkout v1.5.2.4; make clean; make ; make test, and that same test passes. Then I did a git checkout v1.5.3-rc5 ; make clean ; make ; make test, and the same test fails. 

I'll try updating to a newer version of Xcode and see if that helps. 

Perrin


----- Original Message ----
From: Junio C Hamano <gitster@pobox.com>
To: Perrin Meyer <perrinmeyer@yahoo.com>
Cc: git@vger.kernel.org
Sent: Wednesday, August 15, 2007 2:33:04 PM
Subject: Re: 1.5.3-rc5 regression on OS X?

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?

[Junio C Hamano]

Perrin Meyer <perrinmeyer@yahoo.com> writes:

> Nothing has changed,, I just did a git checkout v1.5.2.4; make clean; make ; make test, and that same test passes. Then I did a git checkout v1.5.3-rc5 ; make clean ; make ; make test, and the same test fails. 
>
> I'll try updating to a newer version of Xcode and see if that helps. 

Before doing so, perhaps bisect between v1.5.2.4 and v1.5.3-rc5
woudl be very helpful.

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: 1.5.3-rc5 regression on OS X?

[Perrin Meyer]

Well, this is my first time using git-bisect (which seems like a VERY cool tool), but iterating through, using 
$ make clean ; make -j 4 ; cd t ; sh t1000-read-tree-m-3way.sh ; cd ..

The first bad commit is:

933bf40a5c6328b6c022b636f45a6f2c48c3838e is first bad commit
commit 933bf40a5c6328b6c022b636f45a6f2c48c3838e
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Thu Aug 9 22:21:29 2007 -0700

    Start moving unpack-trees to "struct tree_desc"
    
    This doesn't actually change any real code, but it changes the interface
    to unpack_trees() to take an array of "struct tree_desc" entries, the same
    way the tree-walk.c functions do.
    
    The reason for this is that we would be much better off if we can do the
    tree-unpacking using the generic "traverse_trees()" functionality instead
    of having to the special "unpack" infrastructure.
    
    This really is a pretty minimal diff, just to change the calling
    convention. It passes all the tests, and looks sane. There were only two
    users of "unpack_trees()": builtin-read-tree and merge-recursive, and I
    tried to keep the changes minimal.
    
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

:100644 100644 a3b17a3bd913d628d70ee625ec8b1808d3b200fa 1967d100f28d70e588fde618aa7ed4a10b76ee1d M      builtin-read-tree.c
:100644 100644 c8539ec0bafce5e238d97d6397a9d08c2c92a09c f7d1b84999d401887e4a4eba091d53f5fe7294c8 M      merge-recursive.c
:100644 100644 dfd985b0ef0193ce311ea77e8c8cb11fc7b1c3e3 5d1ffd1a32a56bdcf4f64b545abd6aa951914ffe M      unpack-trees.c
:100644 100644 fee7da43822b63e5b1f24444e5c51c43d3ff5760 9cd39a28a907ef6f0eedc764bbae586353ec2ca5 M      unpack-trees.h


I'll install the newest Xcode and see if that fixes things. 

Perrin

----- Original Message ----
From: Junio C Hamano <gitster@pobox.com>
To: Perrin Meyer <perrinmeyer@yahoo.com>
Cc: git@vger.kernel.org
Sent: Wednesday, August 15, 2007 3:05:18 PM
Subject: Re: 1.5.3-rc5 regression on OS X?

Perrin Meyer <perrinmeyer@yahoo.com> writes:

> Nothing has changed,, I just did a git checkout v1.5.2.4; make clean; make ; make test, and that same test passes. Then I did a git checkout v1.5.3-rc5 ; make clean ; make ; make test, and the same test fails. 
>
> I'll try updating to a newer version of Xcode and see if that helps. 

Before doing so, perhaps bisect between v1.5.2.4 and v1.5.3-rc5
woudl be very helpful.

Re: 1.5.3-rc5 regression on OS X?

[Junio C Hamano]

Perrin Meyer <perrinmeyer@yahoo.com> writes:

> Well, this is my first time using git-bisect (which seems like a VERY cool tool), but iterating through, using 
> $ make clean ; make -j 4 ; cd t ; sh t1000-read-tree-m-3way.sh ; cd ..
>
> The first bad commit is:
>
> 933bf40a5c6328b6c022b636f45a6f2c48c3838e is first bad commit
> ...
> I'll install the newest Xcode and see if that fixes things. 

Thanks, but before doing so could you try the patch I just sent
out?

Re: 1.5.3-rc5 regression on OS X?

[Perrin Meyer]

Yes, this patch fixes the error I was seeing in t1000-read-tree-m-3way.sh

Thanks,

Perrin


----- Original Message ----
From: Junio C Hamano <gitster@pobox.com>
To: Perrin Meyer <perrinmeyer@yahoo.com>
Cc: git@vger.kernel.org
Sent: Wednesday, August 15, 2007 3:59:17 PM
Subject: Re: 1.5.3-rc5 regression on OS X?

Perrin Meyer <perrinmeyer@yahoo.com> writes:

> Well, this is my first time using git-bisect (which seems like a VERY cool tool), but iterating through, using 
> $ make clean ; make -j 4 ; cd t ; sh t1000-read-tree-m-3way.sh ; cd ..
>
> The first bad commit is:
>
> 933bf40a5c6328b6c022b636f45a6f2c48c3838e is first bad commit
> ...
> I'll install the newest Xcode and see if that fixes things. 

Thanks, but before doing so could you try the patch I just sent
out?

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