* Re: Fixup of a fixup not working right
From: Johannes Schindelin @ 2016-09-05 7:53 UTC (permalink / raw)
To: Philip Oakley; +Cc: Junio C Hamano, Robert Dailey, Git
In-Reply-To: <8F7471262D1C45D9A7BD1C29277004AD@PhilipOakley>
Hi Philip,
On Sun, 4 Sep 2016, Philip Oakley wrote:
> From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
>
> > The point is that fixup! messages are really special, and are always
> > intended to be squashed into the referenced commit *before* the latter
> > hits `master`.
>
> I think it's here that we have the hidden use case. I agree that all fixups
> should be squashed before they hit the blessed golden repository.
>
> I suspect that some use cases have intermediate repositories that
> contain a 'master' branch (it's just a name ;-) that isn't blessed and
> golden, e.g. at the team review repo level. In such cases it is possible
> for a fixup! to be passed up as part of the review, though it's not the
> current norm/expectation.
In such a case (which can totally arise when criss-crossing Pull Requests
on GitHub, for example, where a Pull Request's purpose may be to fix up
commits in another Pull Request before the latter is merged), the most
appropriate course of action is... to not reorder the fixup!s prematurely.
> > In short, I am opposed to this change.
>
> It's not like G4W doesn't need fixup!s on the side branches e.g. 5eaffe9
> ("fixup! Handle new t1501 test case properly with MinGW", 2016-07-12)
Yeah, well, Git for Windows' `master` branch is special, in that it is
constantly rebased (as "merging rebases", to keep fast-forwardability). I
would not necessarily use Git for Windows as a role model in this respect.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/4] add: document the chmod option
From: Johannes Schindelin @ 2016-09-05 7:44 UTC (permalink / raw)
To: Thomas Gummerer
Cc: Junio C Hamano, Jeff King, Jan Keromnes, git, Ingo Brückl,
Edward Thomson
In-Reply-To: <20160904113954.21697-2-t.gummerer@gmail.com>
Hi Thomas,
On Sun, 4 Sep 2016, Thomas Gummerer wrote:
> +--chmod=(+|-)::
There is an "x" missing ;-)
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2 05/38] refs: create a base class "ref_store" for files_ref_store
From: Stefan Beller @ 2016-09-05 5:17 UTC (permalink / raw)
To: Michael Haggerty, David Turner
Cc: Junio C Hamano, Ramsay Jones, Eric Sunshine, Jeff King,
Nguyễn Thái Ngọc Duy, git, Stefan Beller
In-Reply-To: <c6ae77bf-e2e2-c108-648d-75b5164b7ba2@alum.mit.edu>
On 04.09.2016 20:53, Michael Haggerty wrote:
> On 09/04/2016 10:40 PM, David Turner wrote:
>> On Sun, 2016-09-04 at 18:08 +0200, Michael Haggerty wrote:
>>
>>> +/* A linked list of ref_stores for submodules: */
>>> +static struct ref_store *submodule_ref_stores;
>>
>> I don't like the per-submodule stores being in a linked list, which
>> requires a linear search. Stefan has, I think, been doing a bunch of
>> work to scale git to support on the order of thousands of submodules,
>> which this potentially breaks. What about a hashmap?
Well I did mostly usability up to this point, not efficiency in terms
of fast execution.
>
> I agree it's not ideal, but this linked list is not new. Before this
> patch the same code was in `files-backend.c`, and before patch [03/38]
> essentially the same linked list was stored as `submodule_ref_caches`.
> So this is not a regression, and I'd rather not address it in this patch
> series.
>
> CC Stefan in case he'd like to put it on his radar. Honestly, I've never
> checked how often the submodule reference cache is used in real life.
I think I need to spend some time on the data structures eventually.
Thanks for cc'ing
Stefan
>
> Michael
>
^ permalink raw reply
* Re: [PATCH v2 7/7] blame: actually use the diff opts parsed from the command line
From: Michael Haggerty @ 2016-09-05 4:12 UTC (permalink / raw)
To: René Scharfe, git
Cc: Stefan Beller, Junio C Hamano, Jeff King, Jakub Narębski,
Jacob Keller
In-Reply-To: <fe96d9e6-b306-0b57-8f24-6106b7e285cb@web.de>
On 08/23/2016 11:56 AM, René Scharfe wrote:
> Am 22.08.2016 um 13:22 schrieb Michael Haggerty:
>> "git blame" already parsed generic diff options from the command line
>> via diff_opt_parse(), but instead of passing the resulting xdl_opts to
>> xdi_diff(), it sent its own xdl_opts, which only reflected the values of
>> the self-parsed options "-w" and "--minimal". Instead, rely on
>> diff_opt_parse() to parse all of the diff options, including "-w" and
>> "--minimal", and pass the resulting xdl_opts to xdi_diff().
>
> Sounds useful: It allows more fine-grained control over which whitespace
> changes to ignore and which diff algorithm to use. There is a bit of
> overlap (e.g. with -b meaning show blank boundaries vs. ignore
> whitespace changes), but with your patch blame's own options still take
> precedence, so there should be no unpleasant surprises.
>
>> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
>> ---
>> Somebody who knows more about how diff operations are configured
>> should please review this. I'm not certain that the change as
>> implemented won't have other unwanted side-effects, though of course
>> I checked that the test suite runs correctly.
>
> I don't qualify, but I'll comment anyway..
>
>> builtin/blame.c | 11 ++--
>> t/t4059-diff-indent.sh | 160
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 165 insertions(+), 6 deletions(-)
>> create mode 100755 t/t4059-diff-indent.sh
>
> This new test doesn't call git blame. Does it belong to a different
> commit? And shouldn't the change to blame.c stand on its own, outside
> of this series?
Thanks for catching this. I squashed the test incorrectly; it was meant
to be part of the previous commit. I'll fix it in v3.
The reason that I would prefer to change `blame` as part of this patch
series is that I think it would be disconcerting for `git diff` and `git
blame` to use different heuristics when computing diffs. It would make
their output inconsistent.
But probably that means passing just the new option to `git blame`
rather than passing all possible `diff` options through.
> [...]
Your other comments may be moot given that changing `git blame` in this
way seems to have been a bad idea in the first place [1]. But I'll keep
them in mind if a new version contains similar code.
Thanks,
Michael
[1]
http://public-inbox.org/git/xmqqtwebwhbg.fsf@gitster.mtv.corp.google.com/
^ permalink raw reply
* Re: [PATCH v2 05/38] refs: create a base class "ref_store" for files_ref_store
From: Michael Haggerty @ 2016-09-05 3:53 UTC (permalink / raw)
To: David Turner
Cc: Junio C Hamano, Ramsay Jones, Eric Sunshine, Jeff King,
Nguyễn Thái Ngọc Duy, git, Stefan Beller
In-Reply-To: <1473021637.6035.27.camel@frank>
On 09/04/2016 10:40 PM, David Turner wrote:
> On Sun, 2016-09-04 at 18:08 +0200, Michael Haggerty wrote:
>
>> +/* A linked list of ref_stores for submodules: */
>> +static struct ref_store *submodule_ref_stores;
>
> I don't like the per-submodule stores being in a linked list, which
> requires a linear search. Stefan has, I think, been doing a bunch of
> work to scale git to support on the order of thousands of submodules,
> which this potentially breaks. What about a hashmap?
I agree it's not ideal, but this linked list is not new. Before this
patch the same code was in `files-backend.c`, and before patch [03/38]
essentially the same linked list was stored as `submodule_ref_caches`.
So this is not a regression, and I'd rather not address it in this patch
series.
CC Stefan in case he'd like to put it on his radar. Honestly, I've never
checked how often the submodule reference cache is used in real life.
Michael
^ permalink raw reply
* [PATCHv3] diff.c: emit moved lines with a different color
From: Stefan Beller @ 2016-09-04 23:42 UTC (permalink / raw)
To: git; +Cc: jnareb, gitster, jacob.keller, Stefan Beller, Stefan Beller
When we color the diff, we'll mark moved lines with a different color.
This is achieved by doing a two passes over the diff. The first pass
will inspect each line of the diff and store the removed lines and the
added lines in its own hash map.
The second pass will check for each added line if that is found in the
set of removed lines. If so it will color the added line differently as
with the new `moved-new` color mode. For each removed line we check the
set of added lines and if found emit that with the new color `moved-old`.
When detecting the moved lines, we cannot just rely on a line being equal,
but we need to take the context into account to detect when the moves were
reordered as we do not want to color moved but per-mutated lines.
To do that we use the hash of the preceding line.
This patch was motivated by e.g. reviewing 3b0c4200 ("apply: move
libified code from builtin/apply.c to apply.{c,h}", 2016-08-08)
Signed-off-by: Stefan Beller <sbeller@google.com>
---
* fixed memleaks
* only run the new code when needed.
* renamed variables to fit in better.
Documentation/config.txt | 5 +-
contrib/completion/git-completion.bash | 2 +
diff.c | 200 +++++++++++++++++++++++++++++++--
diff.h | 5 +-
4 files changed, 202 insertions(+), 10 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..f4f51c2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -980,8 +980,9 @@ color.diff.<slot>::
of `context` (context text - `plain` is a historical synonym),
`meta` (metainformation), `frag`
(hunk header), 'func' (function in hunk header), `old` (removed lines),
- `new` (added lines), `commit` (commit headers), or `whitespace`
- (highlighting whitespace errors).
+ `new` (added lines), `commit` (commit headers), `whitespace`
+ (highlighting whitespace errors), `moved-old` (removed lines that
+ reappear), `moved-new` (added lines that were removed elsewhere).
color.decorate.<slot>::
Use customized color for 'git log --decorate' output. `<slot>` is one
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9c8f738..b558d12 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2115,6 +2115,8 @@ _git_config ()
color.diff.old
color.diff.plain
color.diff.whitespace
+ color.diff.moved-old
+ color.diff.moved-new
color.grep
color.grep.context
color.grep.filename
diff --git a/diff.c b/diff.c
index 534c12e..7fde014 100644
--- a/diff.c
+++ b/diff.c
@@ -18,6 +18,7 @@
#include "ll-merge.h"
#include "string-list.h"
#include "argv-array.h"
+#include "git-compat-util.h"
#ifdef NO_FAST_WORKING_DIRECTORY
#define FAST_WORKING_DIRECTORY 0
@@ -42,6 +43,16 @@ static int diff_dirstat_permille_default = 30;
static struct diff_options default_diff_options;
static long diff_algorithm;
+static struct hashmap *moved_add;
+static struct hashmap *moved_del;
+static int hash_prev_added;
+static int hash_prev_removed;
+struct moved_entry {
+ struct hashmap_entry ent;
+ char *line;
+ int hash_prev_line;
+};
+
static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_NORMAL, /* CONTEXT */
@@ -52,6 +63,8 @@ static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_YELLOW, /* COMMIT */
GIT_COLOR_BG_RED, /* WHITESPACE */
GIT_COLOR_NORMAL, /* FUNCINFO */
+ GIT_COLOR_BLUE, /* NEW MOVED */
+ GIT_COLOR_MAGENTA, /* OLD MOVED */
};
static int parse_diff_color_slot(const char *var)
@@ -72,6 +85,10 @@ static int parse_diff_color_slot(const char *var)
return DIFF_WHITESPACE;
if (!strcasecmp(var, "func"))
return DIFF_FUNCINFO;
+ if (!strcasecmp(var, "moved-old"))
+ return DIFF_FILE_MOVED_OLD;
+ if (!strcasecmp(var, "moved-new"))
+ return DIFF_FILE_MOVED_NEW;
return -1;
}
@@ -287,6 +304,25 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
return git_default_config(var, value, cb);
}
+static int moved_entry_cmp(const struct moved_entry *a,
+ const struct moved_entry *b,
+ const void *unused)
+{
+ return strcmp(a->line, b->line) &&
+ a->hash_prev_line == b->hash_prev_line;
+}
+
+static struct moved_entry *prepare_entry(const char *line,
+ unsigned long len,
+ int hash_prev_line)
+{
+ struct moved_entry *ret = xmalloc(sizeof(*ret));
+ ret->ent.hash = memhash(line, len) ^ hash_prev_line;
+ ret->line = xmemdupz(line, len);
+ ret->hash_prev_line = hash_prev_line;
+ return ret;
+}
+
static char *quote_two(const char *one, const char *two)
{
int need_one = quote_c_style(one, NULL, NULL, 1);
@@ -537,16 +573,38 @@ static void emit_add_line(const char *reset,
struct emit_callback *ecbdata,
const char *line, int len)
{
+ enum color_diff color = DIFF_FILE_NEW;
+ unsigned ws_error_highlight = WSEH_NEW;
+
+ if (ecbdata->opt->color_moved) {
+ int hash = memhash(line, len);
+ struct moved_entry *keydata = prepare_entry(line, len,
+ hash_prev_added);
+ if (hashmap_get(moved_del, keydata, keydata))
+ color = DIFF_FILE_MOVED_NEW;
+ hash_prev_added = hash;
+ }
emit_line_checked(reset, ecbdata, line, len,
- DIFF_FILE_NEW, WSEH_NEW, '+');
+ color, ws_error_highlight, '+');
}
static void emit_del_line(const char *reset,
struct emit_callback *ecbdata,
const char *line, int len)
{
+ enum color_diff color = DIFF_FILE_OLD;
+ unsigned ws_error_highlight = WSEH_OLD;
+
+ if (ecbdata->opt->color_moved) {
+ int hash = memhash(line, len);
+ struct moved_entry *keydata = prepare_entry(line, len,
+ hash_prev_removed);
+ if (hashmap_get(moved_add, keydata, keydata))
+ color = DIFF_FILE_MOVED_OLD;
+ hash_prev_removed = hash;
+ }
emit_line_checked(reset, ecbdata, line, len,
- DIFF_FILE_OLD, WSEH_OLD, '-');
+ color, ws_error_highlight, '-');
}
static void emit_context_line(const char *reset,
@@ -555,6 +613,11 @@ static void emit_context_line(const char *reset,
{
emit_line_checked(reset, ecbdata, line, len,
DIFF_CONTEXT, WSEH_CONTEXT, ' ');
+ if (ecbdata->opt->color_moved) {
+ int h = memhash(line, len);
+ hash_prev_removed = h;
+ hash_prev_added = h;
+ }
}
static void emit_hunk_header(struct emit_callback *ecbdata,
@@ -1323,6 +1386,41 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
}
}
+static void fn_prepare_consume(void *priv, char *line, unsigned long len)
+{
+ struct moved_entry *d;
+ int hash = memhash(line + 1, len - 1);
+
+ switch (line[0]) {
+ case ' ':
+ hashmap_add(moved_del, prepare_entry(line + 1, len - 1, 0));
+ hashmap_add(moved_add, prepare_entry(line + 1, len - 1, 0));
+ hash_prev_added = hash;
+ hash_prev_removed = hash;
+ break;
+ case '+':
+ if (hash_prev_added) {
+ d = prepare_entry(line + 1, len - 1, hash_prev_added);
+ hashmap_add(moved_add, d);
+ }
+ hash_prev_added = hash;
+ hash_prev_removed = 0;
+ break;
+ case '-':
+ if (hash_prev_removed) {
+ d = prepare_entry(line + 1, len - 1, hash_prev_removed);
+ hashmap_add(moved_del, d);
+ }
+ hash_prev_added = 0;
+ hash_prev_removed = hash;
+ break;
+ default:
+ hash_prev_added = 0;
+ hash_prev_removed = 0;
+ break;
+ }
+}
+
static char *pprint_rename(const char *a, const char *b)
{
const char *old = a;
@@ -2279,6 +2377,56 @@ struct userdiff_driver *get_textconv(struct diff_filespec *one)
return userdiff_get_textconv(one->driver);
}
+static void prepare_moved_lines(struct diff_filepair *p, struct diff_options *o)
+{
+ mmfile_t mf1, mf2;
+ xpparam_t xpp;
+ xdemitconf_t xecfg;
+ struct emit_callback ecbdata;
+ struct diff_filespec *one = p->one;
+ struct diff_filespec *two = p->two;
+ struct userdiff_driver *textconv_one = NULL;
+ struct userdiff_driver *textconv_two = NULL;
+
+ if ((!one->mode || S_ISGITLINK(one->mode)) ||
+ (!two->mode || S_ISGITLINK(two->mode)))
+ return;
+
+ if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
+ textconv_one = get_textconv(one);
+ textconv_two = get_textconv(two);
+ }
+
+ if (!DIFF_OPT_TST(o, TEXT) &&
+ ( (!textconv_one && diff_filespec_is_binary(one)) ||
+ (!textconv_two && diff_filespec_is_binary(two)) ))
+ return;
+
+ mf1.size = fill_textconv(textconv_one, one, &mf1.ptr);
+ mf2.size = fill_textconv(textconv_two, two, &mf2.ptr);
+
+ memset(&xpp, 0, sizeof(xpp));
+ memset(&xecfg, 0, sizeof(xecfg));
+ memset(&ecbdata, 0, sizeof(ecbdata));
+
+ xpp.flags = o->xdl_opts;
+ xecfg.ctxlen = 1;
+
+ if (o->word_diff)
+ init_diff_words_data(&ecbdata, o, one, two);
+ if (xdi_diff_outf(&mf1, &mf2, fn_prepare_consume, &ecbdata,
+ &xpp, &xecfg))
+ die("unable to generate generate moved color output for %s",
+ one->path);
+ if (o->word_diff)
+ free_diff_words_data(&ecbdata);
+ if (textconv_one)
+ free(mf1.ptr);
+ if (textconv_two)
+ free(mf2.ptr);
+ xdiff_clear_find_func(&xecfg);
+}
+
static void builtin_diff(const char *name_a,
const char *name_b,
struct diff_filespec *one,
@@ -3295,6 +3443,7 @@ void diff_setup(struct diff_options *options)
options->xdl_opts |= diff_algorithm;
if (diff_compaction_heuristic)
DIFF_XDL_SET(options, COMPACTION_HEURISTIC);
+ options->color_moved = options->use_color;
options->orderfile = diff_order_file_cfg;
@@ -3413,6 +3562,9 @@ void diff_setup_done(struct diff_options *options)
if (DIFF_OPT_TST(options, FOLLOW_RENAMES) && options->pathspec.nr != 1)
die(_("--follow requires exactly one pathspec"));
+
+ if (!options->use_color || external_diff())
+ options->color_moved = 0;
}
static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
@@ -4622,6 +4774,43 @@ void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)
warning(rename_limit_advice, varname, needed);
}
+static void diff_flush_format_patch(struct diff_options *o)
+{
+ int i;
+ struct diff_queue_struct *q = &diff_queued_diff;
+ if (o->color_moved) {
+ moved_add = xmalloc(sizeof(*moved_add));
+ moved_del = xmalloc(sizeof(*moved_del));
+ hashmap_init(moved_add, (hashmap_cmp_fn)moved_entry_cmp, 0);
+ hashmap_init(moved_del, (hashmap_cmp_fn)moved_entry_cmp, 0);
+
+ for (i = 0; i < q->nr; i++) {
+ struct diff_filepair *p = q->queue[i];
+ if (check_pair_status(p) && !diff_unmodified_pair(p))
+ prepare_moved_lines(p, o);
+ }
+ }
+
+ for (i = 0; i < q->nr; i++) {
+ struct diff_filepair *p = q->queue[i];
+ if (check_pair_status(p))
+ diff_flush_patch(p, o);
+ }
+
+ if (o->color_moved) {
+ struct hashmap_iter iter;
+ struct moved_entry *e;
+ hashmap_iter_init(moved_add, &iter);
+ while ((e = hashmap_iter_next(&iter)))
+ free(e->line);
+ hashmap_iter_init(moved_del, &iter);
+ while ((e = hashmap_iter_next(&iter)))
+ free(e->line);
+ hashmap_free(moved_add, 1);
+ hashmap_free(moved_del, 1);
+ }
+}
+
void diff_flush(struct diff_options *options)
{
struct diff_queue_struct *q = &diff_queued_diff;
@@ -4696,6 +4885,7 @@ void diff_flush(struct diff_options *options)
if (!options->file)
die_errno("Could not open /dev/null");
options->close_file = 1;
+ options->color_moved = 0;
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
if (check_pair_status(p))
@@ -4716,11 +4906,7 @@ void diff_flush(struct diff_options *options)
}
}
- for (i = 0; i < q->nr; i++) {
- struct diff_filepair *p = q->queue[i];
- if (check_pair_status(p))
- diff_flush_patch(p, options);
- }
+ diff_flush_format_patch(options);
}
if (output_format & DIFF_FORMAT_CALLBACK)
diff --git a/diff.h b/diff.h
index 7883729..b0331b9 100644
--- a/diff.h
+++ b/diff.h
@@ -122,6 +122,7 @@ struct diff_options {
unsigned int filter;
int use_color;
+ int color_moved;
int context;
int interhunkcontext;
int break_opt;
@@ -189,7 +190,9 @@ enum color_diff {
DIFF_FILE_NEW = 5,
DIFF_COMMIT = 6,
DIFF_WHITESPACE = 7,
- DIFF_FUNCINFO = 8
+ DIFF_FUNCINFO = 8,
+ DIFF_FILE_MOVED_NEW = 9,
+ DIFF_FILE_MOVED_OLD = 10
};
const char *diff_get_color(int diff_use_color, enum color_diff ix);
#define diff_get_color_opt(o, ix) \
--
2.10.0.1.ga424fb3
^ permalink raw reply related
* Re: [RFC/PATCH 0/2] Color moved code differently
From: Junio C Hamano @ 2016-09-04 22:19 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <xmqqpookqi8k.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> I do not think you should step outside diff_flush(). Only when
> producing textual diff, you would have to run the textual diff
> twice by going over the q twice:
>
> * The first pass would run diff_flush_patch(), which would call
> into xdiff the usual way, but the callback from xdiff would
> capture the removed lines and the added lines without making any
> output.
>
> * The second pass would run diff_flush_patch(), but the callback
> from xdiff would be called with additional information, namely,
> the removed and the added lines captured in the first pass.
> ...
> The fn_out_consume() function working in the "second pass of
> moved from/moved to mode" would inspect line[] and see if it is
> an added or a removed line, and then:
>
> - if it is an added line, and it appears as a removed line
> elsewhere in the patchset (you obtained the information in the
> first pass), you show it as "this was moved from elsewhere".
>
> - if it is a removed line, and it appears as an added line
> elsewhere in the patchset (you obtained the information in the
> first pass), you show it as "this was moved to elsewhere".
>
> Or something like that.
Actually, the first pass above ends up queuing majority of the diff
output in-core _anyway_, so it would make much more sense not to
rely on the reproducibility of xdiff machinery by not calling it
twice. Instead, I would imagine that the first pass can queue all
what it receives in the callback, then with the whole-patch picture
at hand, you can call updated fn_out_consume() yourself (instead of
letting xdiff machinery to call you), and that would become the
"second pass".
^ permalink raw reply
* Re: [PATCH v2 05/38] refs: create a base class "ref_store" for files_ref_store
From: David Turner @ 2016-09-04 20:40 UTC (permalink / raw)
To: Michael Haggerty
Cc: Junio C Hamano, Ramsay Jones, Eric Sunshine, Jeff King,
Nguyễn Thái Ngọc Duy, git
In-Reply-To: <39277c75122a374b360057d6e1f6d889a90f6fb4.1473003902.git.mhagger@alum.mit.edu>
On Sun, 2016-09-04 at 18:08 +0200, Michael Haggerty wrote:
> +/* A linked list of ref_stores for submodules: */
> +static struct ref_store *submodule_ref_stores;
I don't like the per-submodule stores being in a linked list, which
requires a linear search. Stefan has, I think, been doing a bunch of
work to scale git to support on the order of thousands of submodules,
which this potentially breaks. What about a hashmap?
^ permalink raw reply
* [PATCH v14 05/41] builtin/apply: make find_header() return -128 instead of die()ing
From: Christian Couder @ 2016-09-04 20:17 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, let's make find_header() return -128 instead of
calling die().
We could make it return -1, unfortunately find_header() already
returns -1 when no header is found.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 40 ++++++++++++++++++++++++++++------------
t/t4254-am-corrupt.sh | 2 +-
2 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index dd7afee..434ba0c 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 if no header was found
+ * -128 in case of error
+ * 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,9 @@ 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);
+ error(_("patch fragment without header at line %d: %.*s"),
+ state->linenr, (int)len-1, line);
+ return -128;
}
if (size < len + 6)
@@ -1468,19 +1477,23 @@ static int find_header(struct apply_state *state,
if (git_hdr_len <= len)
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);
+ if (!patch->def_name) {
+ 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);
+ return -128;
+ }
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);
+ if (!patch->is_delete && !patch->new_name) {
+ error("git diff header lacks filename information "
+ "(line %d)", state->linenr);
+ return -128;
+ }
patch->is_toplevel_relative = 1;
*hdrsize = git_hdr_len;
return offset;
@@ -1996,6 +2009,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 == -128)
+ exit(128);
+
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.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 08/41] builtin/apply: make parse_whitespace_option() return -1 instead of die()ing
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, 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 10aaba7..06a76f2 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,
@@ -4589,7 +4589,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;
}
@@ -4626,8 +4627,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.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 07/41] builtin/apply: make parse_single_patch() return -1 on error
From: Christian Couder @ 2016-09-04 20:17 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_single_patch() should return a negative
integer 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 c07d142..10aaba7 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1671,6 +1671,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,
@@ -1688,8 +1692,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;
@@ -1725,9 +1731,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: "
@@ -2029,6 +2035,9 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
size - offset - hdrsize,
patch);
+ if (patchsize < 0)
+ return -128;
+
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.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 09/41] builtin/apply: make parse_ignorewhitespace_option() return -1 instead of die()ing
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.
To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", 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 06a76f2..ecb1f7a 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)
@@ -4629,8 +4629,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.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 10/41] builtin/apply: move init_apply_state() to apply.c
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we must make 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 | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
apply.h | 10 ++++++
builtin/apply.c | 91 -------------------------------------------------------
4 files changed, 105 insertions(+), 91 deletions(-)
create mode 100644 apply.c
diff --git a/Makefile b/Makefile
index d96ecb7..c6ac236 100644
--- a/Makefile
+++ b/Makefile
@@ -691,6 +691,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..c858ca4
--- /dev/null
+++ b/apply.c
@@ -0,0 +1,94 @@
+#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;
+ string_list_init(&state->fn_table, 0);
+ string_list_init(&state->limit_by_name, 0);
+ string_list_init(&state->symlink_changes, 0);
+ 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 7493a40..08c0a25 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 ecb1f7a..bb6ff77 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)
@@ -4539,13 +4493,6 @@ static int apply_patch(struct apply_state *state,
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)
{
@@ -4604,44 +4551,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;
- string_list_init(&state->fn_table, 0);
- string_list_init(&state->limit_by_name, 0);
- string_list_init(&state->symlink_changes, 0);
- 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.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 06/41] builtin/apply: make parse_chunk() return a negative integer on error
From: Christian Couder @ 2016-09-04 20:17 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing 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 a negative integer
instead of calling die() or exit().
As parse_chunk() is called only by apply_patch() which already
returns either -1 or -128 when an error happened, let's make it also
return -1 or -128.
This makes it compatible with what find_header() and parse_binary()
already return.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 434ba0c..c07d142 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1996,22 +1996,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 if no header was found or parse_binary() failed,
+ * -128 on another error,
+ * 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 == -128)
- exit(128);
-
if (offset < 0)
return offset;
@@ -2071,8 +2071,10 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
* empty to us here.
*/
if ((state->apply || state->check) &&
- (!patch->is_binary && !metadata_changes(patch)))
- die(_("patch with only garbage at line %d"), state->linenr);
+ (!patch->is_binary && !metadata_changes(patch))) {
+ error(_("patch with only garbage at line %d"), state->linenr);
+ return -128;
+ }
}
return offset + hdrsize + patchsize;
@@ -4455,6 +4457,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 == -128) {
+ res = -128;
+ goto end;
+ }
break;
}
if (state->apply_in_reverse)
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 36/41] usage: add get_error_routine() and get_warn_routine()
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
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 d189b4f..bac8613 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.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 37/41] apply: change error_routine when silent
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To avoid printing anything when applying with
`state->apply_verbosity == verbosity_silent`, let's save the
existing warn and error routines before applying, and let's
replace them with a routine that does nothing.
Then after applying, let's restore the saved routines.
Note that, as we need to restore the saved routines in all
cases, we cannot return early any more in apply_all_patches().
Helped-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 21 ++++++++++++++++++++-
apply.h | 8 ++++++++
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/apply.c b/apply.c
index ddbb0a2..27baf0e 100644
--- a/apply.c
+++ b/apply.c
@@ -112,6 +112,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 *msg, va_list params)
+{
+ /* do nothing */
+}
+
int check_apply_state(struct apply_state *state, int force_apply)
{
int is_not_gitdir = !startup_info->have_repository;
@@ -144,6 +149,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->apply_verbosity <= verbosity_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;
}
@@ -4864,7 +4876,7 @@ int apply_all_patches(struct apply_state *state,
state->newfd = -1;
}
- return !!errs;
+ res = !!errs;
end:
if (state->newfd >= 0) {
@@ -4872,5 +4884,12 @@ int apply_all_patches(struct apply_state *state,
state->newfd = -1;
}
+ if (state->apply_verbosity <= verbosity_silent) {
+ set_error_routine(state->saved_error_routine);
+ set_warn_routine(state->saved_warn_routine);
+ }
+
+ if (res > -1)
+ return res;
return (res == -1 ? 1 : 128);
}
diff --git a/apply.h b/apply.h
index f015403..902346b 100644
--- a/apply.h
+++ b/apply.h
@@ -94,6 +94,14 @@ struct apply_state {
*/
struct string_list fn_table;
+ /*
+ * This is to save reporting routines before using
+ * set_error_routine() or set_warn_routine() to install muting
+ * routines when in verbosity_silent mode.
+ */
+ 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 apply_ws_error_action ws_error_action;
enum apply_ws_ignore ws_ignore_action;
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 38/41] apply: refactor `git apply` option parsing
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
Parsing `git apply` options can be useful to other commands that
want to call the libified apply functionality, because this way
they can easily pass some options from their own command line to
the libified apply functionality.
This will be used by `git am` in a following patch.
To make this possible, let's refactor the `git apply` option
parsing code into a new libified apply_parse_options() function.
Doing that makes it possible to remove some functions definitions
from "apply.h" and make them static in "apply.c".
Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++-------
apply.h | 18 +++-------
builtin/apply.c | 74 ++--------------------------------------
3 files changed, 97 insertions(+), 98 deletions(-)
diff --git a/apply.c b/apply.c
index 27baf0e..2dad0db 100644
--- a/apply.c
+++ b/apply.c
@@ -4730,16 +4730,16 @@ static int apply_patch(struct apply_state *state,
return res;
}
-int apply_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;
}
-int apply_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);
@@ -4747,9 +4747,9 @@ int apply_option_parse_include(const struct option *opt,
return 0;
}
-int apply_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);
@@ -4757,8 +4757,8 @@ int apply_option_parse_p(const struct option *opt,
return 0;
}
-int apply_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)
@@ -4768,8 +4768,8 @@ int apply_option_parse_space_change(const struct option *opt,
return 0;
}
-int apply_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;
@@ -4778,8 +4778,8 @@ int apply_option_parse_whitespace(const struct option *opt,
return 0;
}
-int apply_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);
@@ -4893,3 +4893,80 @@ int apply_all_patches(struct apply_state *state,
return res;
return (res == -1 ? 1 : 128);
}
+
+int apply_parse_options(int argc, const char **argv,
+ struct apply_state *state,
+ int *force_apply, int *options,
+ const char * const *apply_usage)
+{
+ struct option builtin_apply_options[] = {
+ { OPTION_CALLBACK, 0, "exclude", state, N_("path"),
+ N_("don't apply changes matching the given path"),
+ 0, apply_option_parse_exclude },
+ { OPTION_CALLBACK, 0, "include", state, N_("path"),
+ N_("apply changes matching the given path"),
+ 0, apply_option_parse_include },
+ { OPTION_CALLBACK, 'p', NULL, state, N_("num"),
+ N_("remove <num> leading slashes from traditional diff paths"),
+ 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,
+ N_("instead of applying the patch, output diffstat for the input")),
+ OPT_NOOP_NOARG(0, "allow-binary-replacement"),
+ OPT_NOOP_NOARG(0, "binary"),
+ OPT_BOOL(0, "numstat", &state->numstat,
+ N_("show number of added and deleted lines in decimal notation")),
+ OPT_BOOL(0, "summary", &state->summary,
+ N_("instead of applying the patch, output a summary for the input")),
+ OPT_BOOL(0, "check", &state->check,
+ N_("instead of applying the patch, see if the patch is applicable")),
+ OPT_BOOL(0, "index", &state->check_index,
+ N_("make sure the patch is applicable to the current index")),
+ OPT_BOOL(0, "cached", &state->cached,
+ N_("apply a patch without touching the working tree")),
+ OPT_BOOL(0, "unsafe-paths", &state->unsafe_paths,
+ N_("accept a patch that touches outside the working area")),
+ OPT_BOOL(0, "apply", force_apply,
+ N_("also apply the patch (use with --stat/--summary/--check)")),
+ OPT_BOOL('3', "3way", &state->threeway,
+ N_( "attempt three-way merge if a patch does not apply")),
+ OPT_FILENAME(0, "build-fake-ancestor", &state->fake_ancestor,
+ N_("build a temporary index based on embedded index information")),
+ /* Think twice before adding "--nul" synonym to this */
+ OPT_SET_INT('z', NULL, &state->line_termination,
+ N_("paths are separated with NUL character"), '\0'),
+ OPT_INTEGER('C', NULL, &state->p_context,
+ 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, apply_option_parse_whitespace },
+ { OPTION_CALLBACK, 0, "ignore-space-change", state, NULL,
+ N_("ignore changes in whitespace when finding context"),
+ 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, 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,
+ N_("don't expect at least one line of context")),
+ OPT_BOOL(0, "reject", &state->apply_with_reject,
+ N_("leave the rejected hunks in corresponding *.rej files")),
+ OPT_BOOL(0, "allow-overlap", &state->allow_overlap,
+ N_("allow overlapping hunks")),
+ OPT__VERBOSE(&state->apply_verbosity, N_("be verbose")),
+ OPT_BIT(0, "inaccurate-eof", options,
+ N_("tolerate incorrectly detected missing new-line at the end of file"),
+ APPLY_OPT_INACCURATE_EOF),
+ OPT_BIT(0, "recount", options,
+ N_("do not trust the line counts in the hunk headers"),
+ APPLY_OPT_RECOUNT),
+ { OPTION_CALLBACK, 0, "directory", state, N_("root"),
+ N_("prepend <root> to all filenames"),
+ 0, apply_option_parse_directory },
+ OPT_END()
+ };
+
+ return parse_options(argc, argv, state->prefix, builtin_apply_options, apply_usage, 0);
+}
diff --git a/apply.h b/apply.h
index 902346b..9fec536 100644
--- a/apply.h
+++ b/apply.h
@@ -111,20 +111,10 @@ struct apply_state {
int applied_after_fixing_ws;
};
-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,
- const char *arg, int unset);
-extern int apply_option_parse_p(const struct option *opt,
- const char *arg,
- int unset);
-extern int apply_option_parse_whitespace(const struct option *opt,
- const char *arg, int unset);
-extern int apply_option_parse_directory(const struct option *opt,
- const char *arg, int unset);
-extern int apply_option_parse_space_change(const struct option *opt,
- const char *arg, int unset);
-
+extern int apply_parse_options(int argc, const char **argv,
+ struct apply_state *state,
+ int *force_apply, int *options,
+ const char * const *apply_usage);
extern int init_apply_state(struct apply_state *state,
const char *prefix,
struct lock_file *lock_file);
diff --git a/builtin/apply.c b/builtin/apply.c
index 7338701..81b9a61 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -18,80 +18,12 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
int ret;
struct apply_state state;
- struct option builtin_apply_options[] = {
- { OPTION_CALLBACK, 0, "exclude", &state, N_("path"),
- N_("don't apply changes matching the given path"),
- 0, apply_option_parse_exclude },
- { OPTION_CALLBACK, 0, "include", &state, N_("path"),
- N_("apply changes matching the given path"),
- 0, apply_option_parse_include },
- { OPTION_CALLBACK, 'p', NULL, &state, N_("num"),
- N_("remove <num> leading slashes from traditional diff paths"),
- 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,
- N_("instead of applying the patch, output diffstat for the input")),
- OPT_NOOP_NOARG(0, "allow-binary-replacement"),
- OPT_NOOP_NOARG(0, "binary"),
- OPT_BOOL(0, "numstat", &state.numstat,
- N_("show number of added and deleted lines in decimal notation")),
- OPT_BOOL(0, "summary", &state.summary,
- N_("instead of applying the patch, output a summary for the input")),
- OPT_BOOL(0, "check", &state.check,
- N_("instead of applying the patch, see if the patch is applicable")),
- OPT_BOOL(0, "index", &state.check_index,
- N_("make sure the patch is applicable to the current index")),
- OPT_BOOL(0, "cached", &state.cached,
- N_("apply a patch without touching the working tree")),
- OPT_BOOL(0, "unsafe-paths", &state.unsafe_paths,
- N_("accept a patch that touches outside the working area")),
- OPT_BOOL(0, "apply", &force_apply,
- N_("also apply the patch (use with --stat/--summary/--check)")),
- OPT_BOOL('3', "3way", &state.threeway,
- N_( "attempt three-way merge if a patch does not apply")),
- OPT_FILENAME(0, "build-fake-ancestor", &state.fake_ancestor,
- N_("build a temporary index based on embedded index information")),
- /* Think twice before adding "--nul" synonym to this */
- OPT_SET_INT('z', NULL, &state.line_termination,
- N_("paths are separated with NUL character"), '\0'),
- OPT_INTEGER('C', NULL, &state.p_context,
- 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, apply_option_parse_whitespace },
- { OPTION_CALLBACK, 0, "ignore-space-change", &state, NULL,
- N_("ignore changes in whitespace when finding context"),
- 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, 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,
- N_("don't expect at least one line of context")),
- OPT_BOOL(0, "reject", &state.apply_with_reject,
- N_("leave the rejected hunks in corresponding *.rej files")),
- OPT_BOOL(0, "allow-overlap", &state.allow_overlap,
- N_("allow overlapping hunks")),
- OPT__VERBOSE(&state.apply_verbosity, N_("be verbose")),
- OPT_BIT(0, "inaccurate-eof", &options,
- N_("tolerate incorrectly detected missing new-line at the end of file"),
- APPLY_OPT_INACCURATE_EOF),
- OPT_BIT(0, "recount", &options,
- N_("do not trust the line counts in the hunk headers"),
- APPLY_OPT_RECOUNT),
- { OPTION_CALLBACK, 0, "directory", &state, N_("root"),
- N_("prepend <root> to all filenames"),
- 0, apply_option_parse_directory },
- OPT_END()
- };
-
if (init_apply_state(&state, prefix, &lock_file))
exit(128);
- argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
- apply_usage, 0);
+ argc = apply_parse_options(argc, argv,
+ &state, &force_apply, &options,
+ apply_usage);
if (check_apply_state(&state, force_apply))
exit(128);
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 39/41] apply: pass apply state to build_fake_ancestor()
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify git apply functionality, we will need to read from a
different index file in get_current_sha1(). This index file will be
stored in "struct apply_state", so let's pass the state to
build_fake_ancestor() which will later pass it to get_current_sha1().
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/apply.c b/apply.c
index 2dad0db..f29387d 100644
--- a/apply.c
+++ b/apply.c
@@ -4042,7 +4042,7 @@ 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 int build_fake_ancestor(struct patch *list, const char *filename)
+static int build_fake_ancestor(struct apply_state *state, struct patch *list)
{
struct patch *patch;
struct index_state result = { NULL };
@@ -4089,12 +4089,13 @@ static int build_fake_ancestor(struct patch *list, const char *filename)
}
}
- hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);
+ hold_lock_file_for_update(&lock, state->fake_ancestor, LOCK_DIE_ON_ERROR);
res = write_locked_index(&result, &lock, COMMIT_LOCK);
discard_index(&result);
if (res)
- return error("Could not write temporary index to %s", filename);
+ return error("Could not write temporary index to %s",
+ state->fake_ancestor);
return 0;
}
@@ -4709,7 +4710,7 @@ static int apply_patch(struct apply_state *state,
}
if (state->fake_ancestor &&
- build_fake_ancestor(list, state->fake_ancestor)) {
+ build_fake_ancestor(state, list)) {
res = -128;
goto end;
}
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 40/41] apply: learn to use a different index file
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
Sometimes we want to apply in a different index file.
Before the apply functionality was libified it was possible to
use the GIT_INDEX_FILE environment variable, for this purpose.
But now, as the apply functionality has been libified, it should
be possible to do that in a libified way.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 27 +++++++++++++++++++++------
apply.h | 1 +
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/apply.c b/apply.c
index f29387d..b1dd998 100644
--- a/apply.c
+++ b/apply.c
@@ -3993,12 +3993,21 @@ static int check_patch_list(struct apply_state *state, struct patch *patch)
return err;
}
+static int read_apply_cache(struct apply_state *state)
+{
+ if (state->index_file)
+ return read_cache_from(state->index_file);
+ else
+ return read_cache();
+}
+
/* This function tries to read the sha1 from the current index */
-static int get_current_sha1(const char *path, unsigned char *sha1)
+static int get_current_sha1(struct apply_state *state, const char *path,
+ unsigned char *sha1)
{
int pos;
- if (read_cache() < 0)
+ if (read_apply_cache(state) < 0)
return -1;
pos = cache_name_pos(path, strlen(path));
if (pos < 0)
@@ -4071,7 +4080,7 @@ static int build_fake_ancestor(struct apply_state *state, struct patch *list)
; /* ok */
} else if (!patch->lines_added && !patch->lines_deleted) {
/* mode-only change: update the current */
- if (get_current_sha1(patch->old_name, sha1))
+ if (get_current_sha1(state, patch->old_name, sha1))
return error("mode change for %s, which is not "
"in current HEAD", name);
} else
@@ -4675,10 +4684,16 @@ static int apply_patch(struct apply_state *state,
state->apply = 0;
state->update_index = state->check_index && state->apply;
- if (state->update_index && state->newfd < 0)
- state->newfd = hold_locked_index(state->lock_file, 1);
+ if (state->update_index && state->newfd < 0) {
+ if (state->index_file)
+ state->newfd = hold_lock_file_for_update(state->lock_file,
+ state->index_file,
+ LOCK_DIE_ON_ERROR);
+ else
+ state->newfd = hold_locked_index(state->lock_file, 1);
+ }
- if (state->check_index && read_cache() < 0) {
+ if (state->check_index && read_apply_cache(state) < 0) {
error(_("unable to read index file"));
res = -128;
goto end;
diff --git a/apply.h b/apply.h
index 9fec536..b3d6783 100644
--- a/apply.h
+++ b/apply.h
@@ -63,6 +63,7 @@ struct apply_state {
int unsafe_paths;
/* Other non boolean parameters */
+ const char *index_file;
enum apply_verbosity apply_verbosity;
const char *fake_ancestor;
const char *patch_input_file;
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 41/41] builtin/am: use apply API in run_apply()
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
This 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 | 65 ++++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 43 insertions(+), 22 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 739b34d..38d5d11 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.
@@ -1522,39 +1523,59 @@ 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;
-
- cp.git_cmd = 1;
-
- if (index_file)
- argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", index_file);
+ 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;
+ static struct lock_file lock_file;
+ int force_apply = 0;
+ int options = 0;
+
+ if (init_apply_state(&apply_state, NULL, &lock_file))
+ die("BUG: init_apply_state() failed");
+
+ argv_array_push(&apply_opts, "apply");
+ argv_array_pushv(&apply_opts, state->git_apply_opts.argv);
+
+ opts_left = apply_parse_options(apply_opts.argc, apply_opts.argv,
+ &apply_state, &force_apply, &options,
+ NULL);
+
+ if (opts_left != 0)
+ die("unknown option passed through to git apply");
+
+ if (index_file) {
+ apply_state.index_file = 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.apply_verbosity = verbosity_silent;
- argv_array_push(&cp.args, "apply");
+ if (check_apply_state(&apply_state, force_apply))
+ die("BUG: check_apply_state() failed");
- argv_array_pushv(&cp.args, state->git_apply_opts.argv);
+ argv_array_push(&apply_paths, am_path(state, "patch"));
- if (index_file)
- argv_array_push(&cp.args, "--cached");
- else
- argv_array_push(&cp.args, "--index");
+ res = apply_all_patches(&apply_state, apply_paths.argc, apply_paths.argv, options);
- 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.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 35/41] usage: add set_warn_routine()
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
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 db89ba7..d189b4f 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.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 26/41] builtin/apply: make try_create_file() return -1 on error
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
To libify `git apply` functionality we have to signal errors to the
caller instead of 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>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 44 +++++++++++++++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index c787ead..3145e03 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4150,38 +4150,48 @@ 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_in_full(fd, buf, size) < 0;
+ if (res)
+ error_errno(_("failed to write to '%s'"), path);
strbuf_release(&nbuf);
- if (close(fd) < 0)
- die_errno(_("closing file '%s'"), path);
- return 0;
+ if (close(fd) < 0 && !res)
+ return error_errno(_("closing file '%s'"), path);
+
+ return res ? -1 : 0;
}
/*
@@ -4195,15 +4205,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(128);
+ 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(128);
+ if (!res)
return;
}
@@ -4222,7 +4241,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(128);
+ if (!res) {
if (!rename(newpath, path))
return;
unlink_or_warn(newpath);
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 34/41] apply: don't print on stdout in verbosity_silent mode
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
When apply_verbosity is set to verbosity_silent nothing should be
printed on both stderr and stdout.
To avoid printing on stdout, we can just skip calling the following
functions:
- stat_patch_list(),
- numstat_patch_list(),
- summary_patch_list().
It is safe to do that because the above functions have no side
effects other than printing:
- stat_patch_list() only computes some local values and then call
show_stats() and print_stat_summary(), those two functions only
compute local values and call printing functions,
- numstat_patch_list() also only computes local values and calls
printing functions,
- summary_patch_list() calls show_file_mode_name(), printf(),
show_rename_copy(), show_mode_change() that are only printing.
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 df85cbc..ddbb0a2 100644
--- a/apply.c
+++ b/apply.c
@@ -4702,13 +4702,13 @@ static int apply_patch(struct apply_state *state,
goto end;
}
- if (state->diffstat)
+ if (state->diffstat && state->apply_verbosity > verbosity_silent)
stat_patch_list(state, list);
- if (state->numstat)
+ if (state->numstat && state->apply_verbosity > verbosity_silent)
numstat_patch_list(state, list);
- if (state->summary)
+ if (state->summary && state->apply_verbosity > verbosity_silent)
summary_patch_list(list);
end:
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 33/41] apply: make it possible to silently apply
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
This changes 'int apply_verbosely' into 'enum apply_verbosity', and
changes the possible values of the variable from a bool to
a tristate.
The previous 'false' state is changed into 'verbosity_normal'.
The previous 'true' state is changed into 'verbosity_verbose'.
The new added state is 'verbosity_silent'. It should prevent
anything to be printed on both stderr and stdout.
This is needed because `git am` wants to first call apply
functionality silently, if it can then fall back on 3-way merge
in case of error.
Printing on stdout, and calls to warning() or error() are not
taken care of in this patch, as that will be done in following
patches.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
apply.c | 62 +++++++++++++++++++++++++++++++++++++--------------------
apply.h | 8 +++++++-
builtin/apply.c | 2 +-
3 files changed, 48 insertions(+), 24 deletions(-)
diff --git a/apply.c b/apply.c
index 41a33d3..df85cbc 100644
--- a/apply.c
+++ b/apply.c
@@ -125,8 +125,11 @@ int check_apply_state(struct apply_state *state, int force_apply)
return error(_("--3way outside a repository"));
state->check_index = 1;
}
- if (state->apply_with_reject)
- state->apply = state->apply_verbosely = 1;
+ if (state->apply_with_reject) {
+ state->apply = 1;
+ if (state->apply_verbosity == verbosity_normal)
+ state->apply_verbosity = verbosity_verbose;
+ }
if (!force_apply && (state->diffstat || state->numstat || state->summary || state->check || state->fake_ancestor))
state->apply = 0;
if (state->check_index && is_not_gitdir)
@@ -1620,8 +1623,9 @@ static void record_ws_error(struct apply_state *state,
return;
err = whitespace_error_string(result);
- fprintf(stderr, "%s:%d: %s.\n%.*s\n",
- state->patch_input_file, linenr, err, len, line);
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf(stderr, "%s:%d: %s.\n%.*s\n",
+ state->patch_input_file, linenr, err, len, line);
free(err);
}
@@ -1816,7 +1820,7 @@ static int parse_single_patch(struct apply_state *state,
return error(_("new file %s depends on old contents"), patch->new_name);
if (0 < patch->is_delete && newlines)
return error(_("deleted file %s still has contents"), patch->old_name);
- if (!patch->is_delete && !newlines && context)
+ if (!patch->is_delete && !newlines && context && state->apply_verbosity > verbosity_silent)
fprintf_ln(stderr,
_("** warning: "
"file %s becomes empty but is not deleted"),
@@ -2911,7 +2915,7 @@ static int apply_one_fragment(struct apply_state *state,
/* Ignore it, we already handled it */
break;
default:
- if (state->apply_verbosely)
+ if (state->apply_verbosity > verbosity_normal)
error(_("invalid start of line: '%c'"), first);
applied_pos = -1;
goto out;
@@ -3026,7 +3030,7 @@ static int apply_one_fragment(struct apply_state *state,
state->apply = 0;
}
- if (state->apply_verbosely && applied_pos != pos) {
+ if (state->apply_verbosity > verbosity_normal && applied_pos != pos) {
int offset = applied_pos - pos;
if (state->apply_in_reverse)
offset = 0 - offset;
@@ -3041,14 +3045,14 @@ static int apply_one_fragment(struct apply_state *state,
* Warn if it was necessary to reduce the number
* of context lines.
*/
- if ((leading != frag->leading) ||
- (trailing != frag->trailing))
+ if ((leading != frag->leading ||
+ trailing != frag->trailing) && state->apply_verbosity > verbosity_silent)
fprintf_ln(stderr, _("Context reduced to (%ld/%ld)"
" to apply fragment at %d"),
leading, trailing, applied_pos+1);
update_image(state, img, applied_pos, &preimage, &postimage);
} else {
- if (state->apply_verbosely)
+ if (state->apply_verbosity > verbosity_normal)
error(_("while searching for:\n%.*s"),
(int)(old - oldlines), oldlines);
}
@@ -3539,7 +3543,8 @@ static int try_threeway(struct apply_state *state,
read_blob_object(&buf, pre_sha1, patch->old_mode))
return error("repository lacks the necessary blob to fall back on 3-way merge.");
- fprintf(stderr, "Falling back to three-way merge...\n");
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf(stderr, "Falling back to three-way merge...\n");
img = strbuf_detach(&buf, &len);
prepare_image(&tmp_image, img, len, 1);
@@ -3569,7 +3574,9 @@ static int try_threeway(struct apply_state *state,
status = three_way_merge(image, patch->new_name,
pre_sha1, our_sha1, post_sha1);
if (status < 0) {
- fprintf(stderr, "Failed to fall back on three-way merge...\n");
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf(stderr,
+ "Failed to fall back on three-way merge...\n");
return status;
}
@@ -3581,9 +3588,15 @@ static int try_threeway(struct apply_state *state,
hashcpy(patch->threeway_stage[0].hash, pre_sha1);
hashcpy(patch->threeway_stage[1].hash, our_sha1);
hashcpy(patch->threeway_stage[2].hash, post_sha1);
- fprintf(stderr, "Applied patch to '%s' with conflicts.\n", patch->new_name);
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf(stderr,
+ "Applied patch to '%s' with conflicts.\n",
+ patch->new_name);
} else {
- fprintf(stderr, "Applied patch to '%s' cleanly.\n", patch->new_name);
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf(stderr,
+ "Applied patch to '%s' cleanly.\n",
+ patch->new_name);
}
return 0;
}
@@ -3956,7 +3969,7 @@ static int check_patch_list(struct apply_state *state, struct patch *patch)
prepare_fn_table(state, patch);
while (patch) {
int res;
- if (state->apply_verbosely)
+ if (state->apply_verbosity > verbosity_normal)
say_patch_name(stderr,
_("Checking patch %s..."), patch);
res = check_patch(state, patch);
@@ -4472,7 +4485,7 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
}
if (!cnt) {
- if (state->apply_verbosely)
+ if (state->apply_verbosity > verbosity_normal)
say_patch_name(stderr,
_("Applied patch %s cleanly."), patch);
return 0;
@@ -4489,7 +4502,8 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
"Applying patch %%s with %d rejects...",
cnt),
cnt);
- say_patch_name(stderr, sb.buf, patch);
+ if (state->apply_verbosity > verbosity_silent)
+ say_patch_name(stderr, sb.buf, patch);
strbuf_release(&sb);
cnt = strlen(patch->new_name);
@@ -4516,10 +4530,12 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
frag;
cnt++, frag = frag->next) {
if (!frag->rejected) {
- fprintf_ln(stderr, _("Hunk #%d applied cleanly."), cnt);
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf_ln(stderr, _("Hunk #%d applied cleanly."), cnt);
continue;
}
- fprintf_ln(stderr, _("Rejected hunk #%d."), cnt);
+ if (state->apply_verbosity > verbosity_silent)
+ fprintf_ln(stderr, _("Rejected hunk #%d."), cnt);
fprintf(rej, "%.*s", frag->size, frag->patch);
if (frag->patch[frag->size-1] != '\n')
fputc('\n', rej);
@@ -4568,8 +4584,10 @@ static int write_out_results(struct apply_state *state, struct patch *list)
struct string_list_item *item;
string_list_sort(&cpath);
- for_each_string_list_item(item, &cpath)
- fprintf(stderr, "U %s\n", item->string);
+ if (state->apply_verbosity > verbosity_silent) {
+ for_each_string_list_item(item, &cpath)
+ fprintf(stderr, "U %s\n", item->string);
+ }
string_list_clear(&cpath, 0);
rerere(0);
@@ -4626,7 +4644,7 @@ static int apply_patch(struct apply_state *state,
listp = &patch->next;
}
else {
- if (state->apply_verbosely)
+ if (state->apply_verbosity > verbosity_normal)
say_patch_name(stderr, _("Skipped patch '%s'."), patch);
free_patch(patch);
skipped_patch++;
diff --git a/apply.h b/apply.h
index 51b983b..f015403 100644
--- a/apply.h
+++ b/apply.h
@@ -13,6 +13,12 @@ enum apply_ws_ignore {
ignore_ws_change
};
+enum apply_verbosity {
+ verbosity_silent = -1,
+ verbosity_normal = 0,
+ verbosity_verbose = 1
+};
+
/*
* We need to keep track of how symlinks in the preimage are
* manipulated by the patches. A patch to add a/b/c where a/b
@@ -51,13 +57,13 @@ struct apply_state {
int allow_overlap;
int apply_in_reverse;
int apply_with_reject;
- int apply_verbosely;
int no_add;
int threeway;
int unidiff_zero;
int unsafe_paths;
/* Other non boolean parameters */
+ enum apply_verbosity apply_verbosity;
const char *fake_ancestor;
const char *patch_input_file;
int line_termination;
diff --git a/builtin/apply.c b/builtin/apply.c
index 9c66474..7338701 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -74,7 +74,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
N_("leave the rejected hunks in corresponding *.rej files")),
OPT_BOOL(0, "allow-overlap", &state.allow_overlap,
N_("allow overlapping hunks")),
- OPT__VERBOSE(&state.apply_verbosely, N_("be verbose")),
+ OPT__VERBOSE(&state.apply_verbosity, N_("be verbose")),
OPT_BIT(0, "inaccurate-eof", &options,
N_("tolerate incorrectly detected missing new-line at the end of file"),
APPLY_OPT_INACCURATE_EOF),
--
2.10.0.41.g9df52c3
^ permalink raw reply related
* [PATCH v14 28/41] builtin/apply: rename option parsing functions
From: Christian Couder @ 2016-09-04 20:18 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Karsten Blees, Nguyen Thai Ngoc Duy, Stefan Beller, Eric Sunshine,
Ramsay Jones, Johannes Sixt, René Scharfe, Stefan Naewe,
Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
As these functions are going to be part of the libified
apply API, let's give them a name that is more specific
to the apply API.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/apply.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 5cd7fd8..1f56303 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4588,16 +4588,16 @@ static int apply_patch(struct apply_state *state,
return res;
}
-static int option_parse_exclude(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_exclude(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
add_name_limit(state, arg, 1);
return 0;
}
-static int option_parse_include(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_include(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
add_name_limit(state, arg, 0);
@@ -4605,9 +4605,9 @@ static int option_parse_include(const struct option *opt,
return 0;
}
-static int option_parse_p(const struct option *opt,
- const char *arg,
- int unset)
+static int apply_option_parse_p(const struct option *opt,
+ const char *arg,
+ int unset)
{
struct apply_state *state = opt->value;
state->p_value = atoi(arg);
@@ -4615,8 +4615,8 @@ static int option_parse_p(const struct option *opt,
return 0;
}
-static int option_parse_space_change(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_space_change(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
if (unset)
@@ -4626,8 +4626,8 @@ static int option_parse_space_change(const struct option *opt,
return 0;
}
-static int option_parse_whitespace(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_whitespace(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
state->whitespace_option = arg;
@@ -4636,8 +4636,8 @@ static int option_parse_whitespace(const struct option *opt,
return 0;
}
-static int option_parse_directory(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_directory(const struct option *opt,
+ const char *arg, int unset)
{
struct apply_state *state = opt->value;
strbuf_reset(&state->root);
@@ -4755,13 +4755,13 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
struct option builtin_apply_options[] = {
{ OPTION_CALLBACK, 0, "exclude", &state, N_("path"),
N_("don't apply changes matching the given path"),
- 0, option_parse_exclude },
+ 0, apply_option_parse_exclude },
{ OPTION_CALLBACK, 0, "include", &state, N_("path"),
N_("apply changes matching the given path"),
- 0, option_parse_include },
+ 0, apply_option_parse_include },
{ OPTION_CALLBACK, 'p', NULL, &state, N_("num"),
N_("remove <num> leading slashes from traditional diff paths"),
- 0, option_parse_p },
+ 0, apply_option_parse_p },
OPT_BOOL(0, "no-add", &state.no_add,
N_("ignore additions made by the patch")),
OPT_BOOL(0, "stat", &state.diffstat,
@@ -4793,13 +4793,13 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
N_("ensure at least <n> lines of context match")),
{ OPTION_CALLBACK, 0, "whitespace", &state, N_("action"),
N_("detect new or modified lines that have whitespace errors"),
- 0, option_parse_whitespace },
+ 0, apply_option_parse_whitespace },
{ OPTION_CALLBACK, 0, "ignore-space-change", &state, NULL,
N_("ignore changes in whitespace when finding context"),
- PARSE_OPT_NOARG, option_parse_space_change },
+ PARSE_OPT_NOARG, apply_option_parse_space_change },
{ OPTION_CALLBACK, 0, "ignore-whitespace", &state, NULL,
N_("ignore changes in whitespace when finding context"),
- PARSE_OPT_NOARG, option_parse_space_change },
+ PARSE_OPT_NOARG, apply_option_parse_space_change },
OPT_BOOL('R', "reverse", &state.apply_in_reverse,
N_("apply the patch in reverse")),
OPT_BOOL(0, "unidiff-zero", &state.unidiff_zero,
@@ -4817,7 +4817,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
RECOUNT),
{ OPTION_CALLBACK, 0, "directory", &state, N_("root"),
N_("prepend <root> to all filenames"),
- 0, option_parse_directory },
+ 0, apply_option_parse_directory },
OPT_END()
};
--
2.10.0.41.g9df52c3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox