[PATCH] rpmbuild doesn't like '-' in version strings

[PATCH] rpmbuild doesn't like '-' in version strings

[John Ellson]

Problem:

	ellson@ellson:git> make rpm
	GIT_VERSION = 1.0.6-g58e3
	sed -e 's/@@VERSION@@/1.0.6-g58e3/g' < git.spec.in > git.spec
	./git-tar-tree HEAD git-1.0.6-g58e3 > git-1.0.6-g58e3.tar
	tar rf git-1.0.6-g58e3.tar git-1.0.6-g58e3/git.spec
	gzip -f -9 git-1.0.6-g58e3.tar
	rpmbuild -ta git-1.0.6-g58e3.tar.gz
	error: line 3: Illegal char '-' in version: Version:    1.0.6-g58e3
	make: *** [rpm] Error 1


Suggested fix:  Use '_' instead of '-'

There is probably a cleaner implementation of the fix, but this works for me.

----------------------------------------


diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 845b9dc..e4e0ab1 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -3,7 +3,9 @@
  GVF=GIT-VERSION-FILE

  VN=$(git-describe --abbrev=4 HEAD 2>/dev/null) || VN=v1.0.GIT
-VN=$(expr "$VN" : v'\(.*\)')
+VN1=$(expr "$VN" : '[^-]*-\(.*\)')
+VN=$(expr "$VN" : v'\([^-]*\)')
+test "$VN1" = "" || VN="$VN"_"$VN1"
  if test -r $GVF
  then
         VC=$(sed -e 's/^GIT_VERSION = //' <$GVF)


--------------------------------------------


Signed-off-by: John Ellson <ellson@research.att.com>

Re: [PATCH] rpmbuild doesn't like '-' in version strings

[Junio C Hamano]

John Ellson <ellson@research.att.com> writes:

> Suggested fix:  Use '_' instead of '-'

I wonder if the right fix is to change the git-describe output
before the current output becomes too widespread.  After all,
somebody might be tempted to use 1.0.6-g58e3 as a tagname ;-).
For example, if we say "1.0.6:58e3" there is no ambiguity, but
probably binary packagers would not like colon either X-<.

More seriously, I do not think git-describe based versioning
scheme meshes well with binary packagers.  There is no guaranee
1.0.6-g58e3 comes before 1.0.6-g4e7a2 (it does not).  To really
fix this problem, I think the rpm target of the main Makefile
needs to be modified to include something monotonicly increasing
(e.g. number of seconds since the base commit encoded in base26,
or something silly like that) between the base version and the
abbreviated object name, but the development being distributed,
my today's version on top of 1.0.6 may be way behind your
yesterday's version, so some centralized coordination (read:
manual version assignment by the maintainer, or automated
assignment but only reserved for the maintainer and unavailable
to mere mortals) might be needed to truly solve this.  In that
sense, maybe leaving the interim version unbuildable for binary
packaging might be considered a feature.

What Linus did with git-describe is good for people who do not
install prebuilt binaries, or know how to override (false)
downgrade refusal by package managers.

I vaguely recall that Debian does not like underscores at random
places, but we know both RPM and Debian can handle multiple
dots, so if you insist using the git-describe based version to
build binary packages, I suspect dot instead of underscore would
be a better choice.  Since I do not understand what that "g"
stands for anyway, how about doing something like this instead?

Note that the part about "describe --stamp" is not really a
serious counterproposal.  If I cut a binary package out of my
master and then one of my topic branches, it is very likely they
will get confused time ordering when they happen to find the
same base revision.  This patch is for discussion only and I
will not even keep it in the proposed updates branch.  I would
however welcome if somebody polishes it up and makes it suitable
for binary packaging purposes ;-).

-- >8 --
[PATCH] sanitize describe output used in GIT_VERSION

It is reported that RPM does not like '-' in version name, which
unfortunately is what the new "git-describe" based interim
versioning scheme gives.

This does three things:

 - GIT-VERSION-GEN avoids '-' characters and replaces them with '.'.

 - git-describe --stamp flag adds number of seconds since the
   base revision until the named revision, between the base
   revision name and the abbreviated object name.

 - GIT-VERSION-GEN uses describe --stamp to make commits
   monotonically increasing to keep binary package managers
   happier, unless you have multiple branches or skewed clocks.

