Trial git RPM's..

Trial git RPM's..

[Linus Torvalds]

Ok, I tagged a "v0.99" thing, and pushed it out. I've also made trial 
RPM's of it: src, ppc64 and x86. They're build on whatever random machines 
I had, and on the ppc64 I chose to do it on my FC4 machine that has newer 
libraries than my YDL one. The x86 thing is FC3, I do believe.

I haven't really verified the RPM's in any other way than a trial 
installation on one machine, and "gitk" seemed to work. Whoop. The idea 
being that this is a good way to check whether the rpm target works, _and_ 
cogito can have something to build against.

They rpm's are at

	http://www.kernel.org/pub/software/scm/git/

or will be once mirrored.

		Linus

Re: Trial git RPM's..

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> Ok, I tagged a "v0.99" thing, and pushed it out. I've also made trial 
> RPM's of it: src, ppc64 and x86. They're build on whatever random machines 
> I had, and on the ppc64 I chose to do it on my FC4 machine that has newer 
> libraries than my YDL one. The x86 thing is FC3, I do believe.
>
> I haven't really verified the RPM's in any other way than a trial 
> installation on one machine, and "gitk" seemed to work. Whoop. The idea 
> being that this is a good way to check whether the rpm target works, _and_ 
> cogito can have something to build against.

A couple of pieces.  The dist target has assumes git-tar-tree is in the
path.  Making it so you have to have git installed to build the rpm.

The man pages are not built. The build dependencies do not call out
the tools necessary to build the man pages.

And it does not pass my torture test of building rpm's on debian,
but that is not a huge problem.

Are you still up for a patch that records who and when made a tag?
I sent one but it seems to have been lost.

Eric

Re: Trial git RPM's..

[Linus Torvalds]

On Mon, 11 Jul 2005, Eric W. Biederman wrote:
> 
> A couple of pieces.  The dist target has assumes git-tar-tree is in the
> path.  Making it so you have to have git installed to build the rpm.

Yes. Maybe we could relax that requirement by using "./git-tar-tree" or 
something? That still requires that you have _built_ git to do the rpm, 
but at least you won't have had to install it.

Does that fit the rpm build process? Or does an rpm build make something 
like that really inconvenient? I don't know, patches welcome.

> The man pages are not built. The build dependencies do not call out
> the tools necessary to build the man pages.

Most people don't have asciidoc, and I'm not sure we want to require it. 
Maybe we could have a separate "make man-rpm" target for that?

> And it does not pass my torture test of building rpm's on debian,
> but that is not a huge problem.

Ok, why is debian problematic? Is there some missing dependency or 
something? I really haven't ever done an rpm, and the git rpm target was 
all done by Chris Wright, so I don't have any clue at all. Again, patches 
welcome.

> Are you still up for a patch that records who and when made a tag?
> I sent one but it seems to have been lost.

I'd really actually prefer for the code to be shared with the commit code, 
so that the user info (name, email, date) would not just be exactly the 
same, but would share the same code so that we don't end up having them 
ever get out of sync. 

That would imply moving parts of "git-tag-script" into mktag.c.

		Linus

Re: Trial git RPM's..

[Horst von Brand]

Linus Torvalds <torvalds@osdl.org> wrote:
> On Mon, 11 Jul 2005, Eric W. Biederman wrote:
> > A couple of pieces.  The dist target has assumes git-tar-tree is in the
> > path.  Making it so you have to have git installed to build the rpm.

> Yes. Maybe we could relax that requirement by using "./git-tar-tree" or 
> something? That still requires that you have _built_ git to do the rpm, 
> but at least you won't have had to install it.

I don't see a problem here. Sure, you need git to build git, so place it in
Build-requires: Need to install the binary to build the next from source,
that's all. Just like gcc ;-)

[...]

> > The man pages are not built. The build dependencies do not call out
> > the tools necessary to build the man pages.

> Most people don't have asciidoc, and I'm not sure we want to require it. 
> Maybe we could have a separate "make man-rpm" target for that?

Would have to be a requirement for building anyway. There is a (not really
nice) SRPM at <http://www.pvv.ntnu.no/~terjeros/rpms/asciidoc/>. Will see
to clean it up.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513

Re: Trial git RPM's..

[Chris Wright]

* Linus Torvalds (torvalds@osdl.org) wrote:
> On Mon, 11 Jul 2005, Eric W. Biederman wrote:
> > 
> > A couple of pieces.  The dist target has assumes git-tar-tree is in the
> > path.  Making it so you have to have git installed to build the rpm.
> 
> Yes. Maybe we could relax that requirement by using "./git-tar-tree" or 
> something? That still requires that you have _built_ git to do the rpm, 
> but at least you won't have had to install it.
> 
> Does that fit the rpm build process? Or does an rpm build make something 
> like that really inconvenient? I don't know, patches welcome.

No, that could be done.  It's not the rpmbuild at that point, it's just
prepping to do the rpmbuild.  It's only an issue for those that are trying
to build an rpm directly from the git source and who've never installed
git before.  Doesn't seem necessary to me, I'm not that fond of it (will
even slightly slow down make dist process if it's done from a clean,
i.e. make clean, dir), but hey...the trivial patch below does this.

> > And it does not pass my torture test of building rpm's on debian,
> > but that is not a huge problem.
> 
> Ok, why is debian problematic? Is there some missing dependency or 
> something? I really haven't ever done an rpm, and the git rpm target was 
> all done by Chris Wright, so I don't have any clue at all. Again, patches 
> welcome.

