threads / patch / 8741

patchgit-submodule: Instead of using only annotated tags, use any tag found in .git/refs/tags

Subject: [PATCH] git-submodule: Instead of using only annotated tags, use any tag found in .git/refs/tags

## tl;dr

4 messages between Jun 26, 2007 and Jun 28, 2007. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Emil Medve· Jun 26, 2007, 23:40 UTC · lore

Some repositories might not use/have annotated tags (for example repositories created with git-cvsimport) and git-submodule status might fail because git-describe might fail to find a tag. This change allows the status of a submodule to be described/displayed relative to lightweight tags as well.

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---
 git-submodule.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to git-submodule.sh +2 −2
diff --git a/git-submodule.sh b/git-submodule.sh
index 89a3885..56ea935 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -155,7 +155,7 @@ modules_list()
 			say "-$sha1 $path"
 			continue;
 		fi
-		revname=$(unset GIT_DIR && cd "$path" && git-describe $sha1)
+		revname=$(unset GIT_DIR && cd "$path" && git-describe --tags $sha1)
 		if git diff-files --quiet -- "$path"
 		then
 			say " $sha1 $path ($revname)"
@@ -163,7 +163,7 @@ modules_list()
 			if test -z "$cached"
 			then
 				sha1=$(unset GIT_DIR && cd "$path" && git-rev-parse --verify HEAD)
-				revname=$(unset GIT_DIR && cd "$path" && git-describe $sha1)
+				revname=$(unset GIT_DIR && cd "$path" && git-describe --tags $sha1)
 			fi
 			say "+$sha1 $path ($revname)"
 		fi
-- 
1.5.2.2.549.gaeb59
Junio C Hamano· Jun 27, 2007, 06:15 UTC · re: Emil Medve · lore

Re: [PATCH] git-submodule: Instead of using only annotated tags, use any tag found in .git/refs/tags

Emil Medve <Emilian.Medve@Freescale.com> writes:
Show 6 quoted lines
> Some repositories might not use/have annotated tags (for
> example repositories created with git-cvsimport) and
> git-submodule status might fail because git-describe might
> fail to find a tag.  This change allows the status of a
> submodule to be described/displayed relative to lightweight
> tags as well.

Certainly that is an improvement, as $revname is purely for commenting and not being able to describe it is not an excuse to fail the command.

But there may not be any tag at all. How about something like this on top?

Show changes to git-submodule.sh +16 −3
diff --git a/git-submodule.sh b/git-submodule.sh
index 56ea935..7b6195b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -133,6 +133,18 @@ modules_update()
 	done
 }
 
+set_name_rev () {
+	revname=$( (
+		unset GIT_DIR &&
+		cd "$1" && {
+			git-describe "$2" 2>/dev/null ||
+			git-describe --tags "$2" 2>/dev/null ||
+			git-describe --contains --tags "$2"
+		}
+	) )
+	test -z "$revname" || revname=" ($revname)"
+}
+
 #
 # List all submodules, prefixed with:
 #  - submodule not initialized
@@ -156,16 +168,17 @@ modules_list()
 			continue;
 		fi
 		revname=$(unset GIT_DIR && cd "$path" && git-describe --tags $sha1)
+		set_name_rev "$path" $"sha1"
 		if git diff-files --quiet -- "$path"
 		then
-			say " $sha1 $path ($revname)"
+			say " $sha1 $path$revname"
 		else
 			if test -z "$cached"
 			then
 				sha1=$(unset GIT_DIR && cd "$path" && git-rev-parse --verify HEAD)
-				revname=$(unset GIT_DIR && cd "$path" && git-describe --tags $sha1)
+				set_name_rev "$path" $"sha1"
 			fi
-			say "+$sha1 $path ($revname)"
+			say "+$sha1 $path$revname"
 		fi
 	done
 }
Medve Emilian-EMMEDVE1· Jun 27, 2007, 12:20 UTC · re: Junio C Hamano · lore

RE: [PATCH] git-submodule: Instead of using only annotated tags, use any tag found in .git/refs/tags

Hello Junio,

You're right and there might be no tag at all, thus --contains might return an undefined. In the spirit of best effort maybe we should try --all (which I don't think that can fail and it will return something more relevant then undefined, i.e. the branch of the commit) if --contains returns undefined. I'll submit a patch to reflect this. Opinions?

While playing with git-describe I noticed that the --all option is maybe not trying first to find a tag as the man page suggests but it goes directly for .git/refs. Here is some output from my git repo clone with yesterday's head on the master branch:

$ git-describe aeb59328453cd4f438345ea79ff04c96bccbbbb8 v1.5.2.2-549-gaeb5932

$ git-describe --all aeb59328453cd4f438345ea79ff04c96bccbbbb8 heads/master

Do you think we want to fix that? If yes, I could look into it and submit a patch.

Cheers, Emil.

