[PATCH] Fix renaming branch without config file

Subsystems: the rest

DORMANTno replies

5 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] Fix renaming branch without config file

From: Geert Bosch <hidden>
Date: 2016-06-15 22:43:03

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 <redacted>
---
 config.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
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

Re: [PATCH] Fix renaming branch without config file

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:03

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

Re: [PATCH] Fix renaming branch without config file

From: Geert Bosch <hidden>
Date: 2016-06-15 22:43:03

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

Re: [PATCH] Fix renaming branch without config file

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:03

Hi,

On Thu, 5 Apr 2007, Geert Bosch wrote:
On Apr 5, 2007, at 11:30, Johannes Schindelin wrote:
quoted
I don't think this is correct. git_config_rename_section() _should_ return
an error.
quoted
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

Re: [PATCH] Fix renaming branch without config file

From: Geert Bosch <hidden>
Date: 2016-06-15 22:43:03

On Apr 5, 2007, at 13:51, Johannes Schindelin wrote:
Ah, yes. You are completely right!
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help