Heh debian rpm build...I missed that bit in Eric's message.  Eric, care
to give details?

thanks,
-chris
--



Use git-tar-tree directly from git source during make dist.  This
handles bootstrap issue with git not being installed.

Signed-off-by: Chris Wright <chrisw@osdl.org>
---

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -175,8 +175,8 @@ git.spec: git.spec.in
 	sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@
 
 GIT_TARNAME=git-$(GIT_VERSION)
-dist: git.spec
-	git-tar-tree HEAD $(GIT_TARNAME) > $(GIT_TARNAME).tar
+dist: git.spec git-tar-tree
+	./git-tar-tree HEAD $(GIT_TARNAME) > $(GIT_TARNAME).tar
 	@mkdir -p $(GIT_TARNAME)
 	@cp git.spec $(GIT_TARNAME)
 	tar rf $(GIT_TARNAME).tar $(GIT_TARNAME)/git.spec

Re: Trial git RPM's..

[Eric W. Biederman]

Chris Wright <chrisw@osdl.org> writes:

> * Linus Torvalds (torvalds@osdl.org) wrote:
>> > And it does not pass my torture test of building rpm's on debian,
>> > but that is not a huge problem.
>> 
>> Ok, why is debian problematic? Is there some missing dependency or 
>> something? I really haven't ever done an rpm, and the git rpm target was 
>> all done by Chris Wright, so I don't have any clue at all. Again, patches 
>> welcome.
>
> Heh debian rpm build...I missed that bit in Eric's message.  Eric, care
> to give details?

Ok paged back in my state.  The practical problem is that rpmbuild try
to lookup the build dependencies which simply aren't present on debian.
Patch will be along shortly once I get the glitches fixed.

One last issue with building packages.  Some distros are still shipping
GNU interactive tools so git as a package name for the rpm is problematic.
At the very least it is extremely confusing that git-0.99 is a more
recent package that git-4.3.20.

Eric

Re: Trial git RPM's..

[Linus Torvalds]

On Tue, 12 Jul 2005, Eric W. Biederman wrote:
> 
> One last issue with building packages.  Some distros are still shipping
> GNU interactive tools so git as a package name for the rpm is problematic.
> At the very least it is extremely confusing that git-0.99 is a more
> recent package that git-4.3.20.

Ahh. Dang, I should have remembered this. We should call the rpm 
"git-core-0.99", not just "git-0.99".

Chris, I assume this is just changing the name in the spec-file from "git" 
to "git-core"?

		Linus

Re: Trial git RPM's..

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> Ahh. Dang, I should have remembered this. We should call the rpm 
> "git-core-0.99", not just "git-0.99".
>
> Chris, I assume this is just changing the name in the spec-file from "git" 
> to "git-core"?

The name of the tarball needs to be updated as well.

Eric

Re: Trial git RPM's..

[Linus Torvalds]

On Tue, 12 Jul 2005, Eric W. Biederman wrote:
> 
> The name of the tarball needs to be updated as well.

Yes, I noticed.

I ended up renaming the spec-file too.

Pushed out,

		Linus

Re: Trial git RPM's..

[Chris Wright]

* Linus Torvalds (torvalds@osdl.org) wrote:
> On Tue, 12 Jul 2005, Eric W. Biederman wrote:
> > The name of the tarball needs to be updated as well.
> 
> Yes, I noticed.
> 
> I ended up renaming the spec-file too.
> 
> Pushed out,

Yup, looks good.
-chris

Re: Trial git RPM's..

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> On Mon, 11 Jul 2005, Eric W. Biederman wrote:
>
>> Are you still up for a patch that records who and when made a tag?
>> I sent one but it seems to have been lost.
>
> I'd really actually prefer for the code to be shared with the commit code, 
> so that the user info (name, email, date) would not just be exactly the 
> same, but would share the same code so that we don't end up having them 
> ever get out of sync. 

Sounds fair.

> That would imply moving parts of "git-tag-script" into mktag.c.

Actually I was looking at doing a git-ident thing that will
just compute who git thinks you are.  And then git-commit-tree can
just popen it to share code.  That looks like how the logic has
been accomplished in other places.

Moving parts of git-tag-script into mktag is hard because you
have to generate a flat file to pass to gpg.  And I don't think
I am ready to hard code the call to gpg into mktag, as some other
signing method may come along.  Although that may be the saner
choice.

Anyway the git-ident thing is easy and informative for debugging
so I will finish coding that up as soon as I get home.

Eric

Re: Trial git RPM's..

[Linus Torvalds]

On Mon, 11 Jul 2005, Eric W. Biederman wrote:
> 
> Actually I was looking at doing a git-ident thing that will
> just compute who git thinks you are.  And then git-commit-tree can
> just popen it to share code.  That looks like how the logic has
> been accomplished in other places.

I hate popen() if there's a reasonable functional interface in a library.

popen() is damn inefficient for doing something like this that is all C 
anyway.

		Linus

Re: Trial git RPM's..

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> On Mon, 11 Jul 2005, Eric W. Biederman wrote:
>> 
>> Actually I was looking at doing a git-ident thing that will
>> just compute who git thinks you are.  And then git-commit-tree can
>> just popen it to share code.  That looks like how the logic has
>> been accomplished in other places.
>
> I hate popen() if there's a reasonable functional interface in a library.
> popen() is damn inefficient for doing something like this that is all C 
> anyway.

Ok two new files then.  The new library function, and then
the utility that calls it.

Eric

[PATCH] tagger id

