* [PATCH v4 01/15] config: introduce set_or_die wrappers
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 02/15] branch: die on error in setting up tracking branch Patrick Steinhardt
` (15 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
A lot of call-sites for the existing family of `git_config_set`
functions do not check for errors that may occur, e.g. when the
configuration file is locked. In many cases we simply want to die
when such a situation arises.
Introduce wrappers that will cause the program to die in those
cases. These wrappers are temporary only to ease the transition
to let `git_config_set` die by default. They will be removed
later on when `git_config_set` itself has been replaced by
`git_config_set_gently`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
cache.h | 4 ++++
config.c | 27 +++++++++++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/cache.h b/cache.h
index dfc459c..c1f844d 100644
--- a/cache.h
+++ b/cache.h
@@ -1508,11 +1508,15 @@ extern int git_config_maybe_bool(const char *, const char *);
extern int git_config_string(const char **, const char *, const char *);
extern int git_config_pathname(const char **, const char *, const char *);
extern int git_config_set_in_file(const char *, const char *, const char *);
+extern void git_config_set_in_file_or_die(const char *, const char *, const char *);
extern int git_config_set(const char *, const char *);
+extern void git_config_set_or_die(const char *, const char *);
extern int git_config_parse_key(const char *, char **, int *);
extern int git_config_key_is_valid(const char *key);
extern int git_config_set_multivar(const char *, const char *, const char *, int);
+extern void git_config_set_multivar_or_die(const char *, const char *, const char *, int);
extern int git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
+extern void git_config_set_multivar_in_file_or_die(const char *, const char *, const char *, const char *, int);
extern int git_config_rename_section(const char *, const char *);
extern int git_config_rename_section_in_file(const char *, const char *, const char *);
extern const char *git_etc_gitconfig(void);
diff --git a/config.c b/config.c
index 86a5eb2..856f7d34 100644
--- a/config.c
+++ b/config.c
@@ -1831,11 +1831,22 @@ int git_config_set_in_file(const char *config_filename,
return git_config_set_multivar_in_file(config_filename, key, value, NULL, 0);
}
+void git_config_set_in_file_or_die(const char *config_filename,
+ const char *key, const char *value)
+{
+ git_config_set_multivar_in_file_or_die(config_filename, key, value, NULL, 0);
+}
+
int git_config_set(const char *key, const char *value)
{
return git_config_set_multivar(key, value, NULL, 0);
}
+void git_config_set_or_die(const char *key, const char *value)
+{
+ git_config_set_multivar_or_die(key, value, NULL, 0);
+}
+
/*
* Auxiliary function to sanity-check and split the key into the section
* identifier and variable name.
@@ -2179,6 +2190,15 @@ write_err_out:
}
+void git_config_set_multivar_in_file_or_die(const char *config_filename,
+ const char *key, const char *value,
+ const char *value_regex, int multi_replace)
+{
+ if (git_config_set_multivar_in_file(config_filename, key, value,
+ value_regex, multi_replace) < 0)
+ die(_("Could not set '%s' to '%s'"), key, value);
+}
+
int git_config_set_multivar(const char *key, const char *value,
const char *value_regex, int multi_replace)
{
@@ -2186,6 +2206,13 @@ int git_config_set_multivar(const char *key, const char *value,
multi_replace);
}
+void git_config_set_multivar_or_die(const char *key, const char *value,
+ const char *value_regex, int multi_replace)
+{
+ git_config_set_multivar_in_file_or_die(NULL, key, value, value_regex,
+ multi_replace);
+}
+
static int section_name_match (const char *buf, const char *name)
{
int i = 0, j = 0, dot = 0;
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v4 02/15] branch: die on error in setting up tracking branch
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 01/15] config: introduce set_or_die wrappers Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 20:49 ` Junio C Hamano
2016-02-02 11:51 ` [PATCH v4 03/15] branch: die on config error when unsetting upstream Patrick Steinhardt
` (14 subsequent siblings)
16 siblings, 1 reply; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
The setup_tracking function calls install_branch_config, which
may fail writing the configuration due to a locked configuration
file or other error conditions. setup_tracking can also fail when
trying to track ambiguous information for a reference. While this
condition is checked for and an error code is returned, this
error is not checked by the caller.
Fix both issues by dying early when error occur.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
branch.c | 19 +++++++++----------
branch.h | 1 +
t/t3200-branch.sh | 9 ++++++++-
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/branch.c b/branch.c
index 7ff3f20..7106369 100644
--- a/branch.c
+++ b/branch.c
@@ -64,16 +64,16 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
}
strbuf_addf(&key, "branch.%s.remote", local);
- git_config_set(key.buf, origin ? origin : ".");
+ git_config_set_or_die(key.buf, origin ? origin : ".");
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.merge", local);
- git_config_set(key.buf, remote);
+ git_config_set_or_die(key.buf, remote);
if (rebasing) {
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.rebase", local);
- git_config_set(key.buf, "true");
+ git_config_set_or_die(key.buf, "true");
}
strbuf_release(&key);
@@ -109,8 +109,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
* to infer the settings for branch.<new_ref>.{remote,merge} from the
* config.
*/
-static int setup_tracking(const char *new_ref, const char *orig_ref,
- enum branch_track track, int quiet)
+static void setup_tracking(const char *new_ref, const char *orig_ref,
+ enum branch_track track, int quiet)
{
struct tracking tracking;
int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
@@ -118,7 +118,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
memset(&tracking, 0, sizeof(tracking));
tracking.spec.dst = (char *)orig_ref;
if (for_each_remote(find_tracked_branch, &tracking))
- return 1;
+ return;
if (!tracking.matches)
switch (track) {
@@ -127,18 +127,17 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
case BRANCH_TRACK_OVERRIDE:
break;
default:
- return 1;
+ return;
}
if (tracking.matches > 1)
- return error(_("Not tracking: ambiguous information for ref %s"),
- orig_ref);
+ die(_("Not tracking: ambiguous information for ref %s"),
+ orig_ref);
install_branch_config(config_flags, new_ref, tracking.remote,
tracking.src ? tracking.src : orig_ref);
free(tracking.src);
- return 0;
}
int read_branch_desc(struct strbuf *buf, const char *branch_name)
diff --git a/branch.h b/branch.h
index 58aa45f..8ce22f8 100644
--- a/branch.h
+++ b/branch.h
@@ -43,6 +43,7 @@ void remove_branch_state(void);
/*
* Configure local branch "local" as downstream to branch "remote"
* from remote "origin". Used by git branch --set-upstream.
+ * Dies if unable to install branch config.
*/
#define BRANCH_CONFIG_VERBOSE 01
extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index cdaf6f6..dd776b3 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -446,6 +446,13 @@ test_expect_success '--set-upstream-to fails on a non-ref' '
test_must_fail git branch --set-upstream-to HEAD^{}
'
+test_expect_success '--set-upstream-to fails on locked config' '
+ test_when_finished "rm -f .git/config.lock" &&
+ >.git/config.lock &&
+ git branch locked &&
+ test_must_fail git branch --set-upstream-to locked
+'
+
test_expect_success 'use --set-upstream-to modify HEAD' '
test_config branch.master.remote foo &&
test_config branch.master.merge foo &&
@@ -579,7 +586,7 @@ test_expect_success 'avoid ambiguous track' '
git config remote.ambi1.fetch refs/heads/lalala:refs/heads/master &&
git config remote.ambi2.url lilili &&
git config remote.ambi2.fetch refs/heads/lilili:refs/heads/master &&
- git branch all1 master &&
+ test_must_fail git branch all1 master &&
test -z "$(git config branch.all1.merge)"
'
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v4 02/15] branch: die on error in setting up tracking branch
2016-02-02 11:51 ` [PATCH v4 02/15] branch: die on error in setting up tracking branch Patrick Steinhardt
@ 2016-02-02 20:49 ` Junio C Hamano
2016-02-08 13:42 ` Patrick Steinhardt
0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2016-02-02 20:49 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jeff King
Patrick Steinhardt <ps@pks.im> writes:
> The setup_tracking function calls install_branch_config, which
> may fail writing the configuration due to a locked configuration
> file or other error conditions. setup_tracking can also fail when
> trying to track ambiguous information for a reference. While this
> condition is checked for and an error code is returned, this
> error is not checked by the caller.
>
> Fix both issues by dying early when error occur.
Hmph. I think create_branch() is written in such a way that all
die() come before the actual ref transaction attempts to create the
branch, but this change means that we have already created the
branch and then die without undoing the damage that is already done.
"The error is not checked by the caller" is very true, but can the
caller do something better than just die? I personally do not think
it is such a big deal if we just died here, but I may have overlooked
something.
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> branch.c | 19 +++++++++----------
> branch.h | 1 +
> t/t3200-branch.sh | 9 ++++++++-
> 3 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/branch.c b/branch.c
> index 7ff3f20..7106369 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -64,16 +64,16 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
> }
>
> strbuf_addf(&key, "branch.%s.remote", local);
> - git_config_set(key.buf, origin ? origin : ".");
> + git_config_set_or_die(key.buf, origin ? origin : ".");
>
> strbuf_reset(&key);
> strbuf_addf(&key, "branch.%s.merge", local);
> - git_config_set(key.buf, remote);
> + git_config_set_or_die(key.buf, remote);
>
> if (rebasing) {
> strbuf_reset(&key);
> strbuf_addf(&key, "branch.%s.rebase", local);
> - git_config_set(key.buf, "true");
> + git_config_set_or_die(key.buf, "true");
> }
> strbuf_release(&key);
>
> @@ -109,8 +109,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
> * to infer the settings for branch.<new_ref>.{remote,merge} from the
> * config.
> */
> -static int setup_tracking(const char *new_ref, const char *orig_ref,
> - enum branch_track track, int quiet)
> +static void setup_tracking(const char *new_ref, const char *orig_ref,
> + enum branch_track track, int quiet)
> {
> struct tracking tracking;
> int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
> @@ -118,7 +118,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
> memset(&tracking, 0, sizeof(tracking));
> tracking.spec.dst = (char *)orig_ref;
> if (for_each_remote(find_tracked_branch, &tracking))
> - return 1;
> + return;
>
> if (!tracking.matches)
> switch (track) {
> @@ -127,18 +127,17 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
> case BRANCH_TRACK_OVERRIDE:
> break;
> default:
> - return 1;
> + return;
> }
>
> if (tracking.matches > 1)
> - return error(_("Not tracking: ambiguous information for ref %s"),
> - orig_ref);
> + die(_("Not tracking: ambiguous information for ref %s"),
> + orig_ref);
>
> install_branch_config(config_flags, new_ref, tracking.remote,
> tracking.src ? tracking.src : orig_ref);
>
> free(tracking.src);
> - return 0;
> }
>
> int read_branch_desc(struct strbuf *buf, const char *branch_name)
> diff --git a/branch.h b/branch.h
> index 58aa45f..8ce22f8 100644
> --- a/branch.h
> +++ b/branch.h
> @@ -43,6 +43,7 @@ void remove_branch_state(void);
> /*
> * Configure local branch "local" as downstream to branch "remote"
> * from remote "origin". Used by git branch --set-upstream.
> + * Dies if unable to install branch config.
> */
> #define BRANCH_CONFIG_VERBOSE 01
> extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote);
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index cdaf6f6..dd776b3 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -446,6 +446,13 @@ test_expect_success '--set-upstream-to fails on a non-ref' '
> test_must_fail git branch --set-upstream-to HEAD^{}
> '
>
> +test_expect_success '--set-upstream-to fails on locked config' '
> + test_when_finished "rm -f .git/config.lock" &&
> + >.git/config.lock &&
> + git branch locked &&
> + test_must_fail git branch --set-upstream-to locked
> +'
> +
> test_expect_success 'use --set-upstream-to modify HEAD' '
> test_config branch.master.remote foo &&
> test_config branch.master.merge foo &&
> @@ -579,7 +586,7 @@ test_expect_success 'avoid ambiguous track' '
> git config remote.ambi1.fetch refs/heads/lalala:refs/heads/master &&
> git config remote.ambi2.url lilili &&
> git config remote.ambi2.fetch refs/heads/lilili:refs/heads/master &&
> - git branch all1 master &&
> + test_must_fail git branch all1 master &&
> test -z "$(git config branch.all1.merge)"
> '
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v4 02/15] branch: die on error in setting up tracking branch
2016-02-02 20:49 ` Junio C Hamano
@ 2016-02-08 13:42 ` Patrick Steinhardt
2016-02-08 14:03 ` Patrick Steinhardt
0 siblings, 1 reply; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-08 13:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
[-- Attachment #1: Type: text/plain, Size: 6160 bytes --]
On Tue, Feb 02, 2016 at 12:49:26PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > The setup_tracking function calls install_branch_config, which
> > may fail writing the configuration due to a locked configuration
> > file or other error conditions. setup_tracking can also fail when
> > trying to track ambiguous information for a reference. While this
> > condition is checked for and an error code is returned, this
> > error is not checked by the caller.
> >
> > Fix both issues by dying early when error occur.
>
> Hmph. I think create_branch() is written in such a way that all
> die() come before the actual ref transaction attempts to create the
> branch, but this change means that we have already created the
> branch and then die without undoing the damage that is already done.
>
> "The error is not checked by the caller" is very true, but can the
> caller do something better than just die? I personally do not think
> it is such a big deal if we just died here, but I may have overlooked
> something.
Well, when dying here we do not record the tracking branch
configuration for the newly set up branch. This is the only thing
that we are missing as right after setting up the tracking branch
we've finished and exit the command.
That being said it's somewhat unfortunate to die here as the user
cannot simply try to repeat creating a branch and hope it works
this time as the branch has already been created and the command
would error out. Maybe we should instead die with an improved
error message hinting the user how to fix the issue, something
along the lines of
"We were unable to set the remote tracking information for the
newly created branch due to <REASON>. After fixing the underlying
problem you may set the remote tracking branch by executing `git
branch --set-upstream-to=<BRANCH>."
> > ---
> > branch.c | 19 +++++++++----------
> > branch.h | 1 +
> > t/t3200-branch.sh | 9 ++++++++-
> > 3 files changed, 18 insertions(+), 11 deletions(-)
> >
> > diff --git a/branch.c b/branch.c
> > index 7ff3f20..7106369 100644
> > --- a/branch.c
> > +++ b/branch.c
> > @@ -64,16 +64,16 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
> > }
> >
> > strbuf_addf(&key, "branch.%s.remote", local);
> > - git_config_set(key.buf, origin ? origin : ".");
> > + git_config_set_or_die(key.buf, origin ? origin : ".");
> >
> > strbuf_reset(&key);
> > strbuf_addf(&key, "branch.%s.merge", local);
> > - git_config_set(key.buf, remote);
> > + git_config_set_or_die(key.buf, remote);
> >
> > if (rebasing) {
> > strbuf_reset(&key);
> > strbuf_addf(&key, "branch.%s.rebase", local);
> > - git_config_set(key.buf, "true");
> > + git_config_set_or_die(key.buf, "true");
> > }
> > strbuf_release(&key);
> >
> > @@ -109,8 +109,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
> > * to infer the settings for branch.<new_ref>.{remote,merge} from the
> > * config.
> > */
> > -static int setup_tracking(const char *new_ref, const char *orig_ref,
> > - enum branch_track track, int quiet)
> > +static void setup_tracking(const char *new_ref, const char *orig_ref,
> > + enum branch_track track, int quiet)
> > {
> > struct tracking tracking;
> > int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
> > @@ -118,7 +118,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
> > memset(&tracking, 0, sizeof(tracking));
> > tracking.spec.dst = (char *)orig_ref;
> > if (for_each_remote(find_tracked_branch, &tracking))
> > - return 1;
> > + return;
> >
> > if (!tracking.matches)
> > switch (track) {
> > @@ -127,18 +127,17 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
> > case BRANCH_TRACK_OVERRIDE:
> > break;
> > default:
> > - return 1;
> > + return;
> > }
> >
> > if (tracking.matches > 1)
> > - return error(_("Not tracking: ambiguous information for ref %s"),
> > - orig_ref);
> > + die(_("Not tracking: ambiguous information for ref %s"),
> > + orig_ref);
> >
> > install_branch_config(config_flags, new_ref, tracking.remote,
> > tracking.src ? tracking.src : orig_ref);
> >
> > free(tracking.src);
> > - return 0;
> > }
> >
> > int read_branch_desc(struct strbuf *buf, const char *branch_name)
> > diff --git a/branch.h b/branch.h
> > index 58aa45f..8ce22f8 100644
> > --- a/branch.h
> > +++ b/branch.h
> > @@ -43,6 +43,7 @@ void remove_branch_state(void);
> > /*
> > * Configure local branch "local" as downstream to branch "remote"
> > * from remote "origin". Used by git branch --set-upstream.
> > + * Dies if unable to install branch config.
> > */
> > #define BRANCH_CONFIG_VERBOSE 01
> > extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote);
> > diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> > index cdaf6f6..dd776b3 100755
> > --- a/t/t3200-branch.sh
> > +++ b/t/t3200-branch.sh
> > @@ -446,6 +446,13 @@ test_expect_success '--set-upstream-to fails on a non-ref' '
> > test_must_fail git branch --set-upstream-to HEAD^{}
> > '
> >
> > +test_expect_success '--set-upstream-to fails on locked config' '
> > + test_when_finished "rm -f .git/config.lock" &&
> > + >.git/config.lock &&
> > + git branch locked &&
> > + test_must_fail git branch --set-upstream-to locked
> > +'
> > +
> > test_expect_success 'use --set-upstream-to modify HEAD' '
> > test_config branch.master.remote foo &&
> > test_config branch.master.merge foo &&
> > @@ -579,7 +586,7 @@ test_expect_success 'avoid ambiguous track' '
> > git config remote.ambi1.fetch refs/heads/lalala:refs/heads/master &&
> > git config remote.ambi2.url lilili &&
> > git config remote.ambi2.fetch refs/heads/lilili:refs/heads/master &&
> > - git branch all1 master &&
> > + test_must_fail git branch all1 master &&
> > test -z "$(git config branch.all1.merge)"
> > '
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v4 02/15] branch: die on error in setting up tracking branch
2016-02-08 13:42 ` Patrick Steinhardt
@ 2016-02-08 14:03 ` Patrick Steinhardt
0 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-08 14:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Patrick Steinhardt
[-- Attachment #1: Type: text/plain, Size: 7301 bytes --]
On Mon, Feb 08, 2016 at 02:42:32PM +0100, Patrick Steinhardt wrote:
> On Tue, Feb 02, 2016 at 12:49:26PM -0800, Junio C Hamano wrote:
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> > > The setup_tracking function calls install_branch_config, which
> > > may fail writing the configuration due to a locked configuration
> > > file or other error conditions. setup_tracking can also fail when
> > > trying to track ambiguous information for a reference. While this
> > > condition is checked for and an error code is returned, this
> > > error is not checked by the caller.
> > >
> > > Fix both issues by dying early when error occur.
> >
> > Hmph. I think create_branch() is written in such a way that all
> > die() come before the actual ref transaction attempts to create the
> > branch, but this change means that we have already created the
> > branch and then die without undoing the damage that is already done.
> >
> > "The error is not checked by the caller" is very true, but can the
> > caller do something better than just die? I personally do not think
> > it is such a big deal if we just died here, but I may have overlooked
> > something.
Hum. I've actually forgot about the function not being static. It
is actually used in transport.c:transport_push() and
builtin/clone.c.
For the clone command I do not regard it as important as on
failure the user can simply re-invoke the command and clone a new
copy. Sure, this may me inefficient for large repos, but it is
still far more sensible than leaving the user with an
inconsistent repository.
In transport.c we skip updating local tracking refs when
we die. As such, we might leave refs that have been deleted on
the remote. I guess those cases are easy to fix when followed
up with a fetch or `fetch --prune`.
As said before, if we want to improve the just-die case we might
propose how the user may fix up his repository afterwards.
>
> Well, when dying here we do not record the tracking branch
> configuration for the newly set up branch. This is the only thing
> that we are missing as right after setting up the tracking branch
> we've finished and exit the command.
>
> That being said it's somewhat unfortunate to die here as the user
> cannot simply try to repeat creating a branch and hope it works
> this time as the branch has already been created and the command
> would error out. Maybe we should instead die with an improved
> error message hinting the user how to fix the issue, something
> along the lines of
>
> "We were unable to set the remote tracking information for the
> newly created branch due to <REASON>. After fixing the underlying
> problem you may set the remote tracking branch by executing `git
> branch --set-upstream-to=<BRANCH>."
>
> > > ---
> > > branch.c | 19 +++++++++----------
> > > branch.h | 1 +
> > > t/t3200-branch.sh | 9 ++++++++-
> > > 3 files changed, 18 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/branch.c b/branch.c
> > > index 7ff3f20..7106369 100644
> > > --- a/branch.c
> > > +++ b/branch.c
> > > @@ -64,16 +64,16 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
> > > }
> > >
> > > strbuf_addf(&key, "branch.%s.remote", local);
> > > - git_config_set(key.buf, origin ? origin : ".");
> > > + git_config_set_or_die(key.buf, origin ? origin : ".");
> > >
> > > strbuf_reset(&key);
> > > strbuf_addf(&key, "branch.%s.merge", local);
> > > - git_config_set(key.buf, remote);
> > > + git_config_set_or_die(key.buf, remote);
> > >
> > > if (rebasing) {
> > > strbuf_reset(&key);
> > > strbuf_addf(&key, "branch.%s.rebase", local);
> > > - git_config_set(key.buf, "true");
> > > + git_config_set_or_die(key.buf, "true");
> > > }
> > > strbuf_release(&key);
> > >
> > > @@ -109,8 +109,8 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
> > > * to infer the settings for branch.<new_ref>.{remote,merge} from the
> > > * config.
> > > */
> > > -static int setup_tracking(const char *new_ref, const char *orig_ref,
> > > - enum branch_track track, int quiet)
> > > +static void setup_tracking(const char *new_ref, const char *orig_ref,
> > > + enum branch_track track, int quiet)
> > > {
> > > struct tracking tracking;
> > > int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
> > > @@ -118,7 +118,7 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
> > > memset(&tracking, 0, sizeof(tracking));
> > > tracking.spec.dst = (char *)orig_ref;
> > > if (for_each_remote(find_tracked_branch, &tracking))
> > > - return 1;
> > > + return;
> > >
> > > if (!tracking.matches)
> > > switch (track) {
> > > @@ -127,18 +127,17 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
> > > case BRANCH_TRACK_OVERRIDE:
> > > break;
> > > default:
> > > - return 1;
> > > + return;
> > > }
> > >
> > > if (tracking.matches > 1)
> > > - return error(_("Not tracking: ambiguous information for ref %s"),
> > > - orig_ref);
> > > + die(_("Not tracking: ambiguous information for ref %s"),
> > > + orig_ref);
> > >
> > > install_branch_config(config_flags, new_ref, tracking.remote,
> > > tracking.src ? tracking.src : orig_ref);
> > >
> > > free(tracking.src);
> > > - return 0;
> > > }
> > >
> > > int read_branch_desc(struct strbuf *buf, const char *branch_name)
> > > diff --git a/branch.h b/branch.h
> > > index 58aa45f..8ce22f8 100644
> > > --- a/branch.h
> > > +++ b/branch.h
> > > @@ -43,6 +43,7 @@ void remove_branch_state(void);
> > > /*
> > > * Configure local branch "local" as downstream to branch "remote"
> > > * from remote "origin". Used by git branch --set-upstream.
> > > + * Dies if unable to install branch config.
> > > */
> > > #define BRANCH_CONFIG_VERBOSE 01
> > > extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote);
> > > diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> > > index cdaf6f6..dd776b3 100755
> > > --- a/t/t3200-branch.sh
> > > +++ b/t/t3200-branch.sh
> > > @@ -446,6 +446,13 @@ test_expect_success '--set-upstream-to fails on a non-ref' '
> > > test_must_fail git branch --set-upstream-to HEAD^{}
> > > '
> > >
> > > +test_expect_success '--set-upstream-to fails on locked config' '
> > > + test_when_finished "rm -f .git/config.lock" &&
> > > + >.git/config.lock &&
> > > + git branch locked &&
> > > + test_must_fail git branch --set-upstream-to locked
> > > +'
> > > +
> > > test_expect_success 'use --set-upstream-to modify HEAD' '
> > > test_config branch.master.remote foo &&
> > > test_config branch.master.merge foo &&
> > > @@ -579,7 +586,7 @@ test_expect_success 'avoid ambiguous track' '
> > > git config remote.ambi1.fetch refs/heads/lalala:refs/heads/master &&
> > > git config remote.ambi2.url lilili &&
> > > git config remote.ambi2.fetch refs/heads/lilili:refs/heads/master &&
> > > - git branch all1 master &&
> > > + test_must_fail git branch all1 master &&
> > > test -z "$(git config branch.all1.merge)"
> > > '
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v4 03/15] branch: die on config error when unsetting upstream
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 01/15] config: introduce set_or_die wrappers Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 02/15] branch: die on error in setting up tracking branch Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 04/15] branch: die on config error when editing branch description Patrick Steinhardt
` (13 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
When we try to unset upstream configurations we do not check
return codes for the `git_config_set` functions. As those may
indicate that we were unable to unset the respective
configuration we may exit successfully without any error message
while in fact the upstream configuration was not unset.
Fix this by dying with an error message when we cannot unset the
configuration.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/branch.c | 4 ++--
t/t3200-branch.sh | 7 +++++++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 3f6c825..0978287 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -791,10 +791,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
die(_("Branch '%s' has no upstream information"), branch->name);
strbuf_addf(&buf, "branch.%s.remote", branch->name);
- git_config_set_multivar(buf.buf, NULL, NULL, 1);
+ git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1);
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.merge", branch->name);
- git_config_set_multivar(buf.buf, NULL, NULL, 1);
+ git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1);
strbuf_release(&buf);
} else if (argc > 0 && argc <= 2) {
struct branch *branch = branch_get(argv[0]);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index dd776b3..a897248 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -473,6 +473,13 @@ test_expect_success '--unset-upstream should fail if given a non-existent branch
test_must_fail git branch --unset-upstream i-dont-exist
'
+test_expect_success '--unset-upstream should fail if config is locked' '
+ test_when_finished "rm -f .git/config.lock" &&
+ git branch --set-upstream-to locked &&
+ >.git/config.lock &&
+ test_must_fail git branch --unset-upstream
+'
+
test_expect_success 'test --unset-upstream on HEAD' '
git branch my14 &&
test_config branch.master.remote foo &&
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v4 04/15] branch: die on config error when editing branch description
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (2 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 03/15] branch: die on config error when unsetting upstream Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 05/15] submodule: die on config error when linking modules Patrick Steinhardt
` (12 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
---
builtin/branch.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 0978287..c043cfc 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -570,7 +570,6 @@ static const char edit_description[] = "BRANCH_DESCRIPTION";
static int edit_branch_description(const char *branch_name)
{
- int status;
struct strbuf buf = STRBUF_INIT;
struct strbuf name = STRBUF_INIT;
@@ -595,11 +594,11 @@ static int edit_branch_description(const char *branch_name)
strbuf_stripspace(&buf, 1);
strbuf_addf(&name, "branch.%s.description", branch_name);
- status = git_config_set(name.buf, buf.len ? buf.buf : NULL);
+ git_config_set_or_die(name.buf, buf.len ? buf.buf : NULL);
strbuf_release(&name);
strbuf_release(&buf);
- return status;
+ return 0;
}
int cmd_branch(int argc, const char **argv, const char *prefix)
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v4 05/15] submodule: die on config error when linking modules
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (3 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 04/15] branch: die on config error when editing branch description Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 06/15] submodule--helper: die on config error when cloning module Patrick Steinhardt
` (11 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
When trying to connect a submodule with its corresponding
repository in '.git/modules' we try to set the core.worktree
setting in the submodule, which may fail due to an error
encountered in `git_config_set_in_file`.
The function is used in the git-mv command when trying to move a
submodule to another location. We already die when renaming a
file fails but do not pay attention to the case where updating
the connection between submodule and its repository fails. As
this leaves the repository in an inconsistent state, as well,
abort the program by dying early and presenting the failure to
the user.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
submodule.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/submodule.c b/submodule.c
index b83939c..589a82c 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1087,11 +1087,9 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
/* Update core.worktree setting */
strbuf_reset(&file_name);
strbuf_addf(&file_name, "%s/config", git_dir);
- if (git_config_set_in_file(file_name.buf, "core.worktree",
- relative_path(real_work_tree, git_dir,
- &rel_path)))
- die(_("Could not set core.worktree in %s"),
- file_name.buf);
+ git_config_set_in_file_or_die(file_name.buf, "core.worktree",
+ relative_path(real_work_tree, git_dir,
+ &rel_path));
strbuf_release(&file_name);
strbuf_release(&rel_path);
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v4 06/15] submodule--helper: die on config error when cloning module
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (4 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 05/15] submodule: die on config error when linking modules Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 18:45 ` Eric Sunshine
2016-02-02 11:51 ` [PATCH v4 07/15] remote: die on config error when setting URL Patrick Steinhardt
` (10 subsequent siblings)
16 siblings, 1 reply; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
When setting the 'core.worktree' option for a newly cloned
submodule we ignore the return value of `git_config_set_in_file`.
As this leaves the submodule in an inconsistent state, we instaed
we want to inform the user that something has gone wrong by
printing an error and aborting the program.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/submodule--helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..c7e1ea2 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -245,8 +245,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
p = git_pathdup_submodule(path, "config");
if (!p)
die(_("could not get submodule directory for '%s'"), path);
- git_config_set_in_file(p, "core.worktree",
- relative_path(sb.buf, sm_gitdir, &rel_path));
+ git_config_set_in_file_or_die(p, "core.worktree",
+ relative_path(sb.buf, sm_gitdir, &rel_path));
strbuf_release(&sb);
strbuf_release(&rel_path);
free(sm_gitdir);
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v4 06/15] submodule--helper: die on config error when cloning module
2016-02-02 11:51 ` [PATCH v4 06/15] submodule--helper: die on config error when cloning module Patrick Steinhardt
@ 2016-02-02 18:45 ` Eric Sunshine
2016-02-08 14:05 ` Patrick Steinhardt
0 siblings, 1 reply; 26+ messages in thread
From: Eric Sunshine @ 2016-02-02 18:45 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Git List, Jeff King, Junio C Hamano
On Tue, Feb 2, 2016 at 6:51 AM, Patrick Steinhardt <ps@pks.im> wrote:
> When setting the 'core.worktree' option for a newly cloned
> submodule we ignore the return value of `git_config_set_in_file`.
> As this leaves the submodule in an inconsistent state, we instaed
> we want to inform the user that something has gone wrong by
s/instaed/instead/
Also, there are too many "we"s, so dropping one would be a good idea:
either "we instead" or "instead we".
> printing an error and aborting the program.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 06/15] submodule--helper: die on config error when cloning module
2016-02-02 18:45 ` Eric Sunshine
@ 2016-02-08 14:05 ` Patrick Steinhardt
0 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-08 14:05 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Git List, Jeff King, Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 682 bytes --]
On Tue, Feb 02, 2016 at 01:45:50PM -0500, Eric Sunshine wrote:
> On Tue, Feb 2, 2016 at 6:51 AM, Patrick Steinhardt <ps@pks.im> wrote:
> > When setting the 'core.worktree' option for a newly cloned
> > submodule we ignore the return value of `git_config_set_in_file`.
> > As this leaves the submodule in an inconsistent state, we instaed
> > we want to inform the user that something has gone wrong by
>
> s/instaed/instead/
>
> Also, there are too many "we"s, so dropping one would be a good idea:
> either "we instead" or "instead we".
>
> > printing an error and aborting the program.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
Thanks, fixed now.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v4 07/15] remote: die on config error when setting URL
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (5 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 06/15] submodule--helper: die on config error when cloning module Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 12:00 ` Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 08/15] remote: die on config error when setting/adding branches Patrick Steinhardt
` (9 subsequent siblings)
16 siblings, 1 reply; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
When invoking `git-remote --set-url` we do not check the return
value when writing the actual new URL to the configuration file,
pretending to the user that the configuration has been set while
it was in fact not persisted.
Fix this problem by dying early when setting the config fails.
Signed-off-by: Patrick Steinhardt
---
builtin/remote.c | 11 ++++++-----
t/t5505-remote.sh | 9 +++++++++
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 2b2ff9b..8b78c3d 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1583,11 +1583,12 @@ static int set_url(int argc, const char **argv)
/* Special cases that add new entry. */
if ((!oldurl && !delete_mode) || add_mode) {
if (add_mode)
- git_config_set_multivar(name_buf.buf, newurl,
- "^$", 0);
+ git_config_set_multivar_or_die(name_buf.buf, newurl,
+ "^$", 0);
else
- git_config_set(name_buf.buf, newurl);
+ git_config_set_or_die(name_buf.buf, newurl);
strbuf_release(&name_buf);
+
return 0;
}
@@ -1608,9 +1609,9 @@ static int set_url(int argc, const char **argv)
regfree(&old_regex);
if (!delete_mode)
- git_config_set_multivar(name_buf.buf, newurl, oldurl, 0);
+ git_config_set_multivar_or_die(name_buf.buf, newurl, oldurl, 0);
else
- git_config_set_multivar(name_buf.buf, NULL, oldurl, 1);
+ git_config_set_multivar_or_die(name_buf.buf, NULL, oldurl, 1);
return 0;
}
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 1a8e3b8..e019f27 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -932,6 +932,15 @@ test_expect_success 'get-url on new remote' '
echo foo | get_url_test --push --all someremote
'
+test_expect_success 'remote set-url with locked config' '
+ test_when_finished "rm -f .git/config.lock" &&
+ git config --get-all remote.someremote.url >expect &&
+ >.git/config.lock &&
+ test_must_fail git remote set-url someremote baz &&
+ git config --get-all remote.someremote.url >actual &&
+ cmp expect actual
+'
+
test_expect_success 'remote set-url bar' '
git remote set-url someremote bar &&
echo bar >expect &&
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v4 08/15] remote: die on config error when setting/adding branches
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (6 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 07/15] remote: die on config error when setting URL Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 09/15] remote: die on config error when manipulating remotes Patrick Steinhardt
` (8 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
When we add or set new branches (e.g. by `git remote add -f` or
`git remote set-branches`) we do not check for error codes when
writing the branches to the configuration file. When persisting
the configuration failed we are left with a remote that has none
or not all of the branches that should have been set without
notifying the user.
Fix this issue by dying early on configuration error.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/remote.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 8b78c3d..eeb6d2e 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -108,8 +108,8 @@ enum {
#define MIRROR_PUSH 2
#define MIRROR_BOTH (MIRROR_FETCH|MIRROR_PUSH)
-static int add_branch(const char *key, const char *branchname,
- const char *remotename, int mirror, struct strbuf *tmp)
+static void add_branch(const char *key, const char *branchname,
+ const char *remotename, int mirror, struct strbuf *tmp)
{
strbuf_reset(tmp);
strbuf_addch(tmp, '+');
@@ -119,7 +119,7 @@ static int add_branch(const char *key, const char *branchname,
else
strbuf_addf(tmp, "refs/heads/%s:refs/remotes/%s/%s",
branchname, remotename, branchname);
- return git_config_set_multivar(key, tmp->buf, "^$", 0);
+ git_config_set_multivar_or_die(key, tmp->buf, "^$", 0);
}
static const char mirror_advice[] =
@@ -206,9 +206,8 @@ static int add(int argc, const char **argv)
if (track.nr == 0)
string_list_append(&track, "*");
for (i = 0; i < track.nr; i++) {
- if (add_branch(buf.buf, track.items[i].string,
- name, mirror, &buf2))
- return 1;
+ add_branch(buf.buf, track.items[i].string,
+ name, mirror, &buf2);
}
}
@@ -1416,21 +1415,17 @@ static int remove_all_fetch_refspecs(const char *remote, const char *key)
return git_config_set_multivar(key, NULL, NULL, 1);
}
-static int add_branches(struct remote *remote, const char **branches,
- const char *key)
+static void add_branches(struct remote *remote, const char **branches,
+ const char *key)
{
const char *remotename = remote->name;
int mirror = remote->mirror;
struct strbuf refspec = STRBUF_INIT;
for (; *branches; branches++)
- if (add_branch(key, *branches, remotename, mirror, &refspec)) {
- strbuf_release(&refspec);
- return 1;
- }
+ add_branch(key, *branches, remotename, mirror, &refspec);
strbuf_release(&refspec);
- return 0;
}
static int set_remote_branches(const char *remotename, const char **branches,
@@ -1449,10 +1444,7 @@ static int set_remote_branches(const char *remotename, const char **branches,
strbuf_release(&key);
return 1;
}
- if (add_branches(remote, branches, key.buf)) {
- strbuf_release(&key);
- return 1;
- }
+ add_branches(remote, branches, key.buf);
strbuf_release(&key);
return 0;
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v4 09/15] remote: die on config error when manipulating remotes
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (7 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 08/15] remote: die on config error when setting/adding branches Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 10/15] clone: die on config error in cmd_clone Patrick Steinhardt
` (7 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
When manipulating remotes we try to set various configuration
values without checking if the values were persisted correctly,
possibly leaving the remote in an inconsistent state.
Fix this issue by dying early and notifying the user about the
error.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/remote.c | 39 ++++++++++++---------------------------
1 file changed, 12 insertions(+), 27 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index eeb6d2e..fe57f77 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -197,8 +197,7 @@ static int add(int argc, const char **argv)
die(_("'%s' is not a valid remote name"), name);
strbuf_addf(&buf, "remote.%s.url", name);
- if (git_config_set(buf.buf, url))
- return 1;
+ git_config_set_or_die(buf.buf, url);
if (!mirror || mirror & MIRROR_FETCH) {
strbuf_reset(&buf);
@@ -214,16 +213,14 @@ static int add(int argc, const char **argv)
if (mirror & MIRROR_PUSH) {
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.mirror", name);
- if (git_config_set(buf.buf, "true"))
- return 1;
+ git_config_set_or_die(buf.buf, "true");
}
if (fetch_tags != TAGS_DEFAULT) {
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.tagopt", name);
- if (git_config_set(buf.buf,
- fetch_tags == TAGS_SET ? "--tags" : "--no-tags"))
- return 1;
+ git_config_set_or_die(buf.buf,
+ fetch_tags == TAGS_SET ? "--tags" : "--no-tags");
}
if (fetch && fetch_remote(name))
@@ -591,25 +588,20 @@ static int migrate_file(struct remote *remote)
strbuf_addf(&buf, "remote.%s.url", remote->name);
for (i = 0; i < remote->url_nr; i++)
- if (git_config_set_multivar(buf.buf, remote->url[i], "^$", 0))
- return error(_("Could not append '%s' to '%s'"),
- remote->url[i], buf.buf);
+ git_config_set_multivar_or_die(buf.buf, remote->url[i], "^$", 0);
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.push", remote->name);
for (i = 0; i < remote->push_refspec_nr; i++)
- if (git_config_set_multivar(buf.buf, remote->push_refspec[i], "^$", 0))
- return error(_("Could not append '%s' to '%s'"),
- remote->push_refspec[i], buf.buf);
+ git_config_set_multivar_or_die(buf.buf, remote->push_refspec[i], "^$", 0);
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.fetch", remote->name);
for (i = 0; i < remote->fetch_refspec_nr; i++)
- if (git_config_set_multivar(buf.buf, remote->fetch_refspec[i], "^$", 0))
- return error(_("Could not append '%s' to '%s'"),
- remote->fetch_refspec[i], buf.buf);
+ git_config_set_multivar_or_die(buf.buf, remote->fetch_refspec[i], "^$", 0);
if (remote->origin == REMOTE_REMOTES)
unlink_or_warn(git_path("remotes/%s", remote->name));
else if (remote->origin == REMOTE_BRANCHES)
unlink_or_warn(git_path("branches/%s", remote->name));
+
return 0;
}
@@ -656,8 +648,7 @@ static int mv(int argc, const char **argv)
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.fetch", rename.new);
- if (git_config_set_multivar(buf.buf, NULL, NULL, 1))
- return error(_("Could not remove config section '%s'"), buf.buf);
+ git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1);
strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old);
for (i = 0; i < oldremote->fetch_refspec_nr; i++) {
char *ptr;
@@ -677,8 +668,7 @@ static int mv(int argc, const char **argv)
"\tPlease update the configuration manually if necessary."),
buf2.buf);
- if (git_config_set_multivar(buf.buf, buf2.buf, "^$", 0))
- return error(_("Could not append '%s'"), buf.buf);
+ git_config_set_multivar_or_die(buf.buf, buf2.buf, "^$", 0);
}
read_branches();
@@ -688,9 +678,7 @@ static int mv(int argc, const char **argv)
if (info->remote_name && !strcmp(info->remote_name, rename.old)) {
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.remote", item->string);
- if (git_config_set(buf.buf, rename.new)) {
- return error(_("Could not set '%s'"), buf.buf);
- }
+ git_config_set_or_die(buf.buf, rename.new);
}
}
@@ -788,10 +776,7 @@ static int rm(int argc, const char **argv)
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.%s",
item->string, *k);
- if (git_config_set(buf.buf, NULL)) {
- strbuf_release(&buf);
- return -1;
- }
+ git_config_set_or_die(buf.buf, NULL);
}
}
}
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v4 10/15] clone: die on config error in cmd_clone
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (8 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 09/15] remote: die on config error when manipulating remotes Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 20:55 ` Junio C Hamano
2016-02-02 11:51 ` [PATCH v4 11/15] init-db: die on config errors when initializing empty repo Patrick Steinhardt
` (6 subsequent siblings)
16 siblings, 1 reply; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
The clone command does not check for error codes returned by
`git_config_set` functions. This may cause the user to end up
with an inconsistent repository without any indication with what
went wrong.
Fix this problem by dying with an error message when we are
unable to write the configuration files to disk.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/clone.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 81e238f..f2a2f9a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -786,12 +786,12 @@ static void write_refspec_config(const char *src_ref_prefix,
/* Configure the remote */
if (value.len) {
strbuf_addf(&key, "remote.%s.fetch", option_origin);
- git_config_set_multivar(key.buf, value.buf, "^$", 0);
+ git_config_set_multivar_or_die(key.buf, value.buf, "^$", 0);
strbuf_reset(&key);
if (option_mirror) {
strbuf_addf(&key, "remote.%s.mirror", option_origin);
- git_config_set(key.buf, "true");
+ git_config_set_or_die(key.buf, "true");
strbuf_reset(&key);
}
}
@@ -949,14 +949,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
src_ref_prefix = "refs/";
strbuf_addstr(&branch_top, src_ref_prefix);
- git_config_set("core.bare", "true");
+ git_config_set_or_die("core.bare", "true");
} else {
strbuf_addf(&branch_top, "refs/remotes/%s/", option_origin);
}
strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
strbuf_addf(&key, "remote.%s.url", option_origin);
- git_config_set(key.buf, repo);
+ git_config_set_or_die(key.buf, repo);
strbuf_reset(&key);
if (option_reference.nr)
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v4 10/15] clone: die on config error in cmd_clone
2016-02-02 11:51 ` [PATCH v4 10/15] clone: die on config error in cmd_clone Patrick Steinhardt
@ 2016-02-02 20:55 ` Junio C Hamano
0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2016-02-02 20:55 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jeff King
Patrick Steinhardt <ps@pks.im> writes:
> The clone command does not check for error codes returned by
> `git_config_set` functions. This may cause the user to end up
> with an inconsistent repository without any indication with what
> went wrong.
>
> Fix this problem by dying with an error message when we are
> unable to write the configuration files to disk.
When this happens, the junk_mode is still JUNK_LEAVE_NONE, so upon
hitting such an error, we'd remove everything and die. And we
haven't wasted the effort for large object transfer yet.
Which all sounds sensible.
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/clone.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 81e238f..f2a2f9a 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -786,12 +786,12 @@ static void write_refspec_config(const char *src_ref_prefix,
> /* Configure the remote */
> if (value.len) {
> strbuf_addf(&key, "remote.%s.fetch", option_origin);
> - git_config_set_multivar(key.buf, value.buf, "^$", 0);
> + git_config_set_multivar_or_die(key.buf, value.buf, "^$", 0);
> strbuf_reset(&key);
>
> if (option_mirror) {
> strbuf_addf(&key, "remote.%s.mirror", option_origin);
> - git_config_set(key.buf, "true");
> + git_config_set_or_die(key.buf, "true");
> strbuf_reset(&key);
> }
> }
> @@ -949,14 +949,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> src_ref_prefix = "refs/";
> strbuf_addstr(&branch_top, src_ref_prefix);
>
> - git_config_set("core.bare", "true");
> + git_config_set_or_die("core.bare", "true");
> } else {
> strbuf_addf(&branch_top, "refs/remotes/%s/", option_origin);
> }
>
> strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
> strbuf_addf(&key, "remote.%s.url", option_origin);
> - git_config_set(key.buf, repo);
> + git_config_set_or_die(key.buf, repo);
> strbuf_reset(&key);
>
> if (option_reference.nr)
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v4 11/15] init-db: die on config errors when initializing empty repo
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (9 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 10/15] clone: die on config error in cmd_clone Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 12/15] sequencer: die on config error when saving replay opts Patrick Steinhardt
` (5 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
When creating an empty repository with `git init-db` we do not
check for error codes returned by `git_config_set` functions.
This may cause the user to end up with an inconsistent repository
without any indication for the user.
Fix this problem by dying early with an error message when we are
unable to write the configuration files to disk.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/init-db.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 07229d6..ef19048 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -227,7 +227,7 @@ static int create_default_files(const char *template_path)
/* This forces creation of new config file */
xsnprintf(repo_version_string, sizeof(repo_version_string),
"%d", GIT_REPO_VERSION);
- git_config_set("core.repositoryformatversion", repo_version_string);
+ git_config_set_or_die("core.repositoryformatversion", repo_version_string);
/* Check filemode trustability */
path = git_path_buf(&buf, "config");
@@ -241,18 +241,18 @@ static int create_default_files(const char *template_path)
if (filemode && !reinit && (st1.st_mode & S_IXUSR))
filemode = 0;
}
- git_config_set("core.filemode", filemode ? "true" : "false");
+ git_config_set_or_die("core.filemode", filemode ? "true" : "false");
if (is_bare_repository())
- git_config_set("core.bare", "true");
+ git_config_set_or_die("core.bare", "true");
else {
const char *work_tree = get_git_work_tree();
- git_config_set("core.bare", "false");
+ git_config_set_or_die("core.bare", "false");
/* allow template config file to override the default */
if (log_all_ref_updates == -1)
- git_config_set("core.logallrefupdates", "true");
+ git_config_set_or_die("core.logallrefupdates", "true");
if (needs_work_tree_config(get_git_dir(), work_tree))
- git_config_set("core.worktree", work_tree);
+ git_config_set_or_die("core.worktree", work_tree);
}
if (!reinit) {
@@ -265,12 +265,12 @@ static int create_default_files(const char *template_path)
S_ISLNK(st1.st_mode))
unlink(path); /* good */
else
- git_config_set("core.symlinks", "false");
+ git_config_set_or_die("core.symlinks", "false");
/* Check if the filesystem is case-insensitive */
path = git_path_buf(&buf, "CoNfIg");
if (!access(path, F_OK))
- git_config_set("core.ignorecase", "true");
+ git_config_set_or_die("core.ignorecase", "true");
probe_utf8_pathname_composition();
}
@@ -386,8 +386,8 @@ int init_db(const char *template_dir, unsigned int flags)
xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_EVERYBODY);
else
die("BUG: invalid value for shared_repository");
- git_config_set("core.sharedrepository", buf);
- git_config_set("receive.denyNonFastforwards", "true");
+ git_config_set_or_die("core.sharedrepository", buf);
+ git_config_set_or_die("receive.denyNonFastforwards", "true");
}
if (!(flags & INIT_DB_QUIET)) {
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v4 12/15] sequencer: die on config error when saving replay opts
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (10 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 11/15] init-db: die on config errors when initializing empty repo Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 13/15] compat: die when unable to set core.precomposeunicode Patrick Steinhardt
` (4 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
When we start picking a range of revisions we save the replay
options that are required to restore state when interrupting and
later continuing picking the revisions. However, we do not check
the return values of the `git_config_set` functions, which may
lead us to store incomplete information. As this may lead us to
fail when trying to continue the sequence the error can be fatal.
Fix this by dying immediately when we are unable to write back
any replay option.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
sequencer.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 8048786..3590248 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -933,31 +933,31 @@ static void save_opts(struct replay_opts *opts)
const char *opts_file = git_path_opts_file();
if (opts->no_commit)
- git_config_set_in_file(opts_file, "options.no-commit", "true");
+ git_config_set_in_file_or_die(opts_file, "options.no-commit", "true");
if (opts->edit)
- git_config_set_in_file(opts_file, "options.edit", "true");
+ git_config_set_in_file_or_die(opts_file, "options.edit", "true");
if (opts->signoff)
- git_config_set_in_file(opts_file, "options.signoff", "true");
+ git_config_set_in_file_or_die(opts_file, "options.signoff", "true");
if (opts->record_origin)
- git_config_set_in_file(opts_file, "options.record-origin", "true");
+ git_config_set_in_file_or_die(opts_file, "options.record-origin", "true");
if (opts->allow_ff)
- git_config_set_in_file(opts_file, "options.allow-ff", "true");
+ git_config_set_in_file_or_die(opts_file, "options.allow-ff", "true");
if (opts->mainline) {
struct strbuf buf = STRBUF_INIT;
strbuf_addf(&buf, "%d", opts->mainline);
- git_config_set_in_file(opts_file, "options.mainline", buf.buf);
+ git_config_set_in_file_or_die(opts_file, "options.mainline", buf.buf);
strbuf_release(&buf);
}
if (opts->strategy)
- git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
+ git_config_set_in_file_or_die(opts_file, "options.strategy", opts->strategy);
if (opts->gpg_sign)
- git_config_set_in_file(opts_file, "options.gpg-sign", opts->gpg_sign);
+ git_config_set_in_file_or_die(opts_file, "options.gpg-sign", opts->gpg_sign);
if (opts->xopts) {
int i;
for (i = 0; i < opts->xopts_nr; i++)
- git_config_set_multivar_in_file(opts_file,
- "options.strategy-option",
- opts->xopts[i], "^$", 0);
+ git_config_set_multivar_in_file_or_die(opts_file,
+ "options.strategy-option",
+ opts->xopts[i], "^$", 0);
}
}
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v4 13/15] compat: die when unable to set core.precomposeunicode
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (11 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 12/15] sequencer: die on config error when saving replay opts Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 14/15] config: rename git_config_set to git_config_set_gently Patrick Steinhardt
` (3 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
When calling `git_config_set` to set 'core.precomposeunicode' we
ignore the return value of the function, which may indicate that
we were unable to write the value back to disk. As the function
is only called by init-db we can and should die when an error
occurs.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
compat/precompose_utf8.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index 079070f..9ff1ebe 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -50,7 +50,8 @@ void probe_utf8_pathname_composition(void)
close(output_fd);
git_path_buf(&path, "%s", auml_nfd);
precomposed_unicode = access(path.buf, R_OK) ? 0 : 1;
- git_config_set("core.precomposeunicode", precomposed_unicode ? "true" : "false");
+ git_config_set_or_die("core.precomposeunicode",
+ precomposed_unicode ? "true" : "false");
git_path_buf(&path, "%s", auml_nfc);
if (unlink(path.buf))
die_errno(_("failed to unlink '%s'"), path.buf);
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v4 14/15] config: rename git_config_set to git_config_set_gently
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (12 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 13/15] compat: die when unable to set core.precomposeunicode Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 11:51 ` [PATCH v4 15/15] config: rename git_config_set_or_die to git_config_set Patrick Steinhardt
` (2 subsequent siblings)
16 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
The desired default behavior for `git_config_set` is to die
whenever an error occurs. Dying is the default for a lot of
internal functions when failures occur and is in this case the
right thing to do for most callers as otherwise we might run into
inconsistent repositories without noticing.
As some code may rely on the actual return values for
`git_config_set` we still require the ability to invoke these
functions without aborting. Rename the existing `git_config_set`
functions to `git_config_set_gently` to keep them available for
those callers.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/clone.c | 2 +-
builtin/config.c | 28 ++++++++++++++--------------
builtin/remote.c | 2 +-
cache.h | 10 +++++-----
config.c | 29 +++++++++++++++--------------
submodule.c | 2 +-
6 files changed, 37 insertions(+), 36 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index f2a2f9a..dccfab3 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -735,7 +735,7 @@ static int checkout(void)
static int write_one_config(const char *key, const char *value, void *data)
{
- return git_config_set_multivar(key, value ? value : "true", "^$", 0);
+ return git_config_set_multivar_gently(key, value ? value : "true", "^$", 0);
}
static void write_config(struct string_list *config)
diff --git a/builtin/config.c b/builtin/config.c
index adc7727..c26d6e7 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -582,7 +582,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
check_write();
check_argc(argc, 2, 2);
value = normalize_value(argv[0], argv[1]);
- ret = git_config_set_in_file(given_config_source.file, argv[0], value);
+ ret = git_config_set_in_file_gently(given_config_source.file, argv[0], value);
if (ret == CONFIG_NOTHING_SET)
error("cannot overwrite multiple values with a single value\n"
" Use a regexp, --add or --replace-all to change %s.", argv[0]);
@@ -592,23 +592,23 @@ int cmd_config(int argc, const char **argv, const char *prefix)
check_write();
check_argc(argc, 2, 3);
value = normalize_value(argv[0], argv[1]);
- return git_config_set_multivar_in_file(given_config_source.file,
- argv[0], value, argv[2], 0);
+ return git_config_set_multivar_in_file_gently(given_config_source.file,
+ argv[0], value, argv[2], 0);
}
else if (actions == ACTION_ADD) {
check_write();
check_argc(argc, 2, 2);
value = normalize_value(argv[0], argv[1]);
- return git_config_set_multivar_in_file(given_config_source.file,
- argv[0], value,
- CONFIG_REGEX_NONE, 0);
+ return git_config_set_multivar_in_file_gently(given_config_source.file,
+ argv[0], value,
+ CONFIG_REGEX_NONE, 0);
}
else if (actions == ACTION_REPLACE_ALL) {
check_write();
check_argc(argc, 2, 3);
value = normalize_value(argv[0], argv[1]);
- return git_config_set_multivar_in_file(given_config_source.file,
- argv[0], value, argv[2], 1);
+ return git_config_set_multivar_in_file_gently(given_config_source.file,
+ argv[0], value, argv[2], 1);
}
else if (actions == ACTION_GET) {
check_argc(argc, 1, 2);
@@ -634,17 +634,17 @@ int cmd_config(int argc, const char **argv, const char *prefix)
check_write();
check_argc(argc, 1, 2);
if (argc == 2)
- return git_config_set_multivar_in_file(given_config_source.file,
- argv[0], NULL, argv[1], 0);
+ return git_config_set_multivar_in_file_gently(given_config_source.file,
+ argv[0], NULL, argv[1], 0);
else
- return git_config_set_in_file(given_config_source.file,
- argv[0], NULL);
+ return git_config_set_in_file_gently(given_config_source.file,
+ argv[0], NULL);
}
else if (actions == ACTION_UNSET_ALL) {
check_write();
check_argc(argc, 1, 2);
- return git_config_set_multivar_in_file(given_config_source.file,
- argv[0], NULL, argv[1], 1);
+ return git_config_set_multivar_in_file_gently(given_config_source.file,
+ argv[0], NULL, argv[1], 1);
}
else if (actions == ACTION_RENAME_SECTION) {
int ret;
diff --git a/builtin/remote.c b/builtin/remote.c
index fe57f77..abd5f67 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1397,7 +1397,7 @@ static int update(int argc, const char **argv)
static int remove_all_fetch_refspecs(const char *remote, const char *key)
{
- return git_config_set_multivar(key, NULL, NULL, 1);
+ return git_config_set_multivar_gently(key, NULL, NULL, 1);
}
static void add_branches(struct remote *remote, const char **branches,
diff --git a/cache.h b/cache.h
index c1f844d..a97f677 100644
--- a/cache.h
+++ b/cache.h
@@ -1469,7 +1469,7 @@ extern int update_server_info(int);
/* git_config_parse_key() returns these negated: */
#define CONFIG_INVALID_KEY 1
#define CONFIG_NO_SECTION_OR_NAME 2
-/* git_config_set(), git_config_set_multivar() return the above or these: */
+/* git_config_set_gently(), git_config_set_multivar_gently() return the above or these: */
#define CONFIG_NO_LOCK -1
#define CONFIG_INVALID_FILE 3
#define CONFIG_NO_WRITE 4
@@ -1507,15 +1507,15 @@ extern int git_config_bool(const char *, const char *);
extern int git_config_maybe_bool(const char *, const char *);
extern int git_config_string(const char **, const char *, const char *);
extern int git_config_pathname(const char **, const char *, const char *);
-extern int git_config_set_in_file(const char *, const char *, const char *);
+extern int git_config_set_in_file_gently(const char *, const char *, const char *);
extern void git_config_set_in_file_or_die(const char *, const char *, const char *);
-extern int git_config_set(const char *, const char *);
+extern int git_config_set_gently(const char *, const char *);
extern void git_config_set_or_die(const char *, const char *);
extern int git_config_parse_key(const char *, char **, int *);
extern int git_config_key_is_valid(const char *key);
-extern int git_config_set_multivar(const char *, const char *, const char *, int);
+extern int git_config_set_multivar_gently(const char *, const char *, const char *, int);
extern void git_config_set_multivar_or_die(const char *, const char *, const char *, int);
-extern int git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
+extern int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, int);
extern void git_config_set_multivar_in_file_or_die(const char *, const char *, const char *, const char *, int);
extern int git_config_rename_section(const char *, const char *);
extern int git_config_rename_section_in_file(const char *, const char *, const char *);
diff --git a/config.c b/config.c
index 856f7d34..e7f42da 100644
--- a/config.c
+++ b/config.c
@@ -1825,10 +1825,10 @@ contline:
return offset;
}
-int git_config_set_in_file(const char *config_filename,
- const char *key, const char *value)
+int git_config_set_in_file_gently(const char *config_filename,
+ const char *key, const char *value)
{
- return git_config_set_multivar_in_file(config_filename, key, value, NULL, 0);
+ return git_config_set_multivar_in_file_gently(config_filename, key, value, NULL, 0);
}
void git_config_set_in_file_or_die(const char *config_filename,
@@ -1837,9 +1837,9 @@ void git_config_set_in_file_or_die(const char *config_filename,
git_config_set_multivar_in_file_or_die(config_filename, key, value, NULL, 0);
}
-int git_config_set(const char *key, const char *value)
+int git_config_set_gently(const char *key, const char *value)
{
- return git_config_set_multivar(key, value, NULL, 0);
+ return git_config_set_multivar_gently(key, value, NULL, 0);
}
void git_config_set_or_die(const char *key, const char *value)
@@ -1961,9 +1961,10 @@ int git_config_key_is_valid(const char *key)
* - the config file is removed and the lock file rename()d to it.
*
*/
-int git_config_set_multivar_in_file(const char *config_filename,
- const char *key, const char *value,
- const char *value_regex, int multi_replace)
+int git_config_set_multivar_in_file_gently(const char *config_filename,
+ const char *key, const char *value,
+ const char *value_regex,
+ int multi_replace)
{
int fd = -1, in_fd = -1;
int ret;
@@ -2194,16 +2195,16 @@ void git_config_set_multivar_in_file_or_die(const char *config_filename,
const char *key, const char *value,
const char *value_regex, int multi_replace)
{
- if (git_config_set_multivar_in_file(config_filename, key, value,
- value_regex, multi_replace) < 0)
+ if (git_config_set_multivar_in_file_gently(config_filename, key, value,
+ value_regex, multi_replace) < 0)
die(_("Could not set '%s' to '%s'"), key, value);
}
-int git_config_set_multivar(const char *key, const char *value,
- const char *value_regex, int multi_replace)
+int git_config_set_multivar_gently(const char *key, const char *value,
+ const char *value_regex, int multi_replace)
{
- return git_config_set_multivar_in_file(NULL, key, value, value_regex,
- multi_replace);
+ return git_config_set_multivar_in_file_gently(NULL, key, value, value_regex,
+ multi_replace);
}
void git_config_set_multivar_or_die(const char *key, const char *value,
diff --git a/submodule.c b/submodule.c
index 589a82c..be8b5cc 100644
--- a/submodule.c
+++ b/submodule.c
@@ -69,7 +69,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)
strbuf_addstr(&entry, "submodule.");
strbuf_addstr(&entry, submodule->name);
strbuf_addstr(&entry, ".path");
- if (git_config_set_in_file(".gitmodules", entry.buf, newpath) < 0) {
+ if (git_config_set_in_file_gently(".gitmodules", entry.buf, newpath) < 0) {
/* Maybe the user already did that, don't error out here */
warning(_("Could not update .gitmodules entry %s"), entry.buf);
strbuf_release(&entry);
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v4 15/15] config: rename git_config_set_or_die to git_config_set
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (13 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 14/15] config: rename git_config_set to git_config_set_gently Patrick Steinhardt
@ 2016-02-02 11:51 ` Patrick Steinhardt
2016-02-02 18:52 ` [PATCH v4 00/15] config: make git_config_set die on failure Stefan Beller
2016-02-02 20:58 ` Junio C Hamano
16 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-02 11:51 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano, Patrick Steinhardt
Rename git_config_set_or_die functions to git_config_set, leading
to the new default behavior of dying whenever a configuration
error occurs.
By now all callers that shall die on error have been transitioned
to the _or_die variants, thus making this patch a simple rename
of the functions.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
branch.c | 6 +++---
builtin/branch.c | 6 +++---
builtin/clone.c | 8 ++++----
builtin/init-db.c | 20 ++++++++++----------
builtin/remote.c | 32 ++++++++++++++++----------------
builtin/submodule--helper.c | 4 ++--
cache.h | 8 ++++----
config.c | 24 ++++++++++++------------
sequencer.c | 22 +++++++++++-----------
submodule.c | 6 +++---
10 files changed, 68 insertions(+), 68 deletions(-)
diff --git a/branch.c b/branch.c
index 7106369..0c11023 100644
--- a/branch.c
+++ b/branch.c
@@ -64,16 +64,16 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
}
strbuf_addf(&key, "branch.%s.remote", local);
- git_config_set_or_die(key.buf, origin ? origin : ".");
+ git_config_set(key.buf, origin ? origin : ".");
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.merge", local);
- git_config_set_or_die(key.buf, remote);
+ git_config_set(key.buf, remote);
if (rebasing) {
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.rebase", local);
- git_config_set_or_die(key.buf, "true");
+ git_config_set(key.buf, "true");
}
strbuf_release(&key);
diff --git a/builtin/branch.c b/builtin/branch.c
index c043cfc..7b45b6b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -594,7 +594,7 @@ static int edit_branch_description(const char *branch_name)
strbuf_stripspace(&buf, 1);
strbuf_addf(&name, "branch.%s.description", branch_name);
- git_config_set_or_die(name.buf, buf.len ? buf.buf : NULL);
+ git_config_set(name.buf, buf.len ? buf.buf : NULL);
strbuf_release(&name);
strbuf_release(&buf);
@@ -790,10 +790,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
die(_("Branch '%s' has no upstream information"), branch->name);
strbuf_addf(&buf, "branch.%s.remote", branch->name);
- git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1);
+ git_config_set_multivar(buf.buf, NULL, NULL, 1);
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.merge", branch->name);
- git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1);
+ git_config_set_multivar(buf.buf, NULL, NULL, 1);
strbuf_release(&buf);
} else if (argc > 0 && argc <= 2) {
struct branch *branch = branch_get(argv[0]);
diff --git a/builtin/clone.c b/builtin/clone.c
index dccfab3..ecdd80a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -786,12 +786,12 @@ static void write_refspec_config(const char *src_ref_prefix,
/* Configure the remote */
if (value.len) {
strbuf_addf(&key, "remote.%s.fetch", option_origin);
- git_config_set_multivar_or_die(key.buf, value.buf, "^$", 0);
+ git_config_set_multivar(key.buf, value.buf, "^$", 0);
strbuf_reset(&key);
if (option_mirror) {
strbuf_addf(&key, "remote.%s.mirror", option_origin);
- git_config_set_or_die(key.buf, "true");
+ git_config_set(key.buf, "true");
strbuf_reset(&key);
}
}
@@ -949,14 +949,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
src_ref_prefix = "refs/";
strbuf_addstr(&branch_top, src_ref_prefix);
- git_config_set_or_die("core.bare", "true");
+ git_config_set("core.bare", "true");
} else {
strbuf_addf(&branch_top, "refs/remotes/%s/", option_origin);
}
strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
strbuf_addf(&key, "remote.%s.url", option_origin);
- git_config_set_or_die(key.buf, repo);
+ git_config_set(key.buf, repo);
strbuf_reset(&key);
if (option_reference.nr)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index ef19048..6223b7d 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -227,7 +227,7 @@ static int create_default_files(const char *template_path)
/* This forces creation of new config file */
xsnprintf(repo_version_string, sizeof(repo_version_string),
"%d", GIT_REPO_VERSION);
- git_config_set_or_die("core.repositoryformatversion", repo_version_string);
+ git_config_set("core.repositoryformatversion", repo_version_string);
/* Check filemode trustability */
path = git_path_buf(&buf, "config");
@@ -241,18 +241,18 @@ static int create_default_files(const char *template_path)
if (filemode && !reinit && (st1.st_mode & S_IXUSR))
filemode = 0;
}
- git_config_set_or_die("core.filemode", filemode ? "true" : "false");
+ git_config_set("core.filemode", filemode ? "true" : "false");
if (is_bare_repository())
- git_config_set_or_die("core.bare", "true");
+ git_config_set("core.bare", "true");
else {
const char *work_tree = get_git_work_tree();
- git_config_set_or_die("core.bare", "false");
+ git_config_set("core.bare", "false");
/* allow template config file to override the default */
if (log_all_ref_updates == -1)
- git_config_set_or_die("core.logallrefupdates", "true");
+ git_config_set("core.logallrefupdates", "true");
if (needs_work_tree_config(get_git_dir(), work_tree))
- git_config_set_or_die("core.worktree", work_tree);
+ git_config_set("core.worktree", work_tree);
}
if (!reinit) {
@@ -265,12 +265,12 @@ static int create_default_files(const char *template_path)
S_ISLNK(st1.st_mode))
unlink(path); /* good */
else
- git_config_set_or_die("core.symlinks", "false");
+ git_config_set("core.symlinks", "false");
/* Check if the filesystem is case-insensitive */
path = git_path_buf(&buf, "CoNfIg");
if (!access(path, F_OK))
- git_config_set_or_die("core.ignorecase", "true");
+ git_config_set("core.ignorecase", "true");
probe_utf8_pathname_composition();
}
@@ -386,8 +386,8 @@ int init_db(const char *template_dir, unsigned int flags)
xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_EVERYBODY);
else
die("BUG: invalid value for shared_repository");
- git_config_set_or_die("core.sharedrepository", buf);
- git_config_set_or_die("receive.denyNonFastforwards", "true");
+ git_config_set("core.sharedrepository", buf);
+ git_config_set("receive.denyNonFastforwards", "true");
}
if (!(flags & INIT_DB_QUIET)) {
diff --git a/builtin/remote.c b/builtin/remote.c
index abd5f67..ecb9b26 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -119,7 +119,7 @@ static void add_branch(const char *key, const char *branchname,
else
strbuf_addf(tmp, "refs/heads/%s:refs/remotes/%s/%s",
branchname, remotename, branchname);
- git_config_set_multivar_or_die(key, tmp->buf, "^$", 0);
+ git_config_set_multivar(key, tmp->buf, "^$", 0);
}
static const char mirror_advice[] =
@@ -197,7 +197,7 @@ static int add(int argc, const char **argv)
die(_("'%s' is not a valid remote name"), name);
strbuf_addf(&buf, "remote.%s.url", name);
- git_config_set_or_die(buf.buf, url);
+ git_config_set(buf.buf, url);
if (!mirror || mirror & MIRROR_FETCH) {
strbuf_reset(&buf);
@@ -213,14 +213,14 @@ static int add(int argc, const char **argv)
if (mirror & MIRROR_PUSH) {
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.mirror", name);
- git_config_set_or_die(buf.buf, "true");
+ git_config_set(buf.buf, "true");
}
if (fetch_tags != TAGS_DEFAULT) {
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.tagopt", name);
- git_config_set_or_die(buf.buf,
- fetch_tags == TAGS_SET ? "--tags" : "--no-tags");
+ git_config_set(buf.buf,
+ fetch_tags == TAGS_SET ? "--tags" : "--no-tags");
}
if (fetch && fetch_remote(name))
@@ -588,15 +588,15 @@ static int migrate_file(struct remote *remote)
strbuf_addf(&buf, "remote.%s.url", remote->name);
for (i = 0; i < remote->url_nr; i++)
- git_config_set_multivar_or_die(buf.buf, remote->url[i], "^$", 0);
+ git_config_set_multivar(buf.buf, remote->url[i], "^$", 0);
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.push", remote->name);
for (i = 0; i < remote->push_refspec_nr; i++)
- git_config_set_multivar_or_die(buf.buf, remote->push_refspec[i], "^$", 0);
+ git_config_set_multivar(buf.buf, remote->push_refspec[i], "^$", 0);
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.fetch", remote->name);
for (i = 0; i < remote->fetch_refspec_nr; i++)
- git_config_set_multivar_or_die(buf.buf, remote->fetch_refspec[i], "^$", 0);
+ git_config_set_multivar(buf.buf, remote->fetch_refspec[i], "^$", 0);
if (remote->origin == REMOTE_REMOTES)
unlink_or_warn(git_path("remotes/%s", remote->name));
else if (remote->origin == REMOTE_BRANCHES)
@@ -648,7 +648,7 @@ static int mv(int argc, const char **argv)
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.fetch", rename.new);
- git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1);
+ git_config_set_multivar(buf.buf, NULL, NULL, 1);
strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old);
for (i = 0; i < oldremote->fetch_refspec_nr; i++) {
char *ptr;
@@ -668,7 +668,7 @@ static int mv(int argc, const char **argv)
"\tPlease update the configuration manually if necessary."),
buf2.buf);
- git_config_set_multivar_or_die(buf.buf, buf2.buf, "^$", 0);
+ git_config_set_multivar(buf.buf, buf2.buf, "^$", 0);
}
read_branches();
@@ -678,7 +678,7 @@ static int mv(int argc, const char **argv)
if (info->remote_name && !strcmp(info->remote_name, rename.old)) {
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.remote", item->string);
- git_config_set_or_die(buf.buf, rename.new);
+ git_config_set(buf.buf, rename.new);
}
}
@@ -776,7 +776,7 @@ static int rm(int argc, const char **argv)
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.%s",
item->string, *k);
- git_config_set_or_die(buf.buf, NULL);
+ git_config_set(buf.buf, NULL);
}
}
}
@@ -1560,10 +1560,10 @@ static int set_url(int argc, const char **argv)
/* Special cases that add new entry. */
if ((!oldurl && !delete_mode) || add_mode) {
if (add_mode)
- git_config_set_multivar_or_die(name_buf.buf, newurl,
+ git_config_set_multivar(name_buf.buf, newurl,
"^$", 0);
else
- git_config_set_or_die(name_buf.buf, newurl);
+ git_config_set(name_buf.buf, newurl);
strbuf_release(&name_buf);
return 0;
@@ -1586,9 +1586,9 @@ static int set_url(int argc, const char **argv)
regfree(&old_regex);
if (!delete_mode)
- git_config_set_multivar_or_die(name_buf.buf, newurl, oldurl, 0);
+ git_config_set_multivar(name_buf.buf, newurl, oldurl, 0);
else
- git_config_set_multivar_or_die(name_buf.buf, NULL, oldurl, 1);
+ git_config_set_multivar(name_buf.buf, NULL, oldurl, 1);
return 0;
}
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index c7e1ea2..f4c3eff 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -245,8 +245,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
p = git_pathdup_submodule(path, "config");
if (!p)
die(_("could not get submodule directory for '%s'"), path);
- git_config_set_in_file_or_die(p, "core.worktree",
- relative_path(sb.buf, sm_gitdir, &rel_path));
+ git_config_set_in_file(p, "core.worktree",
+ relative_path(sb.buf, sm_gitdir, &rel_path));
strbuf_release(&sb);
strbuf_release(&rel_path);
free(sm_gitdir);
diff --git a/cache.h b/cache.h
index a97f677..876106a 100644
--- a/cache.h
+++ b/cache.h
@@ -1508,15 +1508,15 @@ extern int git_config_maybe_bool(const char *, const char *);
extern int git_config_string(const char **, const char *, const char *);
extern int git_config_pathname(const char **, const char *, const char *);
extern int git_config_set_in_file_gently(const char *, const char *, const char *);
-extern void git_config_set_in_file_or_die(const char *, const char *, const char *);
+extern void git_config_set_in_file(const char *, const char *, const char *);
extern int git_config_set_gently(const char *, const char *);
-extern void git_config_set_or_die(const char *, const char *);
+extern void git_config_set(const char *, const char *);
extern int git_config_parse_key(const char *, char **, int *);
extern int git_config_key_is_valid(const char *key);
extern int git_config_set_multivar_gently(const char *, const char *, const char *, int);
-extern void git_config_set_multivar_or_die(const char *, const char *, const char *, int);
+extern void git_config_set_multivar(const char *, const char *, const char *, int);
extern int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, int);
-extern void git_config_set_multivar_in_file_or_die(const char *, const char *, const char *, const char *, int);
+extern void git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
extern int git_config_rename_section(const char *, const char *);
extern int git_config_rename_section_in_file(const char *, const char *, const char *);
extern const char *git_etc_gitconfig(void);
diff --git a/config.c b/config.c
index e7f42da..325c3ea 100644
--- a/config.c
+++ b/config.c
@@ -1831,10 +1831,10 @@ int git_config_set_in_file_gently(const char *config_filename,
return git_config_set_multivar_in_file_gently(config_filename, key, value, NULL, 0);
}
-void git_config_set_in_file_or_die(const char *config_filename,
- const char *key, const char *value)
+void git_config_set_in_file(const char *config_filename,
+ const char *key, const char *value)
{
- git_config_set_multivar_in_file_or_die(config_filename, key, value, NULL, 0);
+ git_config_set_multivar_in_file(config_filename, key, value, NULL, 0);
}
int git_config_set_gently(const char *key, const char *value)
@@ -1842,9 +1842,9 @@ int git_config_set_gently(const char *key, const char *value)
return git_config_set_multivar_gently(key, value, NULL, 0);
}
-void git_config_set_or_die(const char *key, const char *value)
+void git_config_set(const char *key, const char *value)
{
- git_config_set_multivar_or_die(key, value, NULL, 0);
+ git_config_set_multivar(key, value, NULL, 0);
}
/*
@@ -2191,9 +2191,9 @@ write_err_out:
}
-void git_config_set_multivar_in_file_or_die(const char *config_filename,
- const char *key, const char *value,
- const char *value_regex, int multi_replace)
+void git_config_set_multivar_in_file(const char *config_filename,
+ const char *key, const char *value,
+ const char *value_regex, int multi_replace)
{
if (git_config_set_multivar_in_file_gently(config_filename, key, value,
value_regex, multi_replace) < 0)
@@ -2207,11 +2207,11 @@ int git_config_set_multivar_gently(const char *key, const char *value,
multi_replace);
}
-void git_config_set_multivar_or_die(const char *key, const char *value,
- const char *value_regex, int multi_replace)
+void git_config_set_multivar(const char *key, const char *value,
+ const char *value_regex, int multi_replace)
{
- git_config_set_multivar_in_file_or_die(NULL, key, value, value_regex,
- multi_replace);
+ git_config_set_multivar_in_file(NULL, key, value, value_regex,
+ multi_replace);
}
static int section_name_match (const char *buf, const char *name)
diff --git a/sequencer.c b/sequencer.c
index 3590248..8048786 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -933,31 +933,31 @@ static void save_opts(struct replay_opts *opts)
const char *opts_file = git_path_opts_file();
if (opts->no_commit)
- git_config_set_in_file_or_die(opts_file, "options.no-commit", "true");
+ git_config_set_in_file(opts_file, "options.no-commit", "true");
if (opts->edit)
- git_config_set_in_file_or_die(opts_file, "options.edit", "true");
+ git_config_set_in_file(opts_file, "options.edit", "true");
if (opts->signoff)
- git_config_set_in_file_or_die(opts_file, "options.signoff", "true");
+ git_config_set_in_file(opts_file, "options.signoff", "true");
if (opts->record_origin)
- git_config_set_in_file_or_die(opts_file, "options.record-origin", "true");
+ git_config_set_in_file(opts_file, "options.record-origin", "true");
if (opts->allow_ff)
- git_config_set_in_file_or_die(opts_file, "options.allow-ff", "true");
+ git_config_set_in_file(opts_file, "options.allow-ff", "true");
if (opts->mainline) {
struct strbuf buf = STRBUF_INIT;
strbuf_addf(&buf, "%d", opts->mainline);
- git_config_set_in_file_or_die(opts_file, "options.mainline", buf.buf);
+ git_config_set_in_file(opts_file, "options.mainline", buf.buf);
strbuf_release(&buf);
}
if (opts->strategy)
- git_config_set_in_file_or_die(opts_file, "options.strategy", opts->strategy);
+ git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
if (opts->gpg_sign)
- git_config_set_in_file_or_die(opts_file, "options.gpg-sign", opts->gpg_sign);
+ git_config_set_in_file(opts_file, "options.gpg-sign", opts->gpg_sign);
if (opts->xopts) {
int i;
for (i = 0; i < opts->xopts_nr; i++)
- git_config_set_multivar_in_file_or_die(opts_file,
- "options.strategy-option",
- opts->xopts[i], "^$", 0);
+ git_config_set_multivar_in_file(opts_file,
+ "options.strategy-option",
+ opts->xopts[i], "^$", 0);
}
}
diff --git a/submodule.c b/submodule.c
index be8b5cc..b3fc6ac 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1087,9 +1087,9 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
/* Update core.worktree setting */
strbuf_reset(&file_name);
strbuf_addf(&file_name, "%s/config", git_dir);
- git_config_set_in_file_or_die(file_name.buf, "core.worktree",
- relative_path(real_work_tree, git_dir,
- &rel_path));
+ git_config_set_in_file(file_name.buf, "core.worktree",
+ relative_path(real_work_tree, git_dir,
+ &rel_path));
strbuf_release(&file_name);
strbuf_release(&rel_path);
--
2.7.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v4 00/15] config: make git_config_set die on failure
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (14 preceding siblings ...)
2016-02-02 11:51 ` [PATCH v4 15/15] config: rename git_config_set_or_die to git_config_set Patrick Steinhardt
@ 2016-02-02 18:52 ` Stefan Beller
2016-02-02 20:58 ` Junio C Hamano
16 siblings, 0 replies; 26+ messages in thread
From: Stefan Beller @ 2016-02-02 18:52 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jeff King, Junio C Hamano
On Tue, Feb 2, 2016 at 3:51 AM, Patrick Steinhardt <ps@pks.im> wrote:
> submodule: die on config error when linking modules
> submodule--helper: die on config error when cloning module
The code in the submodule related patches looks fine to me.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v4 00/15] config: make git_config_set die on failure
2016-02-02 11:51 [PATCH v4 00/15] config: make git_config_set die on failure Patrick Steinhardt
` (15 preceding siblings ...)
2016-02-02 18:52 ` [PATCH v4 00/15] config: make git_config_set die on failure Stefan Beller
@ 2016-02-02 20:58 ` Junio C Hamano
2016-02-04 8:56 ` Patrick Steinhardt
16 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2016-02-02 20:58 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jeff King
Patrick Steinhardt <ps@pks.im> writes:
> Patrick Steinhardt (15):
> config: introduce set_or_die wrappers
> branch: die on error in setting up tracking branch
> branch: die on config error when unsetting upstream
> branch: die on config error when editing branch description
> submodule: die on config error when linking modules
> submodule--helper: die on config error when cloning module
> remote: die on config error when setting URL
> remote: die on config error when setting/adding branches
> remote: die on config error when manipulating remotes
> clone: die on config error in cmd_clone
> init-db: die on config errors when initializing empty repo
> sequencer: die on config error when saving replay opts
> compat: die when unable to set core.precomposeunicode
> config: rename git_config_set to git_config_set_gently
> config: rename git_config_set_or_die to git_config_set
They mostly looked sensible; I commented on things that I noticed,
but I don't think I spotted anything that needs a huge fix.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v4 00/15] config: make git_config_set die on failure
2016-02-02 20:58 ` Junio C Hamano
@ 2016-02-04 8:56 ` Patrick Steinhardt
0 siblings, 0 replies; 26+ messages in thread
From: Patrick Steinhardt @ 2016-02-04 8:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
[-- Attachment #1: Type: text/plain, Size: 1247 bytes --]
On Tue, Feb 02, 2016 at 12:58:42PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > Patrick Steinhardt (15):
> > config: introduce set_or_die wrappers
> > branch: die on error in setting up tracking branch
> > branch: die on config error when unsetting upstream
> > branch: die on config error when editing branch description
> > submodule: die on config error when linking modules
> > submodule--helper: die on config error when cloning module
> > remote: die on config error when setting URL
> > remote: die on config error when setting/adding branches
> > remote: die on config error when manipulating remotes
> > clone: die on config error in cmd_clone
> > init-db: die on config errors when initializing empty repo
> > sequencer: die on config error when saving replay opts
> > compat: die when unable to set core.precomposeunicode
> > config: rename git_config_set to git_config_set_gently
> > config: rename git_config_set_or_die to git_config_set
>
> They mostly looked sensible; I commented on things that I noticed,
> but I don't think I spotted anything that needs a huge fix.
>
> Thanks.
Thanks for the feedback. Will post v5 next week.
Patrick
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread