Re: [PATCH] tagger id
Junio C Hamano [EMAIL PROTECTED] 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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tagger id
Junio C Hamano [EMAIL PROTECTED] 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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tagger id
[EMAIL PROTECTED] (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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tagger id
Junio C Hamano [EMAIL PROTECTED] writes: [EMAIL PROTECTED] (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='[EMAIL PROTECTED]' GIT_COMMITTER_NAME='Junio C Hamano' GIT_COMMITTER_EMAIL='[EMAIL PROTECTED]' $ 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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tagger id
Dear diary, on Tue, Jul 12, 2005 at 05:04:23PM CEST, I got a letter where Eric W. Biederman [EMAIL PROTECTED] 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.. - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tagger id
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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tagger id
Linus Torvalds [EMAIL PROTECTED] 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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tagger id
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. - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html