[Eric W. Biederman]

This patch adds a command git-id for use on
the command line to see what git will set your id too,
and for use in scripts (git-tag-script) so they can get your git id.

The common code for computing the git-id is moved to ident.c

Fix parse_date to not mind being passed a constant date
to parse.

The code to compute the identifier has been restructured
to at least make a reasonable stab at error handling.  The
original version had so many unchecked return values it was
just scary to think about.

Eric

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -47,7 +47,7 @@ PROG=   git-update-cache git-diff-files 
 	git-diff-stages git-rev-parse git-patch-id git-pack-objects \
 	git-unpack-objects git-verify-pack git-receive-pack git-send-pack \
 	git-prune-packed git-fetch-pack git-upload-pack git-clone-pack \
-	git-show-index
+	git-show-index git-id
 
 all: $(PROG)
 
@@ -57,7 +57,7 @@ install: $(PROG) $(SCRIPTS)
 
 LIB_OBJS=read-cache.o sha1_file.o usage.o object.o commit.o tree.o blob.o \
 	 tag.o date.o index.o diff-delta.o patch-delta.o entry.o path.o \
-	 epoch.o refs.o csum-file.o pack-check.o pkt-line.o connect.o
+	 epoch.o refs.o csum-file.o pack-check.o pkt-line.o connect.o ident.o
 LIB_FILE=libgit.a
 LIB_H=cache.h object.h blob.h tree.h commit.h tag.h delta.h epoch.h csum-file.h \
 	pack.h pkt-line.h refs.h
diff --git a/cache.h b/cache.h
--- a/cache.h
+++ b/cache.h
@@ -208,9 +208,14 @@ extern void *read_object_with_reference(
 					unsigned char *sha1_ret);
 
 const char *show_date(unsigned long time, int timezone);
