update-ref logs: problem with committer info?

update-ref logs: problem with committer info?

[Ramsay Jones]

Hi all,

As a follow-up to the previous (two) email, this concerns the ability
to override the committer info from the config file. As before, this
depends on the relative call sites of setup_ident() and git_config().

In particular, the call to setup_ident(), from log_ref_write(),
which in turn is called from write_ref_sha1() in file refs.c.
Here, the call to setup_ident() seems to me to be misplaced and to
almost guarantee that it will over-write the values set by an
earlier call to git_config(). It is possible, I suppose, that there
has been no previous call to git_config(); just not very likely, and
even if that were so, then it should be the responsibility of the
*callers* of write_ref_sha1() to ensure this happens.

Indeed, tracing the callers of write_ref_sha1(), we find cmd_update_ref()
in builtin-update-ref.c and pull() in fetch.c. Adding a call to
setup_ident() in cmd_update_ref() takes care of that route. The callers
of pull() are to be found in http-fetch.c, local-fetch.c and ssh-fetch.c.
Adding calls to setup_ident() in these callers is equally simple, so it
only remains to remove the call from refs.c.

Now, as in the previous email, I can't compile http-fetch.c or test
the result. (Yes, I could test a local fetch, but I'm feeling lazy!)
In addition, I feel that somebody who knows the code better than I
should take a look.

comments?

Ramsay

diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index 00333c7..83094ab 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -12,6 +12,7 @@ int cmd_update_ref(int argc, const char 
 	unsigned char sha1[20], oldsha1[20];
 	int i;
 
+	setup_ident();
 	setup_git_directory();
 	git_config(git_default_config);
 
diff --git a/http-fetch.c b/http-fetch.c
index 44eba5f..fe3a4fd 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -1222,6 +1222,7 @@ int main(int argc, char **argv)
 	int arg = 1;
 	int rc = 0;
 
+	setup_ident();
 	setup_git_directory();
 	git_config(git_default_config);
 
diff --git a/local-fetch.c b/local-fetch.c
index ffa4887..d059a51 100644
--- a/local-fetch.c
+++ b/local-fetch.c
@@ -207,6 +207,7 @@ int main(int argc, char **argv)
 	char *commit_id;
 	int arg = 1;
 
+	setup_ident();
 	setup_git_directory();
 	git_config(git_default_config);
 
diff --git a/refs.c b/refs.c
index 713ca46..a4060d8 100644
--- a/refs.c
+++ b/refs.c
@@ -379,7 +379,6 @@ static int log_ref_write(struct ref_lock
 			lock->log_file, strerror(errno));
 	}
 
-	setup_ident();
 	comitter = git_committer_info(1);
 	if (msg) {
 		maxlen = strlen(comitter) + strlen(msg) + 2*40 + 5;
diff --git a/ssh-fetch.c b/ssh-fetch.c
index 1e59cd2..a42d17e 100644
--- a/ssh-fetch.c
+++ b/ssh-fetch.c
@@ -131,6 +131,7 @@ int main(int argc, char **argv)
 	prog = getenv("GIT_SSH_PUSH");
 	if (!prog) prog = "git-ssh-upload";
 
+	setup_ident();
 	setup_git_directory();
 	git_config(git_default_config);
 

Re: update-ref logs: problem with committer info?

[Shawn Pearce]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:
> diff --git a/builtin-update-ref.c b/builtin-update-ref.c
> index 00333c7..83094ab 100644
> --- a/builtin-update-ref.c
> +++ b/builtin-update-ref.c
> @@ -12,6 +12,7 @@ int cmd_update_ref(int argc, const char 
>  	unsigned char sha1[20], oldsha1[20];
>  	int i;
>  
> +	setup_ident();
>  	setup_git_directory();
>  	git_config(git_default_config);
> diff --git a/refs.c b/refs.c
> index 713ca46..a4060d8 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -379,7 +379,6 @@ static int log_ref_write(struct ref_lock
>  			lock->log_file, strerror(errno));
>  	}
>  
> -	setup_ident();
>  	comitter = git_committer_info(1);
>  	if (msg) {
>  		maxlen = strlen(comitter) + strlen(msg) + 2*40 + 5;

These two changes were already fixed by me in 0b0fe4a6 on July
10th.  That change is in `next`, in `master` and in v1.4.2-rc3.
So I expect it to be available in a final release real-soon-now.
Maybe you should consider running a newer version of GIT?

> diff --git a/http-fetch.c b/http-fetch.c
> index 44eba5f..fe3a4fd 100644
> --- a/http-fetch.c
> +++ b/http-fetch.c
> @@ -1222,6 +1222,7 @@ int main(int argc, char **argv)
>  	int arg = 1;
>  	int rc = 0;
>  
> +	setup_ident();
>  	setup_git_directory();
>  	git_config(git_default_config);
>  
> diff --git a/local-fetch.c b/local-fetch.c
> index ffa4887..d059a51 100644
> --- a/local-fetch.c
> +++ b/local-fetch.c
> @@ -207,6 +207,7 @@ int main(int argc, char **argv)
>  	char *commit_id;
>  	int arg = 1;
>  
> +	setup_ident();
>  	setup_git_directory();
>  	git_config(git_default_config);
>  
> diff --git a/ssh-fetch.c b/ssh-fetch.c
> index 1e59cd2..a42d17e 100644
> --- a/ssh-fetch.c
> +++ b/ssh-fetch.c
> @@ -131,6 +131,7 @@ int main(int argc, char **argv)
>  	prog = getenv("GIT_SSH_PUSH");
>  	if (!prog) prog = "git-ssh-upload";
>  
> +	setup_ident();
>  	setup_git_directory();
>  	git_config(git_default_config);
>  

These changes aren't in `next` right now, but should be.  Junio,
can you apply them?

-- 
Shawn.

RE: update-ref logs: problem with committer info?

[Ramsay Jones]

On Sat 2006-08-05 at 3:56, spearce@spearce.org wrote:

> Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:
> > diff --git a/builtin-update-ref.c b/builtin-update-ref.c
> > index 00333c7..83094ab 100644
> > --- a/builtin-update-ref.c
> > +++ b/builtin-update-ref.c
> > @@ -12,6 +12,7 @@ int cmd_update_ref(int argc, const char
> >  	unsigned char sha1[20], oldsha1[20];
> >  	int i;
> >
> > +	setup_ident();
> >  	setup_git_directory();
> >  	git_config(git_default_config);
> > diff --git a/refs.c b/refs.c
> > index 713ca46..a4060d8 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -379,7 +379,6 @@ static int log_ref_write(struct ref_lock
> >  			lock->log_file, strerror(errno));
> >  	}
> >
> > -	setup_ident();
> >  	comitter = git_committer_info(1);
> >  	if (msg) {
> >  		maxlen = strlen(comitter) + strlen(msg) + 2*40 + 5;
>
> These two changes were already fixed by me in 0b0fe4a6 on July
> 10th.  That change is in `next`, in `master` and in v1.4.2-rc3.
> So I expect it to be available in a final release real-soon-now.
> Maybe you should consider running a newer version of GIT?
>

Yes, I am quite a long way behind, at version 1.4.1, which is light-years
in git-time!  Unfortunately, since I can't pull from the git repo (I don't
have internet access from Linux), I won't be able to update until the v1.4.2
tar-ball is posted. Yep, not exactly ideal.

OK, for now I will refrain from contributing, at least until v1.4.2 is out.

Sorry for the noise.

Ramsay

Re: update-ref logs: problem with committer info?

[Shawn Pearce]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:
> 
> Yes, I am quite a long way behind, at version 1.4.1, which is light-years
> in git-time!  Unfortunately, since I can't pull from the git repo (I don't
> have internet access from Linux), I won't be able to update until the v1.4.2
> tar-ball is posted. Yep, not exactly ideal.

Yea, that's not much fun.  I found that tracking GIT with GIT is
really the easiest way to stay current.  Not having direct net
access makes that nearly impossible.
 
> OK, for now I will refrain from contributing, at least until v1.4.2 is out.
> 
> Sorry for the noise.

Its not noise, three of your five changes were current bugs that
needed fixing.  :-)

-- 
Shawn.

RE: update-ref logs: problem with committer info?

[Robin Rosenberg]

> Yes, I am quite a long way behind, at version 1.4.1, which is light-years
> in git-time!  Unfortunately, since I can't pull from the git repo (I don't
> have internet access from Linux), I won't be able to update until the
> v1.4.2 tar-ball is posted. Yep, not exactly ideal.

Couldn't you track git with another machine? Then copy that clone to
your linux box and then do a local pull from the copied clone. 

There's also the http_proxy, but I guess you know that already.

-- robin