---
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index 0efe82a..ca0e781 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -8,7 +8,7 @@ git-describe - Show the most recent tag 
 
 SYNOPSIS
 --------
-'git-describe' [--all] [--tags] [--abbrev=<n>] <committish>...
+'git-describe' [--all] [--tags] [--abbrev=<n>] [--stamp] <committish>...
 
 DESCRIPTION
 -----------
@@ -35,6 +35,12 @@ OPTIONS
 	Instead of using the default 8 hexadecimal digits as the
 	abbreviated object name, use <n> digits.
 
+--stamp::
+	Add a timestamp string, expressed as number of seconds
+	between the commit times of the base commit and the
+	named commit, encoded in base26 (a-z), between the base
+	version name and abbreviated object name.
+
 
 EXAMPLES
 --------
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 845b9dc..f6ed589 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -2,8 +2,8 @@
 
 GVF=GIT-VERSION-FILE
 
-VN=$(git-describe --abbrev=4 HEAD 2>/dev/null) || VN=v1.0.GIT
-VN=$(expr "$VN" : v'\(.*\)')
+VN=$(git-describe --abbrev=4 --stamp HEAD 2>/dev/null) || VN=v1.0.GIT
+VN=$(echo "$VN" | sed -e 's/^v//' -e 's/-g\([^-]*\)$/-\1/' -e 's/-/./g')
 if test -r $GVF
 then
 	VC=$(sed -e 's/^GIT_VERSION = //' <$GVF)
diff --git a/describe.c b/describe.c
index 84d96b5..0414100 100644
--- a/describe.c
+++ b/describe.c
@@ -13,6 +13,7 @@ static int tags = 0;	/* But allow any ta
 
 #define DEFAULT_ABBREV 8 /* maybe too many */
 static int abbrev = DEFAULT_ABBREV;
+static int stamp = 0; /* show timestamps between base and abbrev */
 
 static int names = 0, allocs = 0;
 static struct commit_name {
@@ -98,6 +99,39 @@ static int compare_names(const void *_a,
 	return (a_date > b_date) ? -1 : (a_date == b_date) ? 0 : 1;
 }
 
+static void describe_it(struct commit *cmit, struct commit_name *name)
+{
+	const char *ab = find_unique_abbrev(cmit->object.sha1, abbrev);
+	if (stamp) {
+		unsigned long t1 = name->commit->date;
+		unsigned long t2 = cmit->date;
+		char tstamp[11]; /* 26^13 < 2^64 < 26^14 */
+		int i;
+
+		/* It had better be the former unless there is something
+		 * really really screwy with your clock.
+		 */
+		if (t1 < t2) {
+			unsigned long t = t2 - t1;
+			for (i = 0; i < sizeof(tstamp) && t; i++) {
+				unsigned long d = t % 26;
+				t /= 26;
+				tstamp[i] = d + 'a';
+			}
+		}
+		else {
+			tstamp[0] = '0';
+			i = 1;
+		}
+		printf("%s-", name->path);
+		while (0 < i)
+			putchar(tstamp[--i]);
+		printf("-g%s\n", ab);
+	}
+	else
+		printf("%s-g%s\n", name->path, ab);
+}
+
 static void describe(struct commit *cmit)
 {
 	struct commit_list *list;
@@ -122,8 +156,7 @@ static void describe(struct commit *cmit
 		struct commit *c = pop_most_recent_commit(&list, SEEN);
 		n = match(c);
 		if (n) {
-			printf("%s-g%s\n", n->path,
-			       find_unique_abbrev(cmit->object.sha1, abbrev));
+			describe_it(cmit, n);
 			return;
 		}
 	}
@@ -146,6 +179,10 @@ int main(int argc, char **argv)
 			tags = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--stamp")) {
+			stamp = 1;
+			continue;
+		}
 		if (!strncmp(arg, "--abbrev=", 9)) {
 			abbrev = strtoul(arg + 9, NULL, 10);
 			if (abbrev < 4 || 40 <= abbrev)

Re: [PATCH] rpmbuild doesn't like '-' in version strings

[Andreas Ericsson]

Junio C Hamano wrote:
> John Ellson <ellson@research.att.com> writes:
> 
> 
>>Suggested fix:  Use '_' instead of '-'
> 
> 
> I wonder if the right fix is to change the git-describe output
> before the current output becomes too widespread.


I don't think there's any major risk of the current output being very 
widespread. It's not very useful for scripting.


>  To really
> fix this problem, I think the rpm target of the main Makefile
> needs to be modified to include something monotonicly increasing
> (e.g. number of seconds since the base commit encoded in base26,
> or something silly like that)


Why not keep it super-simple and just print the number of commits since 
whatever tag is found? It only counts commits on the current branch, so 
a merge shows up as a single commit. That should more or less be ok 
though and goes well with the topic branch model.


>  If I cut a binary package out of my
> master and then one of my topic branches, it is very likely they
> will get confused time ordering when they happen to find the
> same base revision.


I suppose this could happen when counting commits as well. People who 
build packages from different branches should be shot on sight though, 
so I wouldn't worry about it.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH] rpmbuild doesn't like '-' in version strings

[Johannes Schindelin]

Hi,

we still could go back to \d.\d.\d.GIT, you know?

Using the output of git-describe only helps if you use git from the 
official repository. But when you have a private branch, they are only 
misleading. And if you use git from the official repository, you are more 
likely to take official versions anyway.

Ciao,
Dscho

Re: [PATCH] rpmbuild doesn't like '-' in version strings

[Ryan Anderson]

On Fri, Jan 06, 2006 at 02:37:27PM -0800, Junio C Hamano wrote:
>
> Since I do not understand what that "g"
> stands for anyway, how about doing something like this instead?

I'm pretty sure the "g" is my fault, indirectly.  When I submitted the
"AUTO_LOCALVERSION" patch to Linux, I prepended a -g to it, so it would
be possible to tell versions based off of a git tree apart from versions
based off of a CVS tree that used md5 to make a short, semi-unique
indicator.  Presumably, a CVS variant would've done something like
-c012345678, etc.

So, it's just a little mnemonic to hint that the extra version string
came from a git tree, as opposed to some other semi-random source. It's
value is rather dubious, overall, though.

-- 

Ryan Anderson
  sometimes Pug Majere

Re: [PATCH] rpmbuild doesn't like '-' in version strings

[John Ellson]

Junio C Hamano wrote:
> John Ellson <ellson@research.att.com> writes:
> 
>> Suggested fix:  Use '_' instead of '-'
> 
> I wonder if the right fix is to change the git-describe output
> before the current output becomes too widespread.  After all,
> somebody might be tempted to use 1.0.6-g58e3 as a tagname ;-).
> For example, if we say "1.0.6:58e3" there is no ambiguity, but
> probably binary packagers would not like colon either X-<.
> 
> More seriously, I do not think git-describe based versioning
> scheme meshes well with binary packagers.  There is no guaranee
> 1.0.6-g58e3 comes before 1.0.6-g4e7a2 (it does not).  To really
> fix this problem, I think the rpm target of the main Makefile
> needs to be modified to include something monotonicly increasing
> (e.g. number of seconds since the base commit encoded in base26,
> or something silly like that) between the base version and the
> abbreviated object name, but the development being distributed,
> my today's version on top of 1.0.6 may be way behind your
> yesterday's version, so some centralized coordination (read:
> manual version assignment by the maintainer, or automated
> assignment but only reserved for the maintainer and unavailable
> to mere mortals) might be needed to truly solve this.  In that
> sense, maybe leaving the interim version unbuildable for binary
> packaging might be considered a feature.


What happened to this?   I don't particularly like my fix either, but
some kind of fix is needed for the "make rpm" target to work.  Its still broken 
because of the '-' in the version string.


The need in rpm versioning is to be able to distinguish and monotonically
order, locally built rpms.   There is no particular need to disambiguate against
somebody else's rpms since if you are merging someone else's changes you
would be doing it in git and not by intermixing rpms.

I think that distinguishing between local branches is not likely to be a prolem. 
  Most developers are likely to use only one for rpm construction, or if there 
is a second experimental branch, then it is likely to contain more-recent 
changes anyway.

So a count of minutes since last tag is probably sufficient.

This could have a hash appended if it is essential to make the rpm version 
unique without losing the ordering of the timestamp.

Something like:

	1.1.2_123456_g9e9b


John

Re: [PATCH] rpmbuild doesn't like '-' in version strings

[Linus Torvalds]

On Sat, 14 Jan 2006, John Ellson wrote:
> 
> What happened to this?   I don't particularly like my fix either, but
> some kind of fix is needed for the "make rpm" target to work.  Its still
> broken because of the '-' in the version string.

Do a "sed" for rpmbuild.

There's absolutely no point in trying to make git-describe use "_" instead 
of "-", since having a "-" in a tag-name is very common ("my-version"), 
and it would be a horrible mistake to munge the tag-names. So even if we 
changed "-g" into "_g" it wouldn't help anything, and just make things 
uglier.

This is an RPM versioning problem, and nothing more. So it should be 
handled by rpmbuild.

		Linus

Re: [PATCH] rpmbuild doesn't like '-' in version strings

[Junio C Hamano]

John Ellson <ellson@research.att.com> writes:

>> ...  In that
>> sense, maybe leaving the interim version unbuildable for binary
>> packaging might be considered a feature.
>
> What happened to this?

I consider leaving the interim version unbuildable for binary packaging
consider a feature.

If you want to build your own version, I think you could locally
tag that head and build, like:

	$ git tag -a "John's GIT 1.1.2+frotz patch" v1.1.2.John0114
	$ make rpmbuild

Of course you can keep a patch with the sed -e 's/-/_/' in
GIT-VERSION-GEN as Linus suggested in your development branch.
I am not yet convinced being able to build a random
unidentifiable binary package is a good thing, and "the number
of minutes/seconds monotonicity" would not work in multiple
branches case (i.e. still leaves the result unordered).

Re: [PATCH] rpmbuild doesn't like '-' in version strings

[John Ellson]

Junio C Hamano wrote:
> John Ellson <ellson@research.att.com> writes:
>   
> I consider leaving the interim version unbuildable for binary packaging
> consider a feature.
>   
Not a very helpful feature IMO.    Who is protected by this?
> If you want to build your own version, I think you could locally
> tag that head and build, like:
>
> 	$ git tag -a "John's GIT 1.1.2+frotz patch" v1.1.2.John0114
> 	$ make rpmbuild
>
> Of course you can keep a patch with the sed -e 's/-/_/' in
> GIT-VERSION-GEN as Linus suggested in your development branch.
>   
Thats basically all I'm looking for.   I agree that is only necessary to 
fix the "make rpm" target.
Further changes are not strictly necessary.   I don't understand why it 
would only be useful to me?

> I am not yet convinced being able to build a random
> unidentifiable binary package is a good thing, and "the number
> of minutes/seconds monotonicity" would not work in multiple
> branches case (i.e. still leaves the result unordered).
>   
Since disparate branches are intrinsically unordered I was suggesting 
that the
hash field would be used to ensure uniqueness only.   The timestamp 
field is only for ordering within a branch.

So if someone builds rpms from two different branches, they might still 
have to force the particular
selection they want with "rpm -Uvh --oldpackage ...", but I think this 
is the best that can be done
in the absence of any intrinsic ordering.

Anyway, this is above and beyond doing something with sed to fix the '-' 
issue.

John
>
>
>   

Re: [PATCH] rpmbuild doesn't like '-' in version strings

[Junio C Hamano]

John Ellson <ellson@research.att.com> writes:

> Anyway, this is above and beyond doing something with sed to
> fix the '-' issue.

What I am saying is that until that issue of ordering is
resolved (and I highly suspect it is unsolvable) I think it is
dangerous and more confusing to let binary packages be built,
and it is better to simply forbid it like in the current
scheme.

BTW, if we _were_ to do a sed, please do s/-/./g instead; the
underscore breaks Debian if I am not mistaken.

Re: [PATCH] rpmbuild doesn't like '-' in version strings

[Junio C Hamano]

John Ellson <ellson@research.att.com> writes:

>> Of course you can keep a patch with the sed -e 's/-/_/' in
>> GIT-VERSION-GEN as Linus suggested in your development branch.
>>
> Thats basically all I'm looking for.   I agree that is only necessary
> to fix the "make rpm" target.
> Further changes are not strictly necessary.   I don't understand why
> it would only be useful to me?

OK, if somebody is RPM savvy enough to do his own binary RPM and
install it, he would know how to override the downgrade guard
the binary package manager would give him, so it probably is not
such a big deal.  A patch to do "sed -e 's/-/./g'" will be
pushed out in the "master" branch shortly.

I hope if we later see complains on the list that some RPM cut
from an interim snapshot would not install, somebody would take
time to answer such complaints, pretty please?