# [PATCH] rename_ref(): only print a warning when config-file update fails 5 messages from 2007-04-06 to 2007-04-07. Participants: Lars Hjemli, Geert Bosch, Junio C Hamano. Thread: https://gitlist.dev/t/7545 ## Lars Hjemli, 2007-04-06 08:33 Subject: [PATCH] rename_ref(): only print a warning when config-file update fails Message-ID: <11758483861506-git-send-email-hjemli@gmail.com> URL: https://gitlist.dev/e/11758483861506-git-send-email-hjemli%40gmail.com ``` 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 --- On 4/5/07, Johannes Schindelin 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 ``` ## Geert Bosch, 2007-04-06 10:35 Subject: Re: [PATCH] rename_ref(): only print a warning when config-file update fails Message-ID: <2FE08503-8936-4EB9-9DBF-08AFE2923BD9@gnat.com> URL: https://gitlist.dev/e/2FE08503-8936-4EB9-9DBF-08AFE2923BD9%40gnat.com In-Reply-To: <11758483861506-git-send-email-hjemli@gmail.com> ``` 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 This makes sense in addition to my patch, if the renaming fails for any of the other possible reasons. -Geert ``` ## Junio C Hamano, 2007-04-06 20:35 Subject: Re: [PATCH] rename_ref(): only print a warning when config-file update fails Message-ID: <7v1wixkzpz.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v1wixkzpz.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <11758483861506-git-send-email-hjemli@gmail.com> ``` Lars Hjemli 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. ``` ## Lars Hjemli, 2007-04-06 23:53 Subject: Re: [PATCH] rename_ref(): only print a warning when config-file update fails Message-ID: <8c5c35580704061653x7e5fd149ud31a8289a4508b54@mail.gmail.com> URL: https://gitlist.dev/e/8c5c35580704061653x7e5fd149ud31a8289a4508b54%40mail.gmail.com In-Reply-To: <7v1wixkzpz.fsf@assigned-by-dhcp.cox.net> ``` On 4/6/07, Junio C Hamano wrote: > Lars Hjemli 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 ``` ## Junio C Hamano, 2007-04-07 00:14 Subject: Re: [PATCH] rename_ref(): only print a warning when config-file update fails Message-ID: <7v6489hwfy.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v6489hwfy.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <8c5c35580704061653x7e5fd149ud31a8289a4508b54@mail.gmail.com> ``` "Lars Hjemli" 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. ```