{"thread":{"id":"15183","subject":"[PATCH] git-apply - Add --include=PATH","startedAt":"2008-08-23T20:37:49Z","lastAt":"2008-08-25T08:05:31Z","messageCount":4,"participants":["Joe Perches","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"88320","messageId":"1219523869.18365.106.camel@localhost","threadId":"15183","inReplyTo":null,"subject":"[PATCH] git-apply - Add --include=PATH","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2008-08-23T20:37:49Z","receivedAt":"2008-08-23T20:37:49Z","isPatch":true,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"Add similar capability to --exclude=\n\nAllows selection of files to patch from a\nlarge patchset.\n\nSigned-off-by: Joe Perches <joe@perches.com>\n\n Documentation/git-apply.txt |    8 +++++++-\n builtin-apply.c             |   27 +++++++++++++++++++++++++--\n 2 files changed, 32 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt\nindex feb51f1..2467e62 100644\n--- a/Documentation/git-apply.txt\n+++ b/Documentation/git-apply.txt\n@@ -14,7 +14,8 @@ SYNOPSIS\n \t  [--allow-binary-replacement | --binary] [--reject] [-z]\n \t  [-pNUM] [-CNUM] [--inaccurate-eof] [--recount] [--cached]\n \t  [--whitespace=<nowarn|warn|fix|error|error-all>]\n-\t  [--exclude=PATH] [--directory=<root>] [--verbose] [<patch>...]\n+\t  [--exclude=PATH] [--include=PATH] [--directory=<root>]\n+\t  [--verbose] [<patch>...]\n \n DESCRIPTION\n -----------\n@@ -137,6 +138,11 @@ discouraged.\n \tbe useful when importing patchsets, where you want to exclude certain\n \tfiles or directories.\n \n+--include=<path-pattern>::\n+\tApply changes to files matching the given path pattern. This can\n+\tbe useful when importing patchsets, where you want to include certain\n+\tfiles or directories.\n+\n --whitespace=<action>::\n \tWhen applying a patch, detect a new or modified line that has\n \twhitespace errors.  What are considered whitespace errors is\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 2216a0b..121a6d0 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -46,7 +46,7 @@ static const char *fake_ancestor;\n static int line_termination = '\\n';\n static unsigned long p_context = ULONG_MAX;\n static const char apply_usage[] =\n-\"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>...\";\n+\"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>] [--exclude=PATH] [--include=PATH] <patch>...\";\n \n static enum ws_error_action {\n \tnowarn_ws_error,\n@@ -2996,10 +2996,16 @@ static struct excludes {\n \tconst char *path;\n } *excludes;\n \n+static struct includes {\n+\tstruct includes *next;\n+\tconst char *path;\n+} *includes;\n+\n static int use_patch(struct patch *p)\n {\n \tconst char *pathname = p->new_name ? p->new_name : p->old_name;\n \tstruct excludes *x = excludes;\n+\tstruct includes *y = includes;\n \twhile (x) {\n \t\tif (fnmatch(x->path, pathname, 0) == 0)\n \t\t\treturn 0;\n@@ -3011,7 +3017,17 @@ static int use_patch(struct patch *p)\n \t\t    memcmp(prefix, pathname, prefix_length))\n \t\t\treturn 0;\n \t}\n-\treturn 1;\n+\n+\tif (!y || !y->path)\n+\t\treturn 1;\n+\n+\twhile (y && y->path) {\n+\t\tif (fnmatch(y->path, pathname, 0) == 0)\n+\t\t\treturn 1;\n+\t\ty = y->next;\n+\t}\n+\n+\treturn 0;\n }\n \n static void prefix_one(char **name)\n@@ -3160,6 +3176,13 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \t\t\texcludes = x;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!prefixcmp(arg, \"--include=\")) {\n+\t\t\tstruct includes *y = xmalloc(sizeof(*y));\n+\t\t\ty->path = arg + 10;\n+\t\t\ty->next = includes;\n+\t\t\tincludes = y;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!prefixcmp(arg, \"-p\")) {\n \t\t\tp_value = atoi(arg + 2);\n \t\t\tp_value_known = 1;\n"},{"id":"88337","messageId":"7viqtrw7up.fsf@gitster.siamese.dyndns.org","threadId":"15183","inReplyTo":"1219523869.18365.106.camel@localhost","subject":"Re: [PATCH] git-apply - Add --include=PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-24T00:54:22Z","receivedAt":"2008-08-24T00:54:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches <joe@perches.com> writes:\n\n> Add similar capability to --exclude=\n>\n> Allows selection of files to patch from a\n> large patchset.\n>\n> Signed-off-by: Joe Perches <joe@perches.com>\n\nThanks; I don't see anything fundamentally wrong with what this patch\ntries to achieve.\n\n> @@ -2996,10 +2996,16 @@ static struct excludes {\n>  \tconst char *path;\n>  } *excludes;\n>  \n> +static struct includes {\n> +\tstruct includes *next;\n> +\tconst char *path;\n> +} *includes;\n\nNow this is ugly.  You can just add a new variable \"*includes\" that is of\nexactly the same type as existing \"*excludes\" without introducing a new\ntype.\n\nYou should then find it disturbing that the shared type is still called\n\"struct excludes\" even though it is now used for things you would want to\ninclude.  You are right.  You can then either rename it to a more neutral\nname, or (even better) use an existing type, such as \"string_list\".\n\nWhich would mean that this patch should be done as two patches:\n\n [1/2] builtin-apply.c: Use \"string_list\" for \"--excludes\".\n [2/2] builtin-apply.c: Add \"--includes\" option.\n\nThe first will be a preparatory step that does not change any externally\nvisible behaviour.  The only thing it will do is to change the type of\n\"excludes\" to \"struct string_list\", to update the option parser to use\nstring_list_append() to add to it, and to update the way \"use_patch()\" to\niterate over the items in the exclude list.\n\nThe second will add a new \"includes\" variable, and do the moral equivalent\nof what your patch did.\n\nThe first patch most likely should introduce a new helper function:\n\n  int string_list_has_match(struct string_list *s, const char *path);\n\nso that the update to \"use_patch()\" that needs to be done in the second\npatch can just pass a different string_list to implement the additional\ncheck.\n"},{"id":"88401","messageId":"1219615063.18365.141.camel@localhost","threadId":"15183","inReplyTo":"7viqtrw7up.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-apply - Add --include=PATH","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2008-08-24T21:57:43Z","receivedAt":"2008-08-24T21:57:43Z","isPatch":true,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"On Sat, 2008-08-23 at 17:54 -0700, Junio C Hamano wrote:\n> Joe Perches <joe@perches.com> writes:\n> > Add similar capability to --exclude=\n> > Allows selection of files to patch from a\n> > large patchset.\n> Thanks; I don't see anything fundamentally wrong with what this patch\n> tries to achieve.\n> \n> > @@ -2996,10 +2996,16 @@ static struct excludes {\n> >  \tconst char *path;\n> >  } *excludes;\n> >  \n> > +static struct includes {\n> > +\tstruct includes *next;\n> > +\tconst char *path;\n> > +} *includes;\n> \n> Now this is ugly.  You can just add a new variable \"*includes\" that is of\n> exactly the same type as existing \"*excludes\" without introducing a new\n> type.\n\nYes, it's slightly ugly, but it was less work and much easier for\na human to parse.  I also didn't want to use \"struct excludes\"\nfor includes which I thought even uglier.\n\n> You should then find it disturbing that the shared type is still called\n> \"struct excludes\" even though it is now used for things you would want to\n> include.  You are right.  You can then either rename it to a more neutral\n> name, or (even better) use an existing type, such as \"string_list\".\n\nI'm on holiday for a few days, but I'll submit 2 patches later:\n\n1. Rename struct excludes to struct path_list\n2. Add --includes\n\ncheers, Joe\n"},{"id":"88436","messageId":"7vhc99h644.fsf@gitster.siamese.dyndns.org","threadId":"15183","inReplyTo":"1219615063.18365.141.camel@localhost","subject":"Re: [PATCH] git-apply - Add --include=PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-25T08:05:31Z","receivedAt":"2008-08-25T08:05:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joe Perches <joe@perches.com> writes:\n\n>> > @@ -2996,10 +2996,16 @@ static struct excludes {\n>> >  \tconst char *path;\n>> >  } *excludes;\n>> >  \n>> > +static struct includes {\n>> > +\tstruct includes *next;\n>> > +\tconst char *path;\n>> > +} *includes;\n>> \n>> Now this is ugly.  You can just add a new variable \"*includes\" that is of\n>> exactly the same type as existing \"*excludes\" without introducing a new\n>> type.\n>\n> Yes, it's slightly ugly, but it was less work and much easier for\n> a human to parse.\n\nAnother consideration is what should happen when you give contradicting\nexcludes and includes list.  For example, it is very plausible you might\nwant to say \"apply to all but header files, except that you want the part\nto one specific header file to also get applied).  Something like:\n\n    $ git apply --include='specific-one.h' --exclude='*.h' --include='*' <patch\n\nIt is easy to declare that all the exclude patterns are processed and used\nto reject paths, and then only after that include patterns, if any, are\nused to limit the remainder.  But that is describing how the code does it,\nand may not match what the users expect.  For example, the users would\nexpect:\n\n    $ git apply --include='specific-one.h' --exclude='s*' <patch\n\nto apply the part for \"specific-one.h\" but no other paths that begin with \"s\".\nHowever, that is not what happens.\n\nIt would be much easier to explain to the end users if the rule were that\ninclude and exclude patterns are examined in the order they are specified\non the command line, and the first match determines the each path's fate.\n\nIn order to support that, you do not want two separate lists.  Instead,\nyou would want to keep a single \"static struct string_list limit_by_name\",\nappend both excluded and included items to the list as you encounter with\nstring_list_append(), but in such a way that you can distinguish which one\nis which later.  Also remember if you have seen any included item.\n\nThen your use_patch() would:\n\n * first check and ignore patches about paths outside of the prefix if any\n   is specified via --directory;\n\n * loop over the limit_by_name.items[] array, checking the path with each\n   element in it with fnmatch().  If you find a match, then you know if it\n   is excluded (return 0) or included (return 1);\n\n * if no patterns match, return 1 (i.e. modify this path) if you did not\n   see any \"include\" pattern.  If you had any \"include\" pattern on the\n   command line, return 0 (i.e. do not modify this path).\n\nPerhaps something like this, but I did not test it.\n\n builtin-apply.c |   48 +++++++++++++++++++++++++++++++++---------------\n 1 files changed, 33 insertions(+), 15 deletions(-)\n\ndiff --git c/builtin-apply.c w/builtin-apply.c\nindex 2216a0b..967ebec 100644\n--- c/builtin-apply.c\n+++ w/builtin-apply.c\n@@ -2991,29 +2991,45 @@ static int write_out_results(struct patch *list, int skipped_patch)\n \n static struct lock_file lock_file;\n \n-static struct excludes {\n-\tstruct excludes *next;\n-\tconst char *path;\n-} *excludes;\n+static struct string_list limit_by_name;\n+static int has_include;\n+static void add_name_limit(const char *name, int exclude)\n+{\n+\tstruct string_list_item *it;\n+\n+\tit = string_list_append(name, &limit_by_name);\n+\tit->util = exclude ? NULL : (void *) 1;\n+}\n \n static int use_patch(struct patch *p)\n {\n \tconst char *pathname = p->new_name ? p->new_name : p->old_name;\n-\tstruct excludes *x = excludes;\n-\twhile (x) {\n-\t\tif (fnmatch(x->path, pathname, 0) == 0)\n-\t\t\treturn 0;\n-\t\tx = x->next;\n-\t}\n+\tint i;\n+\n+\t/* Paths outside are not touched regardless of \"--include\" */\n \tif (0 < prefix_length) {\n \t\tint pathlen = strlen(pathname);\n \t\tif (pathlen <= prefix_length ||\n \t\t    memcmp(prefix, pathname, prefix_length))\n \t\t\treturn 0;\n \t}\n-\treturn 1;\n+\n+\t/* See if it matches any of exclude/include rule */\n+\tfor (i = 0; i < limit_by_name.nr; i++) {\n+\t\tstruct string_list_item *it = &limit_by_name.items[i];\n+\t\tif (!fnmatch(it->string, pathname, 0))\n+\t\t\treturn (it->util != NULL);\n+\t}\n+\n+\t/*\n+\t * If we had any include, a path that does not match any rule is\n+\t * not used.  Otherwise, we saw bunch of exclude rules (or none)\n+\t * and such a path is used.\n+\t */\n+\treturn !has_include;\n }\n \n+\n static void prefix_one(char **name)\n {\n \tchar *old_name = *name;\n@@ -3154,10 +3170,12 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!prefixcmp(arg, \"--exclude=\")) {\n-\t\t\tstruct excludes *x = xmalloc(sizeof(*x));\n-\t\t\tx->path = arg + 10;\n-\t\t\tx->next = excludes;\n-\t\t\texcludes = x;\n+\t\t\tadd_name_limit(arg + 10, 1);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!prefixcmp(arg, \"--include=\")) {\n+\t\t\tadd_name_limit(arg + 10, 0);\n+\t\t\thas_include = 1;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!prefixcmp(arg, \"-p\")) {\n"}]}