-void parse_date(char *date, char *buf, int bufsize);
+void parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 
+int git_ident(char *buf, size_t bufsize,
+	const char *env_name, const char *env_email, const char *env_date);
+int git_committer_ident(char *buf, size_t bufsize);
+int git_author_ident(char *buf, size_t bufsize);
+
 static inline void *xmalloc(size_t size)
 {
 	void *ret = malloc(size);
diff --git a/commit-tree.c b/commit-tree.c
--- a/commit-tree.c
+++ b/commit-tree.c
@@ -5,9 +5,10 @@
  */
 #include "cache.h"
 
-#include <pwd.h>
 #include <time.h>
 #include <ctype.h>
+#include <string.h>
+#include <errno.h>
 
 #define BLOCKING (1ul << 14)
 
@@ -45,39 +46,6 @@ static void add_buffer(char **bufp, unsi
 	memcpy(buf + size, one_line, len);
 }
 
-static void remove_special(char *p)
-{
-	char c;
-	char *dst = p, *src = p;
-
-	for (;;) {
-		c = *src;
-		src++;
-		switch(c) {
-		case '\n': case '<': case '>':
-			continue;
-		}
-		*dst++ = c;
-		if (!c)
-			break;
-	}
-
-	/*
-	 * Go back, and remove crud from the end: some people
-	 * have commas etc in their gecos field
-	 */
-	dst--;
-	while (--dst >= p) {
-		unsigned char c = *dst;
-		switch (c) {
-		case ',': case ';': case '.':
-			*dst = 0;
-			continue;
-		}
-		break;
-	}
-}
-
 static void check_valid(unsigned char *sha1, const char *expect)
 {
 	void *buf;
@@ -114,16 +82,13 @@ static int new_parent(int idx)
 
 int main(int argc, char **argv)
 {
-	int i, len;
+	int i;
 	int parents = 0;
 	unsigned char tree_sha1[20];
 	unsigned char commit_sha1[20];
-	char *gecos, *realgecos, *commitgecos;
-	char *email, *commitemail, realemail[1000];
-	char date[50], realdate[50];
-	char *audate, *cmdate;
+	char committer[1000];
+	char author[1000];
 	char comment[1000];
-	struct passwd *pw;
 	char *buffer;
 	unsigned int size;
 
@@ -142,35 +107,12 @@ int main(int argc, char **argv)
 	}
 	if (!parents)
 		fprintf(stderr, "Committing initial tree %s\n", argv[1]);
-	pw = getpwuid(getuid());
-	if (!pw)
-		die("You don't exist. Go away!");
-	realgecos = pw->pw_gecos;
-	len = strlen(pw->pw_name);
-	memcpy(realemail, pw->pw_name, len);
-	realemail[len] = '@';
-	gethostname(realemail+len+1, sizeof(realemail)-len-1);
-	if (!strchr(realemail+len+1, '.')) {
-		strcat(realemail, ".");
-		getdomainname(realemail+strlen(realemail), sizeof(realemail)-strlen(realemail)-1);
+	if (git_author_ident(author, sizeof(author)) < 0) {
+		die("Bad author! %s", strerror(errno));
+	}
+	if (git_committer_ident(committer, sizeof(committer)) < 0) {
+		die("Bad Committer! %s", strerror(errno));
 	}
-
-	datestamp(realdate, sizeof(realdate));
-	strcpy(date, realdate);
-
-	commitgecos = gitenv("GIT_COMMITTER_NAME") ? : realgecos;
-	commitemail = gitenv("GIT_COMMITTER_EMAIL") ? : realemail;
-	gecos = gitenv("GIT_AUTHOR_NAME") ? : realgecos;
-	email = gitenv("GIT_AUTHOR_EMAIL") ? : realemail;
-	audate = gitenv("GIT_AUTHOR_DATE");
-	if (audate)
-		parse_date(audate, date, sizeof(date));
-	cmdate = gitenv("GIT_COMMITTER_DATE");
-	if (cmdate)
-		parse_date(cmdate, realdate, sizeof(realdate));
-
-	remove_special(gecos); remove_special(realgecos); remove_special(commitgecos);
-	remove_special(email); remove_special(realemail); remove_special(commitemail);
 
 	init_buffer(&buffer, &size);
 	add_buffer(&buffer, &size, "tree %s\n", sha1_to_hex(tree_sha1));
@@ -184,8 +126,8 @@ int main(int argc, char **argv)
 		add_buffer(&buffer, &size, "parent %s\n", sha1_to_hex(parent_sha1[i]));
 
 	/* Person/date information */
-	add_buffer(&buffer, &size, "author %s <%s> %s\n", gecos, email, date);
-	add_buffer(&buffer, &size, "committer %s <%s> %s\n\n", commitgecos, commitemail, realdate);
+	add_buffer(&buffer, &size, "author %s <%s> %s\n", author);
+	add_buffer(&buffer, &size, "committer %s <%s> %s\n\n", committer);
 
 	/* And add the comment */
 	while (fgets(comment, sizeof(comment), stdin) != NULL)
diff --git a/date.c b/date.c
--- a/date.c
+++ b/date.c
@@ -224,7 +224,7 @@ static int is_date(int year, int month, 
 	return 0;
 }
 
-static int match_multi_number(unsigned long num, char c, char *date, char *end, struct tm *tm)
+static int match_multi_number(unsigned long num, char c, const char *date, char *end, struct tm *tm)
 {
 	long num2, num3;
 
@@ -270,7 +270,7 @@ static int match_multi_number(unsigned l
 /*
  * We've seen a digit. Time? Year? Date? 
  */
-static int match_digit(char *date, struct tm *tm, int *offset, int *tm_gmt)
+static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt)
 {
 	int n;
 	char *end;
@@ -361,7 +361,7 @@ static int match_digit(char *date, struc
 	return n;
 }
 
-static int match_tz(char *date, int *offp)
+static int match_tz(const char *date, int *offp)
 {
 	char *end;
 	int offset = strtoul(date+1, &end, 10);
@@ -388,7 +388,7 @@ static int match_tz(char *date, int *off
 
 /* Gr. strptime is crap for this; it doesn't have a way to require RFC2822
    (i.e. English) day/month names, and it doesn't work correctly with %z. */
-void parse_date(char *date, char *result, int maxlen)
+void parse_date(const char *date, char *result, int maxlen)
 {
 	struct tm tm;
 	int offset, sign, tm_gmt;
diff --git a/git-tag-script b/git-tag-script
--- a/git-tag-script
+++ b/git-tag-script
@@ -7,6 +7,7 @@ name="$1"
 
 object=${2:-$(cat "$GIT_DIR"/HEAD)}
 type=$(git-cat-file -t $object) || exit 1
+tagger=$(git-id) || exit 1
 
 ( echo "#"
   echo "# Write a tag message"
@@ -17,8 +18,9 @@ grep -v '^#' < .editmsg | git-stripspace
 
 [ -s .tagmsg ] || exit
 
-( echo -e "object $object\ntype $type\ntag $name\n"; cat .tagmsg ) > .tmp-tag
+( echo -e "object $object\ntype $type\ntag $name\ntagger $tagger\n"; cat .tagmsg ) > .tmp-tag
 rm -f .tmp-tag.asc .tagmsg
 gpg -bsa .tmp-tag && cat .tmp-tag.asc >> .tmp-tag
+mkdir -p "$GIT_DIR/refs/tags"
 git-mktag < .tmp-tag > "$GIT_DIR/refs/tags/$name"
 #rm .tmp-tag .tmp-tag.sig
diff --git a/id.c b/id.c
new file mode 100644
--- /dev/null
+++ b/id.c
@@ -0,0 +1,36 @@
+#include "cache.h"
+#include <stdio.h>
+#include <errno.h>
+#include <string.h>
+
+static char *id_usage = "git-id [--author | --committer]";
+
+int main(int argc, char **argv)
+{
+	char buf[1000];
+	int (*ident)(char *buf, size_t bufsize);
+	int i;
+
+	ident = git_committer_ident;
+	for(i = 1; i < argc; i++) {
+		char *arg = argv[i];
+		if (strcmp(arg, "--author") == 0) {
+			ident = git_author_ident;
+		}
+		else if (strcmp(arg, "--committer") == 0) {
+			ident = git_committer_ident;
+		}
+		else {
+			usage(id_usage);
+		}
+	}
+
+
+	if (ident(buf, sizeof(buf)) < 0) {
+		die("Cannot resolve ident: %s\n", strerror(errno));
+	}
+
+	printf("%s\n", buf);
+
+	return 0;
+}
diff --git a/ident.c b/ident.c
new file mode 100644
--- /dev/null
+++ b/ident.c
@@ -0,0 +1,144 @@
+/*
+ * GIT - The information manager from hell
+ *
+ * Copyright (C) Eric Biederman, 2005
+ */
+#include "cache.h"
+
+#include <pwd.h>
+#include <time.h>
+#include <ctype.h>
+#include <errno.h>
+#include <string.h>
+#include <stdio.h>
+
+#define MAX_NAME 1000
+#define MAX_EMAIL 1000
+#define MAX_DATE 50
+static void remove_special(char *p)
+{
+	char c;
+	char *dst = p, *src = p;
+
+	for (;;) {
+		c = *src;
+		src++;
+		switch(c) {
+		case '\n': case '<': case '>':
+			continue;
+		}
+		*dst++ = c;
+		if (!c)
+			break;
+	}
+
+	/*
+	 * Go back, and remove crud from the end: some people
+	 * have commas etc in their gecos field
+	 */
+	dst--;
+	while (--dst >= p) {
+		unsigned char c = *dst;
+		switch (c) {
+		case ',': case ';': case '.':
+			*dst = 0;
+			continue;
+		}
+		break;
+	}
+}
+
+int git_ident(char *buf, size_t bufsize,
+	const char *env_name, const char *env_email, const char *env_date)
+{
+	int len;
+	char name[MAX_NAME];
+	char email[MAX_EMAIL];
+	char date[MAX_DATE];
+	struct passwd *pw;
+	int count;
+	
+	/* Lookup the user in the password file */
+	pw = getpwuid(getuid());
+	if (!pw)
+		return -1;
+
+	/* Get the users full name */
+	strncpy(name, pw->pw_gecos, sizeof(name));
+	name[sizeof(name) - 1] = '\0';
+
+	/* Get the email address with error handling */
+	len = strlen(pw->pw_name);
+	if (len >= (sizeof(email) - 2)) {
+		/* Bad user name length */
+		errno = ENOMEM;
+		return -1;
+	}
+	memcpy(email, pw->pw_name, len);
+	email[len] = '@';
+	email[len + 1] = '\0';
+
+	if (gethostname(email+len+1, sizeof(email)-len-1) < 0) {
+		return -1;
+	}
+	email[sizeof(email) - 1] = '\0';
+	len = strlen(email);
+	if (!strchr(email+len+1, '.')) {
+		if (len >= (sizeof(email) - 1)) {
+			errno = ENOMEM;
+			return -1;
+		}
+		email[len] = '.';
+		if (getdomainname(email+len+1, sizeof(email) - len - 1) < 0) {
+			return -1;
+		}
+	}
+	/* Get the date */
+	datestamp(date, sizeof(date));
+
+	if (env_name) {
+		strncpy(name, env_name, sizeof(name));
+		name[sizeof(name) - 1] = '\0';
+	}
+	if (env_email) {
+		strncpy(email, env_email, sizeof(email));
+		email[sizeof(email) - 1] = '\0';
+	}
+	if (env_date) {
+		parse_date(env_date, date, sizeof(date));
+	}
+	remove_special(name);
+	remove_special(email);
+	count = snprintf(buf, bufsize, "%s <%s> %s", name, email, date);
+	if (count > bufsize) {
+		errno = ENOMEM;
+		return -1;
+	}
+	if (count < 0) {
+		return -1;
+	}
+	return 0;
+}
+
+int git_committer_ident(char *buf, size_t bufsize)
+{
+	const char *name;
+	const char *email;
+	const char *date;
+	name  = gitenv("GIT_COMMITTER_NAME");
+	email = gitenv("GIT_COMMITTER_EMAIL");
+	date  = gitenv("GIT_COMMITTER_DATE");
+	return git_ident(buf, bufsize, name, email, date);
+}
+
+int git_author_ident(char *buf, size_t bufsize)
+{
+	const char *name;
+	const char *email;
+	const char *date;
+	name  = gitenv("GIT_AUTHOR_NAME");
+	email = gitenv("GIT_AUTHOR_EMAIL");
+	date  = gitenv("GIT_AUTHOR_DATE");
+	return git_ident(buf, bufsize, name, email, date);
+}
+
diff --git a/mktag.c b/mktag.c
--- a/mktag.c
+++ b/mktag.c
@@ -42,7 +42,7 @@ static int verify_tag(char *buffer, unsi
 	int typelen;
 	char type[20];
 	unsigned char sha1[20];
-	const char *object, *type_line, *tag_line;
+	const char *object, *type_line, *tag_line, *tagger_line;
 
 	if (size < 64 || size > MAXSIZE-1)
 		return -1;
@@ -92,6 +92,12 @@ static int verify_tag(char *buffer, unsi
 		return -1;
 	}
 
+	/* Verify the tagger line */
+	tagger_line = tag_line;
+	
+	if (memcmp(tagger_line, "tagger", 6) || (tagger_line[6] == '\n'))
+		return -1;
+
 	/* The actual stuff afterwards we don't care about.. */
 	return 0;
 }
@@ -119,7 +125,7 @@ int main(int argc, char **argv)
 		size += ret;
 	}
 
-	// Verify it for some basic sanity: it needs to start with "object <sha1>\ntype "
+	// Verify it for some basic sanity: it needs to start with "object <sha1>\ntype\ntagger "
 	if (verify_tag(buffer, size) < 0)
 		die("invalid tag signature file");
 

Re: [PATCH] tagger id

[Eric W. Biederman]

ebiederm@xmission.com (Eric W. Biederman) writes:

> This patch adds a command git-id for use on
> the command line to see what git will set your id too,
> and for use in scripts (git-tag-script) so they can get your git id.
>
> The common code for computing the git-id is moved to ident.c
>
> Fix parse_date to not mind being passed a constant date
> to parse.
>
> The code to compute the identifier has been restructured
> to at least make a reasonable stab at error handling.  The
> original version had so many unchecked return values it was
> just scary to think about.

Well except for a small bug, but serious bug...

> diff --git a/commit-tree.c b/commit-tree.c
> --- a/commit-tree.c
> +++ b/commit-tree.c
> @@ -184,8 +126,8 @@ int main(int argc, char **argv)
>  		add_buffer(&buffer, &size, "parent %s\n",
> sha1_to_hex(parent_sha1[i]));
>  
>  	/* Person/date information */
> -	add_buffer(&buffer, &size, "author %s <%s> %s\n", gecos, email, date);
> - add_buffer(&buffer, &size, "committer %s <%s> %s\n\n", commitgecos,
> commitemail, realdate);
> +	add_buffer(&buffer, &size, "author %s <%s> %s\n", author);
> +	add_buffer(&buffer, &size, "committer %s <%s> %s\n\n", committer);

This should be:
> +	add_buffer(&buffer, &size, "author %s\n", author);
> +	add_buffer(&buffer, &size, "committer %s\n\n", committer);

>  
>  	/* And add the comment */
>  	while (fgets(comment, sizeof(comment), stdin) != NULL)
> diff --git a/id.c b/id.c
> new file mode 100644
> --- /dev/null
> +++ b/id.c
> @@ -0,0 +1,36 @@
> +#include "cache.h"
> +#include <stdio.h>
> +#include <errno.h>
> +#include <string.h>
> +
> +static char *id_usage = "git-id [--author | --committer]";
> +
> +int main(int argc, char **argv)
> +{
> +	char buf[1000];
> +	int (*ident)(char *buf, size_t bufsize);
> +	int i;
> +
> +	ident = git_committer_ident;

Should this default to git_author_ident or git_committer_ident?
I'm not really certain how we expect to use the different flavors.

Eric

Re: [PATCH] tagger id

[Junio C Hamano]

ebiederm@xmission.com (Eric W. Biederman) writes:

> Should this default to git_author_ident or git_committer_ident?
> I'm not really certain how we expect to use the different flavors.

The only in-tree user after your patch is applied is the tagger
stuff, so in that sense committer_ident may make more sense.
Having said that, for something like this that would not be used
constantly and interatively by the users, my preference is not
to have any default at all, and always require --author or
--committer.  You have to type a bit more when doing the script,
but that needs to be done only once.  You will be sure which one
you are asking from the command two weeks after you wrote the
script so it is not a big loss.

I am not seriously suggesting the below as an alternative, but
have you thought about doing an inverse function of your
computation for the case when the user has all the environment
variables, and have script eval its output, like this [*1*]:

    $ git-id
    GIT_AUTHOR_NAME='Junio C Hamano'
    GIT_AUTHOR_EMAIL='junkio@cox.net'
    GIT_COMMITTER_NAME='Junio C Hamano'
    GIT_COMMITTER_EMAIL='junkio@cox.net'
    $ eval "`git-id`"
    $ tagger="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"

Having names and emails available separately may turn out to be
easier to use in other situation.  Just a thought.

By the way, I do not particularly like the name "git-id".  There
could be IDs for different kinds (not just people) we would want
later (file IDs, for example).  Naming what you are computing
_the_ "id" feels a bit too generic.  I do not have a better
alternative to suggest, though.

[Footnote]

*1* This makes its output syntax a bit too specific to the shell
and unfriendly to Porcelain written in other languages.  The
only non-shell Porcelains I am aware of are done in Perl (I do
not remember hearing its name) and Python (StGIT), both of which
have reasonable regexp support to grok something like this, so
it would not be a big issue.

Re: [PATCH] tagger id

[Eric W. Biederman]

Junio C Hamano <junkio@cox.net> writes:

> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> Should this default to git_author_ident or git_committer_ident?
>> I'm not really certain how we expect to use the different flavors.
>
> The only in-tree user after your patch is applied is the tagger
> stuff, so in that sense committer_ident may make more sense.

There is also the commit path, and that comes from C.  I'm not
quite certain how we should be using the environmental variables.

> Having said that, for something like this that would not be used
> constantly and interatively by the users, my preference is not
> to have any default at all, and always require --author or
> --committer.  You have to type a bit more when doing the script,
> but that needs to be done only once.  You will be sure which one
> you are asking from the command two weeks after you wrote the
> script so it is not a big loss.

Make sense.  Although I'm not quite certain we actually need the
information twice.  Possibly we just have GIT_AUTHOR_NAME and
GIT_AUTHOR_EMAIL, and then have commit-write take a flag to
override the author bit.  That would certainly make it less
confusing when setting up environmental variables for git.
And that would also give us a better name.

> I am not seriously suggesting the below as an alternative, but
> have you thought about doing an inverse function of your
> computation for the case when the user has all the environment
> variables, and have script eval its output, like this [*1*]:
>
>     $ git-id
>     GIT_AUTHOR_NAME='Junio C Hamano'
>     GIT_AUTHOR_EMAIL='junkio@cox.net'
>     GIT_COMMITTER_NAME='Junio C Hamano'
>     GIT_COMMITTER_EMAIL='junkio@cox.net'
>     $ eval "`git-id`"
>     $ tagger="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
>
> Having names and emails available separately may turn out to be
> easier to use in other situation.  Just a thought.

Part of the request was to put all of this information together
in a common place.  And note that it is actually:
tagger="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE"
Where the date is a human unreadable string of the number of seconds
since the epoch (aka 1 Jan 1970 UTC).

> By the way, I do not particularly like the name "git-id".  There
> could be IDs for different kinds (not just people) we would want
> later (file IDs, for example).  Naming what you are computing
> _the_ "id" feels a bit too generic.  I do not have a better
> alternative to suggest, though.

Agreed.  Something like git-author or git-author-stamp is probably
better.

> *1* This makes its output syntax a bit too specific to the shell
> and unfriendly to Porcelain written in other languages.  The
> only non-shell Porcelains I am aware of are done in Perl (I do
> not remember hearing its name) and Python (StGIT), both of which
> have reasonable regexp support to grok something like this, so
> it would not be a big issue.

And in git-commit-script this is actually parsed by sed which makes
it so the shell can parse the information as well so I think
we are fine in that sense.

Eric

Re: [PATCH] tagger id

[Petr Baudis]

Dear diary, on Tue, Jul 12, 2005 at 05:04:23PM CEST, I got a letter
where "Eric W. Biederman" <ebiederm@xmission.com> told me that...
> > By the way, I do not particularly like the name "git-id".  There
> > could be IDs for different kinds (not just people) we would want
> > later (file IDs, for example).  Naming what you are computing
> > _the_ "id" feels a bit too generic.  I do not have a better
> > alternative to suggest, though.
> 
> Agreed.  Something like git-author or git-author-stamp is probably
> better.

Since that "infriges" the author/committer distinction, I would prefer
git-person-id.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
<Espy> be careful, some twit might quote you out of context..

Re: [PATCH] tagger id

[Junio C Hamano]

Eric W. Biederman <ebiederm <at> xmission.com> writes:

> 
> Junio C Hamano <junkio <at> cox.net> writes:
>
> > The only in-tree user after your patch is applied is the tagger
> > stuff, so in that sense committer_ident may make more sense.
> 
> There is also the commit path, and that comes from C.  I'm not
> quite certain how we should be using the environmental variables.

But there you would not have "default" issue, would you?

> Part of the request was to put all of this information together
> in a common place.  And note that it is actually:
> tagger="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE"
> Where the date is a human unreadable string of the number of seconds
> since the epoch (aka 1 Jan 1970 UTC).

This may sound whacy, but how about having git-env command that

 (1) parrots GIT_* environment variables if the user has one; or
 (2) shows the values of environment variables the user _could_ have had
     to cause the program to behave the same way, when it the user does
     not have them?

Synopsis.

  $ git-env [--values-only] [<variable name>...]

Examples.

 $ git-env GIT_COMMITER_DATE GIT_AUTHOR_NAME
 GIT_COMMITTER_DATE='1121202267 -0700'
 GIT_AUTHOR_NAME='Junio C Hamano'
 $ unset GIT_OBJECT_DIRECTORY
 $ GIT_DIR=foo git-env --values-only GIT_OBJECT_DIRECTORY
 foo/objects
 $ git-env
 GIT_DIR=.git
 GIT_OBJECT_DIRECTORY=.git/objects
 ...
 $ eval "`git-env GIT_COMMITTER_DATE GIT_COMMITTER_EMAIL GIT_COMMITTER_DATE`"
 $ tagger="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE"

We could add a couple of "variable name"s that we do _not_ use
from the environment as a shorthand as well while we are at it,
so that you can say:

 $ git-env GIT_COMMITTER_ID
 GIT_COMMITTER_ID='Junio C Hamano <junkio@cox.net> 1121202267 -0700'

Once we go this route, it may even make sense to have that GIT_COMMITTER_ID
environment variable as well.  I don't know..

Re: [PATCH] tagger id

[Eric W. Biederman]

Junio C Hamano <junkio@twinsun.com> writes:

> Eric W. Biederman <ebiederm <at> xmission.com> writes:
>
>
>> Part of the request was to put all of this information together
>> in a common place.  And note that it is actually:
>> tagger="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE"
>> Where the date is a human unreadable string of the number of seconds
>> since the epoch (aka 1 Jan 1970 UTC).
>
> This may sound whacy, but how about having git-env command that
>
>  (1) parrots GIT_* environment variables if the user has one; or
>  (2) shows the values of environment variables the user _could_ have had
>      to cause the program to behave the same way, when it the user does
>      not have them?

I like the general idea.  But I hate the code implications for
echoing environmental variables that git cares about.  Especially
since we really don't care about the environmental variables.  Instead
we are doing this because we are doing things that the are best
not done in shell.

So I have made the program git-var [ -l | <variable name ]

Without the implicit tying we can just export those values
that we find interesting. -l will list all of the variables
and their values like env, while specifying an single variable
will just read that variables value.

Eric

Re: [PATCH] tagger id

[Linus Torvalds]

Eric,
 I ended up coding the ident stuff a bit differently, and I didn't do done
the tag/git-id part yet. Can you check out my latest commit (pushed out, 
but it will probably take a few minutes to mirror out), and do the final 
tag stuff based on that? 

		Linus

Re: [PATCH] tagger id

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> Eric,
>  I ended up coding the ident stuff a bit differently, and I didn't do done
> the tag/git-id part yet. Can you check out my latest commit (pushed out, 
> but it will probably take a few minutes to mirror out), and do the final 
> tag stuff based on that? 

For the most part it looks sane.   I'm not really thrilled that
setup_ident() calls die, and when complaining about the user
name we should probably complain that their sysadmin hated
then if it is over a 1000 characters not their parents :)

I'm also not at all thrilled with global variables.  Globals aren't
the source of all evil but they have a lot better claim than goto.
At least real_email and friends are file local.  If you like
it and the code works git is you project and I won't complain again.

Since we are still looking at this there is one change in the user
interface I would like to make to simplify things for the end user.
The only time when GIT_COMMITTER != GIT_AUTHOR is in git_commit_script
when we you are making a new commit based on an old commit. Can
we add a command line option to git-commit-write, --author
that will allow the author field to be overridden.  Allowing us
to get down to a single set of GIT variables for specifying who
the user is?

That also simplifies the tagging case and answers the question 
which environment variables tags should look at, to see who the
user is. 

Eric

Re: [PATCH] tagger id

[Junio C Hamano]

Eric W. Biederman <ebiederm <at> xmission.com> writes:

> Since we are still looking at this there is one change in the user
> interface I would like to make to simplify things for the end user.
> The only time when GIT_COMMITTER != GIT_AUTHOR is in git_commit_script
> when we you are making a new commit based on an old commit...

I am afraid I do not follow you.  For a "project lead" person like Linus, who
takes an e-mail submission of patches, GIT_AUTHOR is almost always different
from the committer, and typically set up by the program that reads the e-mail
to snarf the From: and Date: lines via environment variables, when the incoming
patches are being processed.  He is saying "I am the COMMITTER, and this commit
I am making is written by this AUTHOR".

AUTHOR can be set to somebody other than yourself and that is a typical mode
of operation for a "project lead" person.

On the other hand, we made COMMITTER overridable only because (1) the
computed value from the system are often not quite right on many systems
with weird GECOS field or domain/e-mail setup, and (2) when converting from
a foreign SCM, we wanted to keep the committer information (and dates), if
available.  Only in (2), which is quite a special case, COMMITTER names
somebody different from yourself.

What this means is that if you are asking the question "who the user is",
the answer _should_ always come from COMMITTER.  

> That also simplifies the tagging case and answers the question 
> which environment variables tags should look at, to see who the
> user is. 

The intent of "tags" (especially the signed kind) is to express "trust":
"This commit is called v2.6.12 and *I* vouch for it."

COMMITTER is the only sensible thing to use there, because (as you said)
what you care is "who I am", not "for whom I am doing this".  

Re: [PATCH] tagger id

[Eric W. Biederman]

Junio C Hamano <junkio@twinsun.com> writes:

> I am afraid I do not follow you.  

I was confused.  My big problem was that we don't really have
an in tree user, and there wasn't a good explanation anywhere.  So it
was hard to track this down. 

I'm going to lobby for a script to import patches from email being
in the git tree just so people can see how this is done, and probably
because there are a lot of people who have been reinventing this
script :)

> The intent of "tags" (especially the signed kind) is to express "trust":
> "This commit is called v2.6.12 and *I* vouch for it."
>
> COMMITTER is the only sensible thing to use there, because (as you said)
> what you care is "who I am", not "for whom I am doing this"

Sounds good.

Eric

Re: Trial git RPM's..

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> On Mon, 11 Jul 2005, Eric W. Biederman wrote:
>> 
>> A couple of pieces.  The dist target has assumes git-tar-tree is in the
>> path.  Making it so you have to have git installed to build the rpm.
>
> Yes. Maybe we could relax that requirement by using "./git-tar-tree" or 
> something? That still requires that you have _built_ git to do the rpm, 
> but at least you won't have had to install it.
>
> Does that fit the rpm build process? Or does an rpm build make something 
> like that really inconvenient? I don't know, patches welcome.

That would be sane.  The reason I worry about having it installed is
that if git-tar-tree changes without building first you will run
the old version instead of the new one.

>> The man pages are not built. The build dependencies do not call out
>> the tools necessary to build the man pages.
>
> Most people don't have asciidoc, and I'm not sure we want to require it. 
> Maybe we could have a separate "make man-rpm" target for that?

Or just have a make man target and only require the rpm to use it.
You certainly want to require making the man pages when building
the rpm.  Which means only those people who build rpms or build
man pages need asciidoc.  

>> And it does not pass my torture test of building rpm's on debian,
>> but that is not a huge problem.
>
> Ok, why is debian problematic? 

Mostly because debian is not rpm based.  If you are real careful
you can build rpm's on debian.   It is almost as bad as complaining
that git does not build on windows with Microsoft's compiler.  I was
getting a really generic error.  I need to look into it deeper to see
if is something that is avoidable.

> Is there some missing dependency or 
> something? I really haven't ever done an rpm, and the git rpm target was 
> all done by Chris Wright, so I don't have any clue at all. Again, patches 
> welcome.

Will do.

Eric

Re: Trial git RPM's..

[Chris Wright]

* Eric W. Biederman (ebiederm@xmission.com) wrote:
> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > Ok, I tagged a "v0.99" thing, and pushed it out. I've also made trial 
> > RPM's of it: src, ppc64 and x86. They're build on whatever random machines 
> > I had, and on the ppc64 I chose to do it on my FC4 machine that has newer 
> > libraries than my YDL one. The x86 thing is FC3, I do believe.
> >
> > I haven't really verified the RPM's in any other way than a trial 
> > installation on one machine, and "gitk" seemed to work. Whoop. The idea 
> > being that this is a good way to check whether the rpm target works, _and_ 
> > cogito can have something to build against.
> 
> A couple of pieces.  The dist target has assumes git-tar-tree is in the
> path.  Making it so you have to have git installed to build the rpm.

Known, and was a reasonable assumption in my environment.  It's simple
bootstrapping issue.

> The man pages are not built. The build dependencies do not call out
> the tools necessary to build the man pages.

That was rather intentional, because the asciidoc package is not common.

thanks,
-chris