* [PATCH v7 01/40] apply: move 'struct apply_state' to apply.h
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 22:49 ` Junio C Hamano
2016-06-13 16:09 ` [PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing Christian Couder
` (38 subsequent siblings)
39 siblings, 1 reply; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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..9a98eae
--- /dev/null
+++ b/apply.h
@@ -0,0 +1,100 @@
+#ifndef APPLY_H
+#define APPLY_H
+
+enum ws_error_action {
+ nowarn_ws_error,
+ warn_on_ws_error,
+ die_on_ws_error,
+ correct_ws_error
+};
+
+enum 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 SYMLINK_GOES_AWAY 01
+#define 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 ws_error_action ws_error_action;
+ enum 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 ecb7f1b..3a0c53a 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 ws_error_action {
- nowarn_ws_error,
- warn_on_ws_error,
- die_on_ws_error,
- correct_ws_error
-};
-
-
-enum 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 SYMLINK_GOES_AWAY 01
-#define 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 ws_error_action ws_error_action;
- enum 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.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v7 01/40] apply: move 'struct apply_state' to apply.h
2016-06-13 16:09 ` [PATCH v7 01/40] apply: move 'struct apply_state' to apply.h Christian Couder
@ 2016-06-13 22:49 ` Junio C Hamano
2016-06-14 11:07 ` Christian Couder
0 siblings, 1 reply; 46+ messages in thread
From: Junio C Hamano @ 2016-06-13 22:49 UTC (permalink / raw)
To: Christian Couder
Cc: git, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy,
Eric Sunshine, Ramsay Jones, Johannes Sixt, René Scharfe,
Christian Couder
Christian Couder <christian.couder@gmail.com> writes:
> 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..9a98eae
> --- /dev/null
> +++ b/apply.h
> @@ -0,0 +1,100 @@
> +#ifndef APPLY_H
> +#define APPLY_H
> +
> +enum ws_error_action {
> + nowarn_ws_error,
> + warn_on_ws_error,
> + die_on_ws_error,
> + correct_ws_error
> +};
> +
> +enum 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 SYMLINK_GOES_AWAY 01
> +#define SYMLINK_IN_RESULT 02
Everything below is agreeable, but all the names that are made
public above by this change do not sound specific enough to "apply".
I wonder if they should get "apply" somewhere in their names to
avoid confusion coming from the namespace contamination...
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v7 01/40] apply: move 'struct apply_state' to apply.h
2016-06-13 22:49 ` Junio C Hamano
@ 2016-06-14 11:07 ` Christian Couder
0 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-14 11:07 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Ævar Arnfjörð,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy,
Eric Sunshine, Ramsay Jones, Johannes Sixt, René Scharfe,
Christian Couder
On Tue, Jun 14, 2016 at 12:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> 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..9a98eae
>> --- /dev/null
>> +++ b/apply.h
>> @@ -0,0 +1,100 @@
>> +#ifndef APPLY_H
>> +#define APPLY_H
>> +
>> +enum ws_error_action {
>> + nowarn_ws_error,
>> + warn_on_ws_error,
>> + die_on_ws_error,
>> + correct_ws_error
>> +};
>> +
>> +enum 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 SYMLINK_GOES_AWAY 01
>> +#define SYMLINK_IN_RESULT 02
>
> Everything below is agreeable, but all the names that are made
> public above by this change do not sound specific enough to "apply".
> I wonder if they should get "apply" somewhere in their names to
> avoid confusion coming from the namespace contamination...
Yeah, I will add "apply" in the names.
Thanks,
Christian.
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
2016-06-13 16:09 ` [PATCH v7 01/40] apply: move 'struct apply_state' to apply.h Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 22:55 ` Junio C Hamano
2016-06-13 16:09 ` [PATCH v7 03/40] builtin/apply: read_patch_file() " Christian Couder
` (37 subsequent siblings)
39 siblings, 1 reply; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 in case of errors instead of dying. For now its only caller
apply_all_patches() will exit(1) when apply_patch() return -1.
In a later patch, apply_all_patches() will return -1 too instead of
exiting.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 54 +++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 39 insertions(+), 15 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 3a0c53a..598e479 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4404,6 +4404,14 @@ static struct lock_file lock_file;
#define INACCURATE_EOF (1<<0)
#define RECOUNT (1<<1)
+/*
+ * Try to apply a patch.
+ *
+ * Returns:
+ * -1 if an error happened
+ * 0 if the patch applied
+ * 1 if the patch did not apply
+ */
static int apply_patch(struct apply_state *state,
int fd,
const char *filename,
@@ -4413,6 +4421,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 +4454,10 @@ static int apply_patch(struct apply_state *state,
offset += nr;
}
- if (!list && !skipped_patch)
- die(_("unrecognized input"));
+ if (!list && !skipped_patch) {
+ res = error(_("unrecognized input"));
+ goto end;
+ }
if (state->whitespace_error && (state->ws_error_action == die_on_ws_error))
state->apply = 0;
@@ -4455,21 +4466,22 @@ 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) {
+ res = error(_("unable to read index file"));
+ 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 +4496,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)
@@ -4625,6 +4638,7 @@ static int apply_all_patches(struct apply_state *state,
int options)
{
int i;
+ int res;
int errs = 0;
int read_stdin = 1;
@@ -4633,7 +4647,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)
+ exit(1);
+ errs |= res;
read_stdin = 0;
continue;
} else if (0 < state->prefix_length)
@@ -4646,12 +4663,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)
+ exit(1);
+ 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)
+ exit(1);
+ errs |= res;
+ }
if (state->whitespace_error) {
if (state->squelch_whitespace_errors &&
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing
2016-06-13 16:09 ` [PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing Christian Couder
@ 2016-06-13 22:55 ` Junio C Hamano
2016-06-14 11:06 ` Christian Couder
0 siblings, 1 reply; 46+ messages in thread
From: Junio C Hamano @ 2016-06-13 22:55 UTC (permalink / raw)
To: Christian Couder
Cc: git, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy,
Eric Sunshine, Ramsay Jones, Johannes Sixt, René Scharfe,
Christian Couder
Christian Couder <christian.couder@gmail.com> writes:
> +/*
> + * Try to apply a patch.
> + *
> + * Returns:
> + * -1 if an error happened
> + * 0 if the patch applied
> + * 1 if the patch did not apply
> + */
> static int apply_patch(struct apply_state *state,
> int fd,
> const char *filename,
> @@ -4413,6 +4421,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 +4454,10 @@ static int apply_patch(struct apply_state *state,
> offset += nr;
> }
>
> - if (!list && !skipped_patch)
> - die(_("unrecognized input"));
> + if (!list && !skipped_patch) {
> + res = error(_("unrecognized input"));
> + goto end;
> + }
Before this patch, the program said "fatal: $message" and exited
with status = 128. All these changes in this step modifies the
external behaviour and make it say "error: $message" and exit with
status = 1 (at least the caller in apply_all_patches() does so).
Will that be an issue for the calling scripts?
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing
2016-06-13 22:55 ` Junio C Hamano
@ 2016-06-14 11:06 ` Christian Couder
2016-06-14 17:50 ` Junio C Hamano
0 siblings, 1 reply; 46+ messages in thread
From: Christian Couder @ 2016-06-14 11:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Ævar Arnfjörð,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy,
Eric Sunshine, Ramsay Jones, Johannes Sixt, René Scharfe,
Christian Couder
On Tue, Jun 14, 2016 at 12:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> +/*
>> + * Try to apply a patch.
>> + *
>> + * Returns:
>> + * -1 if an error happened
>> + * 0 if the patch applied
>> + * 1 if the patch did not apply
>> + */
>> static int apply_patch(struct apply_state *state,
>> int fd,
>> const char *filename,
>> @@ -4413,6 +4421,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 +4454,10 @@ static int apply_patch(struct apply_state *state,
>> offset += nr;
>> }
>>
>> - if (!list && !skipped_patch)
>> - die(_("unrecognized input"));
>> + if (!list && !skipped_patch) {
>> + res = error(_("unrecognized input"));
>> + goto end;
>> + }
>
> Before this patch, the program said "fatal: $message" and exited
> with status = 128. All these changes in this step modifies the
> external behaviour and make it say "error: $message" and exit with
> status = 1 (at least the caller in apply_all_patches() does so).
>
> Will that be an issue for the calling scripts?
Hopefully the scripts don't check the specific error code and message.
I will add something about this in the commit message.
Do you think something else that should be done about this?
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing
2016-06-14 11:06 ` Christian Couder
@ 2016-06-14 17:50 ` Junio C Hamano
0 siblings, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2016-06-14 17:50 UTC (permalink / raw)
To: Christian Couder
Cc: git, Jeff King, Ævar Arnfjörð,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy,
Eric Sunshine, Ramsay Jones, Johannes Sixt, René Scharfe,
Christian Couder
Christian Couder <christian.couder@gmail.com> writes:
>> Before this patch, the program said "fatal: $message" and exited
>> with status = 128. All these changes in this step modifies the
>> external behaviour and make it say "error: $message" and exit with
>> status = 1 (at least the caller in apply_all_patches() does so).
>>
>> Will that be an issue for the calling scripts?
>
> Hopefully the scripts don't check the specific error code and message.
The error codes are designed to be a vehicle that carries
information in a robust way, so it is perfectly understandable if
the callers used it to tell hopelessly broken input that caused
"die" and an otherwise sensible patch that does not happen to
apply. Whoever is changing the external behaviour is responsible
for making sure the existing callers are not broken.
Hope is not a sound software engineering practice.
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v7 03/40] builtin/apply: read_patch_file() return -1 instead of die()ing
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
2016-06-13 16:09 ` [PATCH v7 01/40] apply: move 'struct apply_state' to apply.h Christian Couder
2016-06-13 16:09 ` [PATCH v7 02/40] builtin/apply: make apply_patch() return -1 instead of die()ing Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 04/40] builtin/apply: make find_header() " Christian Couder
` (36 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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().
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 598e479..2ff8450 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)
@@ -4424,7 +4425,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))
+ return -1;
offset = 0;
while (offset < buf.len) {
struct patch *patch;
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 04/40] builtin/apply: make find_header() return -1 instead of die()ing
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (2 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 03/40] builtin/apply: read_patch_file() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 05/40] builtin/apply: make parse_chunk() return a negative integer on error Christian Couder
` (35 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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, find_header() should return -1 instead of calling
die().
Unfortunately find_header() already returns -1 when no header is found,
so let's make it return -2 instead in this case.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 33 ++++++++++++++++++++++-----------
t/t4254-am-corrupt.sh | 2 +-
2 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 2ff8450..630c01c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1419,6 +1419,14 @@ static int parse_fragment_header(const char *line, int len, struct fragment *fra
return offset;
}
+/*
+ * Find file diff header
+ *
+ * Returns:
+ * -1 in case of error
+ * -2 if no header was found
+ * the size of the header in bytes (called "offset") otherwise
+ */
static int find_header(struct apply_state *state,
const char *line,
unsigned long size,
@@ -1452,8 +1460,8 @@ static int find_header(struct apply_state *state,
struct fragment dummy;
if (parse_fragment_header(line, len, &dummy) < 0)
continue;
- die(_("patch fragment without header at line %d: %.*s"),
- state->linenr, (int)len-1, line);
+ return error(_("patch fragment without header at line %d: %.*s"),
+ state->linenr, (int)len-1, line);
}
if (size < len + 6)
@@ -1469,18 +1477,18 @@ static int find_header(struct apply_state *state,
continue;
if (!patch->old_name && !patch->new_name) {
if (!patch->def_name)
- die(Q_("git diff header lacks filename information when removing "
- "%d leading pathname component (line %d)",
- "git diff header lacks filename information when removing "
- "%d leading pathname components (line %d)",
- state->p_value),
- state->p_value, state->linenr);
+ return error(Q_("git diff header lacks filename information when removing "
+ "%d leading pathname component (line %d)",
+ "git diff header lacks filename information when removing "
+ "%d leading pathname components (line %d)",
+ state->p_value),
+ state->p_value, state->linenr);
patch->old_name = xstrdup(patch->def_name);
patch->new_name = xstrdup(patch->def_name);
}
if (!patch->is_delete && !patch->new_name)
- die("git diff header lacks filename information "
- "(line %d)", state->linenr);
+ return error("git diff header lacks filename information "
+ "(line %d)", state->linenr);
patch->is_toplevel_relative = 1;
*hdrsize = git_hdr_len;
return offset;
@@ -1505,7 +1513,7 @@ static int find_header(struct apply_state *state,
state->linenr += 2;
return offset;
}
- return -1;
+ return -2;
}
static void record_ws_error(struct apply_state *state,
@@ -1996,6 +2004,9 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
int hdrsize, patchsize;
int offset = find_header(state, buffer, size, &hdrsize, patch);
+ if (offset == -1)
+ exit(1);
+
if (offset < 0)
return offset;
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
index 85716dd..9bd7dd2 100755
--- a/t/t4254-am-corrupt.sh
+++ b/t/t4254-am-corrupt.sh
@@ -29,7 +29,7 @@ test_expect_success 'try to apply corrupted patch' '
'
test_expect_success 'compare diagnostic; ensure file is still here' '
- echo "fatal: git diff header lacks filename information (line 4)" >expected &&
+ echo "error: git diff header lacks filename information (line 4)" >expected &&
test_path_is_file f &&
test_cmp expected actual
'
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 05/40] builtin/apply: make parse_chunk() return a negative integer on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (3 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 04/40] builtin/apply: make find_header() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 06/40] builtin/apply: make parse_single_patch() return -1 " Christian Couder
` (34 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing or exit()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_chunk() should return -1 instead of calling
die() or exit().
As parse_chunk() is called only by apply_patch() which already
returns -1 when an error happened, let's make apply_patch() return -1
when parse_chunk() returns -1.
If find_header() returns -2 because no patch header has been found, it
is ok for parse_chunk() to also return -2. If find_header() returns -1
because an error happened, it is ok for parse_chunk() to do the same.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 630c01c..a160338 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1991,22 +1991,22 @@ static int use_patch(struct apply_state *state, struct patch *p)
return !state->has_include;
}
-
/*
* Read the patch text in "buffer" that extends for "size" bytes; stop
* reading after seeing a single patch (i.e. changes to a single file).
* Create fragments (i.e. patch hunks) and hang them to the given patch.
- * Return the number of bytes consumed, so that the caller can call us
- * again for the next patch.
+ *
+ * Returns:
+ * -1 on error,
+ * -2 if no header was found,
+ * the number of bytes consumed otherwise,
+ * so that the caller can call us again for the next patch.
*/
static int parse_chunk(struct apply_state *state, char *buffer, unsigned long size, struct patch *patch)
{
int hdrsize, patchsize;
int offset = find_header(state, buffer, size, &hdrsize, patch);
- if (offset == -1)
- exit(1);
-
if (offset < 0)
return offset;
@@ -2067,7 +2067,7 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
*/
if ((state->apply || state->check) &&
(!patch->is_binary && !metadata_changes(patch)))
- die(_("patch with only garbage at line %d"), state->linenr);
+ return error(_("patch with only garbage at line %d"), state->linenr);
}
return offset + hdrsize + patchsize;
@@ -4449,6 +4449,10 @@ static int apply_patch(struct apply_state *state,
nr = parse_chunk(state, buf.buf + offset, buf.len - offset, patch);
if (nr < 0) {
free_patch(patch);
+ if (nr == -1) {
+ res = -1;
+ goto end;
+ }
break;
}
if (state->apply_in_reverse)
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 06/40] builtin/apply: make parse_single_patch() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (4 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 05/40] builtin/apply: make parse_chunk() return a negative integer on error Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 07/40] builtin/apply: make parse_whitespace_option() return -1 instead of die()ing Christian Couder
` (33 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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_single_patch() should return -1 instead of
calling die().
Let's do that by using error() and let's adjust the related test
cases accordingly.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 17 +++++++++++++----
t/t4012-diff-binary.sh | 4 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index a160338..2671586 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1666,6 +1666,10 @@ static int parse_fragment(struct apply_state *state,
*
* The (fragment->patch, fragment->size) pair points into the memory given
* by the caller, not a copy, when we return.
+ *
+ * Returns:
+ * -1 in case of error,
+ * the number of bytes in the patch otherwise.
*/
static int parse_single_patch(struct apply_state *state,
const char *line,
@@ -1683,8 +1687,10 @@ static int parse_single_patch(struct apply_state *state,
fragment = xcalloc(1, sizeof(*fragment));
fragment->linenr = state->linenr;
len = parse_fragment(state, line, size, patch, fragment);
- if (len <= 0)
- die(_("corrupt patch at line %d"), state->linenr);
+ if (len <= 0) {
+ free(fragment);
+ return error(_("corrupt patch at line %d"), state->linenr);
+ }
fragment->patch = line;
fragment->size = len;
oldlines += fragment->oldlines;
@@ -1720,9 +1726,9 @@ static int parse_single_patch(struct apply_state *state,
patch->is_delete = 0;
if (0 < patch->is_new && oldlines)
- die(_("new file %s depends on old contents"), patch->new_name);
+ return error(_("new file %s depends on old contents"), patch->new_name);
if (0 < patch->is_delete && newlines)
- die(_("deleted file %s still has contents"), patch->old_name);
+ return error(_("deleted file %s still has contents"), patch->old_name);
if (!patch->is_delete && !newlines && context)
fprintf_ln(stderr,
_("** warning: "
@@ -2024,6 +2030,9 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
size - offset - hdrsize,
patch);
+ if (patchsize < 0)
+ return -1;
+
if (!patchsize) {
static const char git_binary[] = "GIT binary patch\n";
int hd = hdrsize + offset;
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 643d729..0a8af76 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -68,7 +68,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt patch correctly' '
sed -e "s/-CIT/xCIT/" <output >broken &&
test_must_fail git apply --stat --summary broken 2>detected &&
detected=$(cat detected) &&
- detected=$(expr "$detected" : "fatal.*at line \\([0-9]*\\)\$") &&
+ detected=$(expr "$detected" : "error.*at line \\([0-9]*\\)\$") &&
detected=$(sed -ne "${detected}p" broken) &&
test "$detected" = xCIT
'
@@ -77,7 +77,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt patch correctly' '
git diff --binary | sed -e "s/-CIT/xCIT/" >broken &&
test_must_fail git apply --stat --summary broken 2>detected &&
detected=$(cat detected) &&
- detected=$(expr "$detected" : "fatal.*at line \\([0-9]*\\)\$") &&
+ detected=$(expr "$detected" : "error.*at line \\([0-9]*\\)\$") &&
detected=$(sed -ne "${detected}p" broken) &&
test "$detected" = xCIT
'
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 07/40] builtin/apply: make parse_whitespace_option() return -1 instead of die()ing
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (5 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 06/40] builtin/apply: make parse_single_patch() return -1 " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 08/40] builtin/apply: make parse_ignorewhitespace_option() " Christian Couder
` (32 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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_whitespace_option() should return -1 instead
of calling die().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 2671586..e56e754 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -27,34 +27,34 @@ static const char * const apply_usage[] = {
NULL
};
-static void 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;
- return;
+ return 0;
}
if (!strcmp(option, "warn")) {
state->ws_error_action = warn_on_ws_error;
- return;
+ return 0;
}
if (!strcmp(option, "nowarn")) {
state->ws_error_action = nowarn_ws_error;
- return;
+ return 0;
}
if (!strcmp(option, "error")) {
state->ws_error_action = die_on_ws_error;
- return;
+ return 0;
}
if (!strcmp(option, "error-all")) {
state->ws_error_action = die_on_ws_error;
state->squelch_whitespace_errors = 0;
- return;
+ return 0;
}
if (!strcmp(option, "strip") || !strcmp(option, "fix")) {
state->ws_error_action = correct_ws_error;
- return;
+ return 0;
}
- die(_("unrecognized whitespace option '%s'"), option);
+ return error(_("unrecognized whitespace option '%s'"), option);
}
static void parse_ignorewhitespace_option(struct apply_state *state,
@@ -4579,7 +4579,8 @@ static int option_parse_whitespace(const struct option *opt,
{
struct apply_state *state = opt->value;
state->whitespace_option = arg;
- parse_whitespace_option(state, arg);
+ if (parse_whitespace_option(state, arg))
+ exit(1);
return 0;
}
@@ -4613,8 +4614,8 @@ static void init_apply_state(struct apply_state *state,
strbuf_init(&state->root, 0);
git_apply_config();
- if (apply_default_whitespace)
- parse_whitespace_option(state, apply_default_whitespace);
+ if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
+ exit(1);
if (apply_default_ignorewhitespace)
parse_ignorewhitespace_option(state, apply_default_ignorewhitespace);
}
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 08/40] builtin/apply: make parse_ignorewhitespace_option() return -1 instead of die()ing
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (6 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 07/40] builtin/apply: make parse_whitespace_option() return -1 instead of die()ing Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 09/40] builtin/apply: move init_apply_state() to apply.c Christian Couder
` (31 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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_ignorewhitespace_option() should return
-1 instead of calling die().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index e56e754..e2f970d 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -57,20 +57,20 @@ static int parse_whitespace_option(struct apply_state *state, const char *option
return error(_("unrecognized whitespace option '%s'"), option);
}
-static void 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") ||
!strcmp(option, "none")) {
state->ws_ignore_action = ignore_ws_none;
- return;
+ return 0;
}
if (!strcmp(option, "change")) {
state->ws_ignore_action = ignore_ws_change;
- return;
+ return 0;
}
- die(_("unrecognized whitespace ignore option '%s'"), option);
+ return error(_("unrecognized whitespace ignore option '%s'"), option);
}
static void set_default_whitespace_mode(struct apply_state *state)
@@ -4616,8 +4616,8 @@ static void init_apply_state(struct apply_state *state,
git_apply_config();
if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
exit(1);
- if (apply_default_ignorewhitespace)
- parse_ignorewhitespace_option(state, apply_default_ignorewhitespace);
+ if (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
+ exit(1);
}
static void clear_apply_state(struct apply_state *state)
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 09/40] builtin/apply: move init_apply_state() to apply.c
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (7 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 08/40] builtin/apply: make parse_ignorewhitespace_option() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 10/40] apply: make init_apply_state() return -1 instead of exit()ing Christian Couder
` (30 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
To libify `git apply` functionality we must make init_apply_state()
usable outside "builtin/apply.c".
Let's do that by moving it into a new "apply.c".
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Makefile | 1 +
apply.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
apply.h | 10 +++++++
builtin/apply.c | 88 -------------------------------------------------------
4 files changed, 102 insertions(+), 88 deletions(-)
create mode 100644 apply.c
diff --git a/Makefile b/Makefile
index 6b05249..6da1209 100644
--- a/Makefile
+++ b/Makefile
@@ -680,6 +680,7 @@ LIB_OBJS += abspath.o
LIB_OBJS += advice.o
LIB_OBJS += alias.o
LIB_OBJS += alloc.o
+LIB_OBJS += apply.o
LIB_OBJS += archive.o
LIB_OBJS += archive-tar.o
LIB_OBJS += archive-zip.o
diff --git a/apply.c b/apply.c
new file mode 100644
index 0000000..1f31bb4
--- /dev/null
+++ b/apply.c
@@ -0,0 +1,91 @@
+#include "cache.h"
+#include "lockfile.h"
+#include "apply.h"
+
+static void git_apply_config(void)
+{
+ git_config_get_string_const("apply.whitespace", &apply_default_whitespace);
+ git_config_get_string_const("apply.ignorewhitespace", &apply_default_ignorewhitespace);
+ git_config(git_default_config, NULL);
+}
+
+int parse_whitespace_option(struct apply_state *state, const char *option)
+{
+ if (!option) {
+ state->ws_error_action = warn_on_ws_error;
+ return 0;
+ }
+ if (!strcmp(option, "warn")) {
+ state->ws_error_action = warn_on_ws_error;
+ return 0;
+ }
+ if (!strcmp(option, "nowarn")) {
+ state->ws_error_action = nowarn_ws_error;
+ return 0;
+ }
+ if (!strcmp(option, "error")) {
+ state->ws_error_action = die_on_ws_error;
+ return 0;
+ }
+ if (!strcmp(option, "error-all")) {
+ state->ws_error_action = die_on_ws_error;
+ state->squelch_whitespace_errors = 0;
+ return 0;
+ }
+ if (!strcmp(option, "strip") || !strcmp(option, "fix")) {
+ state->ws_error_action = correct_ws_error;
+ return 0;
+ }
+ return error(_("unrecognized whitespace option '%s'"), option);
+}
+
+int parse_ignorewhitespace_option(struct apply_state *state,
+ const char *option)
+{
+ if (!option || !strcmp(option, "no") ||
+ !strcmp(option, "false") || !strcmp(option, "never") ||
+ !strcmp(option, "none")) {
+ state->ws_ignore_action = ignore_ws_none;
+ return 0;
+ }
+ if (!strcmp(option, "change")) {
+ state->ws_ignore_action = ignore_ws_change;
+ return 0;
+ }
+ return error(_("unrecognized whitespace ignore option '%s'"), option);
+}
+
+void init_apply_state(struct apply_state *state,
+ const char *prefix,
+ struct lock_file *lock_file)
+{
+ memset(state, 0, sizeof(*state));
+ state->prefix = prefix;
+ state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
+ state->lock_file = lock_file;
+ state->newfd = -1;
+ state->apply = 1;
+ state->line_termination = '\n';
+ state->p_value = 1;
+ state->p_context = UINT_MAX;
+ state->squelch_whitespace_errors = 5;
+ state->ws_error_action = warn_on_ws_error;
+ state->ws_ignore_action = ignore_ws_none;
+ state->linenr = 1;
+ strbuf_init(&state->root, 0);
+
+ git_apply_config();
+ if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
+ exit(1);
+ if (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
+ exit(1);
+}
+
+void clear_apply_state(struct apply_state *state)
+{
+ string_list_clear(&state->limit_by_name, 0);
+ string_list_clear(&state->symlink_changes, 0);
+ strbuf_release(&state->root);
+
+ /* &state->fn_table is cleared at the end of apply_patch() */
+}
diff --git a/apply.h b/apply.h
index 9a98eae..77502be 100644
--- a/apply.h
+++ b/apply.h
@@ -97,4 +97,14 @@ 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 void 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 e2f970d..bc15545 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -27,52 +27,6 @@ static const char * const apply_usage[] = {
NULL
};
-static int parse_whitespace_option(struct apply_state *state, const char *option)
-{
- if (!option) {
- state->ws_error_action = warn_on_ws_error;
- return 0;
- }
- if (!strcmp(option, "warn")) {
- state->ws_error_action = warn_on_ws_error;
- return 0;
- }
- if (!strcmp(option, "nowarn")) {
- state->ws_error_action = nowarn_ws_error;
- return 0;
- }
- if (!strcmp(option, "error")) {
- state->ws_error_action = die_on_ws_error;
- return 0;
- }
- if (!strcmp(option, "error-all")) {
- state->ws_error_action = die_on_ws_error;
- state->squelch_whitespace_errors = 0;
- return 0;
- }
- if (!strcmp(option, "strip") || !strcmp(option, "fix")) {
- state->ws_error_action = correct_ws_error;
- return 0;
- }
- return error(_("unrecognized whitespace option '%s'"), option);
-}
-
-static int parse_ignorewhitespace_option(struct apply_state *state,
- const char *option)
-{
- if (!option || !strcmp(option, "no") ||
- !strcmp(option, "false") || !strcmp(option, "never") ||
- !strcmp(option, "none")) {
- state->ws_ignore_action = ignore_ws_none;
- return 0;
- }
- if (!strcmp(option, "change")) {
- state->ws_ignore_action = ignore_ws_change;
- return 0;
- }
- return error(_("unrecognized whitespace ignore option '%s'"), option);
-}
-
static void set_default_whitespace_mode(struct apply_state *state)
{
if (!state->whitespace_option && !apply_default_whitespace)
@@ -4529,13 +4483,6 @@ end:
return res;
}
-static void git_apply_config(void)
-{
- git_config_get_string_const("apply.whitespace", &apply_default_whitespace);
- git_config_get_string_const("apply.ignorewhitespace", &apply_default_ignorewhitespace);
- git_config(git_default_config, NULL);
-}
-
static int option_parse_exclude(const struct option *opt,
const char *arg, int unset)
{
@@ -4594,41 +4541,6 @@ static int option_parse_directory(const struct option *opt,
return 0;
}
-static void init_apply_state(struct apply_state *state,
- const char *prefix,
- struct lock_file *lock_file)
-{
- memset(state, 0, sizeof(*state));
- state->prefix = prefix;
- state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
- state->lock_file = lock_file;
- state->newfd = -1;
- state->apply = 1;
- state->line_termination = '\n';
- state->p_value = 1;
- state->p_context = UINT_MAX;
- state->squelch_whitespace_errors = 5;
- state->ws_error_action = warn_on_ws_error;
- state->ws_ignore_action = ignore_ws_none;
- state->linenr = 1;
- strbuf_init(&state->root, 0);
-
- git_apply_config();
- if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
- exit(1);
- if (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
- exit(1);
-}
-
-static void clear_apply_state(struct apply_state *state)
-{
- string_list_clear(&state->limit_by_name, 0);
- string_list_clear(&state->symlink_changes, 0);
- strbuf_release(&state->root);
-
- /* &state->fn_table is cleared at the end of apply_patch() */
-}
-
static void check_apply_state(struct apply_state *state, int force_apply)
{
int is_not_gitdir = !startup_info->have_repository;
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 10/40] apply: make init_apply_state() return -1 instead of exit()ing
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (8 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 09/40] builtin/apply: move init_apply_state() to apply.c Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 11/40] builtin/apply: make check_apply_state() return -1 instead of die()ing Christian Couder
` (29 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 1f31bb4..c5a9ee2 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;
@@ -76,9 +76,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 77502be..7d3a03b 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 bc15545..2ae1243 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4728,7 +4728,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(1);
argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
apply_usage, 0);
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 11/40] builtin/apply: make check_apply_state() return -1 instead of die()ing
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (9 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 10/40] apply: make init_apply_state() return -1 instead of exit()ing Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 12/40] builtin/apply: move check_apply_state() to apply.c Christian Couder
` (28 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 2ae1243..d60ffce 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4541,17 +4541,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)
@@ -4559,16 +4559,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,
@@ -4734,7 +4736,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(1);
ret = apply_all_patches(&state, argc, argv, options);
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 12/40] builtin/apply: move check_apply_state() to apply.c
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (10 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 11/40] builtin/apply: make check_apply_state() return -1 instead of die()ing Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 13/40] builtin/apply: make apply_all_patches() return -1 on error Christian Couder
` (27 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 c5a9ee2..84dae3d 100644
--- a/apply.c
+++ b/apply.c
@@ -90,3 +90,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 7d3a03b..1f2277e 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 d60ffce..a27fdd3 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4541,38 +4541,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.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 13/40] builtin/apply: make apply_all_patches() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (11 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 12/40] builtin/apply: move check_apply_state() to apply.c Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 14/40] builtin/apply: make parse_traditional_patch() " Christian Couder
` (26 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
To finish libifying the apply functionality, apply_all_patches() should not
die() or exit() in case of error, but return -1.
While doing that we must take care that file descriptors are properly closed
and, if needed, reset 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 | 40 +++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index a27fdd3..c27be35 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4558,7 +4558,7 @@ static int apply_all_patches(struct apply_state *state,
if (!strcmp(arg, "-")) {
res = apply_patch(state, 0, "<stdin>", options);
if (res < 0)
- exit(1);
+ goto rollback_end;
errs |= res;
read_stdin = 0;
continue;
@@ -4568,21 +4568,23 @@ 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));
+ goto rollback_end;
+ }
read_stdin = 0;
set_default_whitespace_mode(state);
res = apply_patch(state, fd, arg, options);
+ close(fd);
if (res < 0)
- exit(1);
+ goto rollback_end;
errs |= res;
- close(fd);
}
set_default_whitespace_mode(state);
if (read_stdin) {
res = apply_patch(state, 0, "<stdin>", options);
if (res < 0)
- exit(1);
+ goto rollback_end;
errs |= res;
}
@@ -4596,11 +4598,13 @@ 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);
+ goto rollback_end;
+ }
if (state->applied_after_fixing_ws && state->apply)
warning("%d line%s applied after"
" fixing whitespace errors.",
@@ -4614,12 +4618,22 @@ 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"));
+ goto rollback_end;
+ }
state->newfd = -1;
}
return !!errs;
+
+rollback_end:
+ if (state->newfd >= 0) {
+ rollback_lock_file(state->lock_file);
+ state->newfd = -1;
+ }
+ return -1;
}
int cmd_apply(int argc, const char **argv, const char *prefix)
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 14/40] builtin/apply: make parse_traditional_patch() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (12 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 13/40] builtin/apply: make apply_all_patches() return -1 on error Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 15/40] builtin/apply: make gitdiff_*() return 1 at end of header Christian Couder
` (25 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 c27be35..eb98116 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,
@@ -1462,7 +1464,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 -1;
*hdrsize = len + nextlen;
state->linenr += 2;
return offset;
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 15/40] builtin/apply: make gitdiff_*() return 1 at end of header
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (13 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 14/40] builtin/apply: make parse_traditional_patch() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 16/40] builtin/apply: make gitdiff_*() return -1 on error Christian Couder
` (24 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 eb98116..1142514 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;
}
@@ -1429,6 +1433,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 -1;
if (git_hdr_len <= len)
continue;
if (!patch->old_name && !patch->new_name) {
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 16/40] builtin/apply: make gitdiff_*() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (14 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 15/40] builtin/apply: make gitdiff_*() return 1 at end of header Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 17/40] builtin/apply: change die_on_unsafe_path() to check_unsafe_path() Christian Couder
` (23 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 1142514..b506369 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.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 17/40] builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (15 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 16/40] builtin/apply: make gitdiff_*() return -1 on error Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 18/40] builtin/apply: make build_fake_ancestor() return -1 on error Christian Couder
` (22 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 -1 using
error() 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 | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index b506369..429fddd 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3697,7 +3697,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;
@@ -3709,9 +3709,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;
}
/*
@@ -3801,8 +3802,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 -1;
/*
* An attempt to read from or delete a path that is beyond a
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 18/40] builtin/apply: make build_fake_ancestor() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (16 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 17/40] builtin/apply: change die_on_unsafe_path() to check_unsafe_path() Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 19/40] builtin/apply: make remove_file() " Christian Couder
` (21 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 429fddd..e74b068 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3889,11 +3889,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...
@@ -3911,31 +3912,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)
@@ -4476,8 +4484,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 = -1;
+ goto end;
+ }
if (state->diffstat)
stat_patch_list(state, list);
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 19/40] builtin/apply: make remove_file() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (17 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 18/40] builtin/apply: make build_fake_ancestor() return -1 on error Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 20/40] builtin/apply: make add_conflicted_stages_file() " Christian Couder
` (20 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 e74b068..694c65b 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4074,17 +4074,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,
@@ -4263,8 +4264,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(1);
+ }
return;
}
if (patch->is_new > 0 || patch->is_copy) {
@@ -4276,8 +4279,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(1);
+ }
if (phase == 1)
create_file(state, patch);
}
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 20/40] builtin/apply: make add_conflicted_stages_file() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (18 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 19/40] builtin/apply: make remove_file() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 21/40] builtin/apply: make add_index_file() " Christian Couder
` (19 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 694c65b..0997384 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4213,7 +4213,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;
@@ -4221,7 +4221,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);
@@ -4236,9 +4236,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)
@@ -4252,9 +4257,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(1);
+ } else
add_index_file(state, path, mode, buf, size);
}
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 21/40] builtin/apply: make add_index_file() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (19 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 20/40] builtin/apply: make add_conflicted_stages_file() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 22/40] builtin/apply: make create_file() " Christian Couder
` (18 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 0997384..005ba78 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4088,11 +4088,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;
@@ -4100,7 +4100,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);
@@ -4111,20 +4111,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)
@@ -4260,8 +4272,10 @@ static void create_file(struct apply_state *state, struct patch *patch)
if (patch->conflicted_threeway) {
if (add_conflicted_stages_file(state, patch))
exit(1);
- } else
- add_index_file(state, path, mode, buf, size);
+ } else {
+ if (add_index_file(state, path, mode, buf, size))
+ exit(1);
+ }
}
/* phase zero is to remove, phase one is to create */
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 22/40] builtin/apply: make create_file() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (20 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 21/40] builtin/apply: make add_index_file() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 23/40] builtin/apply: make write_out_one_result() " Christian Couder
` (17 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 005ba78..76d473c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4258,7 +4258,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;
@@ -4269,13 +4269,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(1);
- } else {
- if (add_index_file(state, path, mode, buf, size))
- exit(1);
- }
+ 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 */
@@ -4291,8 +4288,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(1);
+ }
return;
}
/*
@@ -4303,8 +4302,10 @@ static void write_out_one_result(struct apply_state *state,
if (remove_file(state, patch, patch->is_rename))
exit(1);
}
- if (phase == 1)
- create_file(state, patch);
+ if (phase == 1) {
+ if (create_file(state, patch))
+ exit(1);
+ }
}
static int write_out_one_reject(struct apply_state *state, struct patch *patch)
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 23/40] builtin/apply: make write_out_one_result() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (21 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 22/40] builtin/apply: make create_file() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 24/40] builtin/apply: make write_out_results() " Christian Couder
` (16 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 76d473c..291e24e 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4276,36 +4276,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(1);
- }
- 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(1);
- }
- 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(1);
- }
- if (phase == 1) {
- if (create_file(state, patch))
- exit(1);
- }
+ 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)
@@ -4392,7 +4385,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(1);
if (phase == 1) {
if (write_out_one_reject(state, l))
errs = 1;
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 24/40] builtin/apply: make write_out_results() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (22 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 23/40] builtin/apply: make write_out_one_result() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 25/40] builtin/apply: make try_create_file() " Christian Couder
` (15 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 291e24e..f35c901 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4372,6 +4372,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;
@@ -4385,8 +4391,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(1);
+ 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;
@@ -4498,10 +4506,17 @@ static int apply_patch(struct apply_state *state,
goto end;
}
- 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 = -1;
+ 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.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 25/40] builtin/apply: make try_create_file() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (23 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 24/40] builtin/apply: make write_out_results() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 26/40] builtin/apply: make create_one_file() " Christian Couder
` (14 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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", try_create_file() should return -1 in case of
error.
Unfortunately try_create_file() currently returns -1 to signal a
recoverable error. To fix that, let's make it return 1 in case of
a recoverable error and -1 in case of an unrecoverable error.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 41 ++++++++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index f35c901..37f0c2e 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4139,38 +4139,45 @@ static int add_index_file(struct apply_state *state,
return 0;
}
+/*
+ * Returns:
+ * -1 if an unrecoverable error happened
+ * 0 if everything went well
+ * 1 if a recoverable error happened
+ */
static int try_create_file(const char *path, unsigned int mode, const char *buf, unsigned long size)
{
- int fd;
+ int fd, res;
struct strbuf nbuf = STRBUF_INIT;
if (S_ISGITLINK(mode)) {
struct stat st;
if (!lstat(path, &st) && S_ISDIR(st.st_mode))
return 0;
- return mkdir(path, 0777);
+ return !!mkdir(path, 0777);
}
if (has_symlinks && S_ISLNK(mode))
/* Although buf:size is counted string, it also is NUL
* terminated.
*/
- return symlink(buf, path);
+ return !!symlink(buf, path);
fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 0100) ? 0777 : 0666);
if (fd < 0)
- return -1;
+ return 1;
if (convert_to_working_tree(path, buf, size, &nbuf)) {
size = nbuf.len;
buf = nbuf.buf;
}
- write_or_die(fd, buf, size);
+ res = !write_or_whine_pipe(fd, buf, size, path);
strbuf_release(&nbuf);
- if (close(fd) < 0)
- die_errno(_("closing file '%s'"), path);
- return 0;
+ if (close(fd) < 0 && !res)
+ return error(_("closing file '%s': %s"), path, strerror(errno));
+
+ return res ? -1 : 0;
}
/*
@@ -4184,15 +4191,24 @@ static void create_one_file(struct apply_state *state,
const char *buf,
unsigned long size)
{
+ int res;
+
if (state->cached)
return;
- if (!try_create_file(path, mode, buf, size))
+
+ res = try_create_file(path, mode, buf, size);
+ if (res < 0)
+ exit(1);
+ if (!res)
return;
if (errno == ENOENT) {
if (safe_create_leading_directories(path))
return;
- if (!try_create_file(path, mode, buf, size))
+ res = try_create_file(path, mode, buf, size);
+ if (res < 0)
+ exit(1);
+ if (!res)
return;
}
@@ -4211,7 +4227,10 @@ static void create_one_file(struct apply_state *state,
for (;;) {
char newpath[PATH_MAX];
mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr);
- if (!try_create_file(newpath, mode, buf, size)) {
+ res = try_create_file(newpath, mode, buf, size);
+ if (res < 0)
+ exit(1);
+ if (!res) {
if (!rename(newpath, path))
return;
unlink_or_warn(newpath);
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 26/40] builtin/apply: make create_one_file() return -1 on error
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (24 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 25/40] builtin/apply: make try_create_file() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 27/40] builtin/apply: rename option parsing functions Christian Couder
` (13 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 37f0c2e..f51dc60 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4184,32 +4184,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(1);
+ 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(1);
+ return -1;
if (!res)
- return;
+ return 0;
}
if (errno == EEXIST || errno == EACCES) {
@@ -4229,10 +4233,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(1);
+ return -1;
if (!res) {
if (!rename(newpath, path))
- return;
+ return 0;
unlink_or_warn(newpath);
break;
}
@@ -4241,7 +4245,8 @@ static void create_one_file(struct apply_state *state,
++nr;
}
}
- die_errno(_("unable to write file '%s' mode %o"), path, mode);
+ return error(_("unable to write file '%s' mode %o: %s"),
+ path, mode, strerror(errno));
}
static int add_conflicted_stages_file(struct apply_state *state,
@@ -4286,7 +4291,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.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 27/40] builtin/apply: rename option parsing functions
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (25 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 26/40] builtin/apply: make create_one_file() " Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 28/40] apply: rename and move opt constants to apply.h Christian Couder
` (12 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 f51dc60..c84add0 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4566,16 +4566,16 @@ end:
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);
@@ -4583,9 +4583,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);
@@ -4593,8 +4593,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)
@@ -4604,8 +4604,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;
@@ -4614,8 +4614,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);
@@ -4729,13 +4729,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,
@@ -4767,13 +4767,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,
@@ -4791,7 +4791,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.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 28/40] apply: rename and move opt constants to apply.h
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (26 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 27/40] builtin/apply: rename option parsing functions Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 30/40] apply: make some parsing functions static again Christian Couder
` (11 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.h | 3 +++
builtin/apply.c | 11 ++++-------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/apply.h b/apply.h
index 1f2277e..3cc3da6 100644
--- a/apply.h
+++ b/apply.h
@@ -108,4 +108,7 @@ 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);
+#define APPLY_OPT_INACCURATE_EOF (1<<0)
+#define APPLY_OPT_RECOUNT (1<<1)
+
#endif
diff --git a/builtin/apply.c b/builtin/apply.c
index c84add0..a06ab55 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4449,9 +4449,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.
*
@@ -4480,8 +4477,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);
@@ -4785,10 +4782,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.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 30/40] apply: make some parsing functions static again
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (27 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 28/40] apply: rename and move opt constants to apply.h Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 31/40] environment: add set_index_file() Christian Couder
` (10 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 4920fa8..713d1c0 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 736f818..89e7982 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.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 31/40] environment: add set_index_file()
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (28 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 30/40] apply: make some parsing functions static again Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 32/40] write_or_die: use warning() instead of fprintf(stderr, ...) Christian Couder
` (9 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
Introduce set_index_file() to be able to temporarily change the index file.
It should be used like this:
/* Save current index file */
old_index_file = get_index_file();
set_index_file((char *)tmp_index_file);
/* Do stuff that will use tmp_index_file as the index file */
...
/* When finished reset the index file */
set_index_file(old_index_file);
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
cache.h | 1 +
environment.c | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/cache.h b/cache.h
index 81d4ac3..28fc0bf 100644
--- a/cache.h
+++ b/cache.h
@@ -461,6 +461,7 @@ extern int is_inside_work_tree(void);
extern const char *get_git_dir(void);
extern const char *get_git_common_dir(void);
extern char *get_object_directory(void);
+extern void set_index_file(char *index_file);
extern char *get_index_file(void);
extern char *get_graft_file(void);
extern int set_git_dir(const char *path);
diff --git a/environment.c b/environment.c
index ca72464..7a53799 100644
--- a/environment.c
+++ b/environment.c
@@ -292,6 +292,16 @@ int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1)
return open(name, O_RDWR|O_CREAT|O_EXCL, 0600);
}
+/*
+ * Temporarily change the index file.
+ * Please save the current index file using get_index_file() before changing
+ * the index file. And when finished, reset it to the saved value.
+ */
+void set_index_file(char *index_file)
+{
+ git_index_file = index_file;
+}
+
char *get_index_file(void)
{
if (!git_index_file)
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 32/40] write_or_die: use warning() instead of fprintf(stderr, ...)
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (29 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 31/40] environment: add set_index_file() Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 33/40] apply: add 'be_silent' variable to 'struct apply_state' Christian Couder
` (8 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
In write_or_whine_pipe() and write_or_whine() when write_in_full()
returns an error, let's print the errno related error message using
warning() instead of fprintf(stderr, ...).
This makes it possible to change the way it is handled by changing
the current warn routine in usage.c.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
write_or_die.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/write_or_die.c b/write_or_die.c
index 49e80aa..c29f677 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -87,8 +87,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg)
{
if (write_in_full(fd, buf, count) < 0) {
check_pipe(errno);
- fprintf(stderr, "%s: write error (%s)\n",
- msg, strerror(errno));
+ warning("%s: write error (%s)\n", msg, strerror(errno));
return 0;
}
@@ -98,8 +97,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg)
int write_or_whine(int fd, const void *buf, size_t count, const char *msg)
{
if (write_in_full(fd, buf, count) < 0) {
- fprintf(stderr, "%s: write error (%s)\n",
- msg, strerror(errno));
+ warning("%s: write error (%s)\n", msg, strerror(errno));
return 0;
}
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 33/40] apply: add 'be_silent' variable to 'struct apply_state'
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (30 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 32/40] write_or_die: use warning() instead of fprintf(stderr, ...) Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 34/40] apply: make 'be_silent' incompatible with 'apply_verbosely' Christian Couder
` (7 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
This variable should prevent anything to be printed on both stderr
and stdout.
Let's not take care of stdout and apply_verbosely for now though,
as that will be taken care of in following patches.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 43 +++++++++++++++++++++++++++++--------------
apply.h | 1 +
2 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/apply.c b/apply.c
index 713d1c0..dbb2515 100644
--- a/apply.c
+++ b/apply.c
@@ -1612,8 +1612,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->be_silent)
+ fprintf(stderr, "%s:%d: %s.\n%.*s\n",
+ state->patch_input_file, linenr, err, len, line);
free(err);
}
@@ -1808,7 +1809,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->be_silent)
fprintf_ln(stderr,
_("** warning: "
"file %s becomes empty but is not deleted"),
@@ -3031,8 +3032,8 @@ 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->be_silent)
fprintf_ln(stderr, _("Context reduced to (%ld/%ld)"
" to apply fragment at %d"),
leading, trailing, applied_pos+1);
@@ -3529,7 +3530,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->be_silent)
+ fprintf(stderr, "Falling back to three-way merge...\n");
img = strbuf_detach(&buf, &len);
prepare_image(&tmp_image, img, len, 1);
@@ -3559,7 +3561,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->be_silent)
+ fprintf(stderr,
+ "Failed to fall back on three-way merge...\n");
return status;
}
@@ -3571,9 +3575,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->be_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->be_silent)
+ fprintf(stderr,
+ "Applied patch to '%s' cleanly.\n",
+ patch->new_name);
}
return 0;
}
@@ -4472,7 +4482,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->be_silent)
+ say_patch_name(stderr, sb.buf, patch);
strbuf_release(&sb);
cnt = strlen(patch->new_name);
@@ -4499,10 +4510,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->be_silent)
+ fprintf_ln(stderr, _("Hunk #%d applied cleanly."), cnt);
continue;
}
- fprintf_ln(stderr, _("Rejected hunk #%d."), cnt);
+ if (!state->be_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);
@@ -4551,8 +4564,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->be_silent) {
+ for_each_string_list_item(item, &cpath)
+ fprintf(stderr, "U %s\n", item->string);
+ }
string_list_clear(&cpath, 0);
rerere(0);
diff --git a/apply.h b/apply.h
index 89e7982..034541a 100644
--- a/apply.h
+++ b/apply.h
@@ -52,6 +52,7 @@ struct apply_state {
int apply_in_reverse;
int apply_with_reject;
int apply_verbosely;
+ int be_silent;
int no_add;
int threeway;
int unidiff_zero;
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 34/40] apply: make 'be_silent' incompatible with 'apply_verbosely'
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (31 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 33/40] apply: add 'be_silent' variable to 'struct apply_state' Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 35/40] apply: don't print on stdout when be_silent is set Christian Couder
` (6 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
It should be an error to have both be_silent and apply_verbosely set,
so let's check that in check_apply_state().
And by the way let's not automatically set apply_verbosely when
be_silent is set.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/apply.c b/apply.c
index dbb2515..dd9b301 100644
--- a/apply.c
+++ b/apply.c
@@ -122,8 +122,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->be_silent)
+ 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)
@@ -135,6 +138,8 @@ int check_apply_state(struct apply_state *state, int force_apply)
}
if (state->check_index)
state->unsafe_paths = 0;
+ if (state->be_silent && state->apply_verbosely)
+ return error(_("incompatible internal 'be_silent' and 'apply_verbosely' flags"));
if (!state->lock_file)
return error("BUG: state->lock_file should not be NULL");
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 35/40] apply: don't print on stdout when be_silent is set
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (32 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 34/40] apply: make 'be_silent' incompatible with 'apply_verbosely' Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 36/40] usage: add set_warn_routine() Christian Couder
` (5 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
This variable should prevent anything to be printed on both stderr
and stdout.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/apply.c b/apply.c
index dd9b301..2529534 100644
--- a/apply.c
+++ b/apply.c
@@ -4679,13 +4679,13 @@ static int apply_patch(struct apply_state *state,
goto end;
}
- if (state->diffstat)
+ if (state->diffstat && !state->be_silent)
stat_patch_list(state, list);
- if (state->numstat)
+ if (state->numstat && !state->be_silent)
numstat_patch_list(state, list);
- if (state->summary)
+ if (state->summary && !state->be_silent)
summary_patch_list(list);
end:
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 36/40] usage: add set_warn_routine()
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (33 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 35/40] apply: don't print on stdout when be_silent is set Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 37/40] usage: add get_error_routine() and get_warn_routine() Christian Couder
` (4 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
There are already set_die_routine() and set_error_routine(),
so let's add set_warn_routine() as this will be needed in a
following commit.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
git-compat-util.h | 1 +
usage.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 49d4029..f78706a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -440,6 +440,7 @@ static inline int const_error(void)
extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
extern void set_error_routine(void (*routine)(const char *err, va_list params));
+extern void set_warn_routine(void (*routine)(const char *warn, va_list params));
extern void set_die_is_recursing_routine(int (*routine)(void));
extern void set_error_handle(FILE *);
diff --git a/usage.c b/usage.c
index 1dad03f..67e5526 100644
--- a/usage.c
+++ b/usage.c
@@ -70,6 +70,11 @@ void set_error_routine(void (*routine)(const char *err, va_list params))
error_routine = routine;
}
+void set_warn_routine(void (*routine)(const char *warn, va_list params))
+{
+ warn_routine = routine;
+}
+
void set_die_is_recursing_routine(int (*routine)(void))
{
die_is_recursing = routine;
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 37/40] usage: add get_error_routine() and get_warn_routine()
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (34 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 36/40] usage: add set_warn_routine() Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 38/40] apply: change error_routine when be_silent is set Christian Couder
` (3 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
Let's make it possible to get the current error_routine and warn_routine,
so that we can store them before using set_error_routine() or
set_warn_routine() to use new ones.
This way we will be able put back the original routines, when we are done
with using new ones.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
git-compat-util.h | 2 ++
usage.c | 10 ++++++++++
2 files changed, 12 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index f78706a..92af27d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -440,7 +440,9 @@ static inline int const_error(void)
extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
extern void set_error_routine(void (*routine)(const char *err, va_list params));
+extern void (*get_error_routine(void))(const char *err, va_list params);
extern void set_warn_routine(void (*routine)(const char *warn, va_list params));
+extern void (*get_warn_routine(void))(const char *warn, va_list params);
extern void set_die_is_recursing_routine(int (*routine)(void));
extern void set_error_handle(FILE *);
diff --git a/usage.c b/usage.c
index 67e5526..2fd3045 100644
--- a/usage.c
+++ b/usage.c
@@ -70,11 +70,21 @@ void set_error_routine(void (*routine)(const char *err, va_list params))
error_routine = routine;
}
+void (*get_error_routine(void))(const char *err, va_list params)
+{
+ return error_routine;
+}
+
void set_warn_routine(void (*routine)(const char *warn, va_list params))
{
warn_routine = routine;
}
+void (*get_warn_routine(void))(const char *warn, va_list params)
+{
+ return warn_routine;
+}
+
void set_die_is_recursing_routine(int (*routine)(void))
{
die_is_recursing = routine;
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 38/40] apply: change error_routine when be_silent is set
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (35 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 37/40] usage: add get_error_routine() and get_warn_routine() Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 39/40] builtin/am: use apply api in run_apply() Christian Couder
` (2 subsequent siblings)
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
To avoid printing anything when applying with be_silent set,
let's save the existing warn and error routines before
applying and replace them with a routine that does nothing.
Then after applying, let's restore the saved routines.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 23 +++++++++++++++++++++--
apply.h | 4 ++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/apply.c b/apply.c
index 2529534..ef49709 100644
--- a/apply.c
+++ b/apply.c
@@ -109,6 +109,11 @@ void clear_apply_state(struct apply_state *state)
/* &state->fn_table is cleared at the end of apply_patch() */
}
+static void mute_routine(const char *bla, va_list params)
+{
+ /* do nothing */
+}
+
int check_apply_state(struct apply_state *state, int force_apply)
{
int is_not_gitdir = !startup_info->have_repository;
@@ -143,6 +148,13 @@ int check_apply_state(struct apply_state *state, int force_apply)
if (!state->lock_file)
return error("BUG: state->lock_file should not be NULL");
+ if (state->be_silent) {
+ state->saved_error_routine = get_error_routine();
+ state->saved_warn_routine = get_warn_routine();
+ set_error_routine(mute_routine);
+ set_warn_routine(mute_routine);
+ }
+
return 0;
}
@@ -4760,6 +4772,7 @@ int apply_all_patches(struct apply_state *state,
{
int i;
int res;
+ int retval = -1;
int errs = 0;
int read_stdin = 1;
@@ -4838,12 +4851,18 @@ int apply_all_patches(struct apply_state *state,
state->newfd = -1;
}
- return !!errs;
+ retval = !!errs;
rollback_end:
if (state->newfd >= 0) {
rollback_lock_file(state->lock_file);
state->newfd = -1;
}
- return -1;
+
+ if (state->be_silent) {
+ set_error_routine(state->saved_error_routine);
+ set_warn_routine(state->saved_warn_routine);
+ }
+
+ return retval;
}
diff --git a/apply.h b/apply.h
index 034541a..c6cf33d 100644
--- a/apply.h
+++ b/apply.h
@@ -89,6 +89,10 @@ struct apply_state {
*/
struct string_list fn_table;
+ /* This is to save some reporting routines */
+ void (*saved_error_routine)(const char *err, va_list params);
+ void (*saved_warn_routine)(const char *warn, va_list params);
+
/* These control whitespace errors */
enum ws_error_action ws_error_action;
enum ws_ignore ws_ignore_action;
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 39/40] builtin/am: use apply api in run_apply()
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (36 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 38/40] apply: change error_routine when be_silent is set Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:09 ` [PATCH v7 40/40] apply: use error_errno() where possible Christian Couder
2016-06-13 16:16 ` [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
This replaces run_apply() implementation with a new one that
uses the apply api that has been previously prepared in
apply.c and apply.h.
This shoud improve performance a lot in certain cases.
As the previous implementation was creating a new `git apply`
process to apply each patch, it could be slow on systems like
Windows where it is costly to create new processes.
Also the new `git apply` process had to read the index from
disk, and when the process was done the calling process
discarded its own index and read back from disk the new
index that had been created by the `git apply` process.
This could be very inefficient with big repositories that
have big index files, especially when the system decided
that it was a good idea to run the `git apply` processes on
a different processor core.
Also eliminating index reads enables further performance
improvements by using:
`git update-index --split-index`
For example here is a benchmark of a multi hundred commit
rebase on the Linux kernel on a Debian laptop with SSD:
command: git rebase --onto 1993b17 52bef0c 29dde7c
Vanilla "next" without split index: 1m54.953s
Vanilla "next" with split index: 1m22.476s
This series on top of "next" without split index: 1m12.034s
This series on top of "next" with split index: 0m15.678s
(using branch "next" from mid April 2016.)
Benchmarked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/am.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 73 insertions(+), 18 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 3dfe70b..8647298 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -28,6 +28,7 @@
#include "rerere.h"
#include "prompt.h"
#include "mailinfo.h"
+#include "apply.h"
/**
* Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -1521,39 +1522,93 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
*/
static int run_apply(const struct am_state *state, const char *index_file)
{
- struct child_process cp = CHILD_PROCESS_INIT;
+ struct argv_array apply_paths = ARGV_ARRAY_INIT;
+ struct argv_array apply_opts = ARGV_ARRAY_INIT;
+ struct apply_state apply_state;
+ int res, opts_left;
+ char *save_index_file;
+ static struct lock_file lock_file;
+
+ struct option am_apply_options[] = {
+ { OPTION_CALLBACK, 0, "whitespace", &apply_state, N_("action"),
+ N_("detect new or modified lines that have whitespace errors"),
+ 0, apply_option_parse_whitespace },
+ { OPTION_CALLBACK, 0, "ignore-space-change", &apply_state, NULL,
+ N_("ignore changes in whitespace when finding context"),
+ PARSE_OPT_NOARG, apply_option_parse_space_change },
+ { OPTION_CALLBACK, 0, "ignore-whitespace", &apply_state, NULL,
+ N_("ignore changes in whitespace when finding context"),
+ PARSE_OPT_NOARG, apply_option_parse_space_change },
+ { OPTION_CALLBACK, 0, "directory", &apply_state, N_("root"),
+ N_("prepend <root> to all filenames"),
+ 0, apply_option_parse_directory },
+ { OPTION_CALLBACK, 0, "exclude", &apply_state, N_("path"),
+ N_("don't apply changes matching the given path"),
+ 0, apply_option_parse_exclude },
+ { OPTION_CALLBACK, 0, "include", &apply_state, N_("path"),
+ N_("apply changes matching the given path"),
+ 0, apply_option_parse_include },
+ OPT_INTEGER('C', NULL, &apply_state.p_context,
+ N_("ensure at least <n> lines of context match")),
+ { OPTION_CALLBACK, 'p', NULL, &apply_state, N_("num"),
+ N_("remove <num> leading slashes from traditional diff paths"),
+ 0, apply_option_parse_p },
+ OPT_BOOL(0, "reject", &apply_state.apply_with_reject,
+ N_("leave the rejected hunks in corresponding *.rej files")),
+ OPT_END()
+ };
- cp.git_cmd = 1;
+ if (index_file) {
+ save_index_file = get_index_file();
+ set_index_file((char *)index_file);
+ }
+
+ if (init_apply_state(&apply_state, NULL, &lock_file))
+ die("init_apply_state() failed");
+
+ argv_array_push(&apply_opts, "apply");
+ argv_array_pushv(&apply_opts, state->git_apply_opts.argv);
+
+ opts_left = parse_options(apply_opts.argc, apply_opts.argv,
+ NULL, am_apply_options, NULL, 0);
+
+ if (opts_left != 0)
+ die("unknown option passed thru to git apply");
if (index_file)
- argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", index_file);
+ apply_state.cached = 1;
+ else
+ apply_state.check_index = 1;
/*
* If we are allowed to fall back on 3-way merge, don't give false
* errors during the initial attempt.
*/
- if (state->threeway && !index_file) {
- cp.no_stdout = 1;
- cp.no_stderr = 1;
- }
+ if (state->threeway && !index_file)
+ apply_state.be_silent = 1;
- argv_array_push(&cp.args, "apply");
+ if (check_apply_state(&apply_state, 0))
+ die("check_apply_state() failed");
- argv_array_pushv(&cp.args, state->git_apply_opts.argv);
+ argv_array_push(&apply_paths, am_path(state, "patch"));
+
+ res = apply_all_patches(&apply_state, apply_paths.argc, apply_paths.argv, 0);
if (index_file)
- argv_array_push(&cp.args, "--cached");
- else
- argv_array_push(&cp.args, "--index");
+ set_index_file(save_index_file);
- argv_array_push(&cp.args, am_path(state, "patch"));
+ argv_array_clear(&apply_paths);
+ argv_array_clear(&apply_opts);
+ clear_apply_state(&apply_state);
- if (run_command(&cp))
- return -1;
+ if (res)
+ return res;
- /* Reload index as git-apply will have modified it. */
- discard_cache();
- read_cache_from(index_file ? index_file : get_index_file());
+ if (index_file) {
+ /* Reload index as apply_all_patches() will have modified it. */
+ discard_cache();
+ read_cache_from(index_file);
+ }
return 0;
}
--
2.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* [PATCH v7 40/40] apply: use error_errno() where possible
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (37 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 39/40] builtin/am: use apply api in run_apply() Christian Couder
@ 2016-06-13 16:09 ` Christian Couder
2016-06-13 16:16 ` [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:09 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
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 | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/apply.c b/apply.c
index ef49709..777798a 100644
--- a/apply.c
+++ b/apply.c
@@ -3505,7 +3505,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;
}
@@ -3664,7 +3664,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) {
@@ -3686,7 +3686,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)
@@ -3745,7 +3745,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;
}
@@ -4260,9 +4260,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);
}
@@ -4316,7 +4316,7 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,
strbuf_release(&nbuf);
if (close(fd) < 0 && !res)
- return error(_("closing file '%s': %s"), path, strerror(errno));
+ return error_errno(_("closing file '%s'"), path);
return res ? -1 : 0;
}
@@ -4386,8 +4386,8 @@ static int create_one_file(struct apply_state *state,
++nr;
}
}
- return error(_("unable to write file '%s' mode %o: %s"),
- path, mode, strerror(errno));
+ return error_errno(_("unable to write file '%s' mode %o"),
+ path, mode);
}
static int add_conflicted_stages_file(struct apply_state *state,
@@ -4514,7 +4514,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.9.0.rc2.411.g3e2ca28
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v7 00/40] libify apply and use lib in am, part 2
2016-06-13 16:09 [PATCH v7 00/40] libify apply and use lib in am, part 2 Christian Couder
` (38 preceding siblings ...)
2016-06-13 16:09 ` [PATCH v7 40/40] apply: use error_errno() where possible Christian Couder
@ 2016-06-13 16:16 ` Christian Couder
39 siblings, 0 replies; 46+ messages in thread
From: Christian Couder @ 2016-06-13 16:16 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Stefan Beller, Matthieu Moy, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Christian Couder
On Mon, Jun 13, 2016 at 6:09 PM, Christian Couder
<christian.couder@gmail.com> wrote:
>
> I will send a diff between this version and the previous one, as a
> reply to this email.
Here is the diff:
diff --git a/apply.c b/apply.c
index cd4cd01..777798a 100644
--- a/apply.c
+++ b/apply.c
@@ -4386,7 +4386,7 @@ static int create_one_file(struct apply_state *state,
++nr;
}
}
- return error_errno(_("unable to write file '%s' mode %o: %s"),
+ return error_errno(_("unable to write file '%s' mode %o"),
path, mode);
}
diff --git a/builtin/am.c b/builtin/am.c
index 43f7316..8647298 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1584,7 +1584,6 @@ static int run_apply(const struct am_state
*state, const char *index_file)
* If we are allowed to fall back on 3-way merge, don't give false
* errors during the initial attempt.
*/
-
if (state->threeway && !index_file)
apply_state.be_silent = 1;
@@ -1600,6 +1599,7 @@ static int run_apply(const struct am_state
*state, const char *index_file)
argv_array_clear(&apply_paths);
argv_array_clear(&apply_opts);
+ clear_apply_state(&apply_state);
if (res)
return res;
diff --git a/builtin/apply.c b/builtin/apply.c
index 93744f8..ddd61de 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -74,8 +74,6 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
OPT_BOOL(0, "allow-overlap", &state.allow_overlap,
N_("allow overlapping hunks")),
OPT__VERBOSE(&state.apply_verbosely, N_("be verbose")),
- OPT_BOOL(0, "silent", &state.be_silent,
- N_("do not print any output")),
OPT_BIT(0, "inaccurate-eof", &options,
N_("tolerate incorrectly detected missing
new-line at the end of file"),
APPLY_OPT_INACCURATE_EOF),
diff --git a/run-command.c b/run-command.c
index c09d6b0..af0c8a1 100644
--- a/run-command.c
+++ b/run-command.c
@@ -85,7 +85,7 @@ static inline void close_pair(int fd[2])
}
#ifndef GIT_WINDOWS_NATIVE
-static void dup_devnull(int to)
+static inline void dup_devnull(int to)
{
int fd = open("/dev/null", O_RDWR);
if (fd < 0)
^ permalink raw reply related [flat|nested] 46+ messages in thread