* [PATCH] Fix rev-list when showing objects involving submodules @ 2007-11-11 23:35 Johannes Schindelin 2007-11-12 19:54 ` Sam Vilain 0 siblings, 1 reply; 5+ messages in thread From: Johannes Schindelin @ 2007-11-11 23:35 UTC (permalink / raw) To: git, gitster The function mark_tree_uninteresting() assumed that the tree entries are blob when they are not trees. This is not so. Since we do not traverse into submodules (yet), the gitlinks should be ignored. Noticed by Ilari on IRC. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- revision.c | 2 +- t/t6008-rev-list-submodule.sh | 42 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletions(-) create mode 100755 t/t6008-rev-list-submodule.sh diff --git a/revision.c b/revision.c index 931f978..81b5a93 100644 --- a/revision.c +++ b/revision.c @@ -69,7 +69,7 @@ void mark_tree_uninteresting(struct tree *tree) while (tree_entry(&desc, &entry)) { if (S_ISDIR(entry.mode)) mark_tree_uninteresting(lookup_tree(entry.sha1)); - else + else if (!S_ISGITLINK(entry.mode)) mark_blob_uninteresting(lookup_blob(entry.sha1)); } diff --git a/t/t6008-rev-list-submodule.sh b/t/t6008-rev-list-submodule.sh new file mode 100755 index 0000000..88e96fb --- /dev/null +++ b/t/t6008-rev-list-submodule.sh @@ -0,0 +1,42 @@ +#!/bin/sh +# +# Copyright (c) 2007 Johannes E. Schindelin +# + +test_description='git rev-list involving submodules that this repo has' + +. ./test-lib.sh + +test_expect_success 'setup' ' + : > file && + git add file && + test_tick && + git commit -m initial && + echo 1 > file && + test_tick && + git commit -m second file && + echo 2 > file && + test_tick && + git commit -m third file && + + rm .git/index && + + : > super-file && + git add super-file && + git submodule add . sub && + git symbolic-ref HEAD refs/heads/super && + test_tick && + git commit -m super-initial && + echo 1 > super-file && + test_tick && + git commit -m super-first super-file && + echo 2 > super-file && + test_tick && + git commit -m super-second super-file +' + +test_expect_success "Ilari's test" ' + git rev-list --objects super master ^super^ +' + +test_done -- 1.5.3.5.1695.g146d07 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rev-list when showing objects involving submodules 2007-11-11 23:35 [PATCH] Fix rev-list when showing objects involving submodules Johannes Schindelin @ 2007-11-12 19:54 ` Sam Vilain 2007-11-12 20:21 ` Johannes Schindelin 0 siblings, 1 reply; 5+ messages in thread From: Sam Vilain @ 2007-11-12 19:54 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, gitster Johannes Schindelin wrote: > The function mark_tree_uninteresting() assumed that the tree entries > are blob when they are not trees. This is not so. Since we do > not traverse into submodules (yet), the gitlinks should be ignored. > > diff --git a/revision.c b/revision.c > index 931f978..81b5a93 100644 > --- a/revision.c > +++ b/revision.c > @@ -69,7 +69,7 @@ void mark_tree_uninteresting(struct tree *tree) > while (tree_entry(&desc, &entry)) { > if (S_ISDIR(entry.mode)) > mark_tree_uninteresting(lookup_tree(entry.sha1)); > - else > + else if (!S_ISGITLINK(entry.mode)) > mark_blob_uninteresting(lookup_blob(entry.sha1)); > } > > Wouldn't it be better to check for what it is, rather than what it is not? Sam. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rev-list when showing objects involving submodules 2007-11-12 19:54 ` Sam Vilain @ 2007-11-12 20:21 ` Johannes Schindelin 2007-11-12 20:43 ` Linus Torvalds 0 siblings, 1 reply; 5+ messages in thread From: Johannes Schindelin @ 2007-11-12 20:21 UTC (permalink / raw) To: Sam Vilain; +Cc: git, gitster Hi, On Tue, 13 Nov 2007, Sam Vilain wrote: > Johannes Schindelin wrote: > > The function mark_tree_uninteresting() assumed that the tree entries > > are blob when they are not trees. This is not so. Since we do > > not traverse into submodules (yet), the gitlinks should be ignored. > > > > diff --git a/revision.c b/revision.c > > index 931f978..81b5a93 100644 > > --- a/revision.c > > +++ b/revision.c > > @@ -69,7 +69,7 @@ void mark_tree_uninteresting(struct tree *tree) > > while (tree_entry(&desc, &entry)) { > > if (S_ISDIR(entry.mode)) > > mark_tree_uninteresting(lookup_tree(entry.sha1)); > > - else > > + else if (!S_ISGITLINK(entry.mode)) > > mark_blob_uninteresting(lookup_blob(entry.sha1)); > > } > > > > > > Wouldn't it be better to check for what it is, rather than what it is not? You mean something like else if (S_ISREG(entry.mod) || S_ISLNK(entry.mod)) Hmm? Sure, I have no preference there. Ciao, Dscho ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rev-list when showing objects involving submodules 2007-11-12 20:21 ` Johannes Schindelin @ 2007-11-12 20:43 ` Linus Torvalds 2007-11-12 21:25 ` Johannes Schindelin 0 siblings, 1 reply; 5+ messages in thread From: Linus Torvalds @ 2007-11-12 20:43 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Sam Vilain, git, gitster On Mon, 12 Nov 2007, Johannes Schindelin wrote: > > You mean something like > > else if (S_ISREG(entry.mod) || S_ISLNK(entry.mod)) > > Hmm? Sure, I have no preference there. Maybe more along the line of something like this? In general, I suspect we should try to start moving away from using the "S_ISLNK()" like things for internal git state. It was a mistake to just assume the numbers all were same across all systems in the first place. So this just converts to the "object_type", and then uses a case statement. Linus --- builtin-pack-objects.c | 2 +- revision.c | 11 +++++++++-- tree-walk.h | 7 +++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 545ece5..4f44658 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -990,7 +990,7 @@ static void add_pbase_object(struct tree_desc *tree, return; if (name[cmplen] != '/') { add_object_entry(entry.sha1, - S_ISDIR(entry.mode) ? OBJ_TREE : OBJ_BLOB, + object_type(entry.mode), fullname, 1); return; } diff --git a/revision.c b/revision.c index 931f978..c054a82 100644 --- a/revision.c +++ b/revision.c @@ -67,10 +67,17 @@ void mark_tree_uninteresting(struct tree *tree) init_tree_desc(&desc, tree->buffer, tree->size); while (tree_entry(&desc, &entry)) { - if (S_ISDIR(entry.mode)) + switch (object_type(entry.mode)) { + case OBJ_TREE: mark_tree_uninteresting(lookup_tree(entry.sha1)); - else + break; + case OBJ_BLOB: mark_blob_uninteresting(lookup_blob(entry.sha1)); + break; + default: + /* Subproject commit - not in this repository */ + break; + } } /* diff --git a/tree-walk.h b/tree-walk.h index db0fbdc..903a7b0 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -7,6 +7,13 @@ struct name_entry { unsigned int mode; }; +static inline enum object_type object_type(unsigned int mode) +{ + return S_ISDIR(mode) ? OBJ_TREE : + S_ISGITLINK(mode) ? OBJ_COMMIT : + OBJ_BLOB; +} + struct tree_desc { const void *buffer; struct name_entry entry; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix rev-list when showing objects involving submodules 2007-11-12 20:43 ` Linus Torvalds @ 2007-11-12 21:25 ` Johannes Schindelin 0 siblings, 0 replies; 5+ messages in thread From: Johannes Schindelin @ 2007-11-12 21:25 UTC (permalink / raw) To: Linus Torvalds; +Cc: Sam Vilain, git, gitster Hi, On Mon, 12 Nov 2007, Linus Torvalds wrote: > On Mon, 12 Nov 2007, Johannes Schindelin wrote: > > > > You mean something like > > > > else if (S_ISREG(entry.mod) || S_ISLNK(entry.mod)) > > > > Hmm? Sure, I have no preference there. > > Maybe more along the line of something like this? > > In general, I suspect we should try to start moving away from using the > "S_ISLNK()" like things for internal git state. It was a mistake to just > assume the numbers all were same across all systems in the first place. > > So this just converts to the "object_type", and then uses a case > statement. Sure. Just add the test from my patch, and we're fixed. Ciao, Dscho ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-11-12 21:26 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-11-11 23:35 [PATCH] Fix rev-list when showing objects involving submodules Johannes Schindelin 2007-11-12 19:54 ` Sam Vilain 2007-11-12 20:21 ` Johannes Schindelin 2007-11-12 20:43 ` Linus Torvalds 2007-11-12 21:25 ` Johannes Schindelin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).