Junio C Hamano <junkio@xxxxxxx> writes:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
>> On Wed, 24 Jan 2007, Junio C Hamano wrote:
>>
>>> Ok, how about this, on top of the previous ones?
>>
>> Thanks!
>>
>>> @@ -653,6 +663,8 @@ int main(int argc, char **argv)
>>> struct stat st;
>>>
>>> setup_git_directory();
>>> + setup_ident();
>>> + git_config(fetch_pack_config);
>>
>> Why do you need setup_ident()?
>
> Because presumably you would be updating the reflog that records
> who did the fetch?
>
> But then we should do the same ignore_missing_committer_name()
> we have in receive-pack to allow anonymous fetchers to fetch
> from outside world, I guess.
This turns out to be more serious than I expected.
The code that uses committer_info() in reflog can barf and die.
And I do not think calling ignore_missing_committer_name()
upfront like recent receive-pack did in the aplication is a
reasonable workaround.
So I am thinking about doing something like the attached patch.
What the patch does.
- git_committer_info() takes one parameter. It used to be "if
this is true, then die() if the name is not available due to
bad GECOS, otherwise issue a warning once but leave the name
empty". The reason was because we wanted to prevent bad
commits from being made by git-commit-tree (and its
callers). The value 0 is only used by "git var -l".
Now it takes -1, 0 or 1. When set to -1, it does not
complain but uses the pw->pw_name when name is not
available. Existing 0 and 1 values mean the same thing as
they used to mean before. 0 means issue warnings and leave
it empty, 1 means barf and die.
- ignore_missing_committer_name() and its existing caller
(receive-pack, to set the reflog) have been removed.
- git-format-patch, to come up with the phoney message ID when
asked to thread, now passes -1 to git_committer_info(). This
codepath uses only the e-mail part, ignoring the name. It
used to barf and die. The other call in the same program
when asked to add signed-off-by line based on committer
identity still passes 1 to make sure it barfs instead of
adding a bogus s-o-b line.
- log_ref_write in refs.c, to come up with the name to record
who initiated the ref update in the reflog, passes -1. It
used to barf and die.
The last change means that git-update-ref, git-branch, and
commit walker backends can now be used in a repository with
reflog by somebody who does not have the user identity required
to make a commit. They all used to barf and die.
I've run tests and all of them seem to pass, and also tried "git
clone" as a user whose GECOS is empty -- git clone works again
now (it was broken when reflog was enabled by default).
But this definitely needs extra sets of eyeballs.
---
diff --git a/builtin-log.c b/builtin-log.c
index 503cd1e..56acc13 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -352,7 +352,7 @@ static void get_patch_ids(struct rev_info *rev, struct diff_options *options, co
static void gen_message_id(char *dest, unsigned int length, char *base)
{
- const char *committer = git_committer_info(1);
+ const char *committer = git_committer_info(-1);
const char *email_start = strrchr(committer, '<');
const char *email_end = strrchr(committer, '>');
if(!email_start || !email_end || email_start > email_end - 1)
diff --git a/cache.h b/cache.h
index 473197d..9486132 100644
--- a/cache.h
+++ b/cache.h
@@ -320,7 +320,6 @@ void datestamp(char *buf, int bufsize);
unsigned long approxidate(const char *);
extern int setup_ident(void);
-extern void ignore_missing_committer_name(void);
extern const char *git_author_info(int);
extern const char *git_committer_info(int);
diff --git a/fetch-pack.c b/fetch-pack.c
index 4df7450..83a1d7b 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -671,8 +671,6 @@ int main(int argc, char **argv)
setup_git_directory();
setup_ident();
- /* don't die if gecos is empty */
- ignore_missing_committer_name();
git_config(fetch_pack_config);
if (0 <= transfer_unpack_limit)
diff --git a/ident.c b/ident.c
index 6ad8fed..f967790 100644
--- a/ident.c
+++ b/ident.c
@@ -180,12 +180,21 @@ static const char *get_ident(const char *name, const char *email,
email = git_default_email;
if (!*name) {
- if (name == git_default_name && env_hint) {
+ struct passwd *pw;
+
+ if (0 <= error_on_no_name &&
+ name == git_default_name && env_hint) {
fprintf(stderr, env_hint, au_env, co_env);
env_hint = NULL; /* warn only once, for "git-var -l" */
}
- if (error_on_no_name)
+ if (0 < error_on_no_name)
die("empty ident %s <%s> not allowed", name, email);
+ pw = getpwuid(getuid());
+ if (!pw)
+ die("You don't exist. Go away!");
+ strlcpy(git_default_name, pw->pw_name,
+ sizeof(git_default_name));
+ name = git_default_name;
}
strcpy(date, git_default_date);
@@ -218,18 +227,3 @@ const char *git_committer_info(int error_on_no_name)
getenv("GIT_COMMITTER_DATE"),
error_on_no_name);
}
-
-void ignore_missing_committer_name()
-{
- /* If we did not get a name from the user's gecos entry then
- * git_default_name is empty; so instead load the username
- * into it as a 'good enough for now' approximation of who
- * this user is.
- */
- if (!*git_default_name) {
- struct passwd *pw = getpwuid(getuid());
- if (!pw)
- die("You don't exist. Go away!");
- strlcpy(git_default_name, pw->pw_name, sizeof(git_default_name));
- }
-}
diff --git a/receive-pack.c b/receive-pack.c
index 8b59b32..7d26326 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -430,8 +430,6 @@ int main(int argc, char **argv)
die("attempt to push into a shallow repository");
setup_ident();
- /* don't die if gecos is empty */
- ignore_missing_committer_name();
git_config(receive_pack_config);
if (0 <= transfer_unpack_limit)
diff --git a/refs.c b/refs.c
index 8117328..4323e9a 100644
--- a/refs.c
+++ b/refs.c
@@ -958,7 +958,7 @@ static int log_ref_write(struct ref_lock *lock,
lock->log_file, strerror(errno));
}
- committer = git_committer_info(1);
+ committer = git_committer_info(-1);
if (msg) {
maxlen = strlen(committer) + strlen(msg) + 2*40 + 5;
logrec = xmalloc(maxlen);
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html