git-fetch question/bug

git-fetch question/bug

[Govind Salinas]

Hey,

I was wrapping git-fetch when I discovered that it does not return
error when the
fetch fails due to it not being a ff...

~/Desktop/Projects/test  (test2)
govind@gnosis: 574> git fetch . remotes/origin/gitnext:test2
From .
 ! [rejected]        origin/gitnext -> test2  (non fast forward)
~/Desktop/Projects/test  (test2)
govind@gnosis: 575> echo $?
0
~/Desktop/Projects/test  (test2)
govind@gnosis: 576> git version
git version 1.5.4.3.587.g0bdd73


So, is this the expected behavior?  I expect it should return non-0
when it does not
perform the requested fetch.

If this is supposed to be this way, then sorry for the noise.

Thanks,
Govind.

Re: git-fetch question/bug

[Junio C Hamano]

"Govind Salinas" <blix@sophiasuchtig.com> writes:

> ... git-fetch when I discovered that it does not return
> error when the
> fetch fails due to it not being a ff...

I think this is a regression introduced when "git-fetch" was
re-implemented in C.  git-fetch--tool's native-store subcommand seems to
have signaled this as an error, and it is reasonable to expect an error
exit from the command in this case.

Probably something like this?

 builtin-fetch.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 55f611e..a2deb3e 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -297,7 +297,7 @@ static int store_updated_refs(const char *url, struct ref *ref_map)
 {
 	FILE *fp;
 	struct commit *commit;
-	int url_len, i, note_len, shown_url = 0;
+	int url_len, i, note_len, shown_url = 0, err = 0;
 	char note[1024];
 	const char *what, *kind;
 	struct ref *rm;
@@ -364,7 +364,8 @@ static int store_updated_refs(const char *url, struct ref *ref_map)
 			note);
 
 		if (ref) {
-			update_local_ref(ref, what, verbose, note);
+			if (update_local_ref(ref, what, verbose, note))
+				err = 1;
 			if (*note) {
 				if (!shown_url) {
 					fprintf(stderr, "From %.*s\n",
@@ -376,7 +377,7 @@ static int store_updated_refs(const char *url, struct ref *ref_map)
 		}
 	}
 	fclose(fp);
-	return 0;
+	return err;
 }
 
 /*

Re: git-fetch question/bug

[Jakub Narebski]

Junio C Hamano wrote:

> "Govind Salinas" <blix@sophiasuchtig.com> writes:
> 
>> ... git-fetch when I discovered that it does not return
>> error when the
>> fetch fails due to it not being a ff...
> 
> I think this is a regression introduced when "git-fetch" was
> re-implemented in C.  git-fetch--tool's native-store subcommand seems to
> have signaled this as an error, and it is reasonable to expect an error
> exit from the command in this case.
> 
> Probably something like this?

[...]

Wouldn't it be better to distinguish somehow remote side errors (like for
example: could not connect to remote server) and local side errors (like
fetch refused because of being not fast-forward)? The former are usually
not recoverable (unless it is URL that is mistyped), the latter could be
usually resolved (forced for example)...

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: git-fetch question/bug

[Daniel Barkalow]

On Thu, 13 Mar 2008, Junio C Hamano wrote:

> "Govind Salinas" <blix@sophiasuchtig.com> writes:
> 
> > ... git-fetch when I discovered that it does not return
> > error when the
> > fetch fails due to it not being a ff...
> 
> I think this is a regression introduced when "git-fetch" was
> re-implemented in C.  git-fetch--tool's native-store subcommand seems to
> have signaled this as an error, and it is reasonable to expect an error
> exit from the command in this case.
> 
> Probably something like this?

Looks right to me, although we should probably note that this probably 
means that if pull does a fetch that fails for some ref that you're not 
merging, it won't do the merge, even though it obviously could. So I think 
fixing this would presently (unless fetch and pull communicate more) be a 
regression for people whose upstreams introduce a new non-ff branch that 
matches their non-force fetch pattern.

	-Daniel
*This .sig left intentionally blank*