* [PATCH v14 33/41] apply: make it possible to silently apply
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
This changes 'int apply_verbosely' into 'enum apply_verbosity', and
changes the possible values of the variable from a bool to
a tristate.
The previous 'false' state is changed into 'verbosity_normal'.
The previous 'true' state is changed into 'verbosity_verbose'.
The new added state is 'verbosity_silent'. It should prevent
anything to be printed on both stderr and stdout.
This is needed because `git am` wants to first call apply
functionality silently, if it can then fall back on 3-way merge
in case of error.
Printing on stdout, and calls to warning() or error() are not
taken care of in this patch, as that will be done in following
patches.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 62 +++++++++++++++++++++++++++++++++++++--------------------
apply.h | 8 +++++++-
builtin/apply.c | 2 +-
3 files changed, 48 insertions(+), 24 deletions(-)
diff --git a/apply.c b/apply.c
index 41a33d3..df85cbc 100644
--- a/apply.c
+++ b/apply.c
@@ -125,8 +125,11 @@ int check_apply_state(struct apply_state *state, int force_apply)
return error(_("--3way outside a repository"));
state->check_index = 1;
}
- if (state->apply_with_reject)
- state->apply = state->apply_verbosely = 1;
+ if (state->apply_with_reject) {
+ state->apply = 1;
+ if (state->apply_verbosity == verbosity_normal)
+ state->apply_verbosity = verbosity_verbose;
+ }
if (!force_apply && (state->diffstat || state->numstat || state->summary || state->check || state->fake_ancestor))
state->apply = 0;
if (state->check_index && is_not_gitdir)
@@ -1620,8 +1623,9 @@ static void record_ws_error(struct apply_state *state,
return;
err = whitespace_error_string(result);
- fprintf(stderr, "%s:%d: %s.\n%.*s\n",
- state->patch_input_file, linenr, err, len, line);
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf(stderr, "%s:%d: %s.\n%.*s\n",
+ state->patch_input_file, linenr, err, len, line);
free(err);
}
@@ -1816,7 +1820,7 @@ static int parse_single_patch(struct apply_state *state,
return error(_("new file %s depends on old contents"), patch->new_name);
if (0 < patch->is_delete && newlines)
return error(_("deleted file %s still has contents"), patch->old_name);
- if (!patch->is_delete && !newlines && context)
+ if (!patch->is_delete && !newlines && context && state->apply_verbosity > verbosity_silent)
fprintf_ln(stderr,
_("** warning: "
"file %s becomes empty but is not deleted"),
@@ -2911,7 +2915,7 @@ static int apply_one_fragment(struct apply_state *state,
/* Ignore it, we already handled it */
break;
default:
- if (state->apply_verbosely)
+ if (state->apply_verbosity > verbosity_normal)
error(_("invalid start of line: '%c'"), first);
applied_pos = -1;
goto out;
@@ -3026,7 +3030,7 @@ static int apply_one_fragment(struct apply_state *state,
state->apply = 0;
}
- if (state->apply_verbosely && applied_pos != pos) {
+ if (state->apply_verbosity > verbosity_normal && applied_pos != pos) {
int offset = applied_pos - pos;
if (state->apply_in_reverse)
offset = 0 - offset;
@@ -3041,14 +3045,14 @@ static int apply_one_fragment(struct apply_state *state,
* Warn if it was necessary to reduce the number
* of context lines.
*/
- if ((leading != frag->leading) ||
- (trailing != frag->trailing))
+ if ((leading != frag->leading ||
+ trailing != frag->trailing) && state->apply_verbosity > verbosity_silent)
fprintf_ln(stderr, _("Context reduced to (%ld/%ld)"
" to apply fragment at %d"),
leading, trailing, applied_pos+1);
update_image(state, img, applied_pos, &preimage, &postimage);
} else {
- if (state->apply_verbosely)
+ if (state->apply_verbosity > verbosity_normal)
error(_("while searching for:\n%.*s"),
(int)(old - oldlines), oldlines);
}
@@ -3539,7 +3543,8 @@ static int try_threeway(struct apply_state *state,
read_blob_object(&buf, pre_sha1, patch->old_mode))
return error("repository lacks the necessary blob to fall back on 3-way merge.");
- fprintf(stderr, "Falling back to three-way merge...\n");
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf(stderr, "Falling back to three-way merge...\n");
img = strbuf_detach(&buf, &len);
prepare_image(&tmp_image, img, len, 1);
@@ -3569,7 +3574,9 @@ static int try_threeway(struct apply_state *state,
status = three_way_merge(image, patch->new_name,
pre_sha1, our_sha1, post_sha1);
if (status < 0) {
- fprintf(stderr, "Failed to fall back on three-way merge...\n");
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf(stderr,
+ "Failed to fall back on three-way merge...\n");
return status;
}
@@ -3581,9 +3588,15 @@ static int try_threeway(struct apply_state *state,
hashcpy(patch->threeway_stage[0].hash, pre_sha1);
hashcpy(patch->threeway_stage[1].hash, our_sha1);
hashcpy(patch->threeway_stage[2].hash, post_sha1);
- fprintf(stderr, "Applied patch to '%s' with conflicts.\n", patch->new_name);
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf(stderr,
+ "Applied patch to '%s' with conflicts.\n",
+ patch->new_name);
} else {
- fprintf(stderr, "Applied patch to '%s' cleanly.\n", patch->new_name);
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf(stderr,
+ "Applied patch to '%s' cleanly.\n",
+ patch->new_name);
}
return 0;
}
@@ -3956,7 +3969,7 @@ static int check_patch_list(struct apply_state *state, struct patch *patch)
prepare_fn_table(state, patch);
while (patch) {
int res;
- if (state->apply_verbosely)
+ if (state->apply_verbosity > verbosity_normal)
say_patch_name(stderr,
_("Checking patch %s..."), patch);
res = check_patch(state, patch);
@@ -4472,7 +4485,7 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
}
if (!cnt) {
- if (state->apply_verbosely)
+ if (state->apply_verbosity > verbosity_normal)
say_patch_name(stderr,
_("Applied patch %s cleanly."), patch);
return 0;
@@ -4489,7 +4502,8 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
"Applying patch %%s with %d rejects...",
cnt),
cnt);
- say_patch_name(stderr, sb.buf, patch);
+ if (state->apply_verbosity > verbosity_silent)
+ say_patch_name(stderr, sb.buf, patch);
strbuf_release(&sb);
cnt = strlen(patch->new_name);
@@ -4516,10 +4530,12 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
frag;
cnt++, frag = frag->next) {
if (!frag->rejected) {
- fprintf_ln(stderr, _("Hunk #%d applied cleanly."), cnt);
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf_ln(stderr, _("Hunk #%d applied cleanly."), cnt);
continue;
}
- fprintf_ln(stderr, _("Rejected hunk #%d."), cnt);
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf_ln(stderr, _("Rejected hunk #%d."), cnt);
fprintf(rej, "%.*s", frag->size, frag->patch);
if (frag->patch[frag->size-1] != '\n')
fputc('\n', rej);
@@ -4568,8 +4584,10 @@ static int write_out_results(struct apply_state *state, struct patch *list)
struct string_list_item *item;
string_list_sort(&cpath);
- for_each_string_list_item(item, &cpath)
- fprintf(stderr, "U %s\n", item->string);
+ if (state->apply_verbosity > verbosity_silent) {
+ for_each_string_list_item(item, &cpath)
+ fprintf(stderr, "U %s\n", item->string);
+ }
string_list_clear(&cpath, 0);
rerere(0);
@@ -4626,7 +4644,7 @@ static int apply_patch(struct apply_state *state,
listp = &patch->next;
}
else {
- if (state->apply_verbosely)
+ if (state->apply_verbosity > verbosity_normal)
say_patch_name(stderr, _("Skipped patch '%s'."), patch);
free_patch(patch);
skipped_patch++;
diff --git a/apply.h b/apply.h
index 51b983b..f015403 100644
--- a/apply.h
+++ b/apply.h
@@ -13,6 +13,12 @@ enum apply_ws_ignore {
ignore_ws_change
};
+enum apply_verbosity {
+ verbosity_silent = -1,
+ verbosity_normal = 0,
+ verbosity_verbose = 1
+};
+
/*
* We need to keep track of how symlinks in the preimage are
* manipulated by the patches. A patch to add a/b/c where a/b
@@ -51,13 +57,13 @@ struct apply_state {
int allow_overlap;
int apply_in_reverse;
int apply_with_reject;
- int apply_verbosely;
int no_add;
int threeway;
int unidiff_zero;
int unsafe_paths;
/* Other non boolean parameters */
+ enum apply_verbosity apply_verbosity;
const char *fake_ancestor;
const char *patch_input_file;
int line_termination;
diff --git a/builtin/apply.c b/builtin/apply.c
index 9c66474..7338701 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -74,7 +74,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
N_("leave the rejected hunks in corresponding *.rej files")),
OPT_BOOL(0, "allow-overlap", &state.allow_overlap,
N_("allow overlapping hunks")),
- OPT__VERBOSE(&state.apply_verbosely, N_("be verbose")),
+ OPT__VERBOSE(&state.apply_verbosity, N_("be verbose")),
OPT_BIT(0, "inaccurate-eof", &options,
N_("tolerate incorrectly detected missing new-line at the end of file"),
APPLY_OPT_INACCURATE_EOF),
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 28/41] builtin/apply: rename option parsing functions
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
As these functions are going to be part of the libified
apply API, let's give them a name that is more specific
to the apply API.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 5cd7fd8..1f56303 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4588,16 +4588,16 @@ static int apply_patch(struct apply_state *state,
return res;
}
-static int option_parse_exclude(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_exclude(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
add_name_limit(state, arg, 1);
return 0;
}
-static int option_parse_include(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_include(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
add_name_limit(state, arg, 0);
@@ -4605,9 +4605,9 @@ static int option_parse_include(const struct option *opt,
return 0;
}
-static int option_parse_p(const struct option *opt,
- const char *arg,
- int unset)
+static int apply_option_parse_p(const struct option *opt,
+ const char *arg,
+ int unset)
{
struct apply_state *state = opt->value;
state->p_value = atoi(arg);
@@ -4615,8 +4615,8 @@ static int option_parse_p(const struct option *opt,
return 0;
}
-static int option_parse_space_change(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_space_change(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
if (unset)
@@ -4626,8 +4626,8 @@ static int option_parse_space_change(const struct option *opt,
return 0;
}
-static int option_parse_whitespace(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_whitespace(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
state->whitespace_option = arg;
@@ -4636,8 +4636,8 @@ static int option_parse_whitespace(const struct option *opt,
return 0;
}
-static int option_parse_directory(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_directory(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
strbuf_reset(&state->root);
@@ -4755,13 +4755,13 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
struct option builtin_apply_options[] = {
{ OPTION_CALLBACK, 0, "exclude", &state, N_("path"),
N_("don't apply changes matching the given path"),
- 0, option_parse_exclude },
+ 0, apply_option_parse_exclude },
{ OPTION_CALLBACK, 0, "include", &state, N_("path"),
N_("apply changes matching the given path"),
- 0, option_parse_include },
+ 0, apply_option_parse_include },
{ OPTION_CALLBACK, 'p', NULL, &state, N_("num"),
N_("remove <num> leading slashes from traditional diff paths"),
- 0, option_parse_p },
+ 0, apply_option_parse_p },
OPT_BOOL(0, "no-add", &state.no_add,
N_("ignore additions made by the patch")),
OPT_BOOL(0, "stat", &state.diffstat,
@@ -4793,13 +4793,13 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
N_("ensure at least <n> lines of context match")),
{ OPTION_CALLBACK, 0, "whitespace", &state, N_("action"),
N_("detect new or modified lines that have whitespace errors"),
- 0, option_parse_whitespace },
+ 0, apply_option_parse_whitespace },
{ OPTION_CALLBACK, 0, "ignore-space-change", &state, NULL,
N_("ignore changes in whitespace when finding context"),
- PARSE_OPT_NOARG, option_parse_space_change },
+ PARSE_OPT_NOARG, apply_option_parse_space_change },
{ OPTION_CALLBACK, 0, "ignore-whitespace", &state, NULL,
N_("ignore changes in whitespace when finding context"),
- PARSE_OPT_NOARG, option_parse_space_change },
+ PARSE_OPT_NOARG, apply_option_parse_space_change },
OPT_BOOL('R', "reverse", &state.apply_in_reverse,
N_("apply the patch in reverse")),
OPT_BOOL(0, "unidiff-zero", &state.unidiff_zero,
@@ -4817,7 +4817,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
RECOUNT),
{ OPTION_CALLBACK, 0, "directory", &state, N_("root"),
N_("prepend <root> to all filenames"),
- 0, option_parse_directory },
+ 0, apply_option_parse_directory },
OPT_END()
};
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 32/41] apply: use error_errno() where possible
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To avoid possible mistakes and to uniformly show the errno
related messages, let's use error_errno() where possible.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/apply.c b/apply.c
index a4dfc64..41a33d3 100644
--- a/apply.c
+++ b/apply.c
@@ -3497,7 +3497,7 @@ static int load_current(struct apply_state *state,
ce = active_cache[pos];
if (lstat(name, &st)) {
if (errno != ENOENT)
- return error(_("%s: %s"), name, strerror(errno));
+ return error_errno("%s", name);
if (checkout_target(&the_index, ce, &st))
return -1;
}
@@ -3647,7 +3647,7 @@ static int check_preimage(struct apply_state *state,
} else if (!state->cached) {
stat_ret = lstat(old_name, st);
if (stat_ret && errno != ENOENT)
- return error(_("%s: %s"), old_name, strerror(errno));
+ return error_errno("%s", old_name);
}
if (state->check_index && !previous) {
@@ -3669,7 +3669,7 @@ static int check_preimage(struct apply_state *state,
} else if (stat_ret < 0) {
if (patch->is_new < 0)
goto is_new;
- return error(_("%s: %s"), old_name, strerror(errno));
+ return error_errno("%s", old_name);
}
if (!state->cached && !previous)
@@ -3728,7 +3728,7 @@ static int check_to_create(struct apply_state *state,
return EXISTS_IN_WORKTREE;
} else if ((errno != ENOENT) && (errno != ENOTDIR)) {
- return error("%s: %s", new_name, strerror(errno));
+ return error_errno("%s", new_name);
}
return 0;
}
@@ -4247,9 +4247,9 @@ static int add_index_file(struct apply_state *state,
if (!state->cached) {
if (lstat(path, &st) < 0) {
free(ce);
- return error(_("unable to stat newly "
- "created file '%s': %s"),
- path, strerror(errno));
+ return error_errno(_("unable to stat newly "
+ "created file '%s'"),
+ path);
}
fill_stat_cache_info(ce, &st);
}
@@ -4503,7 +4503,7 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
rej = fopen(namebuf, "w");
if (!rej)
- return error(_("cannot open %s: %s"), namebuf, strerror(errno));
+ return error_errno(_("cannot open %s"), namebuf);
/* Normal git tools never deal with .rej, so do not pretend
* this is a git patch by saying --git or giving extended
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 31/41] apply: make some parsing functions static again
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
Some parsing functions that were used in both "apply.c" and
"builtin/apply.c" are now only used in the former, so they
can be made static to "apply.c".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 6 +++---
apply.h | 5 -----
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/apply.c b/apply.c
index f640224..a4dfc64 100644
--- a/apply.c
+++ b/apply.c
@@ -27,7 +27,7 @@ static void git_apply_config(void)
git_config(git_default_config, NULL);
}
-int parse_whitespace_option(struct apply_state *state, const char *option)
+static int parse_whitespace_option(struct apply_state *state, const char *option)
{
if (!option) {
state->ws_error_action = warn_on_ws_error;
@@ -57,8 +57,8 @@ int parse_whitespace_option(struct apply_state *state, const char *option)
return error(_("unrecognized whitespace option '%s'"), option);
}
-int parse_ignorewhitespace_option(struct apply_state *state,
- const char *option)
+static int parse_ignorewhitespace_option(struct apply_state *state,
+ const char *option)
{
if (!option || !strcmp(option, "no") ||
!strcmp(option, "false") || !strcmp(option, "never") ||
diff --git a/apply.h b/apply.h
index 28cbe6c..51b983b 100644
--- a/apply.h
+++ b/apply.h
@@ -97,11 +97,6 @@ struct apply_state {
int applied_after_fixing_ws;
};
-extern int parse_whitespace_option(struct apply_state *state,
- const char *option);
-extern int parse_ignorewhitespace_option(struct apply_state *state,
- const char *option);
-
extern int apply_option_parse_exclude(const struct option *opt,
const char *arg, int unset);
extern int apply_option_parse_include(const struct option *opt,
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 27/41] builtin/apply: make create_one_file() return -1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", create_one_file() should return -1 instead of
calling exit().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 3145e03..5cd7fd8 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4198,32 +4198,36 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,
* We optimistically assume that the directories exist,
* which is true 99% of the time anyway. If they don't,
* we create them and try again.
+ *
+ * Returns:
+ * -1 on error
+ * 0 otherwise
*/
-static void create_one_file(struct apply_state *state,
- char *path,
- unsigned mode,
- const char *buf,
- unsigned long size)
+static int create_one_file(struct apply_state *state,
+ char *path,
+ unsigned mode,
+ const char *buf,
+ unsigned long size)
{
int res;
if (state->cached)
- return;
+ return 0;
res = try_create_file(path, mode, buf, size);
if (res < 0)
- exit(128);
+ return -1;
if (!res)
- return;
+ return 0;
if (errno == ENOENT) {
if (safe_create_leading_directories(path))
- return;
+ return 0;
res = try_create_file(path, mode, buf, size);
if (res < 0)
- exit(128);
+ return -1;
if (!res)
- return;
+ return 0;
}
if (errno == EEXIST || errno == EACCES) {
@@ -4243,10 +4247,10 @@ static void create_one_file(struct apply_state *state,
mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr);
res = try_create_file(newpath, mode, buf, size);
if (res < 0)
- exit(128);
+ return -1;
if (!res) {
if (!rename(newpath, path))
- return;
+ return 0;
unlink_or_warn(newpath);
break;
}
@@ -4255,7 +4259,8 @@ static void create_one_file(struct apply_state *state,
++nr;
}
}
- die_errno(_("unable to write file '%s' mode %o"), path, mode);
+ return error_errno(_("unable to write file '%s' mode %o"),
+ path, mode);
}
static int add_conflicted_stages_file(struct apply_state *state,
@@ -4300,7 +4305,8 @@ static int create_file(struct apply_state *state, struct patch *patch)
if (!mode)
mode = S_IFREG | 0644;
- create_one_file(state, path, mode, buf, size);
+ if (create_one_file(state, path, mode, buf, size))
+ return -1;
if (patch->conflicted_threeway)
return add_conflicted_stages_file(state, patch);
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 29/41] apply: rename and move opt constants to apply.h
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
The constants for the "inaccurate-eof" and the "recount" options will
be used in both "apply.c" and "builtin/apply.c", so they need to go
into "apply.h", and therefore they need a name that is more specific
to the API they belong to.
Helped-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.h | 7 +++++++
builtin/apply.c | 11 ++++-------
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/apply.h b/apply.h
index 53f09b5..48abd8e 100644
--- a/apply.h
+++ b/apply.h
@@ -108,4 +108,11 @@ extern int init_apply_state(struct apply_state *state,
extern void clear_apply_state(struct apply_state *state);
extern int check_apply_state(struct apply_state *state, int force_apply);
+/*
+ * Some aspects of the apply behavior are controlled by the following
+ * bits in the "options" parameter passed to apply_all_patches().
+ */
+#define APPLY_OPT_INACCURATE_EOF (1<<0) /* accept inaccurate eof */
+#define APPLY_OPT_RECOUNT (1<<1) /* accept inaccurate line count */
+
#endif
diff --git a/builtin/apply.c b/builtin/apply.c
index 1f56303..da31af2 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4463,9 +4463,6 @@ static int write_out_results(struct apply_state *state, struct patch *list)
static struct lock_file lock_file;
-#define INACCURATE_EOF (1<<0)
-#define RECOUNT (1<<1)
-
/*
* Try to apply a patch.
*
@@ -4495,8 +4492,8 @@ static int apply_patch(struct apply_state *state,
int nr;
patch = xcalloc(1, sizeof(*patch));
- patch->inaccurate_eof = !!(options & INACCURATE_EOF);
- patch->recount = !!(options & RECOUNT);
+ patch->inaccurate_eof = !!(options & APPLY_OPT_INACCURATE_EOF);
+ patch->recount = !!(options & APPLY_OPT_RECOUNT);
nr = parse_chunk(state, buf.buf + offset, buf.len - offset, patch);
if (nr < 0) {
free_patch(patch);
@@ -4811,10 +4808,10 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
OPT__VERBOSE(&state.apply_verbosely, N_("be verbose")),
OPT_BIT(0, "inaccurate-eof", &options,
N_("tolerate incorrectly detected missing new-line at the end of file"),
- INACCURATE_EOF),
+ APPLY_OPT_INACCURATE_EOF),
OPT_BIT(0, "recount", &options,
N_("do not trust the line counts in the hunk headers"),
- RECOUNT),
+ APPLY_OPT_RECOUNT),
{ OPTION_CALLBACK, 0, "directory", &state, N_("root"),
N_("prepend <root> to all filenames"),
0, apply_option_parse_directory },
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 25/41] builtin/apply: make write_out_results() return -1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", write_out_results() should return -1 instead of
calling exit().
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 003acec..c787ead 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4383,6 +4383,12 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
return -1;
}
+/*
+ * Returns:
+ * -1 if an error happened
+ * 0 if the patch applied cleanly
+ * 1 if the patch did not apply cleanly
+ */
static int write_out_results(struct apply_state *state, struct patch *list)
{
int phase;
@@ -4396,8 +4402,10 @@ static int write_out_results(struct apply_state *state, struct patch *list)
if (l->rejected)
errs = 1;
else {
- if (write_out_one_result(state, l, phase))
- exit(128);
+ if (write_out_one_result(state, l, phase)) {
+ string_list_clear(&cpath, 0);
+ return -1;
+ }
if (phase == 1) {
if (write_out_one_reject(state, l))
errs = 1;
@@ -4517,10 +4525,17 @@ static int apply_patch(struct apply_state *state,
}
}
- if (state->apply && write_out_results(state, list)) {
- /* with --3way, we still need to write the index out */
- res = state->apply_with_reject ? -1 : 1;
- goto end;
+ if (state->apply) {
+ int write_res = write_out_results(state, list);
+ if (write_res < 0) {
+ res = -128;
+ goto end;
+ }
+ if (write_res > 0) {
+ /* with --3way, we still need to write the index out */
+ res = state->apply_with_reject ? -1 : 1;
+ goto end;
+ }
}
if (state->fake_ancestor &&
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 21/41] builtin/apply: make add_conflicted_stages_file() return -1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", add_conflicted_stages_file() should return -1
instead of calling die().
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 27fb6e2..ad0b875 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4224,7 +4224,7 @@ static void create_one_file(struct apply_state *state,
die_errno(_("unable to write file '%s' mode %o"), path, mode);
}
-static void add_conflicted_stages_file(struct apply_state *state,
+static int add_conflicted_stages_file(struct apply_state *state,
struct patch *patch)
{
int stage, namelen;
@@ -4232,7 +4232,7 @@ static void add_conflicted_stages_file(struct apply_state *state,
struct cache_entry *ce;
if (!state->update_index)
- return;
+ return 0;
namelen = strlen(patch->new_name);
ce_size = cache_entry_size(namelen);
mode = patch->new_mode ? patch->new_mode : (S_IFREG | 0644);
@@ -4247,9 +4247,14 @@ static void add_conflicted_stages_file(struct apply_state *state,
ce->ce_flags = create_ce_flags(stage);
ce->ce_namelen = namelen;
hashcpy(ce->sha1, patch->threeway_stage[stage - 1].hash);
- if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)
- die(_("unable to add cache entry for %s"), patch->new_name);
+ if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
+ free(ce);
+ return error(_("unable to add cache entry for %s"),
+ patch->new_name);
+ }
}
+
+ return 0;
}
static void create_file(struct apply_state *state, struct patch *patch)
@@ -4263,9 +4268,10 @@ static void create_file(struct apply_state *state, struct patch *patch)
mode = S_IFREG | 0644;
create_one_file(state, path, mode, buf, size);
- if (patch->conflicted_threeway)
- add_conflicted_stages_file(state, patch);
- else
+ if (patch->conflicted_threeway) {
+ if (add_conflicted_stages_file(state, patch))
+ exit(128);
+ } else
add_index_file(state, path, mode, buf, size);
}
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 24/41] builtin/apply: make write_out_one_result() return -1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", write_out_one_result() should just return what
remove_file() and create_file() are returning instead of calling
exit().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 38 ++++++++++++++++----------------------
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index fdfeab0..003acec 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4287,36 +4287,29 @@ static int create_file(struct apply_state *state, struct patch *patch)
}
/* phase zero is to remove, phase one is to create */
-static void write_out_one_result(struct apply_state *state,
- struct patch *patch,
- int phase)
+static int write_out_one_result(struct apply_state *state,
+ struct patch *patch,
+ int phase)
{
if (patch->is_delete > 0) {
- if (phase == 0) {
- if (remove_file(state, patch, 1))
- exit(128);
- }
- return;
+ if (phase == 0)
+ return remove_file(state, patch, 1);
+ return 0;
}
if (patch->is_new > 0 || patch->is_copy) {
- if (phase == 1) {
- if (create_file(state, patch))
- exit(128);
- }
- return;
+ if (phase == 1)
+ return create_file(state, patch);
+ return 0;
}
/*
* Rename or modification boils down to the same
* thing: remove the old, write the new
*/
- if (phase == 0) {
- if (remove_file(state, patch, patch->is_rename))
- exit(128);
- }
- if (phase == 1) {
- if (create_file(state, patch))
- exit(128);
- }
+ if (phase == 0)
+ return remove_file(state, patch, patch->is_rename);
+ if (phase == 1)
+ return create_file(state, patch);
+ return 0;
}
static int write_out_one_reject(struct apply_state *state, struct patch *patch)
@@ -4403,7 +4396,8 @@ static int write_out_results(struct apply_state *state, struct patch *list)
if (l->rejected)
errs = 1;
else {
- write_out_one_result(state, l, phase);
+ if (write_out_one_result(state, l, phase))
+ exit(128);
if (phase == 1) {
if (write_out_one_reject(state, l))
errs = 1;
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 18/41] builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", die_on_unsafe_path() should return a negative
integer instead of calling die(), so while doing that let's change
its name to check_unsafe_path().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 6b16173..166e94d 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3704,7 +3704,7 @@ static int path_is_beyond_symlink(struct apply_state *state, const char *name_)
return ret;
}
-static void die_on_unsafe_path(struct patch *patch)
+static int check_unsafe_path(struct patch *patch)
{
const char *old_name = NULL;
const char *new_name = NULL;
@@ -3716,9 +3716,10 @@ static void die_on_unsafe_path(struct patch *patch)
new_name = patch->new_name;
if (old_name && !verify_path(old_name))
- die(_("invalid path '%s'"), old_name);
+ return error(_("invalid path '%s'"), old_name);
if (new_name && !verify_path(new_name))
- die(_("invalid path '%s'"), new_name);
+ return error(_("invalid path '%s'"), new_name);
+ return 0;
}
/*
@@ -3808,8 +3809,8 @@ static int check_patch(struct apply_state *state, struct patch *patch)
}
}
- if (!state->unsafe_paths)
- die_on_unsafe_path(patch);
+ if (!state->unsafe_paths && check_unsafe_path(patch))
+ return -128;
/*
* An attempt to read from or delete a path that is beyond a
@@ -3837,10 +3838,14 @@ static int check_patch_list(struct apply_state *state, struct patch *patch)
prepare_symlink_changes(state, patch);
prepare_fn_table(state, patch);
while (patch) {
+ int res;
if (state->apply_verbosely)
say_patch_name(stderr,
_("Checking patch %s..."), patch);
- err |= check_patch(state, patch);
+ res = check_patch(state, patch);
+ if (res == -128)
+ return -128;
+ err |= res;
patch = patch->next;
}
return err;
@@ -4472,11 +4477,16 @@ static int apply_patch(struct apply_state *state,
goto end;
}
- if ((state->check || state->apply) &&
- check_patch_list(state, list) < 0 &&
- !state->apply_with_reject) {
- res = -1;
- goto end;
+ if (state->check || state->apply) {
+ int r = check_patch_list(state, list);
+ if (r == -128) {
+ res = -128;
+ goto end;
+ }
+ if (r < 0 && !state->apply_with_reject) {
+ res = -1;
+ goto end;
+ }
}
if (state->apply && write_out_results(state, list)) {
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 22/41] builtin/apply: make add_index_file() return -1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", add_index_file() should return -1 instead of
calling die().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 48 +++++++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index ad0b875..a646900 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4099,11 +4099,11 @@ static int remove_file(struct apply_state *state, struct patch *patch, int rmdir
return 0;
}
-static void add_index_file(struct apply_state *state,
- const char *path,
- unsigned mode,
- void *buf,
- unsigned long size)
+static int add_index_file(struct apply_state *state,
+ const char *path,
+ unsigned mode,
+ void *buf,
+ unsigned long size)
{
struct stat st;
struct cache_entry *ce;
@@ -4111,7 +4111,7 @@ static void add_index_file(struct apply_state *state,
unsigned ce_size = cache_entry_size(namelen);
if (!state->update_index)
- return;
+ return 0;
ce = xcalloc(1, ce_size);
memcpy(ce->name, path, namelen);
@@ -4122,20 +4122,32 @@ static void add_index_file(struct apply_state *state,
const char *s;
if (!skip_prefix(buf, "Subproject commit ", &s) ||
- get_sha1_hex(s, ce->sha1))
- die(_("corrupt patch for submodule %s"), path);
+ get_sha1_hex(s, ce->sha1)) {
+ free(ce);
+ return error(_("corrupt patch for submodule %s"), path);
+ }
} else {
if (!state->cached) {
- if (lstat(path, &st) < 0)
- die_errno(_("unable to stat newly created file '%s'"),
- path);
+ if (lstat(path, &st) < 0) {
+ free(ce);
+ return error(_("unable to stat newly "
+ "created file '%s': %s"),
+ path, strerror(errno));
+ }
fill_stat_cache_info(ce, &st);
}
- if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)
- die(_("unable to create backing store for newly created file %s"), path);
+ if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0) {
+ free(ce);
+ return error(_("unable to create backing store "
+ "for newly created file %s"), path);
+ }
}
- if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)
- die(_("unable to add cache entry for %s"), path);
+ if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
+ free(ce);
+ return error(_("unable to add cache entry for %s"), path);
+ }
+
+ return 0;
}
static int try_create_file(const char *path, unsigned int mode, const char *buf, unsigned long size)
@@ -4271,8 +4283,10 @@ static void create_file(struct apply_state *state, struct patch *patch)
if (patch->conflicted_threeway) {
if (add_conflicted_stages_file(state, patch))
exit(128);
- } else
- add_index_file(state, path, mode, buf, size);
+ } else {
+ if (add_index_file(state, path, mode, buf, size))
+ exit(128);
+ }
}
/* phase zero is to remove, phase one is to create */
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 23/41] builtin/apply: make create_file() return -1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", create_file() should just return what
add_conflicted_stages_file() and add_index_file() are returning
instead of calling exit().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index a646900..fdfeab0 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4269,7 +4269,7 @@ static int add_conflicted_stages_file(struct apply_state *state,
return 0;
}
-static void create_file(struct apply_state *state, struct patch *patch)
+static int create_file(struct apply_state *state, struct patch *patch)
{
char *path = patch->new_name;
unsigned mode = patch->new_mode;
@@ -4280,13 +4280,10 @@ static void create_file(struct apply_state *state, struct patch *patch)
mode = S_IFREG | 0644;
create_one_file(state, path, mode, buf, size);
- if (patch->conflicted_threeway) {
- if (add_conflicted_stages_file(state, patch))
- exit(128);
- } else {
- if (add_index_file(state, path, mode, buf, size))
- exit(128);
- }
+ if (patch->conflicted_threeway)
+ return add_conflicted_stages_file(state, patch);
+ else
+ return add_index_file(state, path, mode, buf, size);
}
/* phase zero is to remove, phase one is to create */
@@ -4302,8 +4299,10 @@ static void write_out_one_result(struct apply_state *state,
return;
}
if (patch->is_new > 0 || patch->is_copy) {
- if (phase == 1)
- create_file(state, patch);
+ if (phase == 1) {
+ if (create_file(state, patch))
+ exit(128);
+ }
return;
}
/*
@@ -4314,8 +4313,10 @@ static void write_out_one_result(struct apply_state *state,
if (remove_file(state, patch, patch->is_rename))
exit(128);
}
- if (phase == 1)
- create_file(state, patch);
+ if (phase == 1) {
+ if (create_file(state, patch))
+ exit(128);
+ }
}
static int write_out_one_reject(struct apply_state *state, struct patch *patch)
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 16/41] builtin/apply: make gitdiff_*() return 1 at end of header
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
The gitdiff_*() functions that are called as p->fn() in parse_git_header()
should return 1 instead of -1 in case of end of header or unrecognized
input, as these are not real errors. It just instructs the parser to break
out.
This makes it possible for gitdiff_*() functions to return -1 in case of a
real error. This will be done in a following patch.
Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index f99498b..eb918e5 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -812,7 +812,7 @@ static int gitdiff_hdrend(struct apply_state *state,
const char *line,
struct patch *patch)
{
- return -1;
+ return 1;
}
/*
@@ -1016,7 +1016,7 @@ static int gitdiff_unrecognized(struct apply_state *state,
const char *line,
struct patch *patch)
{
- return -1;
+ return 1;
}
/*
@@ -1248,9 +1248,13 @@ static int parse_git_header(struct apply_state *state,
for (i = 0; i < ARRAY_SIZE(optable); i++) {
const struct opentry *p = optable + i;
int oplen = strlen(p->str);
+ int res;
if (len < oplen || memcmp(p->str, line, oplen))
continue;
- if (p->fn(state, line + oplen, patch) < 0)
+ res = p->fn(state, line + oplen, patch);
+ if (res < 0)
+ return -1;
+ if (res > 0)
return offset;
break;
}
@@ -1430,6 +1434,8 @@ static int find_header(struct apply_state *state,
*/
if (!memcmp("diff --git ", line, 11)) {
int git_hdr_len = parse_git_header(state, line, len, size, patch);
+ if (git_hdr_len < 0)
+ return -128;
if (git_hdr_len <= len)
continue;
if (!patch->old_name && !patch->new_name) {
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 20/41] builtin/apply: make remove_file() return -1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", remove_file() should return -1 instead of
calling die().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 575981b..27fb6e2 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4085,17 +4085,18 @@ static void patch_stats(struct apply_state *state, struct patch *patch)
}
}
-static void remove_file(struct apply_state *state, struct patch *patch, int rmdir_empty)
+static int remove_file(struct apply_state *state, struct patch *patch, int rmdir_empty)
{
if (state->update_index) {
if (remove_file_from_cache(patch->old_name) < 0)
- die(_("unable to remove %s from index"), patch->old_name);
+ return error(_("unable to remove %s from index"), patch->old_name);
}
if (!state->cached) {
if (!remove_or_warn(patch->old_mode, patch->old_name) && rmdir_empty) {
remove_path(patch->old_name);
}
}
+ return 0;
}
static void add_index_file(struct apply_state *state,
@@ -4274,8 +4275,10 @@ static void write_out_one_result(struct apply_state *state,
int phase)
{
if (patch->is_delete > 0) {
- if (phase == 0)
- remove_file(state, patch, 1);
+ if (phase == 0) {
+ if (remove_file(state, patch, 1))
+ exit(128);
+ }
return;
}
if (patch->is_new > 0 || patch->is_copy) {
@@ -4287,8 +4290,10 @@ static void write_out_one_result(struct apply_state *state,
* Rename or modification boils down to the same
* thing: remove the old, write the new
*/
- if (phase == 0)
- remove_file(state, patch, patch->is_rename);
+ if (phase == 0) {
+ if (remove_file(state, patch, patch->is_rename))
+ exit(128);
+ }
if (phase == 1)
create_file(state, patch);
}
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 19/41] builtin/apply: make build_fake_ancestor() return -1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", build_fake_ancestor() should return -1 instead
of calling die().
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 41 ++++++++++++++++++++++++++---------------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 166e94d..575981b 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3900,11 +3900,12 @@ static int preimage_sha1_in_gitlink_patch(struct patch *p, unsigned char sha1[20
}
/* Build an index that contains the just the files needed for a 3way merge */
-static void build_fake_ancestor(struct patch *list, const char *filename)
+static int build_fake_ancestor(struct patch *list, const char *filename)
{
struct patch *patch;
struct index_state result = { NULL };
static struct lock_file lock;
+ int res;
/* Once we start supporting the reverse patch, it may be
* worth showing the new sha1 prefix, but until then...
@@ -3922,31 +3923,38 @@ static void build_fake_ancestor(struct patch *list, const char *filename)
if (!preimage_sha1_in_gitlink_patch(patch, sha1))
; /* ok, the textual part looks sane */
else
- die("sha1 information is lacking or useless for submodule %s",
- name);
+ return error("sha1 information is lacking or "
+ "useless for submodule %s", name);
} else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {
; /* ok */
} else if (!patch->lines_added && !patch->lines_deleted) {
/* mode-only change: update the current */
if (get_current_sha1(patch->old_name, sha1))
- die("mode change for %s, which is not "
- "in current HEAD", name);
+ return error("mode change for %s, which is not "
+ "in current HEAD", name);
} else
- die("sha1 information is lacking or useless "
- "(%s).", name);
+ return error("sha1 information is lacking or useless "
+ "(%s).", name);
ce = make_cache_entry(patch->old_mode, sha1, name, 0, 0);
if (!ce)
- die(_("make_cache_entry failed for path '%s'"), name);
- if (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD))
- die ("Could not add %s to temporary index", name);
+ return error(_("make_cache_entry failed for path '%s'"),
+ name);
+ if (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD)) {
+ free(ce);
+ return error("Could not add %s to temporary index",
+ name);
+ }
}
hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);
- if (write_locked_index(&result, &lock, COMMIT_LOCK))
- die ("Could not write temporary index to %s", filename);
-
+ res = write_locked_index(&result, &lock, COMMIT_LOCK);
discard_index(&result);
+
+ if (res)
+ return error("Could not write temporary index to %s", filename);
+
+ return 0;
}
static void stat_patch_list(struct apply_state *state, struct patch *patch)
@@ -4495,8 +4503,11 @@ static int apply_patch(struct apply_state *state,
goto end;
}
- if (state->fake_ancestor)
- build_fake_ancestor(list, state->fake_ancestor);
+ if (state->fake_ancestor &&
+ build_fake_ancestor(list, state->fake_ancestor)) {
+ res = -128;
+ goto end;
+ }
if (state->diffstat)
stat_patch_list(state, list);
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 13/41] builtin/apply: move check_apply_state() to apply.c
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we must make check_apply_state()
usable outside "builtin/apply.c".
Let's do that by moving it into "apply.c".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 32 ++++++++++++++++++++++++++++++++
apply.h | 1 +
builtin/apply.c | 32 --------------------------------
3 files changed, 33 insertions(+), 32 deletions(-)
diff --git a/apply.c b/apply.c
index 6e0e992..2eac3e3 100644
--- a/apply.c
+++ b/apply.c
@@ -93,3 +93,35 @@ void clear_apply_state(struct apply_state *state)
/* &state->fn_table is cleared at the end of apply_patch() */
}
+
+int check_apply_state(struct apply_state *state, int force_apply)
+{
+ int is_not_gitdir = !startup_info->have_repository;
+
+ if (state->apply_with_reject && state->threeway)
+ return error("--reject and --3way cannot be used together.");
+ if (state->cached && state->threeway)
+ return error("--cached and --3way cannot be used together.");
+ if (state->threeway) {
+ if (is_not_gitdir)
+ return error(_("--3way outside a repository"));
+ state->check_index = 1;
+ }
+ if (state->apply_with_reject)
+ state->apply = state->apply_verbosely = 1;
+ if (!force_apply && (state->diffstat || state->numstat || state->summary || state->check || state->fake_ancestor))
+ state->apply = 0;
+ if (state->check_index && is_not_gitdir)
+ return error(_("--index outside a repository"));
+ if (state->cached) {
+ if (is_not_gitdir)
+ return error(_("--cached outside a repository"));
+ state->check_index = 1;
+ }
+ if (state->check_index)
+ state->unsafe_paths = 0;
+ if (!state->lock_file)
+ return error("BUG: state->lock_file should not be NULL");
+
+ return 0;
+}
diff --git a/apply.h b/apply.h
index e18a18a..53f09b5 100644
--- a/apply.h
+++ b/apply.h
@@ -106,5 +106,6 @@ extern int init_apply_state(struct apply_state *state,
const char *prefix,
struct lock_file *lock_file);
extern void clear_apply_state(struct apply_state *state);
+extern int check_apply_state(struct apply_state *state, int force_apply);
#endif
diff --git a/builtin/apply.c b/builtin/apply.c
index bb89e07..075ada4 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4551,38 +4551,6 @@ static int option_parse_directory(const struct option *opt,
return 0;
}
-static int check_apply_state(struct apply_state *state, int force_apply)
-{
- int is_not_gitdir = !startup_info->have_repository;
-
- if (state->apply_with_reject && state->threeway)
- return error("--reject and --3way cannot be used together.");
- if (state->cached && state->threeway)
- return error("--cached and --3way cannot be used together.");
- if (state->threeway) {
- if (is_not_gitdir)
- return error(_("--3way outside a repository"));
- state->check_index = 1;
- }
- if (state->apply_with_reject)
- state->apply = state->apply_verbosely = 1;
- if (!force_apply && (state->diffstat || state->numstat || state->summary || state->check || state->fake_ancestor))
- state->apply = 0;
- if (state->check_index && is_not_gitdir)
- return error(_("--index outside a repository"));
- if (state->cached) {
- if (is_not_gitdir)
- return error(_("--cached outside a repository"));
- state->check_index = 1;
- }
- if (state->check_index)
- state->unsafe_paths = 0;
- if (!state->lock_file)
- return error("BUG: state->lock_file should not be NULL");
-
- return 0;
-}
-
static int apply_all_patches(struct apply_state *state,
int argc,
const char **argv,
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 17/41] builtin/apply: make gitdiff_*() return -1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", gitdiff_*() functions should return -1 instead
of calling die().
A previous patch made it possible for gitdiff_*() functions to
return -1 in case of error. Let's take advantage of that to
make gitdiff_verify_name() return -1 on error, and to have
gitdiff_oldname() and gitdiff_newname() directly return
what gitdiff_verify_name() returns.
Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index eb918e5..6b16173 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -827,54 +827,56 @@ static int gitdiff_hdrend(struct apply_state *state,
#define DIFF_OLD_NAME 0
#define DIFF_NEW_NAME 1
-static void gitdiff_verify_name(struct apply_state *state,
- const char *line,
- int isnull,
- char **name,
- int side)
+static int gitdiff_verify_name(struct apply_state *state,
+ const char *line,
+ int isnull,
+ char **name,
+ int side)
{
if (!*name && !isnull) {
*name = find_name(state, line, NULL, state->p_value, TERM_TAB);
- return;
+ return 0;
}
if (*name) {
int len = strlen(*name);
char *another;
if (isnull)
- die(_("git apply: bad git-diff - expected /dev/null, got %s on line %d"),
- *name, state->linenr);
+ return error(_("git apply: bad git-diff - expected /dev/null, got %s on line %d"),
+ *name, state->linenr);
another = find_name(state, line, NULL, state->p_value, TERM_TAB);
- if (!another || memcmp(another, *name, len + 1))
- die((side == DIFF_NEW_NAME) ?
+ if (!another || memcmp(another, *name, len + 1)) {
+ free(another);
+ return error((side == DIFF_NEW_NAME) ?
_("git apply: bad git-diff - inconsistent new filename on line %d") :
_("git apply: bad git-diff - inconsistent old filename on line %d"), state->linenr);
+ }
free(another);
} else {
/* expect "/dev/null" */
if (memcmp("/dev/null", line, 9) || line[9] != '\n')
- die(_("git apply: bad git-diff - expected /dev/null on line %d"), state->linenr);
+ return error(_("git apply: bad git-diff - expected /dev/null on line %d"), state->linenr);
}
+
+ return 0;
}
static int gitdiff_oldname(struct apply_state *state,
const char *line,
struct patch *patch)
{
- gitdiff_verify_name(state, line,
- patch->is_new, &patch->old_name,
- DIFF_OLD_NAME);
- return 0;
+ return gitdiff_verify_name(state, line,
+ patch->is_new, &patch->old_name,
+ DIFF_OLD_NAME);
}
static int gitdiff_newname(struct apply_state *state,
const char *line,
struct patch *patch)
{
- gitdiff_verify_name(state, line,
- patch->is_delete, &patch->new_name,
- DIFF_NEW_NAME);
- return 0;
+ return gitdiff_verify_name(state, line,
+ patch->is_delete, &patch->new_name,
+ DIFF_NEW_NAME);
}
static int gitdiff_oldmode(struct apply_state *state,
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 15/41] builtin/apply: make parse_traditional_patch() return -1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", parse_traditional_patch() should return -1
instead of calling die().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 5530ba1..f99498b 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -755,10 +755,10 @@ static int has_epoch_timestamp(const char *nameline)
* files, we can happily check the index for a match, but for creating a
* new file we should try to match whatever "patch" does. I have no idea.
*/
-static void parse_traditional_patch(struct apply_state *state,
- const char *first,
- const char *second,
- struct patch *patch)
+static int parse_traditional_patch(struct apply_state *state,
+ const char *first,
+ const char *second,
+ struct patch *patch)
{
char *name;
@@ -803,7 +803,9 @@ static void parse_traditional_patch(struct apply_state *state,
}
}
if (!name)
- die(_("unable to find filename in patch at line %d"), state->linenr);
+ return error(_("unable to find filename in patch at line %d"), state->linenr);
+
+ return 0;
}
static int gitdiff_hdrend(struct apply_state *state,
@@ -1467,7 +1469,8 @@ static int find_header(struct apply_state *state,
continue;
/* Ok, we'll consider it a patch */
- parse_traditional_patch(state, line, line+len, patch);
+ if (parse_traditional_patch(state, line, line+len, patch))
+ return -128;
*hdrsize = len + nextlen;
state->linenr += 2;
return offset;
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 14/41] builtin/apply: make apply_all_patches() return 128 or 1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To finish libifying the apply functionality, apply_all_patches() should not
die() or exit() in case of error, but return either 128 or 1, so that it
gives the same exit code as when die() or exit(1) is called. This way
scripts relying on the exit code don't need to be changed.
While doing that we must take care that file descriptors are properly closed
and, if needed, reset to a sensible value.
Also, according to the lockfile API, when finished with a lockfile, one
should either commit it or roll it back.
This is even more important now that the same lockfile can be passed
to init_apply_state() many times to be reused by series of calls to
the apply lib functions.
Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 075ada4..5530ba1 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4578,15 +4578,18 @@ static int apply_all_patches(struct apply_state *state,
arg);
fd = open(arg, O_RDONLY);
- if (fd < 0)
- die_errno(_("can't open patch '%s'"), arg);
+ if (fd < 0) {
+ error(_("can't open patch '%s': %s"), arg, strerror(errno));
+ res = -128;
+ goto end;
+ }
read_stdin = 0;
set_default_whitespace_mode(state);
res = apply_patch(state, fd, arg, options);
+ close(fd);
if (res < 0)
goto end;
errs |= res;
- close(fd);
}
set_default_whitespace_mode(state);
if (read_stdin) {
@@ -4606,11 +4609,14 @@ static int apply_all_patches(struct apply_state *state,
squelched),
squelched);
}
- if (state->ws_error_action == die_on_ws_error)
- die(Q_("%d line adds whitespace errors.",
- "%d lines add whitespace errors.",
- state->whitespace_error),
- state->whitespace_error);
+ if (state->ws_error_action == die_on_ws_error) {
+ error(Q_("%d line adds whitespace errors.",
+ "%d lines add whitespace errors.",
+ state->whitespace_error),
+ state->whitespace_error);
+ res = -128;
+ goto end;
+ }
if (state->applied_after_fixing_ws && state->apply)
warning("%d line%s applied after"
" fixing whitespace errors.",
@@ -4624,15 +4630,24 @@ static int apply_all_patches(struct apply_state *state,
}
if (state->update_index) {
- if (write_locked_index(&the_index, state->lock_file, COMMIT_LOCK))
- die(_("Unable to write new index file"));
+ res = write_locked_index(&the_index, state->lock_file, COMMIT_LOCK);
+ if (res) {
+ error(_("Unable to write new index file"));
+ res = -128;
+ goto end;
+ }
state->newfd = -1;
}
return !!errs;
end:
- exit(res == -1 ? 1 : 128);
+ if (state->newfd >= 0) {
+ rollback_lock_file(state->lock_file);
+ state->newfd = -1;
+ }
+
+ return (res == -1 ? 1 : 128);
}
int cmd_apply(int argc, const char **argv, const char *prefix)
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 11/41] apply: make init_apply_state() return -1 instead of exit()ing
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", init_apply_state() should return -1 instead of
calling exit().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 11 ++++++-----
apply.h | 6 +++---
builtin/apply.c | 3 ++-
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/apply.c b/apply.c
index c858ca4..6e0e992 100644
--- a/apply.c
+++ b/apply.c
@@ -55,9 +55,9 @@ int parse_ignorewhitespace_option(struct apply_state *state,
return error(_("unrecognized whitespace ignore option '%s'"), option);
}
-void init_apply_state(struct apply_state *state,
- const char *prefix,
- struct lock_file *lock_file)
+int init_apply_state(struct apply_state *state,
+ const char *prefix,
+ struct lock_file *lock_file)
{
memset(state, 0, sizeof(*state));
state->prefix = prefix;
@@ -79,9 +79,10 @@ void init_apply_state(struct apply_state *state,
git_apply_config();
if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
- exit(1);
+ return -1;
if (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
- exit(1);
+ return -1;
+ return 0;
}
void clear_apply_state(struct apply_state *state)
diff --git a/apply.h b/apply.h
index 08c0a25..e18a18a 100644
--- a/apply.h
+++ b/apply.h
@@ -102,9 +102,9 @@ extern int parse_whitespace_option(struct apply_state *state,
extern int parse_ignorewhitespace_option(struct apply_state *state,
const char *option);
-extern void init_apply_state(struct apply_state *state,
- const char *prefix,
- struct lock_file *lock_file);
+extern int init_apply_state(struct apply_state *state,
+ const char *prefix,
+ struct lock_file *lock_file);
extern void clear_apply_state(struct apply_state *state);
#endif
diff --git a/builtin/apply.c b/builtin/apply.c
index bb6ff77..61fd316 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4741,7 +4741,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
OPT_END()
};
- init_apply_state(&state, prefix, &lock_file);
+ if (init_apply_state(&state, prefix, &lock_file))
+ exit(128);
argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
apply_usage, 0);
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 12/41] builtin/apply: make check_apply_state() return -1 instead of die()ing
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", check_apply_state() should return -1 instead of
calling die().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 61fd316..bb89e07 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4551,17 +4551,17 @@ static int option_parse_directory(const struct option *opt,
return 0;
}
-static void check_apply_state(struct apply_state *state, int force_apply)
+static int check_apply_state(struct apply_state *state, int force_apply)
{
int is_not_gitdir = !startup_info->have_repository;
if (state->apply_with_reject && state->threeway)
- die("--reject and --3way cannot be used together.");
+ return error("--reject and --3way cannot be used together.");
if (state->cached && state->threeway)
- die("--cached and --3way cannot be used together.");
+ return error("--cached and --3way cannot be used together.");
if (state->threeway) {
if (is_not_gitdir)
- die(_("--3way outside a repository"));
+ return error(_("--3way outside a repository"));
state->check_index = 1;
}
if (state->apply_with_reject)
@@ -4569,16 +4569,18 @@ static void check_apply_state(struct apply_state *state, int force_apply)
if (!force_apply && (state->diffstat || state->numstat || state->summary || state->check || state->fake_ancestor))
state->apply = 0;
if (state->check_index && is_not_gitdir)
- die(_("--index outside a repository"));
+ return error(_("--index outside a repository"));
if (state->cached) {
if (is_not_gitdir)
- die(_("--cached outside a repository"));
+ return error(_("--cached outside a repository"));
state->check_index = 1;
}
if (state->check_index)
state->unsafe_paths = 0;
if (!state->lock_file)
- die("BUG: state->lock_file should not be NULL");
+ return error("BUG: state->lock_file should not be NULL");
+
+ return 0;
}
static int apply_all_patches(struct apply_state *state,
@@ -4747,7 +4749,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
apply_usage, 0);
- check_apply_state(&state, force_apply);
+ if (check_apply_state(&state, force_apply))
+ exit(128);
ret = apply_all_patches(&state, argc, argv, options);
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 03/41] builtin/apply: make apply_patch() return -1 or -128 instead of die()ing
From: Christian Couder @ 2016-09-04 20:17 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors
to the caller instead of die()ing.
As a first step in this direction, let's make apply_patch() return
-1 or -128 in case of errors instead of dying. For now its only
caller apply_all_patches() will exit(128) when apply_patch()
return -128 and it will exit(1) when it returns -1.
We exit() with code 128 because that was what die() was doing
and we want to keep the distinction between exiting with code 1
and exiting with code 128.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 60 ++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 45 insertions(+), 15 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index ed923ca..435030a 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4404,6 +4404,15 @@ static struct lock_file lock_file;
#define INACCURATE_EOF (1<<0)
#define RECOUNT (1<<1)
+/*
+ * Try to apply a patch.
+ *
+ * Returns:
+ * -128 if a bad error happened (like patch unreadable)
+ * -1 if patch did not apply and user cannot deal with it
+ * 0 if the patch applied
+ * 1 if the patch did not apply but user might fix it
+ */
static int apply_patch(struct apply_state *state,
int fd,
const char *filename,
@@ -4413,6 +4422,7 @@ static int apply_patch(struct apply_state *state,
struct strbuf buf = STRBUF_INIT; /* owns the patch text */
struct patch *list = NULL, **listp = &list;
int skipped_patch = 0;
+ int res = 0;
state->patch_input_file = filename;
read_patch_file(&buf, fd);
@@ -4445,8 +4455,11 @@ static int apply_patch(struct apply_state *state,
offset += nr;
}
- if (!list && !skipped_patch)
- die(_("unrecognized input"));
+ if (!list && !skipped_patch) {
+ error(_("unrecognized input"));
+ res = -128;
+ goto end;
+ }
if (state->whitespace_error && (state->ws_error_action == die_on_ws_error))
state->apply = 0;
@@ -4455,21 +4468,23 @@ static int apply_patch(struct apply_state *state,
if (state->update_index && state->newfd < 0)
state->newfd = hold_locked_index(state->lock_file, 1);
- if (state->check_index) {
- if (read_cache() < 0)
- die(_("unable to read index file"));
+ if (state->check_index && read_cache() < 0) {
+ error(_("unable to read index file"));
+ res = -128;
+ goto end;
}
if ((state->check || state->apply) &&
check_patch_list(state, list) < 0 &&
- !state->apply_with_reject)
- exit(1);
+ !state->apply_with_reject) {
+ res = -1;
+ goto end;
+ }
if (state->apply && write_out_results(state, list)) {
- if (state->apply_with_reject)
- exit(1);
/* with --3way, we still need to write the index out */
- return 1;
+ res = state->apply_with_reject ? -1 : 1;
+ goto end;
}
if (state->fake_ancestor)
@@ -4484,10 +4499,11 @@ static int apply_patch(struct apply_state *state,
if (state->summary)
summary_patch_list(list);
+end:
free_patch_list(list);
strbuf_release(&buf);
string_list_clear(&state->fn_table, 0);
- return 0;
+ return res;
}
static void git_apply_config(void)
@@ -4628,6 +4644,7 @@ static int apply_all_patches(struct apply_state *state,
int options)
{
int i;
+ int res;
int errs = 0;
int read_stdin = 1;
@@ -4636,7 +4653,10 @@ static int apply_all_patches(struct apply_state *state,
int fd;
if (!strcmp(arg, "-")) {
- errs |= apply_patch(state, 0, "<stdin>", options);
+ res = apply_patch(state, 0, "<stdin>", options);
+ if (res < 0)
+ goto end;
+ errs |= res;
read_stdin = 0;
continue;
} else if (0 < state->prefix_length)
@@ -4649,12 +4669,19 @@ static int apply_all_patches(struct apply_state *state,
die_errno(_("can't open patch '%s'"), arg);
read_stdin = 0;
set_default_whitespace_mode(state);
- errs |= apply_patch(state, fd, arg, options);
+ res = apply_patch(state, fd, arg, options);
+ if (res < 0)
+ goto end;
+ errs |= res;
close(fd);
}
set_default_whitespace_mode(state);
- if (read_stdin)
- errs |= apply_patch(state, 0, "<stdin>", options);
+ if (read_stdin) {
+ res = apply_patch(state, 0, "<stdin>", options);
+ if (res < 0)
+ goto end;
+ errs |= res;
+ }
if (state->whitespace_error) {
if (state->squelch_whitespace_errors &&
@@ -4690,6 +4717,9 @@ static int apply_all_patches(struct apply_state *state,
}
return !!errs;
+
+end:
+ exit(res == -1 ? 1 : 128);
}
int cmd_apply(int argc, const char **argv, const char *prefix)
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 04/41] builtin/apply: read_patch_file() return -1 instead of die()ing
From: Christian Couder @ 2016-09-04 20:17 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing. Let's do that by returning -1 instead of
die()ing in read_patch_file().
Helped-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 435030a..dd7afee 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -335,10 +335,10 @@ static void say_patch_name(FILE *output, const char *fmt, struct patch *patch)
#define SLOP (16)
-static void read_patch_file(struct strbuf *sb, int fd)
+static int read_patch_file(struct strbuf *sb, int fd)
{
if (strbuf_read(sb, fd, 0) < 0)
- die_errno("git apply: failed to read");
+ return error_errno("git apply: failed to read");
/*
* Make sure that we have some slop in the buffer
@@ -347,6 +347,7 @@ static void read_patch_file(struct strbuf *sb, int fd)
*/
strbuf_grow(sb, SLOP);
memset(sb->buf + sb->len, 0, SLOP);
+ return 0;
}
static unsigned long linelen(const char *buffer, unsigned long size)
@@ -4425,7 +4426,8 @@ static int apply_patch(struct apply_state *state,
int res = 0;
state->patch_input_file = filename;
- read_patch_file(&buf, fd);
+ if (read_patch_file(&buf, fd) < 0)
+ return -128;
offset = 0;
while (offset < buf.len) {
struct patch *patch;
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 02/41] apply: move 'struct apply_state' to apply.h
From: Christian Couder @ 2016-09-04 20:17 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we must make 'struct apply_state'
usable outside "builtin/apply.c".
Let's do that by creating a new "apply.h" and moving
'struct apply_state' there.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.h | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
builtin/apply.c | 98 +-----------------------------------------------------
2 files changed, 101 insertions(+), 97 deletions(-)
create mode 100644 apply.h
diff --git a/apply.h b/apply.h
new file mode 100644
index 0000000..7493a40
--- /dev/null
+++ b/apply.h
@@ -0,0 +1,100 @@
+#ifndef APPLY_H
+#define APPLY_H
+
+enum apply_ws_error_action {
+ nowarn_ws_error,
+ warn_on_ws_error,
+ die_on_ws_error,
+ correct_ws_error
+};
+
+enum apply_ws_ignore {
+ ignore_ws_none,
+ ignore_ws_change
+};
+
+/*
+ * We need to keep track of how symlinks in the preimage are
+ * manipulated by the patches. A patch to add a/b/c where a/b
+ * is a symlink should not be allowed to affect the directory
+ * the symlink points at, but if the same patch removes a/b,
+ * it is perfectly fine, as the patch removes a/b to make room
+ * to create a directory a/b so that a/b/c can be created.
+ *
+ * See also "struct string_list symlink_changes" in "struct
+ * apply_state".
+ */
+#define APPLY_SYMLINK_GOES_AWAY 01
+#define APPLY_SYMLINK_IN_RESULT 02
+
+struct apply_state {
+ const char *prefix;
+ int prefix_length;
+
+ /* These are lock_file related */
+ struct lock_file *lock_file;
+ int newfd;
+
+ /* These control what gets looked at and modified */
+ int apply; /* this is not a dry-run */
+ int cached; /* apply to the index only */
+ int check; /* preimage must match working tree, don't actually apply */
+ int check_index; /* preimage must match the indexed version */
+ int update_index; /* check_index && apply */
+
+ /* These control cosmetic aspect of the output */
+ int diffstat; /* just show a diffstat, and don't actually apply */
+ int numstat; /* just show a numeric diffstat, and don't actually apply */
+ int summary; /* just report creation, deletion, etc, and don't actually apply */
+
+ /* These boolean parameters control how the apply is done */
+ int allow_overlap;
+ int apply_in_reverse;
+ int apply_with_reject;
+ int apply_verbosely;
+ int no_add;
+ int threeway;
+ int unidiff_zero;
+ int unsafe_paths;
+
+ /* Other non boolean parameters */
+ const char *fake_ancestor;
+ const char *patch_input_file;
+ int line_termination;
+ struct strbuf root;
+ int p_value;
+ int p_value_known;
+ unsigned int p_context;
+
+ /* Exclude and include path parameters */
+ struct string_list limit_by_name;
+ int has_include;
+
+ /* Various "current state" */
+ int linenr; /* current line number */
+ struct string_list symlink_changes; /* we have to track symlinks */
+
+ /*
+ * For "diff-stat" like behaviour, we keep track of the biggest change
+ * we've seen, and the longest filename. That allows us to do simple
+ * scaling.
+ */
+ int max_change;
+ int max_len;
+
+ /*
+ * Records filenames that have been touched, in order to handle
+ * the case where more than one patches touch the same file.
+ */
+ struct string_list fn_table;
+
+ /* These control whitespace errors */
+ enum apply_ws_error_action ws_error_action;
+ enum apply_ws_ignore ws_ignore_action;
+ const char *whitespace_option;
+ int whitespace_error;
+ int squelch_whitespace_errors;
+ int applied_after_fixing_ws;
+};
+
+#endif
diff --git a/builtin/apply.c b/builtin/apply.c
index ab8f0bd..ed923ca 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -20,103 +20,7 @@
#include "xdiff-interface.h"
#include "ll-merge.h"
#include "rerere.h"
-
-enum apply_ws_error_action {
- nowarn_ws_error,
- warn_on_ws_error,
- die_on_ws_error,
- correct_ws_error
-};
-
-
-enum apply_ws_ignore {
- ignore_ws_none,
- ignore_ws_change
-};
-
-/*
- * We need to keep track of how symlinks in the preimage are
- * manipulated by the patches. A patch to add a/b/c where a/b
- * is a symlink should not be allowed to affect the directory
- * the symlink points at, but if the same patch removes a/b,
- * it is perfectly fine, as the patch removes a/b to make room
- * to create a directory a/b so that a/b/c can be created.
- *
- * See also "struct string_list symlink_changes" in "struct
- * apply_state".
- */
-#define APPLY_SYMLINK_GOES_AWAY 01
-#define APPLY_SYMLINK_IN_RESULT 02
-
-struct apply_state {
- const char *prefix;
- int prefix_length;
-
- /* These are lock_file related */
- struct lock_file *lock_file;
- int newfd;
-
- /* These control what gets looked at and modified */
- int apply; /* this is not a dry-run */
- int cached; /* apply to the index only */
- int check; /* preimage must match working tree, don't actually apply */
- int check_index; /* preimage must match the indexed version */
- int update_index; /* check_index && apply */
-
- /* These control cosmetic aspect of the output */
- int diffstat; /* just show a diffstat, and don't actually apply */
- int numstat; /* just show a numeric diffstat, and don't actually apply */
- int summary; /* just report creation, deletion, etc, and don't actually apply */
-
- /* These boolean parameters control how the apply is done */
- int allow_overlap;
- int apply_in_reverse;
- int apply_with_reject;
- int apply_verbosely;
- int no_add;
- int threeway;
- int unidiff_zero;
- int unsafe_paths;
-
- /* Other non boolean parameters */
- const char *fake_ancestor;
- const char *patch_input_file;
- int line_termination;
- struct strbuf root;
- int p_value;
- int p_value_known;
- unsigned int p_context;
-
- /* Exclude and include path parameters */
- struct string_list limit_by_name;
- int has_include;
-
- /* Various "current state" */
- int linenr; /* current line number */
- struct string_list symlink_changes; /* we have to track symlinks */
-
- /*
- * For "diff-stat" like behaviour, we keep track of the biggest change
- * we've seen, and the longest filename. That allows us to do simple
- * scaling.
- */
- int max_change;
- int max_len;
-
- /*
- * Records filenames that have been touched, in order to handle
- * the case where more than one patches touch the same file.
- */
- struct string_list fn_table;
-
- /* These control whitespace errors */
- enum apply_ws_error_action ws_error_action;
- enum apply_ws_ignore ws_ignore_action;
- const char *whitespace_option;
- int whitespace_error;
- int squelch_whitespace_errors;
- int applied_after_fixing_ws;
-};
+#include "apply.h"
static const char * const apply_usage[] = {
N_("git apply [<options>] [<patch>...]"),
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 01/41] apply: make some names more specific
From: Christian Couder @ 2016-09-04 20:17 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To prepare for some structs and constants being moved from
builtin/apply.c to apply.h, we should give them some more
specific names to avoid possible name collisions in the global
namespace.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 1a488f9..ab8f0bd 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -21,7 +21,7 @@
#include "ll-merge.h"
#include "rerere.h"
-enum ws_error_action {
+enum apply_ws_error_action {
nowarn_ws_error,
warn_on_ws_error,
die_on_ws_error,
@@ -29,7 +29,7 @@ enum ws_error_action {
};
-enum ws_ignore {
+enum apply_ws_ignore {
ignore_ws_none,
ignore_ws_change
};
@@ -45,8 +45,8 @@ enum ws_ignore {
* See also "struct string_list symlink_changes" in "struct
* apply_state".
*/
-#define SYMLINK_GOES_AWAY 01
-#define SYMLINK_IN_RESULT 02
+#define APPLY_SYMLINK_GOES_AWAY 01
+#define APPLY_SYMLINK_IN_RESULT 02
struct apply_state {
const char *prefix;
@@ -110,8 +110,8 @@ struct apply_state {
struct string_list fn_table;
/* These control whitespace errors */
- enum ws_error_action ws_error_action;
- enum ws_ignore ws_ignore_action;
+ enum apply_ws_error_action ws_error_action;
+ enum apply_ws_ignore ws_ignore_action;
const char *whitespace_option;
int whitespace_error;
int squelch_whitespace_errors;
@@ -3750,11 +3750,11 @@ static void prepare_symlink_changes(struct apply_state *state, struct patch *pat
if ((patch->old_name && S_ISLNK(patch->old_mode)) &&
(patch->is_rename || patch->is_delete))
/* the symlink at patch->old_name is removed */
- register_symlink_changes(state, patch->old_name, SYMLINK_GOES_AWAY);
+ register_symlink_changes(state, patch->old_name, APPLY_SYMLINK_GOES_AWAY);
if (patch->new_name && S_ISLNK(patch->new_mode))
/* the symlink at patch->new_name is created or remains */
- register_symlink_changes(state, patch->new_name, SYMLINK_IN_RESULT);
+ register_symlink_changes(state, patch->new_name, APPLY_SYMLINK_IN_RESULT);
}
}
@@ -3769,9 +3769,9 @@ static int path_is_beyond_symlink_1(struct apply_state *state, struct strbuf *na
break;
name->buf[name->len] = '\0';
change = check_symlink_changes(state, name->buf);
- if (change & SYMLINK_IN_RESULT)
+ if (change & APPLY_SYMLINK_IN_RESULT)
return 1;
- if (change & SYMLINK_GOES_AWAY)
+ if (change & APPLY_SYMLINK_GOES_AWAY)
/*
* This cannot be "return 0", because we may
* see a new one created at a higher level.
--
2.10.0.41.g9df52c3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox