Git development
 help / color / mirror / Atom feed
* [RFC] t5500 considered dangerous
From: Johannes Schindelin @ 2005-12-26 23:35 UTC (permalink / raw)
  To: git


'corrupt a pack and see if verify catches' is bound to fail sometimes for 
non-obvious reasons:

The test case corrupts a pack, and then also the index of that pack, by 
writing "0" at certain offsets, and then tests if git-verify-pack fails.

However, said pack contains a commit, which is variable by virtue of 
storing two dates. Therefore, the deflated objects are no longer 
deterministic, and neither the pack. What happens is that in roughly every 
256th run, there *is* a 0 at one of these offsets, and the test fails.

This patch makes the test deterministic by skipping the commit.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	I just hunted down a bug which showed up very rarely, and it did 
	not make the hunting easier.

 t/t5300-pack-object.sh |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index df6549c..d6b6697 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -20,12 +20,11 @@ test_expect_success \
      done &&
      cat c >d && echo foo >>d && git-update-index --add d &&
      tree=`git-write-tree` &&
-     commit=`git-commit-tree $tree </dev/null` && {
+     {
 	 echo $tree &&
-	 echo $commit &&
 	 git-ls-tree $tree | sed -e "s/.* \\([0-9a-f]*\\)	.*/\\1/"
      } >obj-list && {
-	 git-diff-tree --root -p $commit &&
+	 git-diff-tree --root -p $tree &&
 	 while read object
 	 do
 	    t=`git-cat-file -t $object` &&
@@ -99,7 +98,7 @@ test_expect_success \
      GIT_OBJECT_DIRECTORY=.git2/objects &&
      export GIT_OBJECT_DIRECTORY &&
      cp test-1-${packname_1}.pack test-1-${packname_1}.idx .git2/objects/pack && {
-	 git-diff-tree --root -p $commit &&
+	 git-diff-tree --root -p $tree &&
 	 while read object
 	 do
 	    t=`git-cat-file -t $object` &&
@@ -114,7 +113,7 @@ test_expect_success \
      export GIT_OBJECT_DIRECTORY &&
      rm -f .git2/objects/pack/test-?.idx &&
      cp test-2-${packname_2}.pack test-2-${packname_2}.idx .git2/objects/pack && {
-	 git-diff-tree --root -p $commit &&
+	 git-diff-tree --root -p $tree &&
 	 while read object
 	 do
 	    t=`git-cat-file -t $object` &&

^ permalink raw reply related

* Re: [RFC] t5500 considered dangerous
From: Johannes Schindelin @ 2005-12-26 23:37 UTC (permalink / raw)
  To: git
In-Reply-To: <Pine.LNX.4.63.0512270018310.3067@wbgn013.biozentrum.uni-wuerzburg.de>

Of course, it is t5300. Sorry for the noise.

^ permalink raw reply

* [PATCH] Do not allow git-init-db to be called twice in the same directory
From: Johannes Schindelin @ 2005-12-26 23:54 UTC (permalink / raw)
  To: git, junkio


t5300 is also adapted, since it relies heavily on init-db not complaining 
when $GIT_DIR already exists.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 init-db.c              |    3 ++-
 t/t5300-pack-object.sh |   16 ++++++----------
 2 files changed, 8 insertions(+), 11 deletions(-)

199557e282c6b7600fdb9f7ce8b203b1d634de8f
diff --git a/init-db.c b/init-db.c
index ff29496..1cb015e 100644
--- a/init-db.c
+++ b/init-db.c
@@ -259,7 +259,8 @@ int main(int argc, char **argv)
 		git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
 		fprintf(stderr, "defaulting to local storage area\n");
 	}
-	safe_create_dir(git_dir, 0);
+	if (mkdir(git_dir, 0777) < 0)
+		die("Cannot initialize repository twice!");
 
 	/* Check to see if the repository version is right.
 	 * Note that a newly created repository does not have
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 7dfb1ab..df6549c 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -38,13 +38,12 @@ test_expect_success \
     'packname_1=$(git-pack-objects --window=0 test-1 <obj-list)'
 
 rm -fr .git2
-mkdir .git2
 
 test_expect_success \
     'unpack without delta' \
-    "GIT_OBJECT_DIRECTORY=.git2/objects &&
+    "GIT_DIR=.git2 git-init-db &&
+     GIT_OBJECT_DIRECTORY=.git2/objects &&
      export GIT_OBJECT_DIRECTORY &&
-     git-init-db &&
      git-unpack-objects -n <test-1-${packname_1}.pack &&
      git-unpack-objects <test-1-${packname_1}.pack"
 
@@ -69,13 +68,12 @@ test_expect_success \
      packname_2=$(git-pack-objects test-2 <obj-list)'
 
 rm -fr .git2
-mkdir .git2
 
 test_expect_success \
     'unpack with delta' \
-    'GIT_OBJECT_DIRECTORY=.git2/objects &&
+    'GIT_DIR=.git2 git-init-db &&
+     GIT_OBJECT_DIRECTORY=.git2/objects &&
      export GIT_OBJECT_DIRECTORY &&
-     git-init-db &&
      git-unpack-objects -n <test-2-${packname_2}.pack &&
      git-unpack-objects <test-2-${packname_2}.pack'
 
@@ -94,13 +92,12 @@ test_expect_success \
 cd "$TRASH"
 
 rm -fr .git2
-mkdir .git2
 
 test_expect_success \
     'use packed objects' \
-    'GIT_OBJECT_DIRECTORY=.git2/objects &&
+    'GIT_DIR=.git2 git-init-db &&
+     GIT_OBJECT_DIRECTORY=.git2/objects &&
      export GIT_OBJECT_DIRECTORY &&
-     git-init-db &&
      cp test-1-${packname_1}.pack test-1-${packname_1}.idx .git2/objects/pack && {
 	 git-diff-tree --root -p $commit &&
 	 while read object
@@ -111,7 +108,6 @@ test_expect_success \
     } >current &&
     diff expect current'
 
-
 test_expect_success \
     'use packed deltified objects' \
     'GIT_OBJECT_DIRECTORY=.git2/objects &&
-- 
1.0.GIT

^ permalink raw reply related

* Re: [PATCH] Do not allow git-init-db to be called twice in the same directory
From: Junio C Hamano @ 2005-12-27  1:18 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, junkio
In-Reply-To: <Pine.LNX.4.63.0512270053030.14928@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> t5300 is also adapted, since it relies heavily on init-db not complaining 
> when $GIT_DIR already exists.

I think it tries not to overwrite things that are already there
and is meant to be safe to run in an already populated
repository; e.g. you should be able to use it to pick up newly
added templates.

Is there a particular reason running the current init-db twice
is undesirable?

^ permalink raw reply

* Re: [RFC] t5300 considered dangerous
From: Junio C Hamano @ 2005-12-27  2:10 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0512270018310.3067@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> The test case corrupts a pack, and then also the index of that pack, by 
> writing "0" at certain offsets, and then tests if git-verify-pack fails.

These certain offsets are:

 . byte    2 in pack
   This corrupts PACK_SIGNATURE ("A" in "PACK").

 . byte    7 in pack
   This corrupts PACK_VERSION ("\002" in "\0\0\0\002").

 . byte   12 in pack
   This corrupts the type/size byte of whatever the first object
   happens to be (the type/size byte has non-zero upper nibble).

 . byte 1200 in idx that is currently 1208 bytes long.
   This tries to corrupt the checksum of the index file itself,

You are right about the last one; this location has 1/256 chance
of validly being 0, and making the set of objects to be packed
constant is one way to fix it.  More appropriate way would be to
measure how long the .idx file is, and corrupt the last 20 bytes;
I was too lazy.

-- >8 --

diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 7dfb1ab..dd719bb 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -163,8 +163,10 @@ test_expect_success \
      else :;
      fi &&
 
+     l=`wc -c <test-3.idx` &&
+     l=`expr "$l" - 20` &&
      cp test-1-${packname_1}.pack test-3.pack &&
-     dd if=/dev/zero of=test-3.idx count=1 bs=1 conv=notrunc seek=1200 &&
+     dd if=/dev/zero of=test-3.idx count=20 bs=1 conv=notrunc seek=$l &&
      if git-verify-pack test-3.pack
      then false
      else :;

^ permalink raw reply related

* Re: [PATCH] Handle symlinks graciously
From: Junio C Hamano @ 2005-12-27  2:33 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0512262231350.21076@wbgn013.biozentrum.uni-wuerzburg.de>

Good catch; thanks.

^ permalink raw reply

* [ANNOUNCE] GIT 1.0.5
From: Junio C Hamano @ 2005-12-27  8:47 UTC (permalink / raw)
  To: git, linux-kernel

This is a minor bugfix release.  There is one real fix by
Johannes that would help people who manage symlinks with git.

Starting from this one I won't be touching debian/ directory,
nor supplying pre-built Debian binaries anymore.
The official maintainer seems to be reasonably quick to package
up things.  The packaging procedure used there seems to be quite
different from what I have, so I'd like to avoid potential
confusion and reduce work by the official maintainer and myself.

Also I've enhanced the post-update hook of git.git repository to
automatically prepare preformatted documentation, which I'll
cover in a separate message.

^ permalink raw reply

* [ANNOUNCE] GIT preformatted documentation available.
From: Junio C Hamano @ 2005-12-27  8:49 UTC (permalink / raw)
  To: git, linux-kernel; +Cc: Willy Tarreau

I was asked to provide pre-formatted man pages (and perhaps html
pages), since the time of kernel developers are better spent on
what they do best, rather than preparing the xmlto toolchain.

I was planning to do a tarball every time I do a "release", but
that would mean it is no better than the current way --- you
could extract manpages out of rpm anyway.

So instead, I'll do independent branches "html" and "man" in
git.git repository to keep the preformatted documentation.

	$ mkdir git.man ; cd git.man
	$ git init-db
        $ ID=$(git fetch-pack -k git://git.kernel.org/pub/scm/git/git man)
        $ expr "$ID" : '\(.*\) .*' >.git/refs/heads/master
        $ cat .git/refs/heads/master >.git/refs/heads/origin
	$ git checkout; /bin/ls -aF
	./  ../  .git/	man1/  man7/
        $ echo >.git/remotes/origin <<\EOF
        URL: git://git.kernel.org/pub/scm/git/git
        Pull: man:origin
        EOF

Would set you up, and you can
to keep them up-to-date, and install them like so:

	$ cd git.man
	$ git pull
	$ cp -a man1 man7 /usr/local/man/

The "html" branch is similar; it is a copy of what is shown at
http://www.kernel.org/pub/software/scm/git/docs/.

This means if you clone from git.git repository, you would end
up with something like this in .git/remotes/origin file:

	URL: git://git.kernel.org/pub/scm/git/git.git
        Pull: master:origin
        Pull: todo:todo
        Pull: html:html
        Pull: man:man
        Pull: pu:pu
        Pull: maint:maint

Typically, for the repository to track GIT itself, you would
want to trim them like this:

	URL: git://git.kernel.org/pub/scm/git/git.git
        Pull: master:origin
        Pull: maint:maint
        Pull: +pu:pu

^ permalink raw reply

* [PATCH] copy_fd: close ifd on error
From: Sam Ravnborg @ 2005-12-27  8:19 UTC (permalink / raw)
  To: git

In copy_fd when write fails we ought to close input file descriptor.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---

Found while looking at the source for inspiration for something else.
I did not see any error related to this.

	Sam

diff --git a/copy.c b/copy.c
index 7100eed..08a3d38 100644
--- a/copy.c
+++ b/copy.c
@@ -22,11 +22,14 @@ int copy_fd(int ifd, int ofd)
 				buf += written;
 				len -= written;
 			}
-			else if (!written)
+			else if (!written) {
+				close(ifd);
 				return error("copy-fd: write returned 0");
-			else
+			} else {
+				close(ifd);
 				return error("copy-fd: write returned %s",
 					     strerror(errno));
+			}
 		}
 	}
 	close(ifd);

^ permalink raw reply related

* Re: [PATCH] copy_fd: close ifd on error
From: Junio C Hamano @ 2005-12-27  9:02 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: git
In-Reply-To: <20051227081905.GA9946@mars.ravnborg.org>

Sam Ravnborg <sam@ravnborg.org> writes:

> In copy_fd when write fails we ought to close input file descriptor.
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>
> Found while looking at the source for inspiration for something else.
> I did not see any error related to this.

Thanks.  I think the only place this matters is git-local-fetch
which nobody uses these days where it can leak handful ifd.
All other users immediately exit upon seeing failure return from
the function.

Of course this would matter much when we do libification.

^ permalink raw reply

* Re: [RFC] t5300 considered dangerous
From: Johannes Schindelin @ 2005-12-27 13:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd5jjffxu.fsf@assigned-by-dhcp.cox.net>

Hi,

On Mon, 26 Dec 2005, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > The test case corrupts a pack, and then also the index of that pack, by 
> > writing "0" at certain offsets, and then tests if git-verify-pack fails.
> 
> These certain offsets are:
> 
>  . byte    2 in pack
>    This corrupts PACK_SIGNATURE ("A" in "PACK").
> 
>  . byte    7 in pack
>    This corrupts PACK_VERSION ("\002" in "\0\0\0\002").
> 
>  . byte   12 in pack
>    This corrupts the type/size byte of whatever the first object
>    happens to be (the type/size byte has non-zero upper nibble).
> 
>  . byte 1200 in idx that is currently 1208 bytes long.
>    This tries to corrupt the checksum of the index file itself,

Thank you for clarifying this.

> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
> index 7dfb1ab..dd719bb 100755
> --- a/t/t5300-pack-object.sh
> +++ b/t/t5300-pack-object.sh
> @@ -163,8 +163,10 @@ test_expect_success \
>       else :;
>       fi &&
>  
> +     l=`wc -c <test-3.idx` &&
> +     l=`expr "$l" - 20` &&

This needs to be

> +     l=`expr $l - 20` &&

since there are textutils out there (notably version 2.1) where "wc -c" 
prints a leading space.

>       cp test-1-${packname_1}.pack test-3.pack &&
> -     dd if=/dev/zero of=test-3.idx count=1 bs=1 conv=notrunc seek=1200 &&
> +     dd if=/dev/zero of=test-3.idx count=20 bs=1 conv=notrunc seek=$l &&
>       if git-verify-pack test-3.pack
>       then false
>       else :;

When the commit is not skipped, I fail to see why this should not fail in 
one out of 256 cases: the input is (partially) pseudo-random. The last 20 
bytes are the SHA-1 which should inherit this pseudo-randomness. So, the 
first byte should be pseudo-random, too.

Something I missed?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Do not allow git-init-db to be called twice in the same directory
From: Johannes Schindelin @ 2005-12-27 14:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbqz3ibhn.fsf@assigned-by-dhcp.cox.net>

Hi,

On Mon, 26 Dec 2005, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > t5300 is also adapted, since it relies heavily on init-db not complaining 
> > when $GIT_DIR already exists.
> 
> I think it tries not to overwrite things that are already there
> and is meant to be safe to run in an already populated
> repository; e.g. you should be able to use it to pick up newly
> added templates.

Ah, that is a use case I missed. I don't work too much with templates 
(only on the shared repositories' machine)...

> Is there a particular reason running the current init-db twice
> is undesirable?

In my case, yes. When I do that, it means that I did not correctly 
change directories. I fscked up my private git repository twice that way.

I like your point, though.

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC] t5300 considered dangerous
From: Johannes Schindelin @ 2005-12-27 14:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0512271452470.17086@wbgn013.biozentrum.uni-wuerzburg.de>

Hi,

On Tue, 27 Dec 2005, Johannes Schindelin wrote:

> On Mon, 26 Dec 2005, Junio C Hamano wrote:
> 
> >       cp test-1-${packname_1}.pack test-3.pack &&
> > -     dd if=/dev/zero of=test-3.idx count=1 bs=1 conv=notrunc seek=1200 &&
> > +     dd if=/dev/zero of=test-3.idx count=20 bs=1 conv=notrunc seek=$l &&
> >       if git-verify-pack test-3.pack
> >       then false
> >       else :;
> 
> When the commit is not skipped, I fail to see why this should not fail in 
> one out of 256 cases: the input is (partially) pseudo-random. The last 20 
> bytes are the SHA-1 which should inherit this pseudo-randomness. So, the 
> first byte should be pseudo-random, too.
> 
> Something I missed?

Yes, I missed the "count=20".

Sorry,
Dscho

^ permalink raw reply

* cannot handle more than 29 refs
From: Jon Nelson @ 2005-12-27 14:43 UTC (permalink / raw)
  To: git


I get this message whenever I use --tags with git-pull with certain 
repositories. The git repository is one, and so is my local repository. 
I googled and got nothing. I grepped the git archive and got only 
show-branch.c.  Can this be safely ignored? Is it a known shortcoming, 
or something else entirely?

--
Jon Nelson <jnelson-git@jamponi.net>

^ permalink raw reply

* Re: [PATCH] Do not allow git-init-db to be called twice in the same directory
From: Junio C Hamano @ 2005-12-27 16:47 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0512271457200.17086@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Is there a particular reason running the current init-db twice
>> is undesirable?
>
> In my case, yes. When I do that, it means that I did not correctly 
> change directories. I fscked up my private git repository twice that way.

Maybe a warning (e.g. "rerunning in an already set up
repository") is in order?

^ permalink raw reply

* Re: cannot handle more than 29 refs
From: Junio C Hamano @ 2005-12-27 16:48 UTC (permalink / raw)
  To: Jon Nelson; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0512270840410.6812@gheavc.wnzcbav.cig>

Known shortcoming.

^ permalink raw reply

* Re: [PATCH] Do not allow git-init-db to be called twice in the same directory
From: Johannes Schindelin @ 2005-12-27 18:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlky6bi6f.fsf@assigned-by-dhcp.cox.net>

Hi,

On Tue, 27 Dec 2005, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Is there a particular reason running the current init-db twice
> >> is undesirable?
> >
> > In my case, yes. When I do that, it means that I did not correctly 
> > change directories. I fscked up my private git repository twice that way.
> 
> Maybe a warning (e.g. "rerunning in an already set up
> repository") is in order?

Yeah. Or a command line parameter "--exists" without which it fails?

Ciao,
Dscho

^ permalink raw reply

* Re: cannot handle more than 29 refs
From: Linus Torvalds @ 2005-12-27 19:18 UTC (permalink / raw)
  To: Jon Nelson; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0512270840410.6812@gheavc.wnzcbav.cig>



On Tue, 27 Dec 2005, Jon Nelson wrote:
> 
> I get this message whenever I use --tags with git-pull with certain 
> repositories.

You shouldn't "pull" tags. You should just "fetch" them.

The error message comes from the fact that the pull is a "fetch + merge", 
and the merge logic tries to figure out what the common parent is with 
"git-show-branch". And the common parent finding is limited to 29 commits.

(Which is a very reasonable limit: the normal case is 2, and doing an 
octopus-merge with more than four or five parents is already insane. Much 
less 30 parents).

> The git repository is one, and so is my local repository. 
> I googled and got nothing. I grepped the git archive and got only 
> show-branch.c.  Can this be safely ignored? Is it a known shortcoming, 
> or something else entirely?

Well, it's a known shortcoming, but it really ends up being a sign of you 
doing the wrong thing and trying to merge all the tags together. The merge 
would almost certainly fail, btw, regardless of anything else.

Junio - I think we should make "git pull" refuse to merge more than one 
head. If somebody wants to do an octopus merge, they can use "git merge" 
instead. Hmm?

		Linus

^ permalink raw reply

* Re: cannot handle more than 29 refs
From: Junio C Hamano @ 2005-12-27 19:46 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0512271113290.14098@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> Junio - I think we should make "git pull" refuse to merge more than one 
> head. If somebody wants to do an octopus merge, they can use "git merge" 
> instead. Hmm?

That would lose the autogenerated merge message.  I can be
talked into making it 'core.expert = true' only, though.

More troubling is 'git pull --tags' should *not* even try to use
them for merging, so if it does that that needs to be fixed.

As I recall how I did (wanted to do) it, the default rule for
'pull' is:

 - if you have explicit head names on the command line (--tags
   does not count -- you are not stating name explicitly),
   all of them are merged into an octopus;

 - otherwise if you used remotes/ shorthand the head(s) on the
   first Pull: line;

 - otherwise HEAD;

And the code _is_ broken.  Here is a proposed fix.

-- >8 --
[PATCH] Do not mark tags fetched via --tags flag as mergeable

Otherwise "git pull --tags" would mistakenly try to merge all of
them, which is never what the user wants.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/git-fetch.sh b/git-fetch.sh
index 767ca61..125bcea 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -192,7 +192,7 @@ then
 		sed -e '
 			/\^/d
 			s/^[^	]*	//
-			s/.*/&:&/')
+			s/.*/.&:&/')
 	if test "$#" -gt 1
 	then
 		# remote URL plus explicit refspecs; we need to merge them.

^ permalink raw reply related

* Re: cannot handle more than 29 refs
From: Jon Nelson @ 2005-12-27 20:49 UTC (permalink / raw)
  Cc: git
In-Reply-To: <Pine.LNX.4.64.0512271113290.14098@g5.osdl.org>

On Tue, 27 Dec 2005, Linus Torvalds wrote:

> 
> 
> On Tue, 27 Dec 2005, Jon Nelson wrote:
> > 
> > I get this message whenever I use --tags with git-pull with certain 
> > repositories.
> 
> You shouldn't "pull" tags. You should just "fetch" them.
> 
> The error message comes from the fact that the pull is a "fetch + merge", 
> and the merge logic tries to figure out what the common parent is with 
> "git-show-branch". And the common parent finding is limited to 29 commits.
> 
> (Which is a very reasonable limit: the normal case is 2, and doing an 
> octopus-merge with more than four or five parents is already insane. Much 
> less 30 parents).

^^^ Aha! I think I understand what's going on here - and you are right, 
I *don't* want to merge from all of those tags, I just want the content, 
so fetch is what I should be doing.  Thanks!

--
Jon Nelson <jnelson-git@jamponi.net>

^ permalink raw reply

* [PATCH 3/6] git-describe: --tags and --abbrev
From: Junio C Hamano @ 2005-12-28  0:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0512241409300.14098@g5.osdl.org>

With --tags, not just annontated tags, but also any ref under
refs/tags/ are used to name the revision.

The number of digits is configurable with the --abbrev=<n> option.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 describe.c |   33 +++++++++++++++++++++++++++------
 1 files changed, 27 insertions(+), 6 deletions(-)

970d400993d27a3228bccd72c0ddba767ebbbfff
diff --git a/describe.c b/describe.c
index ba49d00..ee38f5f 100644
--- a/describe.c
+++ b/describe.c
@@ -5,9 +5,14 @@
 
 #define SEEN (1u << 0)
 
-static const char describe_usage[] = "git-describe [--all] <committish>*";
+static const char describe_usage[] =
+"git-describe [--all] [--tags] [--abbrev=<n>] <committish>*";
 
 static int all = 0;	/* Default to annotated tags only */
+static int tags = 0;	/* But allow any tags if --tags is specified */
+
+#define DEFAULT_ABBREV 8 /* maybe too many */
+static int abbrev = DEFAULT_ABBREV;
 
 static int names = 0, allocs = 0;
 static struct commit_name {
@@ -50,13 +55,19 @@ static int get_name(const char *path, co
 	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
 	if (!commit)
 		return 0;
+	/* If --all, then any refs are used.
+	 * If --tags, then any tags are used.
+	 * Otherwise only annotated tags are used.
+	 */
 	if (!all) {
-		struct object *object;
 		if (strncmp(path, "refs/tags/", 10))
 			return 0;
-		object = parse_object(sha1);
-		if (object->type != tag_type)
-			return 0;
+		if (!tags) {
+			struct object *object;
+			object = parse_object(sha1);
+			if (object->type != tag_type)
+				return 0;
+		}
 	}
 	add_to_known_names(all ? path : path + 10, commit);
 	return 0;
@@ -96,7 +107,7 @@ static void describe(struct commit *cmit
 		n = match(c);
 		if (n) {
 			printf("%s-g%s\n", n->path,
-			       find_unique_abbrev(cmit->object.sha1, 8));
+			       find_unique_abbrev(cmit->object.sha1, abbrev));
 			return;
 		}
 	}
@@ -115,6 +126,16 @@ int main(int argc, char **argv)
 			all = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--tags")) {
+			tags = 1;
+			continue;
+		}
+		if (!strncmp(arg, "--abbrev=", 9)) {
+			abbrev = strtoul(arg + 9, NULL, 10);
+			if (abbrev < 4 || 40 <= abbrev)
+				abbrev = DEFAULT_ABBREV;
+			continue;
+		}
 		if (get_sha1(arg, sha1) < 0)
 			usage(describe_usage);
 		cmit = lookup_commit_reference(sha1);
-- 
1.0.5-geb9d

^ permalink raw reply related

* [PATCH 1/6] git-describe: really prefer tags only.
From: Junio C Hamano @ 2005-12-28  0:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0512241409300.14098@g5.osdl.org>

Often there are references other than annotated tags under
refs/tags hierarchy that are used to "keep things just in case".
default to use annotated tags only, still leaving the option to
use any ref with --all flag.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 Linus Torvalds <torvalds@osdl.org> writes:

 > On Sat, 24 Dec 2005, Linus Torvalds wrote:
 >> 
 >> This is useful for two things:
 >> 
 >>  - automatic version naming in Makefiles, for example. We could use it in 
 >>    git itself: when doing "git --version", we could use this to give a 
 >>    much more useful description of exactly what version was installed.
 >
 > This trivial patch fails to do that correctly, but maybe somebody could 
 > fix it. 

 Here is a 6-patch series towards that.  Will be in the proposed
 updates later tonight.

 describe.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

170c7e67e7792e0d5ce2328c1d7b8d4139a7c7e7
diff --git a/describe.c b/describe.c
index ebfa429..e1b6588 100644
--- a/describe.c
+++ b/describe.c
@@ -1,12 +1,13 @@
 #include "cache.h"
 #include "commit.h"
+#include "tag.h"
 #include "refs.h"
 
 #define SEEN (1u << 0)
 
 static const char describe_usage[] = "git-describe [--all] <committish>*";
 
-static int all = 0;	/* Default to tags only */
+static int all = 0;	/* Default to annotated tags only */
 
 static int names = 0, allocs = 0;
 static struct commit_name {
@@ -49,9 +50,15 @@ static int get_name(const char *path, co
 	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
 	if (!commit)
 		return 0;
-	if (!all && strncmp(path, "refs/tags/", 10))
-		return 0;
-	add_to_known_names(path, commit);
+	if (!all) {
+		struct object *object;
+		if (strncmp(path, "refs/tags/", 10))
+			return 0;
+		object = parse_object(sha1);
+		if (object->type != tag_type)
+			return 0;
+	}
+	add_to_known_names(all ? path : path + 10, commit);
 	return 0;
 }
 
-- 
1.0.5-geb9d

^ permalink raw reply related

* [PATCH 2/6] git-describe: use find_unique_abbrev()
From: Junio C Hamano @ 2005-12-28  0:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0512241409300.14098@g5.osdl.org>

Just in case 8 hexadecimal digits are not enough.  We could use
shorter default if we wanted to.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 describe.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

298f9203cf85d5de28a8380bd26f85206530b7d0
diff --git a/describe.c b/describe.c
index e1b6588..ba49d00 100644
--- a/describe.c
+++ b/describe.c
@@ -95,7 +95,8 @@ static void describe(struct commit *cmit
 		struct commit *c = pop_most_recent_commit(&list, SEEN);
 		n = match(c);
 		if (n) {
-			printf("%s-g%.8s\n", n->path, sha1_to_hex(cmit->object.sha1));
+			printf("%s-g%s\n", n->path,
+			       find_unique_abbrev(cmit->object.sha1, 8));
 			return;
 		}
 	}
-- 
1.0.5-geb9d

^ permalink raw reply related

* Re: Add a "git-describe" command
From: Junio C Hamano @ 2005-12-28  0:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0512241409300.14098@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> The problem is not that it generates GIT_VERSION wrong. The problem is 
> two-fold:
>  - it should notice when "git-describe" doesn't exist, and fall back on 
>    the old less-than-descriptive behaviour
>  - it doesn't do dependencies correctly (ie it should now make "git" 
>    depend on the version number, but it doesn't, so it doesn't re-build 
>    git after a commit/pull)
>
> but at least it shows the _idea_ of using git-describe.
>
> With this I get
>
> 	[torvalds@g5 git]$ git --version
> 	git version v1.0.4-g6e9961d6
>
> which I think is better than "1.0.GIT" which doesn't say anything about 
> what the _actual_ version was.

I do not think 1.0.GIT is a good name either, so obviously there
is a room for improvement.

One problem with git-describe is that getting tags is a concious
user action, and you need to do "git fetch --tags" from time to
time in order to see the v1.0.4-g6e9961d6 name.  It could well
say v1.0.3-g6649c466 if the end user has built on top of
otherwise fully up-to-date maint branch without fetching v1.0.4
nor v1.0.5 tags, so when we learn that the v1.0.3-g6649c466 does
not work for an end-user, it would not help us to look for
regression between v1.0.3 and v1.0.4.  All it tells us is that
it is not older than v1.0.3, and it may well be the v1.0.5
without any local modification.

^ permalink raw reply

* [PATCH 6/6] Makefile: use git-describe to mark the git version.
From: Junio C Hamano @ 2005-12-28  0:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0512241409300.14098@g5.osdl.org>

Note: with this commit, the GIT maintainer workflow must change.
GIT-VERSION-GEN is now the file to munge when the default
version needs to be changed, not Makefile.  The tag needs to be
pushed into the repository to build the official tarball and
binary package beforehand.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---
 * Only lightly tested.  Especially bootstrapping might be
   fishy, but I'll be heading off to Japan for fishful new year
   meals in a few days ;-), so...

 .gitignore      |    1 +
 GIT-VERSION-GEN |   18 ++++++++++++++++++
 Makefile        |   10 ++++++++--
 3 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100755 GIT-VERSION-GEN

8e517cdeb5644b9857c8a9d8ce204ec9b7405297
diff --git a/.gitignore b/.gitignore
index 6bd508e..47d76f4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,4 @@
+GIT-VERSION-FILE
 git
 git-add
 git-am
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
new file mode 100755
index 0000000..196402c
--- /dev/null
+++ b/GIT-VERSION-GEN
@@ -0,0 +1,18 @@
+#!/bin/sh
+
+GVF=GIT-VERSION-FILE
+
+VN=$(git describe --abbrev=4 HEAD 2>/dev/null) || VN=v1.0.GIT
+VN=$(expr "$VN" : v'\(.*\)')
+if test -r $GVF
+then
+	VC=$(sed -e 's/^GIT_VERSION = //' <$GVF)
+else
+	VC=unset
+fi
+test "$VN" = "$VC" || {
+	echo >&2 "GIT_VERSION = $VN"
+	echo "GIT_VERSION = $VN" >$GVF
+}
+
+
diff --git a/Makefile b/Makefile
index 47e7898..6f1d123 100644
--- a/Makefile
+++ b/Makefile
@@ -55,7 +55,9 @@ all:
 # Define USE_STDEV below if you want git to care about the underlying device
 # change being considered an inode change from the update-cache perspective.
 
-GIT_VERSION = 1.0.GIT
+GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
+	@sh ./GIT-VERSION-GEN
+-include GIT-VERSION-FILE
 
 # CFLAGS and LDFLAGS are for the users to override from the command line.
 
@@ -369,7 +371,7 @@ all: $(ALL_PROGRAMS)
 all:
 	$(MAKE) -C templates
 
-git$X: git.c $(LIB_FILE) Makefile
+git$X: git.c $(LIB_FILE) Makefile GIT-VERSION-FILE
 	$(CC) -DGIT_EXEC_PATH='"$(bindir)"' -DGIT_VERSION='"$(GIT_VERSION)"' \
 		$(CFLAGS) $(COMPAT_CFLAGS) -o $@ $(filter %.c,$^) $(LIB_FILE)
 
@@ -499,6 +501,7 @@ deb: dist
 ### Cleaning rules
 
 clean:
+	rm -f GIT-VERSION-FILE
 	rm -f *.o mozilla-sha1/*.o arm/*.o ppc/*.o compat/*.o $(LIB_FILE)
 	rm -f $(PROGRAMS) $(SIMPLE_PROGRAMS) git$X
 	rm -f $(filter-out gitk,$(SCRIPTS))
@@ -511,3 +514,6 @@ clean:
 	$(MAKE) -C templates clean
 	$(MAKE) -C t/ clean
 
+.PHONY: all install clean
+.PHONY: .FORCE-GIT-VERSION-FILE
+
-- 
1.0.5-geb9d

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox