* [PATCH] parse-opt: migrate builtin-apply.
@ 2008-12-27 4:22 Miklos Vajna
2008-12-27 7:29 ` Junio C Hamano
0 siblings, 1 reply; 10+ messages in thread
From: Miklos Vajna @ 2008-12-27 4:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The only notable user-visible/incompatible change is that the
--build-fake-ancestor option now conforms to gitcli(7).
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
I know that we do care about incompatible changes a lot, though I think
this is the right direction and probably --build-fake-ancestor is not a
heavily used switch, so I hope that part is OK.
Documentation/git-apply.txt | 4 +-
builtin-apply.c | 360 ++++++++++++++++++++++++++++---------------
2 files changed, 234 insertions(+), 130 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index e726510..9400f6a 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[verse]
'git apply' [--stat] [--numstat] [--summary] [--check] [--index]
- [--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --reverse]
+ [--apply] [--no-add] [--build-fake-ancestor=<file>] [-R | --reverse]
[--allow-binary-replacement | --binary] [--reject] [-z]
[-pNUM] [-CNUM] [--inaccurate-eof] [--recount] [--cached]
[--whitespace=<nowarn|warn|fix|error|error-all>]
@@ -64,7 +64,7 @@ OPTIONS
cached data, apply the patch, and store the result in the index,
without using the working tree. This implies '--index'.
---build-fake-ancestor <file>::
+--build-fake-ancestor=<file>::
Newer 'git-diff' output has embedded 'index information'
for each blob to help identify the original version that
the patch applies to. When this flag is given, and if
diff --git a/builtin-apply.c b/builtin-apply.c
index 07244b0..c2a587f 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -14,6 +14,7 @@
#include "builtin.h"
#include "string-list.h"
#include "dir.h"
+#include "parse-options.h"
/*
* --check turns on checking that the working tree matches the
@@ -46,8 +47,10 @@ static int no_add;
static const char *fake_ancestor;
static int line_termination = '\n';
static unsigned long p_context = ULONG_MAX;
-static const char apply_usage[] =
-"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
+static const char * const apply_usage[] = {
+ "git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...",
+ NULL
+};
static enum ws_error_action {
nowarn_ws_error,
@@ -61,6 +64,8 @@ static int applied_after_fixing_ws;
static const char *patch_input_file;
static const char *root;
static int root_len;
+static int read_stdin = 1;
+static int options;
static void parse_whitespace_option(const char *option)
{
@@ -3135,150 +3140,249 @@ static int git_apply_config(const char *var, const char *value, void *cb)
return git_default_config(var, value, cb);
}
+static int option_parse_stdin(const struct option *opt,
+ const char *arg, int unset)
+{
+ int *errs = opt->value;
+
+ *errs |= apply_patch(0, "<stdin>", options);
+ read_stdin = 0;
+ return 0;
+}
+
+static int option_parse_exclude(const struct option *opt,
+ const char *arg, int unset)
+{
+ add_name_limit(arg, 1);
+ return 0;
+}
+
+static int option_parse_include(const struct option *opt,
+ const char *arg, int unset)
+{
+ add_name_limit(arg, 0);
+ has_include = 1;
+ return 0;
+}
+
+static int option_parse_p(const struct option *opt,
+ const char *arg, int unset)
+{
+ p_value = atoi(arg);
+ p_value_known = 1;
+ return 0;
+}
+
+static int option_parse_stat(const struct option *opt,
+ const char *arg, int unset)
+{
+ apply = 0;
+ diffstat = 1;
+ return 0;
+}
+
+static int option_parse_numstat(const struct option *opt,
+ const char *arg, int unset)
+{
+ apply = 0;
+ numstat = 1;
+ return 0;
+}
+
+static int option_parse_summary(const struct option *opt,
+ const char *arg, int unset)
+{
+ apply = 0;
+ summary = 1;
+ return 0;
+}
+
+static int option_parse_check(const struct option *opt,
+ const char *arg, int unset)
+{
+ apply = 0;
+ check = 1;
+ return 0;
+}
+
+static int option_parse_index(const struct option *opt,
+ const char *arg, int unset)
+{
+ int *is_not_gitdir = opt->value;
+
+ if (*is_not_gitdir)
+ die("--index outside a repository");
+ check_index = 1;
+ return 0;
+}
+
+static int option_parse_cached(const struct option *opt,
+ const char *arg, int unset)
+{
+ int *is_not_gitdir = opt->value;
+
+ if (*is_not_gitdir)
+ die("--cached outside a repository");
+ check_index = 1;
+ cached = 1;
+ return 0;
+}
+
+static int option_parse_ancestor(const struct option *opt,
+ const char *arg, int unset)
+{
+ apply = 0;
+ fake_ancestor = arg;
+ return 0;
+}
+
+static int option_parse_z(const struct option *opt,
+ const char *arg, int unset)
+{
+ if (unset)
+ line_termination = '\n';
+ else
+ line_termination = 0;
+ return 0;
+}
+
+static int option_parse_whitespace(const struct option *opt,
+ const char *arg, int unset)
+{
+ const char **whitespace_option = opt->value;
+
+ *whitespace_option = arg;
+ parse_whitespace_option(arg);
+ return 0;
+}
+
+static int option_parse_reject(const struct option *opt,
+ const char *arg, int unset)
+{
+ apply = apply_with_reject = apply_verbosely = 1;
+ return 0;
+}
+
+static int option_parse_inaccurate(const struct option *opt,
+ const char *arg, int unset)
+{
+ options |= INACCURATE_EOF;
+ return 0;
+}
+
+static int option_parse_recount(const struct option *opt,
+ const char *arg, int unset)
+{
+ options |= RECOUNT;
+ return 0;
+}
+
+static int option_parse_directory(const struct option *opt,
+ const char *arg, int unset)
+{
+ root_len = strlen(arg);
+ if (root_len && arg[root_len - 1] != '/') {
+ char *new_root;
+ root = new_root = xmalloc(root_len + 2);
+ strcpy(new_root, arg);
+ strcpy(new_root + root_len++, "/");
+ } else
+ root = arg;
+ return 0;
+}
int cmd_apply(int argc, const char **argv, const char *unused_prefix)
{
int i;
- int read_stdin = 1;
- int options = 0;
int errs = 0;
int is_not_gitdir;
+ int binary;
const char *whitespace_option = NULL;
+ struct option builtin_apply_options[] = {
+ { OPTION_CALLBACK, '-', NULL, &errs, NULL,
+ "read the patch from the standard input",
+ PARSE_OPT_NOARG, option_parse_stdin },
+ { OPTION_CALLBACK, 0, "exclude", NULL, "path",
+ "don´t apply changes matching the given path",
+ 0, option_parse_exclude },
+ { OPTION_CALLBACK, 0, "include", NULL, "path",
+ "apply changes matching the given path",
+ 0, option_parse_include },
+ { OPTION_CALLBACK, 'p', NULL, NULL, "num",
+ "remove <num> leading slashes from traditional diff paths",
+ 0, option_parse_p },
+ OPT_BOOLEAN(0, "no-add", &no_add,
+ "ignore additions made by the patch"),
+ { OPTION_CALLBACK, 0, "stat", NULL, NULL,
+ "instead of applying the patch, output diffstat for the input",
+ PARSE_OPT_NOARG, option_parse_stat },
+ OPT_BOOLEAN(0, "allow-binary-replacement", &binary,
+ "now no-op"),
+ OPT_BOOLEAN(0, "binary", &binary,
+ "now no-op"),
+ { OPTION_CALLBACK, 0, "numstat", NULL, NULL,
+ "shows number of added and deleted lines in decimal notation",
+ PARSE_OPT_NOARG, option_parse_numstat },
+ { OPTION_CALLBACK, 0, "summary", NULL, NULL,
+ "instead of applying the patch, output a summary for the input",
+ PARSE_OPT_NOARG, option_parse_summary },
+ { OPTION_CALLBACK, 0, "check", NULL, NULL,
+ "instead of applying the patch, see if the patch is applicable",
+ PARSE_OPT_NOARG, option_parse_check },
+ { OPTION_CALLBACK, 0, "index", &is_not_gitdir, NULL,
+ "make sure the patch is applicable to the current index",
+ PARSE_OPT_NOARG, option_parse_index },
+ { OPTION_CALLBACK, 0, "cached", &is_not_gitdir, NULL,
+ "apply a patch without touching the working tree",
+ PARSE_OPT_NOARG, option_parse_cached },
+ OPT_BOOLEAN(0, "apply", &apply,
+ "also apply the patch (use with --stat/--summary/--check)"),
+ { OPTION_CALLBACK, 0, "build-fake-ancestor", NULL, "file",
+ "build a temporary index based on embedded index information",
+ 0, option_parse_ancestor },
+ { OPTION_CALLBACK, 'z', NULL, NULL, NULL,
+ "paths are separated with NUL character",
+ PARSE_OPT_NOARG, option_parse_z },
+ OPT_INTEGER('C', NULL, &p_context,
+ "ensure at least <n> lines of context match"),
+ { OPTION_CALLBACK, 0, "whitespace", &whitespace_option, "action",
+ "detect new or modified lines that have whitespace errors",
+ 0, option_parse_whitespace },
+ OPT_BOOLEAN('R', "reverse", &apply_in_reverse,
+ "apply the patch in reverse"),
+ OPT_BOOLEAN(0, "unidiff-zero", &unidiff_zero,
+ "don't expect at least one line of context"),
+ { OPTION_CALLBACK, 0, "reject", NULL, NULL,
+ "leave the rejected hunks in corresponding *.rej files",
+ PARSE_OPT_NOARG, option_parse_reject },
+ OPT__VERBOSE(&apply_verbosely),
+ { OPTION_CALLBACK, 0, "inaccurate-eof", NULL, NULL,
+ "tolerate incorrectly detected missing new-line at the end of file",
+ PARSE_OPT_NOARG, option_parse_inaccurate },
+ { OPTION_CALLBACK, 0, "recount", NULL, NULL,
+ "do not trust the line counts in the hunk headers",
+ PARSE_OPT_NOARG, option_parse_recount },
+ { OPTION_CALLBACK, 0, "directory", NULL, "root",
+ "prepend <root> to all filenames",
+ 0, option_parse_directory },
+ OPT_END()
+ };
+
prefix = setup_git_directory_gently(&is_not_gitdir);
prefix_length = prefix ? strlen(prefix) : 0;
git_config(git_apply_config, NULL);
if (apply_default_whitespace)
parse_whitespace_option(apply_default_whitespace);
- for (i = 1; i < argc; i++) {
+ argc = parse_options(argc, argv, builtin_apply_options,
+ apply_usage, 0);
+
+ for (i = 0; i < argc; i++) {
const char *arg = argv[i];
- char *end;
int fd;
- if (!strcmp(arg, "-")) {
- errs |= apply_patch(0, "<stdin>", options);
- read_stdin = 0;
- continue;
- }
- if (!prefixcmp(arg, "--exclude=")) {
- add_name_limit(arg + 10, 1);
- continue;
- }
- if (!prefixcmp(arg, "--include=")) {
- add_name_limit(arg + 10, 0);
- has_include = 1;
- continue;
- }
- if (!prefixcmp(arg, "-p")) {
- p_value = atoi(arg + 2);
- p_value_known = 1;
- continue;
- }
- if (!strcmp(arg, "--no-add")) {
- no_add = 1;
- continue;
- }
- if (!strcmp(arg, "--stat")) {
- apply = 0;
- diffstat = 1;
- continue;
- }
- if (!strcmp(arg, "--allow-binary-replacement") ||
- !strcmp(arg, "--binary")) {
- continue; /* now no-op */
- }
- if (!strcmp(arg, "--numstat")) {
- apply = 0;
- numstat = 1;
- continue;
- }
- if (!strcmp(arg, "--summary")) {
- apply = 0;
- summary = 1;
- continue;
- }
- if (!strcmp(arg, "--check")) {
- apply = 0;
- check = 1;
- continue;
- }
- if (!strcmp(arg, "--index")) {
- if (is_not_gitdir)
- die("--index outside a repository");
- check_index = 1;
- continue;
- }
- if (!strcmp(arg, "--cached")) {
- if (is_not_gitdir)
- die("--cached outside a repository");
- check_index = 1;
- cached = 1;
- continue;
- }
- if (!strcmp(arg, "--apply")) {
- apply = 1;
- continue;
- }
- if (!strcmp(arg, "--build-fake-ancestor")) {
- apply = 0;
- if (++i >= argc)
- die ("need a filename");
- fake_ancestor = argv[i];
- continue;
- }
- if (!strcmp(arg, "-z")) {
- line_termination = 0;
- continue;
- }
- if (!prefixcmp(arg, "-C")) {
- p_context = strtoul(arg + 2, &end, 0);
- if (*end != '\0')
- die("unrecognized context count '%s'", arg + 2);
- continue;
- }
- if (!prefixcmp(arg, "--whitespace=")) {
- whitespace_option = arg + 13;
- parse_whitespace_option(arg + 13);
- continue;
- }
- if (!strcmp(arg, "-R") || !strcmp(arg, "--reverse")) {
- apply_in_reverse = 1;
- continue;
- }
- if (!strcmp(arg, "--unidiff-zero")) {
- unidiff_zero = 1;
- continue;
- }
- if (!strcmp(arg, "--reject")) {
- apply = apply_with_reject = apply_verbosely = 1;
- continue;
- }
- if (!strcmp(arg, "-v") || !strcmp(arg, "--verbose")) {
- apply_verbosely = 1;
- continue;
- }
- if (!strcmp(arg, "--inaccurate-eof")) {
- options |= INACCURATE_EOF;
- continue;
- }
- if (!strcmp(arg, "--recount")) {
- options |= RECOUNT;
- continue;
- }
- if (!prefixcmp(arg, "--directory=")) {
- arg += strlen("--directory=");
- root_len = strlen(arg);
- if (root_len && arg[root_len - 1] != '/') {
- char *new_root;
- root = new_root = xmalloc(root_len + 2);
- strcpy(new_root, arg);
- strcpy(new_root + root_len++, "/");
- } else
- root = arg;
- continue;
- }
if (0 < prefix_length)
arg = prefix_filename(prefix, prefix_length, arg);
--
1.6.1.rc1.35.gae26e.dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] parse-opt: migrate builtin-apply.
2008-12-27 4:22 [PATCH] parse-opt: migrate builtin-apply Miklos Vajna
@ 2008-12-27 7:29 ` Junio C Hamano
2008-12-27 14:05 ` Miklos Vajna
0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2008-12-27 7:29 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
Miklos Vajna <vmiklos@frugalware.org> writes:
> The only notable user-visible/incompatible change is that the
> --build-fake-ancestor option now conforms to gitcli(7).
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> I know that we do care about incompatible changes a lot, though I think
> this is the right direction and probably --build-fake-ancestor is not a
> heavily used switch, so I hope that part is OK.
An acceptable justification for such a plumbing change is if (1) the old
syntax is still supported the same way as the original implementation,
*and* if (2) the new syntax is something that could not possibly have been
a valid input to the original implementation with a different meaning.
I think the condition (1) holds but (2) does not hold for your patch; even
though I think the latter breakage is excusable in this particular case,
it is not for the reason you cited.
That is,
(1) The parseopt parser allows both of these forms:
$ git apply --build-fake-ancestor file <input
$ git apply --build-fake-ancestor=file <input
The former has been how existing scripts that use the plumbing have
been feeding the file, and it is still supported.
(2) A script that used "git apply" and relied on the behaviour of
the original implementation could have fed a patch from a file
whose name is "--build-fake-ancestor=some-string", with this command
line:
$ git apply --build-fake-ancestor=file
Now such a script would break with the new parser.
The reason you are excused to break such an insane script is definitely
not because --build-fake-ancestor is a rarely used option. The whole
defence depends on the fact that --build-fake-ancestor=something is a very
unlikely name for any sane script to be using for its temporary file. It
could still be an end user input, but at that point you could simply doubt
the sanity of the end user and dismiss the issue away.
I am not fundamentally opposed to using parseopt in git-apply, and I think
the change to add a new and saner meaning to "--build-fake-ancestor=file"
on the command line is a good thing in the longer term. But your
justification for such a change should be given in such a way to show
clearly that you have thought things through. It has to be much better
than "it is not a heavily used switch anyway".
The saddest part of the story that pisses me off about this patch is that
you did not seem to have even run the test suite before sending it. t4105
and t4252 fail for me, at least.
I did not look at the patch very closely, but do you really need that many
option callbacks? My gut feeling is that many of them should be just
setting a boolean flag, and you can postprocess to get the correct "apply"
behaviour.
For example, you start with "apply" set to true, and let parseopt set
"diffstat" upon seeing "--stat", and set "cmdline_apply" upon seeing
"--apply". After parseopt returns, you determine the final value of
"apply" by using "diffstat" (and friends that would normally drop "apply")
and "cmdline_apply" (which would override such droppages). That way I
think you can lose many callback functions whose sole purpose is to drop
"apply" option, no?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] parse-opt: migrate builtin-apply.
2008-12-27 7:29 ` Junio C Hamano
@ 2008-12-27 14:05 ` Miklos Vajna
2008-12-27 14:22 ` [PATCH v2] " Miklos Vajna
0 siblings, 1 reply; 10+ messages in thread
From: Miklos Vajna @ 2008-12-27 14:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 4579 bytes --]
On Fri, Dec 26, 2008 at 11:29:42PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> Miklos Vajna <vmiklos@frugalware.org> writes:
>
> > The only notable user-visible/incompatible change is that the
> > --build-fake-ancestor option now conforms to gitcli(7).
> >
> > Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> > ---
> >
> > I know that we do care about incompatible changes a lot, though I think
> > this is the right direction and probably --build-fake-ancestor is not a
> > heavily used switch, so I hope that part is OK.
>
> An acceptable justification for such a plumbing change is if (1) the old
> syntax is still supported the same way as the original implementation,
> *and* if (2) the new syntax is something that could not possibly have been
> a valid input to the original implementation with a different meaning.
>
> I think the condition (1) holds but (2) does not hold for your patch; even
> though I think the latter breakage is excusable in this particular case,
> it is not for the reason you cited.
>
> That is,
>
> (1) The parseopt parser allows both of these forms:
>
> $ git apply --build-fake-ancestor file <input
> $ git apply --build-fake-ancestor=file <input
>
> The former has been how existing scripts that use the plumbing have
> been feeding the file, and it is still supported.
>
> (2) A script that used "git apply" and relied on the behaviour of
> the original implementation could have fed a patch from a file
> whose name is "--build-fake-ancestor=some-string", with this command
> line:
>
> $ git apply --build-fake-ancestor=file
>
> Now such a script would break with the new parser.
>
> The reason you are excused to break such an insane script is definitely
> not because --build-fake-ancestor is a rarely used option. The whole
> defence depends on the fact that --build-fake-ancestor=something is a very
> unlikely name for any sane script to be using for its temporary file. It
> could still be an end user input, but at that point you could simply doubt
> the sanity of the end user and dismiss the issue away.
>
> I am not fundamentally opposed to using parseopt in git-apply, and I think
> the change to add a new and saner meaning to "--build-fake-ancestor=file"
> on the command line is a good thing in the longer term. But your
> justification for such a change should be given in such a way to show
> clearly that you have thought things through. It has to be much better
> than "it is not a heavily used switch anyway".
I was not aware about parsepont allows both options, I just -
incorrectly - thought git-log uses paseopt and there I remember
--since=foo works, but not --since foo. So actually the commit message
is incorrect, the backwards-incompatible change is not to accept a patch
file named --build-fake-ancestor=something without passing '--' first.
> The saddest part of the story that pisses me off about this patch is that
> you did not seem to have even run the test suite before sending it. t4105
> and t4252 fail for me, at least.
Hm, I did:
$ ./t4105-apply-fuzz.sh
* ok 1: setup
* ok 2: unmodified patch
* ok 3: minus offset
* ok 4: plus offset
* ok 5: big offset
* ok 6: fuzz with no offset
* ok 7: fuzz with minus offset
* ok 8: fuzz with plus offset
* ok 9: fuzz with big offset
* passed all 9 test(s)
$ ./t4252-am-options.sh
* ok 1: setup
* ok 2: interrupted am --whitespace=fix
* ok 3: interrupted am -C1
* ok 4: interrupted am -p2
* ok 5: interrupted am -C1 -p2
* passed all 5 test(s)
$ git show -s --pretty=oneline
05d26caf212b58998b7e559991f3a25fd8cbf3f0 parse-opt: migrate builtin-apply.
What testcases did fail for you?
> I did not look at the patch very closely, but do you really need that many
> option callbacks? My gut feeling is that many of them should be just
> setting a boolean flag, and you can postprocess to get the correct "apply"
> behaviour.
>
> For example, you start with "apply" set to true, and let parseopt set
> "diffstat" upon seeing "--stat", and set "cmdline_apply" upon seeing
> "--apply". After parseopt returns, you determine the final value of
> "apply" by using "diffstat" (and friends that would normally drop "apply")
> and "cmdline_apply" (which would override such droppages). That way I
> think you can lose many callback functions whose sole purpose is to drop
> "apply" option, no?
Yes, you are right. I'll send an updated patch in a bit.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] parse-opt: migrate builtin-apply.
2008-12-27 14:05 ` Miklos Vajna
@ 2008-12-27 14:22 ` Miklos Vajna
2008-12-27 21:47 ` Junio C Hamano
2008-12-27 21:53 ` René Scharfe
0 siblings, 2 replies; 10+ messages in thread
From: Miklos Vajna @ 2008-12-27 14:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The only incompatible change is that the user how have to use '--'
before a patch named --build-fake-ancestor=something.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
Diffstat against v1:
builtin-apply.c | 125 +++++++++++++------------------------------------------
1 files changed, 29 insertions(+), 96 deletions(-)
due to the removal of option_parse_stat, option_parse_numstat,
option_parse_summary, option_parse_check, option_parse_index,
option_parse_cached, option_parse_ancestor and option_parse_reject.
Documentation/git-apply.txt | 4 +-
builtin-apply.c | 293 ++++++++++++++++++++++++-------------------
2 files changed, 167 insertions(+), 130 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index e726510..9400f6a 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[verse]
'git apply' [--stat] [--numstat] [--summary] [--check] [--index]
- [--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --reverse]
+ [--apply] [--no-add] [--build-fake-ancestor=<file>] [-R | --reverse]
[--allow-binary-replacement | --binary] [--reject] [-z]
[-pNUM] [-CNUM] [--inaccurate-eof] [--recount] [--cached]
[--whitespace=<nowarn|warn|fix|error|error-all>]
@@ -64,7 +64,7 @@ OPTIONS
cached data, apply the patch, and store the result in the index,
without using the working tree. This implies '--index'.
---build-fake-ancestor <file>::
+--build-fake-ancestor=<file>::
Newer 'git-diff' output has embedded 'index information'
for each blob to help identify the original version that
the patch applies to. When this flag is given, and if
diff --git a/builtin-apply.c b/builtin-apply.c
index 07244b0..98a988c 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -14,6 +14,7 @@
#include "builtin.h"
#include "string-list.h"
#include "dir.h"
+#include "parse-options.h"
/*
* --check turns on checking that the working tree matches the
@@ -46,8 +47,10 @@ static int no_add;
static const char *fake_ancestor;
static int line_termination = '\n';
static unsigned long p_context = ULONG_MAX;
-static const char apply_usage[] =
-"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
+static const char * const apply_usage[] = {
+ "git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...",
+ NULL
+};
static enum ws_error_action {
nowarn_ws_error,
@@ -61,6 +64,8 @@ static int applied_after_fixing_ws;
static const char *patch_input_file;
static const char *root;
static int root_len;
+static int read_stdin = 1;
+static int options;
static void parse_whitespace_option(const char *option)
{
@@ -3135,150 +3140,182 @@ static int git_apply_config(const char *var, const char *value, void *cb)
return git_default_config(var, value, cb);
}
+static int option_parse_stdin(const struct option *opt,
+ const char *arg, int unset)
+{
+ int *errs = opt->value;
+
+ *errs |= apply_patch(0, "<stdin>", options);
+ read_stdin = 0;
+ return 0;
+}
+
+static int option_parse_exclude(const struct option *opt,
+ const char *arg, int unset)
+{
+ add_name_limit(arg, 1);
+ return 0;
+}
+
+static int option_parse_include(const struct option *opt,
+ const char *arg, int unset)
+{
+ add_name_limit(arg, 0);
+ has_include = 1;
+ return 0;
+}
+
+static int option_parse_p(const struct option *opt,
+ const char *arg, int unset)
+{
+ p_value = atoi(arg);
+ p_value_known = 1;
+ return 0;
+}
+
+static int option_parse_z(const struct option *opt,
+ const char *arg, int unset)
+{
+ if (unset)
+ line_termination = '\n';
+ else
+ line_termination = 0;
+ return 0;
+}
+
+static int option_parse_whitespace(const struct option *opt,
+ const char *arg, int unset)
+{
+ const char **whitespace_option = opt->value;
+
+ *whitespace_option = arg;
+ parse_whitespace_option(arg);
+ return 0;
+}
+
+static int option_parse_inaccurate(const struct option *opt,
+ const char *arg, int unset)
+{
+ options |= INACCURATE_EOF;
+ return 0;
+}
+
+static int option_parse_recount(const struct option *opt,
+ const char *arg, int unset)
+{
+ options |= RECOUNT;
+ return 0;
+}
+
+static int option_parse_directory(const struct option *opt,
+ const char *arg, int unset)
+{
+ root_len = strlen(arg);
+ if (root_len && arg[root_len - 1] != '/') {
+ char *new_root;
+ root = new_root = xmalloc(root_len + 2);
+ strcpy(new_root, arg);
+ strcpy(new_root + root_len++, "/");
+ } else
+ root = arg;
+ return 0;
+}
int cmd_apply(int argc, const char **argv, const char *unused_prefix)
{
int i;
- int read_stdin = 1;
- int options = 0;
int errs = 0;
int is_not_gitdir;
+ int binary;
+ int force_apply = 0;
const char *whitespace_option = NULL;
+ struct option builtin_apply_options[] = {
+ { OPTION_CALLBACK, '-', NULL, &errs, NULL,
+ "read the patch from the standard input",
+ PARSE_OPT_NOARG, option_parse_stdin },
+ { OPTION_CALLBACK, 0, "exclude", NULL, "path",
+ "don´t apply changes matching the given path",
+ 0, option_parse_exclude },
+ { OPTION_CALLBACK, 0, "include", NULL, "path",
+ "apply changes matching the given path",
+ 0, option_parse_include },
+ { OPTION_CALLBACK, 'p', NULL, NULL, "num",
+ "remove <num> leading slashes from traditional diff paths",
+ 0, option_parse_p },
+ OPT_BOOLEAN(0, "no-add", &no_add,
+ "ignore additions made by the patch"),
+ OPT_BOOLEAN(0, "stat", &diffstat,
+ "instead of applying the patch, output diffstat for the input"),
+ OPT_BOOLEAN(0, "allow-binary-replacement", &binary,
+ "now no-op"),
+ OPT_BOOLEAN(0, "binary", &binary,
+ "now no-op"),
+ OPT_BOOLEAN(0, "numstat", &numstat,
+ "shows number of added and deleted lines in decimal notation"),
+ OPT_BOOLEAN(0, "summary", &summary,
+ "instead of applying the patch, output a summary for the input"),
+ OPT_BOOLEAN(0, "check", &check,
+ "instead of applying the patch, see if the patch is applicable"),
+ OPT_BOOLEAN(0, "index", &check_index,
+ "make sure the patch is applicable to the current index"),
+ OPT_BOOLEAN(0, "cached", &cached,
+ "apply a patch without touching the working tree"),
+ OPT_BOOLEAN(0, "apply", &force_apply,
+ "also apply the patch (use with --stat/--summary/--check)"),
+ OPT_STRING(0, "build-fake-ancestor", &fake_ancestor, "file",
+ "build a temporary index based on embedded index information"),
+ { OPTION_CALLBACK, 'z', NULL, NULL, NULL,
+ "paths are separated with NUL character",
+ PARSE_OPT_NOARG, option_parse_z },
+ OPT_INTEGER('C', NULL, &p_context,
+ "ensure at least <n> lines of context match"),
+ { OPTION_CALLBACK, 0, "whitespace", &whitespace_option, "action",
+ "detect new or modified lines that have whitespace errors",
+ 0, option_parse_whitespace },
+ OPT_BOOLEAN('R', "reverse", &apply_in_reverse,
+ "apply the patch in reverse"),
+ OPT_BOOLEAN(0, "unidiff-zero", &unidiff_zero,
+ "don't expect at least one line of context"),
+ OPT_BOOLEAN(0, "reject", &apply_with_reject,
+ "leave the rejected hunks in corresponding *.rej files"),
+ OPT__VERBOSE(&apply_verbosely),
+ { OPTION_CALLBACK, 0, "inaccurate-eof", NULL, NULL,
+ "tolerate incorrectly detected missing new-line at the end of file",
+ PARSE_OPT_NOARG, option_parse_inaccurate },
+ { OPTION_CALLBACK, 0, "recount", NULL, NULL,
+ "do not trust the line counts in the hunk headers",
+ PARSE_OPT_NOARG, option_parse_recount },
+ { OPTION_CALLBACK, 0, "directory", NULL, "root",
+ "prepend <root> to all filenames",
+ 0, option_parse_directory },
+ OPT_END()
+ };
+
prefix = setup_git_directory_gently(&is_not_gitdir);
prefix_length = prefix ? strlen(prefix) : 0;
git_config(git_apply_config, NULL);
if (apply_default_whitespace)
parse_whitespace_option(apply_default_whitespace);
- for (i = 1; i < argc; i++) {
+ argc = parse_options(argc, argv, builtin_apply_options,
+ apply_usage, 0);
+ if (apply_with_reject)
+ apply = apply_verbosely = 1;
+ if (!force_apply && (diffstat || numstat || summary || check || fake_ancestor))
+ apply = 0;
+ if (check_index && is_not_gitdir)
+ die("--index outside a repository");
+ if (cached) {
+ if (is_not_gitdir)
+ die("--cached outside a repository");
+ check_index = 1;
+ }
+ for (i = 0; i < argc; i++) {
const char *arg = argv[i];
- char *end;
int fd;
- if (!strcmp(arg, "-")) {
- errs |= apply_patch(0, "<stdin>", options);
- read_stdin = 0;
- continue;
- }
- if (!prefixcmp(arg, "--exclude=")) {
- add_name_limit(arg + 10, 1);
- continue;
- }
- if (!prefixcmp(arg, "--include=")) {
- add_name_limit(arg + 10, 0);
- has_include = 1;
- continue;
- }
- if (!prefixcmp(arg, "-p")) {
- p_value = atoi(arg + 2);
- p_value_known = 1;
- continue;
- }
- if (!strcmp(arg, "--no-add")) {
- no_add = 1;
- continue;
- }
- if (!strcmp(arg, "--stat")) {
- apply = 0;
- diffstat = 1;
- continue;
- }
- if (!strcmp(arg, "--allow-binary-replacement") ||
- !strcmp(arg, "--binary")) {
- continue; /* now no-op */
- }
- if (!strcmp(arg, "--numstat")) {
- apply = 0;
- numstat = 1;
- continue;
- }
- if (!strcmp(arg, "--summary")) {
- apply = 0;
- summary = 1;
- continue;
- }
- if (!strcmp(arg, "--check")) {
- apply = 0;
- check = 1;
- continue;
- }
- if (!strcmp(arg, "--index")) {
- if (is_not_gitdir)
- die("--index outside a repository");
- check_index = 1;
- continue;
- }
- if (!strcmp(arg, "--cached")) {
- if (is_not_gitdir)
- die("--cached outside a repository");
- check_index = 1;
- cached = 1;
- continue;
- }
- if (!strcmp(arg, "--apply")) {
- apply = 1;
- continue;
- }
- if (!strcmp(arg, "--build-fake-ancestor")) {
- apply = 0;
- if (++i >= argc)
- die ("need a filename");
- fake_ancestor = argv[i];
- continue;
- }
- if (!strcmp(arg, "-z")) {
- line_termination = 0;
- continue;
- }
- if (!prefixcmp(arg, "-C")) {
- p_context = strtoul(arg + 2, &end, 0);
- if (*end != '\0')
- die("unrecognized context count '%s'", arg + 2);
- continue;
- }
- if (!prefixcmp(arg, "--whitespace=")) {
- whitespace_option = arg + 13;
- parse_whitespace_option(arg + 13);
- continue;
- }
- if (!strcmp(arg, "-R") || !strcmp(arg, "--reverse")) {
- apply_in_reverse = 1;
- continue;
- }
- if (!strcmp(arg, "--unidiff-zero")) {
- unidiff_zero = 1;
- continue;
- }
- if (!strcmp(arg, "--reject")) {
- apply = apply_with_reject = apply_verbosely = 1;
- continue;
- }
- if (!strcmp(arg, "-v") || !strcmp(arg, "--verbose")) {
- apply_verbosely = 1;
- continue;
- }
- if (!strcmp(arg, "--inaccurate-eof")) {
- options |= INACCURATE_EOF;
- continue;
- }
- if (!strcmp(arg, "--recount")) {
- options |= RECOUNT;
- continue;
- }
- if (!prefixcmp(arg, "--directory=")) {
- arg += strlen("--directory=");
- root_len = strlen(arg);
- if (root_len && arg[root_len - 1] != '/') {
- char *new_root;
- root = new_root = xmalloc(root_len + 2);
- strcpy(new_root, arg);
- strcpy(new_root + root_len++, "/");
- } else
- root = arg;
- continue;
- }
if (0 < prefix_length)
arg = prefix_filename(prefix, prefix_length, arg);
--
1.6.1.rc1.35.gae26e.dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] parse-opt: migrate builtin-apply.
2008-12-27 14:22 ` [PATCH v2] " Miklos Vajna
@ 2008-12-27 21:47 ` Junio C Hamano
2009-01-05 19:12 ` Pierre Habouzit
2008-12-27 21:53 ` René Scharfe
1 sibling, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2008-12-27 21:47 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
Miklos Vajna <vmiklos@frugalware.org> writes:
> +static int option_parse_inaccurate(const struct option *opt,
> + const char *arg, int unset)
> +{
> + options |= INACCURATE_EOF;
> + return 0;
> +}
> +
> +static int option_parse_recount(const struct option *opt,
> + const char *arg, int unset)
> +{
> + options |= RECOUNT;
> + return 0;
> +}
I still haven't applied and ran the testsuite myself, but these makes me
wonder if there isn't a built-in "bit" type support in parse_options().
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] parse-opt: migrate builtin-apply.
2008-12-27 14:22 ` [PATCH v2] " Miklos Vajna
2008-12-27 21:47 ` Junio C Hamano
@ 2008-12-27 21:53 ` René Scharfe
2008-12-27 23:03 ` [PATCH] " Miklos Vajna
1 sibling, 1 reply; 10+ messages in thread
From: René Scharfe @ 2008-12-27 21:53 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Junio C Hamano, git
Miklos Vajna schrieb:
> -static const char apply_usage[] =
> -"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
> +static const char * const apply_usage[] = {
> + "git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...",
> + NULL
> +};
A useful convention with parse_options is to display "[options]" as a
place holder instead of listing all options explicitly in the usage
string. They are listed and explained in the full help message anyway
shown by "git apply -?").
> +static int option_parse_inaccurate(const struct option *opt,
> + const char *arg, int unset)
> +{
> + options |= INACCURATE_EOF;
> + return 0;
> +}
> +
> +static int option_parse_recount(const struct option *opt,
> + const char *arg, int unset)
> +{
> + options |= RECOUNT;
> + return 0;
> +}
OPT_BIT?
> + OPT_INTEGER('C', NULL, &p_context,
> + "ensure at least <n> lines of context match"),
p_context is an unsigned long variable; OPT_INTEGER expects a pointer
to an int. You'd either need an OPT_ULONG macro or change p_context
to in int. Doing the latter fixed the two test cases t4105 and t4252
for me.
René
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] parse-opt: migrate builtin-apply.
2008-12-27 21:53 ` René Scharfe
@ 2008-12-27 23:03 ` Miklos Vajna
2008-12-28 0:05 ` Jacob Helwig
0 siblings, 1 reply; 10+ messages in thread
From: Miklos Vajna @ 2008-12-27 23:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Rene Scharfe, git
The only incompatible change is that the user how have to use '--'
before a patch named --build-fake-ancestor=something.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Sat, Dec 27, 2008 at 10:53:43PM +0100, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> Miklos Vajna schrieb:
> > -static const char apply_usage[] =
> > -"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
> > +static const char * const apply_usage[] = {
> > + "git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...",
> > + NULL
> > +};
>
> A useful convention with parse_options is to display "[options]" as a
> place holder instead of listing all options explicitly in the usage
> string. They are listed and explained in the full help message anyway
> shown by "git apply -?").
True, changed.
> > +static int option_parse_recount(const struct option *opt,
> > + const char *arg, int unset)
> > +{
> > + options |= RECOUNT;
> > + return 0;
> > +}
>
> OPT_BIT?
I haven't discovered it, you are right. :-)
> > + OPT_INTEGER('C', NULL, &p_context,
> > + "ensure at least <n> lines of context match"),
>
> p_context is an unsigned long variable; OPT_INTEGER expects a pointer
> to an int. You'd either need an OPT_ULONG macro or change p_context
> to in int. Doing the latter fixed the two test cases t4105 and t4252
> for me.
Thanks, the trick was that the test did not fail on i686, just on
x86_64. I have changed it as you suggested and it now passes for me on
x86_64 as well.
Documentation/git-apply.txt | 4 +-
builtin-apply.c | 281 +++++++++++++++++++++++--------------------
2 files changed, 154 insertions(+), 131 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index e726510..9400f6a 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[verse]
'git apply' [--stat] [--numstat] [--summary] [--check] [--index]
- [--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --reverse]
+ [--apply] [--no-add] [--build-fake-ancestor=<file>] [-R | --reverse]
[--allow-binary-replacement | --binary] [--reject] [-z]
[-pNUM] [-CNUM] [--inaccurate-eof] [--recount] [--cached]
[--whitespace=<nowarn|warn|fix|error|error-all>]
@@ -64,7 +64,7 @@ OPTIONS
cached data, apply the patch, and store the result in the index,
without using the working tree. This implies '--index'.
---build-fake-ancestor <file>::
+--build-fake-ancestor=<file>::
Newer 'git-diff' output has embedded 'index information'
for each blob to help identify the original version that
the patch applies to. When this flag is given, and if
diff --git a/builtin-apply.c b/builtin-apply.c
index 07244b0..7603658 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -14,6 +14,7 @@
#include "builtin.h"
#include "string-list.h"
#include "dir.h"
+#include "parse-options.h"
/*
* --check turns on checking that the working tree matches the
@@ -45,9 +46,11 @@ static int apply_verbosely;
static int no_add;
static const char *fake_ancestor;
static int line_termination = '\n';
-static unsigned long p_context = ULONG_MAX;
-static const char apply_usage[] =
-"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
+static unsigned int p_context = UINT_MAX;
+static const char * const apply_usage[] = {
+ "git apply [options] [<patch>...]",
+ NULL
+};
static enum ws_error_action {
nowarn_ws_error,
@@ -61,6 +64,8 @@ static int applied_after_fixing_ws;
static const char *patch_input_file;
static const char *root;
static int root_len;
+static int read_stdin = 1;
+static int options;
static void parse_whitespace_option(const char *option)
{
@@ -3135,150 +3140,168 @@ static int git_apply_config(const char *var, const char *value, void *cb)
return git_default_config(var, value, cb);
}
+static int option_parse_stdin(const struct option *opt,
+ const char *arg, int unset)
+{
+ int *errs = opt->value;
+
+ *errs |= apply_patch(0, "<stdin>", options);
+ read_stdin = 0;
+ return 0;
+}
+
+static int option_parse_exclude(const struct option *opt,
+ const char *arg, int unset)
+{
+ add_name_limit(arg, 1);
+ return 0;
+}
+
+static int option_parse_include(const struct option *opt,
+ const char *arg, int unset)
+{
+ add_name_limit(arg, 0);
+ has_include = 1;
+ return 0;
+}
+
+static int option_parse_p(const struct option *opt,
+ const char *arg, int unset)
+{
+ p_value = atoi(arg);
+ p_value_known = 1;
+ return 0;
+}
+
+static int option_parse_z(const struct option *opt,
+ const char *arg, int unset)
+{
+ if (unset)
+ line_termination = '\n';
+ else
+ line_termination = 0;
+ return 0;
+}
+
+static int option_parse_whitespace(const struct option *opt,
+ const char *arg, int unset)
+{
+ const char **whitespace_option = opt->value;
+
+ *whitespace_option = arg;
+ parse_whitespace_option(arg);
+ return 0;
+}
+
+static int option_parse_directory(const struct option *opt,
+ const char *arg, int unset)
+{
+ root_len = strlen(arg);
+ if (root_len && arg[root_len - 1] != '/') {
+ char *new_root;
+ root = new_root = xmalloc(root_len + 2);
+ strcpy(new_root, arg);
+ strcpy(new_root + root_len++, "/");
+ } else
+ root = arg;
+ return 0;
+}
int cmd_apply(int argc, const char **argv, const char *unused_prefix)
{
int i;
- int read_stdin = 1;
- int options = 0;
int errs = 0;
int is_not_gitdir;
+ int binary;
+ int force_apply = 0;
const char *whitespace_option = NULL;
+ struct option builtin_apply_options[] = {
+ { OPTION_CALLBACK, '-', NULL, &errs, NULL,
+ "read the patch from the standard input",
+ PARSE_OPT_NOARG, option_parse_stdin },
+ { OPTION_CALLBACK, 0, "exclude", NULL, "path",
+ "don´t apply changes matching the given path",
+ 0, option_parse_exclude },
+ { OPTION_CALLBACK, 0, "include", NULL, "path",
+ "apply changes matching the given path",
+ 0, option_parse_include },
+ { OPTION_CALLBACK, 'p', NULL, NULL, "num",
+ "remove <num> leading slashes from traditional diff paths",
+ 0, option_parse_p },
+ OPT_BOOLEAN(0, "no-add", &no_add,
+ "ignore additions made by the patch"),
+ OPT_BOOLEAN(0, "stat", &diffstat,
+ "instead of applying the patch, output diffstat for the input"),
+ OPT_BOOLEAN(0, "allow-binary-replacement", &binary,
+ "now no-op"),
+ OPT_BOOLEAN(0, "binary", &binary,
+ "now no-op"),
+ OPT_BOOLEAN(0, "numstat", &numstat,
+ "shows number of added and deleted lines in decimal notation"),
+ OPT_BOOLEAN(0, "summary", &summary,
+ "instead of applying the patch, output a summary for the input"),
+ OPT_BOOLEAN(0, "check", &check,
+ "instead of applying the patch, see if the patch is applicable"),
+ OPT_BOOLEAN(0, "index", &check_index,
+ "make sure the patch is applicable to the current index"),
+ OPT_BOOLEAN(0, "cached", &cached,
+ "apply a patch without touching the working tree"),
+ OPT_BOOLEAN(0, "apply", &force_apply,
+ "also apply the patch (use with --stat/--summary/--check)"),
+ OPT_STRING(0, "build-fake-ancestor", &fake_ancestor, "file",
+ "build a temporary index based on embedded index information"),
+ { OPTION_CALLBACK, 'z', NULL, NULL, NULL,
+ "paths are separated with NUL character",
+ PARSE_OPT_NOARG, option_parse_z },
+ OPT_INTEGER('C', NULL, &p_context,
+ "ensure at least <n> lines of context match"),
+ { OPTION_CALLBACK, 0, "whitespace", &whitespace_option, "action",
+ "detect new or modified lines that have whitespace errors",
+ 0, option_parse_whitespace },
+ OPT_BOOLEAN('R', "reverse", &apply_in_reverse,
+ "apply the patch in reverse"),
+ OPT_BOOLEAN(0, "unidiff-zero", &unidiff_zero,
+ "don't expect at least one line of context"),
+ OPT_BOOLEAN(0, "reject", &apply_with_reject,
+ "leave the rejected hunks in corresponding *.rej files"),
+ OPT__VERBOSE(&apply_verbosely),
+ OPT_BIT(0, "inaccurate-eof", &options,
+ "tolerate incorrectly detected missing new-line at the end of file",
+ INACCURATE_EOF),
+ OPT_BIT(0, "recount", &options,
+ "do not trust the line counts in the hunk headers",
+ RECOUNT),
+ { OPTION_CALLBACK, 0, "directory", NULL, "root",
+ "prepend <root> to all filenames",
+ 0, option_parse_directory },
+ OPT_END()
+ };
+
prefix = setup_git_directory_gently(&is_not_gitdir);
prefix_length = prefix ? strlen(prefix) : 0;
git_config(git_apply_config, NULL);
if (apply_default_whitespace)
parse_whitespace_option(apply_default_whitespace);
- for (i = 1; i < argc; i++) {
+ argc = parse_options(argc, argv, builtin_apply_options,
+ apply_usage, 0);
+ if (apply_with_reject)
+ apply = apply_verbosely = 1;
+ if (!force_apply && (diffstat || numstat || summary || check || fake_ancestor))
+ apply = 0;
+ if (check_index && is_not_gitdir)
+ die("--index outside a repository");
+ if (cached) {
+ if (is_not_gitdir)
+ die("--cached outside a repository");
+ check_index = 1;
+ }
+ for (i = 0; i < argc; i++) {
const char *arg = argv[i];
- char *end;
int fd;
- if (!strcmp(arg, "-")) {
- errs |= apply_patch(0, "<stdin>", options);
- read_stdin = 0;
- continue;
- }
- if (!prefixcmp(arg, "--exclude=")) {
- add_name_limit(arg + 10, 1);
- continue;
- }
- if (!prefixcmp(arg, "--include=")) {
- add_name_limit(arg + 10, 0);
- has_include = 1;
- continue;
- }
- if (!prefixcmp(arg, "-p")) {
- p_value = atoi(arg + 2);
- p_value_known = 1;
- continue;
- }
- if (!strcmp(arg, "--no-add")) {
- no_add = 1;
- continue;
- }
- if (!strcmp(arg, "--stat")) {
- apply = 0;
- diffstat = 1;
- continue;
- }
- if (!strcmp(arg, "--allow-binary-replacement") ||
- !strcmp(arg, "--binary")) {
- continue; /* now no-op */
- }
- if (!strcmp(arg, "--numstat")) {
- apply = 0;
- numstat = 1;
- continue;
- }
- if (!strcmp(arg, "--summary")) {
- apply = 0;
- summary = 1;
- continue;
- }
- if (!strcmp(arg, "--check")) {
- apply = 0;
- check = 1;
- continue;
- }
- if (!strcmp(arg, "--index")) {
- if (is_not_gitdir)
- die("--index outside a repository");
- check_index = 1;
- continue;
- }
- if (!strcmp(arg, "--cached")) {
- if (is_not_gitdir)
- die("--cached outside a repository");
- check_index = 1;
- cached = 1;
- continue;
- }
- if (!strcmp(arg, "--apply")) {
- apply = 1;
- continue;
- }
- if (!strcmp(arg, "--build-fake-ancestor")) {
- apply = 0;
- if (++i >= argc)
- die ("need a filename");
- fake_ancestor = argv[i];
- continue;
- }
- if (!strcmp(arg, "-z")) {
- line_termination = 0;
- continue;
- }
- if (!prefixcmp(arg, "-C")) {
- p_context = strtoul(arg + 2, &end, 0);
- if (*end != '\0')
- die("unrecognized context count '%s'", arg + 2);
- continue;
- }
- if (!prefixcmp(arg, "--whitespace=")) {
- whitespace_option = arg + 13;
- parse_whitespace_option(arg + 13);
- continue;
- }
- if (!strcmp(arg, "-R") || !strcmp(arg, "--reverse")) {
- apply_in_reverse = 1;
- continue;
- }
- if (!strcmp(arg, "--unidiff-zero")) {
- unidiff_zero = 1;
- continue;
- }
- if (!strcmp(arg, "--reject")) {
- apply = apply_with_reject = apply_verbosely = 1;
- continue;
- }
- if (!strcmp(arg, "-v") || !strcmp(arg, "--verbose")) {
- apply_verbosely = 1;
- continue;
- }
- if (!strcmp(arg, "--inaccurate-eof")) {
- options |= INACCURATE_EOF;
- continue;
- }
- if (!strcmp(arg, "--recount")) {
- options |= RECOUNT;
- continue;
- }
- if (!prefixcmp(arg, "--directory=")) {
- arg += strlen("--directory=");
- root_len = strlen(arg);
- if (root_len && arg[root_len - 1] != '/') {
- char *new_root;
- root = new_root = xmalloc(root_len + 2);
- strcpy(new_root, arg);
- strcpy(new_root + root_len++, "/");
- } else
- root = arg;
- continue;
- }
if (0 < prefix_length)
arg = prefix_filename(prefix, prefix_length, arg);
--
1.6.1.rc1.35.gae26e.dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] parse-opt: migrate builtin-apply.
2008-12-27 23:03 ` [PATCH] " Miklos Vajna
@ 2008-12-28 0:05 ` Jacob Helwig
0 siblings, 0 replies; 10+ messages in thread
From: Jacob Helwig @ 2008-12-28 0:05 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Junio C Hamano, Rene Scharfe, Git
s/how have to use/now has to use/ ;-)
On Dec 27, 2008, at 17:03, Miklos Vajna <vmiklos@frugalware.org> wrote:
> The only incompatible change is that the user how have to use '--'
> before a patch named --build-fake-ancestor=something.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> On Sat, Dec 27, 2008 at 10:53:43PM +0100, René Scharfe <rene.scharfe@lsrfire.ath.c
> x> wrote:
>> Miklos Vajna schrieb:
>>> -static const char apply_usage[] =
>>> -"git apply [--stat] [--numstat] [--summary] [--check] [--index]
>>> [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-
>>> replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-
>>> CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
>>> +static const char * const apply_usage[] = {
>>> + "git apply [--stat] [--numstat] [--summary] [--check] [--
>>> index] [--cached] [--apply] [--no-add] [--index-info] [--allow-
>>> binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-
>>> pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>]
>>> <patch>...",
>>> + NULL
>>> +};
>>
>> A useful convention with parse_options is to display "[options]" as a
>> place holder instead of listing all options explicitly in the usage
>> string. They are listed and explained in the full help message
>> anyway
>> shown by "git apply -?").
>
> True, changed.
>
>>> +static int option_parse_recount(const struct option *opt,
>>> + const char *arg, int unset)
>>> +{
>>> + options |= RECOUNT;
>>> + return 0;
>>> +}
>>
>> OPT_BIT?
>
> I haven't discovered it, you are right. :-)
>
>>> + OPT_INTEGER('C', NULL, &p_context,
>>> + "ensure at least <n> lines of context
>>> match"),
>>
>> p_context is an unsigned long variable; OPT_INTEGER expects a pointer
>> to an int. You'd either need an OPT_ULONG macro or change p_context
>> to in int. Doing the latter fixed the two test cases t4105 and t4252
>> for me.
>
> Thanks, the trick was that the test did not fail on i686, just on
> x86_64. I have changed it as you suggested and it now passes for me on
> x86_64 as well.
>
> Documentation/git-apply.txt | 4 +-
> builtin-apply.c | 281 ++++++++++++++++++++++
> +--------------------
> 2 files changed, 154 insertions(+), 131 deletions(-)
>
> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
> index e726510..9400f6a 100644
> --- a/Documentation/git-apply.txt
> +++ b/Documentation/git-apply.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
> --------
> [verse]
> 'git apply' [--stat] [--numstat] [--summary] [--check] [--index]
> - [--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --
> reverse]
> + [--apply] [--no-add] [--build-fake-ancestor=<file>] [-R | --
> reverse]
> [--allow-binary-replacement | --binary] [--reject] [-z]
> [-pNUM] [-CNUM] [--inaccurate-eof] [--recount] [--cached]
> [--whitespace=<nowarn|warn|fix|error|error-all>]
> @@ -64,7 +64,7 @@ OPTIONS
> cached data, apply the patch, and store the result in the index,
> without using the working tree. This implies '--index'.
>
> ---build-fake-ancestor <file>::
> +--build-fake-ancestor=<file>::
> Newer 'git-diff' output has embedded 'index information'
> for each blob to help identify the original version that
> the patch applies to. When this flag is given, and if
> diff --git a/builtin-apply.c b/builtin-apply.c
> index 07244b0..7603658 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -14,6 +14,7 @@
> #include "builtin.h"
> #include "string-list.h"
> #include "dir.h"
> +#include "parse-options.h"
>
> /*
> * --check turns on checking that the working tree matches the
> @@ -45,9 +46,11 @@ static int apply_verbosely;
> static int no_add;
> static const char *fake_ancestor;
> static int line_termination = '\n';
> -static unsigned long p_context = ULONG_MAX;
> -static const char apply_usage[] =
> -"git apply [--stat] [--numstat] [--summary] [--check] [--index] [--
> cached] [--apply] [--no-add] [--index-info] [--allow-binary-
> replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM]
> [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>...";
> +static unsigned int p_context = UINT_MAX;
> +static const char * const apply_usage[] = {
> + "git apply [options] [<patch>...]",
> + NULL
> +};
>
> static enum ws_error_action {
> nowarn_ws_error,
> @@ -61,6 +64,8 @@ static int applied_after_fixing_ws;
> static const char *patch_input_file;
> static const char *root;
> static int root_len;
> +static int read_stdin = 1;
> +static int options;
>
> static void parse_whitespace_option(const char *option)
> {
> @@ -3135,150 +3140,168 @@ static int git_apply_config(const char
> *var, const char *value, void *cb)
> return git_default_config(var, value, cb);
> }
>
> +static int option_parse_stdin(const struct option *opt,
> + const char *arg, int unset)
> +{
> + int *errs = opt->value;
> +
> + *errs |= apply_patch(0, "<stdin>", options);
> + read_stdin = 0;
> + return 0;
> +}
> +
> +static int option_parse_exclude(const struct option *opt,
> + const char *arg, int unset)
> +{
> + add_name_limit(arg, 1);
> + return 0;
> +}
> +
> +static int option_parse_include(const struct option *opt,
> + const char *arg, int unset)
> +{
> + add_name_limit(arg, 0);
> + has_include = 1;
> + return 0;
> +}
> +
> +static int option_parse_p(const struct option *opt,
> + const char *arg, int unset)
> +{
> + p_value = atoi(arg);
> + p_value_known = 1;
> + return 0;
> +}
> +
> +static int option_parse_z(const struct option *opt,
> + const char *arg, int unset)
> +{
> + if (unset)
> + line_termination = '\n';
> + else
> + line_termination = 0;
> + return 0;
> +}
> +
> +static int option_parse_whitespace(const struct option *opt,
> + const char *arg, int unset)
> +{
> + const char **whitespace_option = opt->value;
> +
> + *whitespace_option = arg;
> + parse_whitespace_option(arg);
> + return 0;
> +}
> +
> +static int option_parse_directory(const struct option *opt,
> + const char *arg, int unset)
> +{
> + root_len = strlen(arg);
> + if (root_len && arg[root_len - 1] != '/') {
> + char *new_root;
> + root = new_root = xmalloc(root_len + 2);
> + strcpy(new_root, arg);
> + strcpy(new_root + root_len++, "/");
> + } else
> + root = arg;
> + return 0;
> +}
>
> int cmd_apply(int argc, const char **argv, const char *unused_prefix)
> {
> int i;
> - int read_stdin = 1;
> - int options = 0;
> int errs = 0;
> int is_not_gitdir;
> + int binary;
> + int force_apply = 0;
>
> const char *whitespace_option = NULL;
>
> + struct option builtin_apply_options[] = {
> + { OPTION_CALLBACK, '-', NULL, &errs, NULL,
> + "read the patch from the standard input",
> + PARSE_OPT_NOARG, option_parse_stdin },
> + { OPTION_CALLBACK, 0, "exclude", NULL, "path",
> + "don´t apply changes matching the given path",
> + 0, option_parse_exclude },
> + { OPTION_CALLBACK, 0, "include", NULL, "path",
> + "apply changes matching the given path",
> + 0, option_parse_include },
> + { OPTION_CALLBACK, 'p', NULL, NULL, "num",
> + "remove <num> leading slashes from traditional diff
> paths",
> + 0, option_parse_p },
> + OPT_BOOLEAN(0, "no-add", &no_add,
> + "ignore additions made by the patch"),
> + OPT_BOOLEAN(0, "stat", &diffstat,
> + "instead of applying the patch, output diffstat for the
> input"),
> + OPT_BOOLEAN(0, "allow-binary-replacement", &binary,
> + "now no-op"),
> + OPT_BOOLEAN(0, "binary", &binary,
> + "now no-op"),
> + OPT_BOOLEAN(0, "numstat", &numstat,
> + "shows number of added and deleted lines in decimal
> notation"),
> + OPT_BOOLEAN(0, "summary", &summary,
> + "instead of applying the patch, output a summary for
> the input"),
> + OPT_BOOLEAN(0, "check", &check,
> + "instead of applying the patch, see if the patch is
> applicable"),
> + OPT_BOOLEAN(0, "index", &check_index,
> + "make sure the patch is applicable to the current
> index"),
> + OPT_BOOLEAN(0, "cached", &cached,
> + "apply a patch without touching the working tree"),
> + OPT_BOOLEAN(0, "apply", &force_apply,
> + "also apply the patch (use with --stat/--summary/--
> check)"),
> + OPT_STRING(0, "build-fake-ancestor", &fake_ancestor, "file",
> + "build a temporary index based on embedded index
> information"),
> + { OPTION_CALLBACK, 'z', NULL, NULL, NULL,
> + "paths are separated with NUL character",
> + PARSE_OPT_NOARG, option_parse_z },
> + OPT_INTEGER('C', NULL, &p_context,
> + "ensure at least <n> lines of context match"),
> + { OPTION_CALLBACK, 0, "whitespace", &whitespace_option,
> "action",
> + "detect new or modified lines that have whitespace
> errors",
> + 0, option_parse_whitespace },
> + OPT_BOOLEAN('R', "reverse", &apply_in_reverse,
> + "apply the patch in reverse"),
> + OPT_BOOLEAN(0, "unidiff-zero", &unidiff_zero,
> + "don't expect at least one line of context"),
> + OPT_BOOLEAN(0, "reject", &apply_with_reject,
> + "leave the rejected hunks in corresponding *.rej files"),
> + OPT__VERBOSE(&apply_verbosely),
> + OPT_BIT(0, "inaccurate-eof", &options,
> + "tolerate incorrectly detected missing new-line at the
> end of file",
> + INACCURATE_EOF),
> + OPT_BIT(0, "recount", &options,
> + "do not trust the line counts in the hunk headers",
> + RECOUNT),
> + { OPTION_CALLBACK, 0, "directory", NULL, "root",
> + "prepend <root> to all filenames",
> + 0, option_parse_directory },
> + OPT_END()
> + };
> +
> prefix = setup_git_directory_gently(&is_not_gitdir);
> prefix_length = prefix ? strlen(prefix) : 0;
> git_config(git_apply_config, NULL);
> if (apply_default_whitespace)
> parse_whitespace_option(apply_default_whitespace);
>
> - for (i = 1; i < argc; i++) {
> + argc = parse_options(argc, argv, builtin_apply_options,
> + apply_usage, 0);
> + if (apply_with_reject)
> + apply = apply_verbosely = 1;
> + if (!force_apply && (diffstat || numstat || summary || check ||
> fake_ancestor))
> + apply = 0;
> + if (check_index && is_not_gitdir)
> + die("--index outside a repository");
> + if (cached) {
> + if (is_not_gitdir)
> + die("--cached outside a repository");
> + check_index = 1;
> + }
> + for (i = 0; i < argc; i++) {
> const char *arg = argv[i];
> - char *end;
> int fd;
>
> - if (!strcmp(arg, "-")) {
> - errs |= apply_patch(0, "<stdin>", options);
> - read_stdin = 0;
> - continue;
> - }
> - if (!prefixcmp(arg, "--exclude=")) {
> - add_name_limit(arg + 10, 1);
> - continue;
> - }
> - if (!prefixcmp(arg, "--include=")) {
> - add_name_limit(arg + 10, 0);
> - has_include = 1;
> - continue;
> - }
> - if (!prefixcmp(arg, "-p")) {
> - p_value = atoi(arg + 2);
> - p_value_known = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--no-add")) {
> - no_add = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--stat")) {
> - apply = 0;
> - diffstat = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--allow-binary-replacement") ||
> - !strcmp(arg, "--binary")) {
> - continue; /* now no-op */
> - }
> - if (!strcmp(arg, "--numstat")) {
> - apply = 0;
> - numstat = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--summary")) {
> - apply = 0;
> - summary = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--check")) {
> - apply = 0;
> - check = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--index")) {
> - if (is_not_gitdir)
> - die("--index outside a repository");
> - check_index = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--cached")) {
> - if (is_not_gitdir)
> - die("--cached outside a repository");
> - check_index = 1;
> - cached = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--apply")) {
> - apply = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--build-fake-ancestor")) {
> - apply = 0;
> - if (++i >= argc)
> - die ("need a filename");
> - fake_ancestor = argv[i];
> - continue;
> - }
> - if (!strcmp(arg, "-z")) {
> - line_termination = 0;
> - continue;
> - }
> - if (!prefixcmp(arg, "-C")) {
> - p_context = strtoul(arg + 2, &end, 0);
> - if (*end != '\0')
> - die("unrecognized context count '%s'", arg + 2);
> - continue;
> - }
> - if (!prefixcmp(arg, "--whitespace=")) {
> - whitespace_option = arg + 13;
> - parse_whitespace_option(arg + 13);
> - continue;
> - }
> - if (!strcmp(arg, "-R") || !strcmp(arg, "--reverse")) {
> - apply_in_reverse = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--unidiff-zero")) {
> - unidiff_zero = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--reject")) {
> - apply = apply_with_reject = apply_verbosely = 1;
> - continue;
> - }
> - if (!strcmp(arg, "-v") || !strcmp(arg, "--verbose")) {
> - apply_verbosely = 1;
> - continue;
> - }
> - if (!strcmp(arg, "--inaccurate-eof")) {
> - options |= INACCURATE_EOF;
> - continue;
> - }
> - if (!strcmp(arg, "--recount")) {
> - options |= RECOUNT;
> - continue;
> - }
> - if (!prefixcmp(arg, "--directory=")) {
> - arg += strlen("--directory=");
> - root_len = strlen(arg);
> - if (root_len && arg[root_len - 1] != '/') {
> - char *new_root;
> - root = new_root = xmalloc(root_len + 2);
> - strcpy(new_root, arg);
> - strcpy(new_root + root_len++, "/");
> - } else
> - root = arg;
> - continue;
> - }
> if (0 < prefix_length)
> arg = prefix_filename(prefix, prefix_length, arg);
>
> --
> 1.6.1.rc1.35.gae26e.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] parse-opt: migrate builtin-apply.
2008-12-27 21:47 ` Junio C Hamano
@ 2009-01-05 19:12 ` Pierre Habouzit
2009-01-05 20:06 ` Miklos Vajna
0 siblings, 1 reply; 10+ messages in thread
From: Pierre Habouzit @ 2009-01-05 19:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Miklos Vajna, git
[-- Attachment #1: Type: text/plain, Size: 807 bytes --]
On Sat, Dec 27, 2008 at 09:47:13PM +0000, Junio C Hamano wrote:
> Miklos Vajna <vmiklos@frugalware.org> writes:
>
> > +static int option_parse_inaccurate(const struct option *opt,
> > + const char *arg, int unset)
> > +{
> > + options |= INACCURATE_EOF;
> > + return 0;
> > +}
> > +
> > +static int option_parse_recount(const struct option *opt,
> > + const char *arg, int unset)
> > +{
> > + options |= RECOUNT;
> > + return 0;
> > +}
>
> I still haven't applied and ran the testsuite myself, but these makes me
> wonder if there isn't a built-in "bit" type support in parse_options().
There is.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] parse-opt: migrate builtin-apply.
2009-01-05 19:12 ` Pierre Habouzit
@ 2009-01-05 20:06 ` Miklos Vajna
0 siblings, 0 replies; 10+ messages in thread
From: Miklos Vajna @ 2009-01-05 20:06 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
[-- Attachment #1: Type: text/plain, Size: 331 bytes --]
On Mon, Jan 05, 2009 at 08:12:22PM +0100, Pierre Habouzit <madcoder@debian.org> wrote:
> > I still haven't applied and ran the testsuite myself, but these makes me
> > wonder if there isn't a built-in "bit" type support in parse_options().
>
> There is.
FYI, in the meantime f26c494 is already in next, using OPT_BIT. :-)
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-01-05 20:07 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-12-27 4:22 [PATCH] parse-opt: migrate builtin-apply Miklos Vajna
2008-12-27 7:29 ` Junio C Hamano
2008-12-27 14:05 ` Miklos Vajna
2008-12-27 14:22 ` [PATCH v2] " Miklos Vajna
2008-12-27 21:47 ` Junio C Hamano
2009-01-05 19:12 ` Pierre Habouzit
2009-01-05 20:06 ` Miklos Vajna
2008-12-27 21:53 ` René Scharfe
2008-12-27 23:03 ` [PATCH] " Miklos Vajna
2008-12-28 0:05 ` Jacob Helwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).