threads / patch / 7526

patchFix renaming branch without config file

Subject: [PATCH] Fix renaming branch without config file

## tl;dr

5 messages between Apr 5, 2007 and Apr 6, 2007. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Geert Bosch· Apr 5, 2007, 14:20 UTC · lore

Make git_config_rename_section return success if no config file exists. Otherwise, renaming a branch would abort, leaving the repository in an inconsistent state.

Signed-off-by: Geert Bosch <bosch@gnat.com>
---
 config.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

Show changes to config.c +3 −2

diff --git a/config.c b/config.c
index 6479855..70d1055 100644
--- a/config.c
+++ b/config.c
@@ -916,8 +916,8 @@ int git_config_rename_section(const char *old_name, const char *new_name)
 	}
 
 	if (!(config_file = fopen(config_filename, "rb"))) {
-		ret = error("Could not open config file!");
-		goto out;
+		/* no config file means nothing to rename, no error */
+		goto unlock_and_out;
 	}
 
 	while (fgets(buf, sizeof(buf), config_file)) {
@@ -951,6 +951,7 @@ int git_config_rename_section(const char *old_name, const char *new_name)
 		}
 	}
 	fclose(config_file);
+ unlock_and_out:
 	if (close(out_fd) || commit_lock_file(lock) < 0)
 			ret = error("Cannot commit config file!");
  out:

-- 
1.4.5-rc0.GIT

Johannes Schindelin· Apr 5, 2007, 15:30 UTC · re: Geert Bosch · lore

Re: [PATCH] Fix renaming branch without config file

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).

Ciao, Dscho

Geert Bosch· Apr 5, 2007, 16:00 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Fix renaming branch without config file

On Apr 5, 2007, at 11:30, Johannes Schindelin wrote:

Show 11 quoted lines

> 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).

Looking at the code, I deduced that git_config_rename_section() returns a positive count of sections renamed, 0 if no matching section exists and negative for another error condition.

The proposed patch makes the behavior of an absent config file the same as the behavior of an empty one, which seems sane to me. There should be no warning, as it is perfectly fine to have no config file. Could you elaborate on why you believe my patch is not correct?

Currently, a repository without "config" file will be messed up by trying to rename a branch, as the rename will mostly succeed but leaves the HEAD dangling, requiring manual file editing to recover.

   -Geert

Johannes Schindelin· Apr 5, 2007, 17:51 UTC · re: Geert Bosch · lore

Re: [PATCH] Fix renaming branch without config file

Hi,

On Thu, 5 Apr 2007, Geert Bosch wrote:

Show 14 quoted lines

> 
> On Apr 5, 2007, at 11:30, Johannes Schindelin wrote:
> > 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).
> 
> Looking at the code, I deduced that git_config_rename_section()
> returns a positive count of sections renamed, 0 if no matching
> section exists and negative for another error condition.

Ah, yes. You are completely right!

> The proposed patch makes the behavior of an absent config file the same 
> as the behavior of an empty one, which seems sane to me. There should be 
> no warning, as it is perfectly fine to have no config file. Could you 
> elaborate on why you believe my patch is not correct?

I elaborate by taking my objections back. Fair enough?

Ciao, Dscho

Geert Bosch· Apr 6, 2007, 11:33 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Fix renaming branch without config file

On Apr 5, 2007, at 13:51, Johannes Schindelin wrote:

Show 10 quoted lines

> Ah, yes. You are completely right!
>
>> The proposed patch makes the behavior of an absent config file the  
>> same
>> as the behavior of an empty one, which seems sane to me. There  
>> should be
>> no warning, as it is perfectly fine to have no config file. Could you
>> elaborate on why you believe my patch is not correct?
>
> I elaborate by taking my objections back. Fair enough?

Sure ;-)

Thanks for commenting on my patch.

   -Geert