threads / bug / 12681

git-fetch question/bug

Subject: git-fetch question/bug

## tl;dr

4 messages between Mar 14, 2008 and Mar 14, 2008.

replies: 3people: 4as markdown or json

Govind Salinas· Mar 14, 2008, 05:08 UTC · lore
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.

Junio C Hamano· Mar 14, 2008, 05:27 UTC · re: Govind Salinas · lore

Re: git-fetch question/bug

"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;
 }
 
 /*
Jakub Narebski· Mar 14, 2008, 07:52 UTC · re: Junio C Hamano · lore

Re: git-fetch question/bug

Junio C Hamano wrote:
Show 12 quoted lines
> "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
Daniel Barkalow· Mar 14, 2008, 15:57 UTC · re: Junio C Hamano · lore

Re: git-fetch question/bug

On Thu, 13 Mar 2008, Junio C Hamano wrote:
Show 12 quoted lines
> "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*

← back to recent threads