[PATCH] rename_ref(): only print a warning when config-file update fails

[PATCH] rename_ref(): only print a warning when config-file update fails

[Lars Hjemli]

If git_config_rename_section() fails, rename_ref() used to return 1, which
left HEAD pointing to an absent refs/heads file (since the actual renaming
had already occurred).

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---

On 4/5/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
> 
> On Thu, 5 Apr 2007, Geert Bosch wrote:
> 
> > Make git_config_rename_section return success if no config file
> > exists.
> 
> I don't think this is correct. git_config_rename_section() _should_ return
> an error.
> 
> > Otherwise, renaming a branch would abort, leaving the repository in an
> > inconsistent state.
> 
> This should take the hint from --rename-section, and print a warning (or
> not).

I think both arguments makes sense. There really is no reason to abort the
rename operation if the config file update fails (for any reason).


 refs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index f471152..2ac6384 100644
--- a/refs.c
+++ b/refs.c
@@ -835,7 +835,7 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 		snprintf(oldsection, 1024, "branch.%s", oldref + 11);
 		snprintf(newsection, 1024, "branch.%s", newref + 11);
 		if (git_config_rename_section(oldsection, newsection) < 0)
-			return 1;
+			error("unable to update config-file");
 	}
 
 	return 0;
-- 
1.5.1.53.g77e6f

Re: [PATCH] rename_ref(): only print a warning when config-file update fails

[Geert Bosch]

On Apr 6, 2007, at 04:33, Lars Hjemli wrote:
> If git_config_rename_section() fails, rename_ref() used to return  
> 1, which
> left HEAD pointing to an absent refs/heads file (since the actual  
> renaming
> had already occurred).
>
> Signed-off-by: Lars Hjemli <hjemli@gmail.com>

This makes sense in addition to my patch, if the renaming
fails for any of the other possible reasons.

   -Geert

Re: [PATCH] rename_ref(): only print a warning when config-file update fails

[Junio C Hamano]

Lars Hjemli <hjemli@gmail.com> writes:

> If git_config_rename_section() fails, rename_ref() used to return 1, which
> left HEAD pointing to an absent refs/heads file (since the actual renaming
> had already occurred).

I wonder if rolling back the rename that was asked is an
option.  We would want to keep these low-level things atomic
whenever possible.

Re: [PATCH] rename_ref(): only print a warning when config-file update fails

[Lars Hjemli]

On 4/6/07, Junio C Hamano <junkio@cox.net> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
> > If git_config_rename_section() fails, rename_ref() used to return 1, which
> > left HEAD pointing to an absent refs/heads file (since the actual renaming
> > had already occurred).
>
> I wonder if rolling back the rename that was asked is an
> option.  We would want to keep these low-level things atomic
> whenever possible.

I was wondering the same thing, i.e. "goto rollback" as an option for
"error()". But I ended up thinking that rename_ref() shouldn't bother
with the config file at all (thus my other patch).

-- 
larsh

Re: [PATCH] rename_ref(): only print a warning when config-file update fails

[Junio C Hamano]

"Lars Hjemli" <hjemli@gmail.com> writes:

>> I wonder if rolling back the rename that was asked is an
>> option.  We would want to keep these low-level things atomic
>> whenever possible.
>
> I was wondering the same thing, i.e. "goto rollback" as an option for
> "error()". But I ended up thinking that rename_ref() shouldn't bother
> with the config file at all (thus my other patch).

I agree that "other patch" is sensible regardless.