This e-mail, and any associated attachments have been classified as: -------------------------------------------------------------------- [x] Public [ ] Freescale Semiconductor Internal Use Only [ ] Freescale Semiconductor Confidential Proprietary

-----Original Message-----
From: Junio C Hamano [mailto:gitster@pobox.com] 
Sent: Wednesday, June 27, 2007 1:15 AM
To: Medve Emilian-EMMEDVE1
Cc: git@vger.kernel.org
Subject: Re: [PATCH] git-submodule: Instead of using only annotated
tags, use any tag found in .git/refs/tags
Emil Medve <Emilian.Medve@Freescale.com> writes:
Show 6 quoted lines
> Some repositories might not use/have annotated tags (for
> example repositories created with git-cvsimport) and
> git-submodule status might fail because git-describe might
> fail to find a tag.  This change allows the status of a
> submodule to be described/displayed relative to lightweight
> tags as well.

Certainly that is an improvement, as $revname is purely for commenting and not being able to describe it is not an excuse to fail the command.

But there may not be any tag at all. How about something like this on top?

Show changes to git-submodule.sh +16 −4
diff --git a/git-submodule.sh b/git-submodule.sh
index 56ea935..7b6195b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -133,6 +133,18 @@ modules_update()
 	done
 }
 
+set_name_rev () {
+	revname=$( (
+		unset GIT_DIR &&
+		cd "$1" && {
+			git-describe "$2" 2>/dev/null ||
+			git-describe --tags "$2" 2>/dev/null ||
+			git-describe --contains --tags "$2"
+		}
+	) )
+	test -z "$revname" || revname=" ($revname)"
+}
+
 #
 # List all submodules, prefixed with:
 #  - submodule not initialized
@@ -156,16 +168,17 @@ modules_list()
 			continue;
 		fi
 		revname=$(unset GIT_DIR && cd "$path" && git-describe
--tags $sha1)
+		set_name_rev "$path" $"sha1"
 		if git diff-files --quiet -- "$path"
 		then
-			say " $sha1 $path ($revname)"
+			say " $sha1 $path$revname"
 		else
 			if test -z "$cached"
 			then
 				sha1=$(unset GIT_DIR && cd "$path" &&
git-rev-parse --verify HEAD)
-				revname=$(unset GIT_DIR && cd "$path" &&
git-describe --tags $sha1)
+				set_name_rev "$path" $"sha1"
 			fi
-			say "+$sha1 $path ($revname)"
+			say "+$sha1 $path$revname"
 		fi
 	done
 }
Shawn O. Pearce· Jun 28, 2007, 05:27 UTC · re: Medve Emilian-EMMEDVE1 · lore

Re: [PATCH] git-submodule: Instead of using only annotated tags, use any tag found in .git/refs/tags

Medve Emilian-EMMEDVE1 <Emilian.Medve@freescale.com> wrote:
Show 10 quoted lines
> While playing with git-describe I noticed that the --all option is maybe
> not trying first to find a tag as the man page suggests but it goes
> directly for .git/refs. Here is some output from my git repo clone with
> yesterday's head on the master branch:
> 
> $ git-describe aeb59328453cd4f438345ea79ff04c96bccbbbb8
> v1.5.2.2-549-gaeb5932
> 
> $ git-describe --all aeb59328453cd4f438345ea79ff04c96bccbbbb8
> heads/master

Yea. Look at what's happening. In the --all case we attach heads/master into the ->util field of aeb5's struct commit*. Since no annotated tag (a ref with prio 2) and no lightweight tag (a ref with prio 1) was found pointing at aeb5 we kept that ->util field pointing at the heads/master ref (which has prio 0).

The --all and --tags options are about selecting what refs can appear in that ->util field. That's _all_ they do.

Later in describe() at l.151 we immediately display a ref if there is one in the ->util field:

    150     n = cmit->util;
    151     if (n) {
    152         printf("%s\n", n->path);
    153         return;
    154     }

So we're favoring a ref that points directly at a commit over any other ref. We only search if we don't have a ref pointing directly at the input commit. Searching is when ranking really gets involved.

> Do you think we want to fix that? If yes, I could look into it and
> submit a patch.

I'm not sure. If we "fixed" this then --all would only ever turn up a head if no annotated tag exists on the entire history of that input commit. Because the "fix" would be to actually not return right away here at l.151, but instead to drop down further into the slower loop where we traverse through commits, pick our candidates, rank them, and then pick the highest priorty ref that is also the closest. The annotated tag would always win over the head.

At which point --all is only ever useful if the repository *never* had an annotated tag along the input branch. I'm not sure that's useful as a description for a commit. If no annotated tag exists the raw commit SHA-1 is probably a better description. Its at least stable with time. ;-)

In my opinion, git-describe is doing *exactly* what the manual page says it does. But both the current implementation and the manual page were last majorly overhauld by me. So take my comments about the documentation with a grain of salt. ;-)

-- 
Shawn.

← back to recent threads