{"thread":{"id":"51734","subject":"[PATCH v5 0/2] Honor .gitattributes with rebase --am","startedAt":"2019-08-25T23:34:02Z","lastAt":"2019-09-02T22:39:58Z","messageCount":10,"participants":["brian m. carlson","Phillip Wood","Johannes Schindelin"],"isPatch":true,"patchVersion":5,"patchTotal":2},"messages":[{"id":"381132","messageId":"20190825233340.10894-1-sandals@crustytoothpaste.net","threadId":"51734","inReplyTo":null,"subject":"[PATCH v5 0/2] Honor .gitattributes with rebase --am","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-08-25T23:33:38Z","receivedAt":"2019-08-25T23:34:02Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This series makes rebase --am honor the .gitattributes file for\nsubsequent patches when a patch changes it.\n\nNote that there are two places we load attributes in ll-merge.c, but\nthis code only handles the one that git am uses.  The two cannot be\nunified because the one in ll_merge_marker_size intentionally doesn't\nload the merge attribute, since it wants to always use the recursive\nstrategy.  Loading it anyway causes t4017 to fail.\n\nChanges from v4:\n* Wrap lines in apply.c.\n* Handle merge and conflict-marker-size attributes.\n* Add tests for am and am -3 in addition to rebase.\n\nChanges from v3:\n* Check for both addition and removal of .gitattributes files.\n* Switch from \"test_config\" to \"git config\".\n\nChanges from v2:\n* Rename has_path_suffix to ends_with_path_components.\n\nChanges from v1:\n* Add has_path_suffix in a separate commit.\n\nbrian m. carlson (2):\n  path: add a function to check for path suffix\n  am: reload .gitattributes after patching it\n\n apply.c           | 11 ++++++++++\n convert.c         | 11 +++++++++-\n convert.h         |  6 ++++++\n ll-merge.c        | 19 +++++++++++++----\n ll-merge.h        |  1 +\n path.c            | 39 +++++++++++++++++++++++++++--------\n path.h            |  3 +++\n t/t3400-rebase.sh | 36 ++++++++++++++++++++++++++++++++\n t/t4150-am.sh     | 52 +++++++++++++++++++++++++++++++++++++++++++++++\n 9 files changed, 164 insertions(+), 14 deletions(-)\n\nRange-diff against v4:\n1:  fa825e4b40 ! 1:  2077a0829e apply: reload .gitattributes after patching it\n    @@ Metadata\n     Author: brian m. carlson <sandals@crustytoothpaste.net>\n     \n      ## Commit message ##\n    -    apply: reload .gitattributes after patching it\n    +    am: reload .gitattributes after patching it\n     \n         When applying multiple patches with git am, or when rebasing using the\n         am backend, it's possible that one of our patches has updated a\n    @@ Commit message\n     \n         To ensure we write the correct data into the working tree, expire the\n         cache after each patch that touches a path ending in \".gitattributes\".\n    +    Since we load these attributes in multiple separate files, we must\n    +    expire them accordingly.\n    +\n    +    Verify that both the am and rebase code paths work correctly, including\n    +    the conflict marker size with am -3.\n     \n         Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n     \n    @@ apply.c: static int apply_patch(struct apply_state *state,\n      \t\t\t*listp = patch;\n      \t\t\tlistp = &patch->next;\n     +\n    -+\t\t\tif ((patch->new_name && ends_with_path_components(patch->new_name, GITATTRIBUTES_FILE)) ||\n    -+\t\t\t    (patch->old_name && ends_with_path_components(patch->old_name, GITATTRIBUTES_FILE)))\n    ++\t\t\tif ((patch->new_name &&\n    ++\t\t\t     ends_with_path_components(patch->new_name,\n    ++\t\t\t\t\t\t       GITATTRIBUTES_FILE)) ||\n    ++\t\t\t    (patch->old_name &&\n    ++\t\t\t     ends_with_path_components(patch->old_name,\n    ++\t\t\t\t\t\t       GITATTRIBUTES_FILE)))\n     +\t\t\t\tflush_attributes = 1;\n      \t\t}\n      \t\telse {\n    @@ apply.c: static int apply_patch(struct apply_state *state,\n      \tstrbuf_release(&buf);\n     \n      ## convert.c ##\n    +@@\n    + #include \"pkt-line.h\"\n    + #include \"sub-process.h\"\n    + #include \"utf8.h\"\n    ++#include \"ll-merge.h\"\n    + \n    + /*\n    +  * convert.c - convert a file when checking it out and checking it in.\n     @@ convert.c: struct conv_attrs {\n      \tconst char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n      };\n    @@ convert.c: static void convert_attrs(const struct index_state *istate,\n     +{\n     +\tattr_check_free(check);\n     +\tcheck = NULL;\n    ++\treset_merge_attributes();\n     +}\n     +\n      int would_convert_to_git_filter_fd(const struct index_state *istate, const char *path)\n    @@ convert.h: void convert_to_git_filter_fd(const struct index_state *istate,\n       *\n       * Streaming conversion support\n     \n    + ## ll-merge.c ##\n    +@@ ll-merge.c: struct ll_merge_driver {\n    + \tchar *cmdline;\n    + };\n    + \n    ++static struct attr_check *merge_attributes;\n    ++static struct attr_check *load_merge_attributes(void)\n    ++{\n    ++\tif (!merge_attributes)\n    ++\t\tmerge_attributes = attr_check_initl(\"merge\", \"conflict-marker-size\", NULL);\n    ++\treturn merge_attributes;\n    ++}\n    ++\n    ++void reset_merge_attributes(void)\n    ++{\n    ++\tattr_check_free(merge_attributes);\n    ++\tmerge_attributes = NULL;\n    ++}\n    ++\n    + /*\n    +  * Built-in low-levels\n    +  */\n    +@@ ll-merge.c: int ll_merge(mmbuffer_t *result_buf,\n    + \t     struct index_state *istate,\n    + \t     const struct ll_merge_options *opts)\n    + {\n    +-\tstatic struct attr_check *check;\n    ++\tstruct attr_check *check = load_merge_attributes();\n    + \tstatic const struct ll_merge_options default_opts;\n    + \tconst char *ll_driver_name = NULL;\n    + \tint marker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n    +@@ ll-merge.c: int ll_merge(mmbuffer_t *result_buf,\n    + \t\tnormalize_file(theirs, path, istate);\n    + \t}\n    + \n    +-\tif (!check)\n    +-\t\tcheck = attr_check_initl(\"merge\", \"conflict-marker-size\", NULL);\n    +-\n    + \tgit_check_attr(istate, path, check);\n    + \tll_driver_name = check->items[0].value;\n    + \tif (check->items[1].value) {\n    +\n    + ## ll-merge.h ##\n    +@@ ll-merge.h: int ll_merge(mmbuffer_t *result_buf,\n    + \t     const struct ll_merge_options *opts);\n    + \n    + int ll_merge_marker_size(struct index_state *istate, const char *path);\n    ++void reset_merge_attributes(void);\n    + \n    + #endif\n    +\n      ## t/t3400-rebase.sh ##\n     @@ t/t3400-rebase.sh: test_expect_success 'rebase --am and --show-current-patch' '\n      \t)\n    @@ t/t3400-rebase.sh: test_expect_success 'rebase --am and --show-current-patch' '\n      test_expect_success 'rebase--merge.sh and --show-current-patch' '\n      \ttest_create_repo conflict-merge &&\n      \t(\n    +\n    + ## t/t4150-am.sh ##\n    +@@ t/t4150-am.sh: test_expect_success 'am --quit keeps HEAD where it is' '\n    + \ttest_cmp expected actual\n    + '\n    + \n    ++test_expect_success 'am and .gitattibutes' '\n    ++\ttest_create_repo attributes &&\n    ++\t(\n    ++\t\tcd attributes &&\n    ++\t\ttest_commit init &&\n    ++\t\tgit config filter.test.clean \"sed -e '\\''s/smudged/clean/g'\\''\" &&\n    ++\t\tgit config filter.test.smudge \"sed -e '\\''s/clean/smudged/g'\\''\" &&\n    ++\n    ++\t\ttest_commit second &&\n    ++\t\tgit checkout -b test HEAD^ &&\n    ++\n    ++\t\techo \"*.txt filter=test conflict-marker-size=10\" >.gitattributes &&\n    ++\t\tgit add .gitattributes &&\n    ++\t\ttest_commit third &&\n    ++\n    ++\t\techo \"This text is smudged.\" >a.txt &&\n    ++\t\tgit add a.txt &&\n    ++\t\ttest_commit fourth &&\n    ++\n    ++\t\tgit checkout -b removal HEAD^ &&\n    ++\t\tgit rm .gitattributes &&\n    ++\t\tgit add -u &&\n    ++\t\ttest_commit fifth &&\n    ++\t\tgit cherry-pick test &&\n    ++\n    ++\t\tgit checkout -b conflict third &&\n    ++\t\techo \"This text is different.\" >a.txt &&\n    ++\t\tgit add a.txt &&\n    ++\t\ttest_commit sixth &&\n    ++\n    ++\t\tgit checkout test &&\n    ++\t\tgit format-patch --stdout master..HEAD >patches &&\n    ++\t\tgit reset --hard master &&\n    ++\t\tgit am patches &&\n    ++\t\tgrep \"smudged\" a.txt &&\n    ++\n    ++\t\tgit checkout removal &&\n    ++\t\tgit reset --hard &&\n    ++\t\tgit format-patch --stdout master..HEAD >patches &&\n    ++\t\tgit reset --hard master &&\n    ++\t\tgit am patches &&\n    ++\t\tgrep \"clean\" a.txt &&\n    ++\n    ++\t\tgit checkout conflict &&\n    ++\t\tgit reset --hard &&\n    ++\t\tgit format-patch --stdout master..HEAD >patches &&\n    ++\t\tgit reset --hard fourth &&\n    ++\t\ttest_must_fail git am -3 patches &&\n    ++\t\tgrep \"<<<<<<<<<<\" a.txt\n    ++\t)\n    ++'\n    ++\n    + test_done\n"},{"id":"381133","messageId":"20190825233340.10894-2-sandals@crustytoothpaste.net","threadId":"51734","inReplyTo":"20190825233340.10894-1-sandals@crustytoothpaste.net","subject":"[PATCH v5 1/2] path: add a function to check for path suffix","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-08-25T23:33:39Z","receivedAt":"2019-08-25T23:34:03Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We have a function to strip the path suffix from a commit, but we don't\nhave one to check for a path suffix. For a plain filename, we can use\nbasename, but that requires an allocation, since POSIX allows it to\nmodify its argument. Refactor strip_path_suffix into a helper function\nand a new function, ends_with_path_components, to meet this need.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n path.c | 39 ++++++++++++++++++++++++++++++---------\n path.h |  3 +++\n 2 files changed, 33 insertions(+), 9 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex 25e97b8c3f..e3da1f3c4e 100644\n--- a/path.c\n+++ b/path.c\n@@ -1221,31 +1221,52 @@ static inline int chomp_trailing_dir_sep(const char *path, int len)\n }\n \n /*\n- * If path ends with suffix (complete path components), returns the\n- * part before suffix (sans trailing directory separators).\n- * Otherwise returns NULL.\n+ * If path ends with suffix (complete path components), returns the offset of\n+ * the last character in the path before the suffix (sans trailing directory\n+ * separators), and -1 otherwise.\n  */\n-char *strip_path_suffix(const char *path, const char *suffix)\n+static ssize_t stripped_path_suffix_offset(const char *path, const char *suffix)\n {\n \tint path_len = strlen(path), suffix_len = strlen(suffix);\n \n \twhile (suffix_len) {\n \t\tif (!path_len)\n-\t\t\treturn NULL;\n+\t\t\treturn -1;\n \n \t\tif (is_dir_sep(path[path_len - 1])) {\n \t\t\tif (!is_dir_sep(suffix[suffix_len - 1]))\n-\t\t\t\treturn NULL;\n+\t\t\t\treturn -1;\n \t\t\tpath_len = chomp_trailing_dir_sep(path, path_len);\n \t\t\tsuffix_len = chomp_trailing_dir_sep(suffix, suffix_len);\n \t\t}\n \t\telse if (path[--path_len] != suffix[--suffix_len])\n-\t\t\treturn NULL;\n+\t\t\treturn -1;\n \t}\n \n \tif (path_len && !is_dir_sep(path[path_len - 1]))\n-\t\treturn NULL;\n-\treturn xstrndup(path, chomp_trailing_dir_sep(path, path_len));\n+\t\treturn -1;\n+\treturn chomp_trailing_dir_sep(path, path_len);\n+}\n+\n+/*\n+ * Returns true if the path ends with components, considering only complete path\n+ * components, and false otherwise.\n+ */\n+int ends_with_path_components(const char *path, const char *components)\n+{\n+\treturn stripped_path_suffix_offset(path, components) != -1;\n+}\n+\n+/*\n+ * If path ends with suffix (complete path components), returns the\n+ * part before suffix (sans trailing directory separators).\n+ * Otherwise returns NULL.\n+ */\n+char *strip_path_suffix(const char *path, const char *suffix)\n+{\n+\tssize_t offset = stripped_path_suffix_offset(path, suffix);\n+\n+\treturn offset == -1 ? NULL : xstrndup(path, offset);\n }\n \n int daemon_avoid_alias(const char *p)\ndiff --git a/path.h b/path.h\nindex 2ba6ca58c8..14d6dcad16 100644\n--- a/path.h\n+++ b/path.h\n@@ -193,4 +193,7 @@ const char *git_path_merge_head(struct repository *r);\n const char *git_path_fetch_head(struct repository *r);\n const char *git_path_shallow(struct repository *r);\n \n+\n+int ends_with_path_components(const char *path, const char *components);\n+\n #endif /* PATH_H */\n"},{"id":"381134","messageId":"20190825233340.10894-3-sandals@crustytoothpaste.net","threadId":"51734","inReplyTo":"20190825233340.10894-1-sandals@crustytoothpaste.net","subject":"[PATCH v5 2/2] am: reload .gitattributes after patching it","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-08-25T23:33:40Z","receivedAt":"2019-08-25T23:34:10Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When applying multiple patches with git am, or when rebasing using the\nam backend, it's possible that one of our patches has updated a\ngitattributes file. Currently, we cache this information, so if a\nfile in a subsequent patch has attributes applied, the file will be\nwritten out with the attributes in place as of the time we started the\nrebase or am operation, not with the attributes applied by the previous\npatch. This problem does not occur when using the -m or -i flags to\nrebase.\n\nTo ensure we write the correct data into the working tree, expire the\ncache after each patch that touches a path ending in \".gitattributes\".\nSince we load these attributes in multiple separate files, we must\nexpire them accordingly.\n\nVerify that both the am and rebase code paths work correctly, including\nthe conflict marker size with am -3.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n apply.c           | 11 ++++++++++\n convert.c         | 11 +++++++++-\n convert.h         |  6 ++++++\n ll-merge.c        | 19 +++++++++++++----\n ll-merge.h        |  1 +\n t/t3400-rebase.sh | 36 ++++++++++++++++++++++++++++++++\n t/t4150-am.sh     | 52 +++++++++++++++++++++++++++++++++++++++++++++++\n 7 files changed, 131 insertions(+), 5 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex cde95369bb..57a61f2881 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4643,6 +4643,7 @@ static int apply_patch(struct apply_state *state,\n \tstruct patch *list = NULL, **listp = &list;\n \tint skipped_patch = 0;\n \tint res = 0;\n+\tint flush_attributes = 0;\n \n \tstate->patch_input_file = filename;\n \tif (read_patch_file(&buf, fd) < 0)\n@@ -4670,6 +4671,14 @@ static int apply_patch(struct apply_state *state,\n \t\t\tpatch_stats(state, patch);\n \t\t\t*listp = patch;\n \t\t\tlistp = &patch->next;\n+\n+\t\t\tif ((patch->new_name &&\n+\t\t\t     ends_with_path_components(patch->new_name,\n+\t\t\t\t\t\t       GITATTRIBUTES_FILE)) ||\n+\t\t\t    (patch->old_name &&\n+\t\t\t     ends_with_path_components(patch->old_name,\n+\t\t\t\t\t\t       GITATTRIBUTES_FILE)))\n+\t\t\t\tflush_attributes = 1;\n \t\t}\n \t\telse {\n \t\t\tif (state->apply_verbosity > verbosity_normal)\n@@ -4746,6 +4755,8 @@ static int apply_patch(struct apply_state *state,\n \tif (state->summary && state->apply_verbosity > verbosity_silent)\n \t\tsummary_patch_list(list);\n \n+\tif (flush_attributes)\n+\t\treset_parsed_attributes();\n end:\n \tfree_patch_list(list);\n \tstrbuf_release(&buf);\ndiff --git a/convert.c b/convert.c\nindex 94ff837649..0e6e9d2d75 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -8,6 +8,7 @@\n #include \"pkt-line.h\"\n #include \"sub-process.h\"\n #include \"utf8.h\"\n+#include \"ll-merge.h\"\n \n /*\n  * convert.c - convert a file when checking it out and checking it in.\n@@ -1293,10 +1294,11 @@ struct conv_attrs {\n \tconst char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n };\n \n+static struct attr_check *check;\n+\n static void convert_attrs(const struct index_state *istate,\n \t\t\t  struct conv_attrs *ca, const char *path)\n {\n-\tstatic struct attr_check *check;\n \tstruct attr_check_item *ccheck = NULL;\n \n \tif (!check) {\n@@ -1339,6 +1341,13 @@ static void convert_attrs(const struct index_state *istate,\n \t\tca->crlf_action = CRLF_AUTO_INPUT;\n }\n \n+void reset_parsed_attributes(void)\n+{\n+\tattr_check_free(check);\n+\tcheck = NULL;\n+\treset_merge_attributes();\n+}\n+\n int would_convert_to_git_filter_fd(const struct index_state *istate, const char *path)\n {\n \tstruct conv_attrs ca;\ndiff --git a/convert.h b/convert.h\nindex 831559f10d..3710969d43 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -94,6 +94,12 @@ void convert_to_git_filter_fd(const struct index_state *istate,\n int would_convert_to_git_filter_fd(const struct index_state *istate,\n \t\t\t\t   const char *path);\n \n+/*\n+ * Reset the internal list of attributes used by convert_to_git and\n+ * convert_to_working_tree.\n+ */\n+void reset_parsed_attributes(void);\n+\n /*****************************************************************\n  *\n  * Streaming conversion support\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 5b8d46aede..d65a8971db 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -32,6 +32,20 @@ struct ll_merge_driver {\n \tchar *cmdline;\n };\n \n+static struct attr_check *merge_attributes;\n+static struct attr_check *load_merge_attributes(void)\n+{\n+\tif (!merge_attributes)\n+\t\tmerge_attributes = attr_check_initl(\"merge\", \"conflict-marker-size\", NULL);\n+\treturn merge_attributes;\n+}\n+\n+void reset_merge_attributes(void)\n+{\n+\tattr_check_free(merge_attributes);\n+\tmerge_attributes = NULL;\n+}\n+\n /*\n  * Built-in low-levels\n  */\n@@ -354,7 +368,7 @@ int ll_merge(mmbuffer_t *result_buf,\n \t     struct index_state *istate,\n \t     const struct ll_merge_options *opts)\n {\n-\tstatic struct attr_check *check;\n+\tstruct attr_check *check = load_merge_attributes();\n \tstatic const struct ll_merge_options default_opts;\n \tconst char *ll_driver_name = NULL;\n \tint marker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n@@ -369,9 +383,6 @@ int ll_merge(mmbuffer_t *result_buf,\n \t\tnormalize_file(theirs, path, istate);\n \t}\n \n-\tif (!check)\n-\t\tcheck = attr_check_initl(\"merge\", \"conflict-marker-size\", NULL);\n-\n \tgit_check_attr(istate, path, check);\n \tll_driver_name = check->items[0].value;\n \tif (check->items[1].value) {\ndiff --git a/ll-merge.h b/ll-merge.h\nindex b9e2af1c88..e78973dd55 100644\n--- a/ll-merge.h\n+++ b/ll-merge.h\n@@ -26,5 +26,6 @@ int ll_merge(mmbuffer_t *result_buf,\n \t     const struct ll_merge_options *opts);\n \n int ll_merge_marker_size(struct index_state *istate, const char *path);\n+void reset_merge_attributes(void);\n \n #endif\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 80b23fd326..23469cc789 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -301,6 +301,42 @@ test_expect_success 'rebase --am and --show-current-patch' '\n \t)\n '\n \n+test_expect_success 'rebase --am and .gitattributes' '\n+\ttest_create_repo attributes &&\n+\t(\n+\t\tcd attributes &&\n+\t\ttest_commit init &&\n+\t\tgit config filter.test.clean \"sed -e '\\''s/smudged/clean/g'\\''\" &&\n+\t\tgit config filter.test.smudge \"sed -e '\\''s/clean/smudged/g'\\''\" &&\n+\n+\t\ttest_commit second &&\n+\t\tgit checkout -b test HEAD^ &&\n+\n+\t\techo \"*.txt filter=test\" >.gitattributes &&\n+\t\tgit add .gitattributes &&\n+\t\ttest_commit third &&\n+\n+\t\techo \"This text is smudged.\" >a.txt &&\n+\t\tgit add a.txt &&\n+\t\ttest_commit fourth &&\n+\n+\t\tgit checkout -b removal HEAD^ &&\n+\t\tgit rm .gitattributes &&\n+\t\tgit add -u &&\n+\t\ttest_commit fifth &&\n+\t\tgit cherry-pick test &&\n+\n+\t\tgit checkout test &&\n+\t\tgit rebase master &&\n+\t\tgrep \"smudged\" a.txt &&\n+\n+\t\tgit checkout removal &&\n+\t\tgit reset --hard &&\n+\t\tgit rebase master &&\n+\t\tgrep \"clean\" a.txt\n+\t)\n+'\n+\n test_expect_success 'rebase--merge.sh and --show-current-patch' '\n \ttest_create_repo conflict-merge &&\n \t(\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 3f7f750cc8..4f1e24ecbe 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -1061,4 +1061,56 @@ test_expect_success 'am --quit keeps HEAD where it is' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'am and .gitattibutes' '\n+\ttest_create_repo attributes &&\n+\t(\n+\t\tcd attributes &&\n+\t\ttest_commit init &&\n+\t\tgit config filter.test.clean \"sed -e '\\''s/smudged/clean/g'\\''\" &&\n+\t\tgit config filter.test.smudge \"sed -e '\\''s/clean/smudged/g'\\''\" &&\n+\n+\t\ttest_commit second &&\n+\t\tgit checkout -b test HEAD^ &&\n+\n+\t\techo \"*.txt filter=test conflict-marker-size=10\" >.gitattributes &&\n+\t\tgit add .gitattributes &&\n+\t\ttest_commit third &&\n+\n+\t\techo \"This text is smudged.\" >a.txt &&\n+\t\tgit add a.txt &&\n+\t\ttest_commit fourth &&\n+\n+\t\tgit checkout -b removal HEAD^ &&\n+\t\tgit rm .gitattributes &&\n+\t\tgit add -u &&\n+\t\ttest_commit fifth &&\n+\t\tgit cherry-pick test &&\n+\n+\t\tgit checkout -b conflict third &&\n+\t\techo \"This text is different.\" >a.txt &&\n+\t\tgit add a.txt &&\n+\t\ttest_commit sixth &&\n+\n+\t\tgit checkout test &&\n+\t\tgit format-patch --stdout master..HEAD >patches &&\n+\t\tgit reset --hard master &&\n+\t\tgit am patches &&\n+\t\tgrep \"smudged\" a.txt &&\n+\n+\t\tgit checkout removal &&\n+\t\tgit reset --hard &&\n+\t\tgit format-patch --stdout master..HEAD >patches &&\n+\t\tgit reset --hard master &&\n+\t\tgit am patches &&\n+\t\tgrep \"clean\" a.txt &&\n+\n+\t\tgit checkout conflict &&\n+\t\tgit reset --hard &&\n+\t\tgit format-patch --stdout master..HEAD >patches &&\n+\t\tgit reset --hard fourth &&\n+\t\ttest_must_fail git am -3 patches &&\n+\t\tgrep \"<<<<<<<<<<\" a.txt\n+\t)\n+'\n+\n test_done\n"},{"id":"381188","messageId":"6ad26ef8-fc94-726a-2243-4c3ff9717ad3@gmail.com","threadId":"51734","inReplyTo":"20190825233340.10894-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v5 0/2] Honor .gitattributes with rebase --am","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-08-26T15:11:08Z","receivedAt":"2019-08-26T15:11:15Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Brian\n\nOn 26/08/2019 00:33, brian m. carlson wrote:\n> This series makes rebase --am honor the .gitattributes file for\n> subsequent patches when a patch changes it.\n> \n> Note that there are two places we load attributes in ll-merge.c, but\n> this code only handles the one that git am uses.  The two cannot be\n> unified because the one in ll_merge_marker_size intentionally doesn't\n> load the merge attribute, since it wants to always use the recursive\n> strategy.  Loading it anyway causes t4017 to fail.\n> \n> Changes from v4:\n> * Wrap lines in apply.c.\n> * Handle merge and conflict-marker-size attributes.\n> * Add tests for am and am -3 in addition to rebase.\n\nI've only had time for a quick look at these but the changes look good \nto me. Thanks for taking the time to add the tests for am and the code \nfor handling the merge attributes\n\nBest Wishes\n\nPhillip\n\n> Changes from v3:\n> * Check for both addition and removal of .gitattributes files.\n> * Switch from \"test_config\" to \"git config\".\n> \n> Changes from v2:\n> * Rename has_path_suffix to ends_with_path_components.\n> \n> Changes from v1:\n> * Add has_path_suffix in a separate commit.\n> \n> brian m. carlson (2):\n>    path: add a function to check for path suffix\n>    am: reload .gitattributes after patching it\n> \n>   apply.c           | 11 ++++++++++\n>   convert.c         | 11 +++++++++-\n>   convert.h         |  6 ++++++\n>   ll-merge.c        | 19 +++++++++++++----\n>   ll-merge.h        |  1 +\n>   path.c            | 39 +++++++++++++++++++++++++++--------\n>   path.h            |  3 +++\n>   t/t3400-rebase.sh | 36 ++++++++++++++++++++++++++++++++\n>   t/t4150-am.sh     | 52 +++++++++++++++++++++++++++++++++++++++++++++++\n>   9 files changed, 164 insertions(+), 14 deletions(-)\n> \n> Range-diff against v4:\n> 1:  fa825e4b40 ! 1:  2077a0829e apply: reload .gitattributes after patching it\n>      @@ Metadata\n>       Author: brian m. carlson <sandals@crustytoothpaste.net>\n>       \n>        ## Commit message ##\n>      -    apply: reload .gitattributes after patching it\n>      +    am: reload .gitattributes after patching it\n>       \n>           When applying multiple patches with git am, or when rebasing using the\n>           am backend, it's possible that one of our patches has updated a\n>      @@ Commit message\n>       \n>           To ensure we write the correct data into the working tree, expire the\n>           cache after each patch that touches a path ending in \".gitattributes\".\n>      +    Since we load these attributes in multiple separate files, we must\n>      +    expire them accordingly.\n>      +\n>      +    Verify that both the am and rebase code paths work correctly, including\n>      +    the conflict marker size with am -3.\n>       \n>           Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n>       \n>      @@ apply.c: static int apply_patch(struct apply_state *state,\n>        \t\t\t*listp = patch;\n>        \t\t\tlistp = &patch->next;\n>       +\n>      -+\t\t\tif ((patch->new_name && ends_with_path_components(patch->new_name, GITATTRIBUTES_FILE)) ||\n>      -+\t\t\t    (patch->old_name && ends_with_path_components(patch->old_name, GITATTRIBUTES_FILE)))\n>      ++\t\t\tif ((patch->new_name &&\n>      ++\t\t\t     ends_with_path_components(patch->new_name,\n>      ++\t\t\t\t\t\t       GITATTRIBUTES_FILE)) ||\n>      ++\t\t\t    (patch->old_name &&\n>      ++\t\t\t     ends_with_path_components(patch->old_name,\n>      ++\t\t\t\t\t\t       GITATTRIBUTES_FILE)))\n>       +\t\t\t\tflush_attributes = 1;\n>        \t\t}\n>        \t\telse {\n>      @@ apply.c: static int apply_patch(struct apply_state *state,\n>        \tstrbuf_release(&buf);\n>       \n>        ## convert.c ##\n>      +@@\n>      + #include \"pkt-line.h\"\n>      + #include \"sub-process.h\"\n>      + #include \"utf8.h\"\n>      ++#include \"ll-merge.h\"\n>      +\n>      + /*\n>      +  * convert.c - convert a file when checking it out and checking it in.\n>       @@ convert.c: struct conv_attrs {\n>        \tconst char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n>        };\n>      @@ convert.c: static void convert_attrs(const struct index_state *istate,\n>       +{\n>       +\tattr_check_free(check);\n>       +\tcheck = NULL;\n>      ++\treset_merge_attributes();\n>       +}\n>       +\n>        int would_convert_to_git_filter_fd(const struct index_state *istate, const char *path)\n>      @@ convert.h: void convert_to_git_filter_fd(const struct index_state *istate,\n>         *\n>         * Streaming conversion support\n>       \n>      + ## ll-merge.c ##\n>      +@@ ll-merge.c: struct ll_merge_driver {\n>      + \tchar *cmdline;\n>      + };\n>      +\n>      ++static struct attr_check *merge_attributes;\n>      ++static struct attr_check *load_merge_attributes(void)\n>      ++{\n>      ++\tif (!merge_attributes)\n>      ++\t\tmerge_attributes = attr_check_initl(\"merge\", \"conflict-marker-size\", NULL);\n>      ++\treturn merge_attributes;\n>      ++}\n>      ++\n>      ++void reset_merge_attributes(void)\n>      ++{\n>      ++\tattr_check_free(merge_attributes);\n>      ++\tmerge_attributes = NULL;\n>      ++}\n>      ++\n>      + /*\n>      +  * Built-in low-levels\n>      +  */\n>      +@@ ll-merge.c: int ll_merge(mmbuffer_t *result_buf,\n>      + \t     struct index_state *istate,\n>      + \t     const struct ll_merge_options *opts)\n>      + {\n>      +-\tstatic struct attr_check *check;\n>      ++\tstruct attr_check *check = load_merge_attributes();\n>      + \tstatic const struct ll_merge_options default_opts;\n>      + \tconst char *ll_driver_name = NULL;\n>      + \tint marker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n>      +@@ ll-merge.c: int ll_merge(mmbuffer_t *result_buf,\n>      + \t\tnormalize_file(theirs, path, istate);\n>      + \t}\n>      +\n>      +-\tif (!check)\n>      +-\t\tcheck = attr_check_initl(\"merge\", \"conflict-marker-size\", NULL);\n>      +-\n>      + \tgit_check_attr(istate, path, check);\n>      + \tll_driver_name = check->items[0].value;\n>      + \tif (check->items[1].value) {\n>      +\n>      + ## ll-merge.h ##\n>      +@@ ll-merge.h: int ll_merge(mmbuffer_t *result_buf,\n>      + \t     const struct ll_merge_options *opts);\n>      +\n>      + int ll_merge_marker_size(struct index_state *istate, const char *path);\n>      ++void reset_merge_attributes(void);\n>      +\n>      + #endif\n>      +\n>        ## t/t3400-rebase.sh ##\n>       @@ t/t3400-rebase.sh: test_expect_success 'rebase --am and --show-current-patch' '\n>        \t)\n>      @@ t/t3400-rebase.sh: test_expect_success 'rebase --am and --show-current-patch' '\n>        test_expect_success 'rebase--merge.sh and --show-current-patch' '\n>        \ttest_create_repo conflict-merge &&\n>        \t(\n>      +\n>      + ## t/t4150-am.sh ##\n>      +@@ t/t4150-am.sh: test_expect_success 'am --quit keeps HEAD where it is' '\n>      + \ttest_cmp expected actual\n>      + '\n>      +\n>      ++test_expect_success 'am and .gitattibutes' '\n>      ++\ttest_create_repo attributes &&\n>      ++\t(\n>      ++\t\tcd attributes &&\n>      ++\t\ttest_commit init &&\n>      ++\t\tgit config filter.test.clean \"sed -e '\\''s/smudged/clean/g'\\''\" &&\n>      ++\t\tgit config filter.test.smudge \"sed -e '\\''s/clean/smudged/g'\\''\" &&\n>      ++\n>      ++\t\ttest_commit second &&\n>      ++\t\tgit checkout -b test HEAD^ &&\n>      ++\n>      ++\t\techo \"*.txt filter=test conflict-marker-size=10\" >.gitattributes &&\n>      ++\t\tgit add .gitattributes &&\n>      ++\t\ttest_commit third &&\n>      ++\n>      ++\t\techo \"This text is smudged.\" >a.txt &&\n>      ++\t\tgit add a.txt &&\n>      ++\t\ttest_commit fourth &&\n>      ++\n>      ++\t\tgit checkout -b removal HEAD^ &&\n>      ++\t\tgit rm .gitattributes &&\n>      ++\t\tgit add -u &&\n>      ++\t\ttest_commit fifth &&\n>      ++\t\tgit cherry-pick test &&\n>      ++\n>      ++\t\tgit checkout -b conflict third &&\n>      ++\t\techo \"This text is different.\" >a.txt &&\n>      ++\t\tgit add a.txt &&\n>      ++\t\ttest_commit sixth &&\n>      ++\n>      ++\t\tgit checkout test &&\n>      ++\t\tgit format-patch --stdout master..HEAD >patches &&\n>      ++\t\tgit reset --hard master &&\n>      ++\t\tgit am patches &&\n>      ++\t\tgrep \"smudged\" a.txt &&\n>      ++\n>      ++\t\tgit checkout removal &&\n>      ++\t\tgit reset --hard &&\n>      ++\t\tgit format-patch --stdout master..HEAD >patches &&\n>      ++\t\tgit reset --hard master &&\n>      ++\t\tgit am patches &&\n>      ++\t\tgrep \"clean\" a.txt &&\n>      ++\n>      ++\t\tgit checkout conflict &&\n>      ++\t\tgit reset --hard &&\n>      ++\t\tgit format-patch --stdout master..HEAD >patches &&\n>      ++\t\tgit reset --hard fourth &&\n>      ++\t\ttest_must_fail git am -3 patches &&\n>      ++\t\tgrep \"<<<<<<<<<<\" a.txt\n>      ++\t)\n>      ++'\n>      ++\n>      + test_done\n> \n"},{"id":"381434","messageId":"nycvar.QRO.7.76.6.1908281327180.46@tvgsbejvaqbjf.bet","threadId":"51734","inReplyTo":"20190825233340.10894-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH v5 2/2] am: reload .gitattributes after patching it","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-08-28T11:30:53Z","receivedAt":"2019-08-28T11:31:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi brian,\n\n[chiming in from the peanut gallery; if my comments don't make any\nsense, please do feel free to completely ignore me.]\n\nOn Sun, 25 Aug 2019, brian m. carlson wrote:\n\n> diff --git a/convert.c b/convert.c\n> index 94ff837649..0e6e9d2d75 100644\n> --- a/convert.c\n> +++ b/convert.c\n> @@ -8,6 +8,7 @@\n>  #include \"pkt-line.h\"\n>  #include \"sub-process.h\"\n>  #include \"utf8.h\"\n> +#include \"ll-merge.h\"\n>\n>  /*\n>   * convert.c - convert a file when checking it out and checking it in.\n> @@ -1293,10 +1294,11 @@ struct conv_attrs {\n>  \tconst char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n>  };\n>\n> +static struct attr_check *check;\n> +\n>  static void convert_attrs(const struct index_state *istate,\n>  \t\t\t  struct conv_attrs *ca, const char *path)\n>  {\n> -\tstatic struct attr_check *check;\n>  \tstruct attr_check_item *ccheck = NULL;\n>\n>  \tif (!check) {\n\nAfter this line:\n\n                check = attr_check_initl(\"crlf\", \"ident\", \"filter\",\n\t\t\t\t\"eol\", \"text\", \"working-tree-encoding\",\n\t\t\t\tNULL);\n\t\tuser_convert_tail = &user_convert;\n\t\tgit_config(read_convert_config, NULL);\n\t}\n\nI am a bit worried about `user_convert`: it seems never to be re-set.\n\nAlso, how thread-safe do we need `reset_parsed_attributes()` to be?\n\nCiao,\nDscho\n"},{"id":"381550","messageId":"20190829230928.GF11334@genre.crustytoothpaste.net","threadId":"51734","inReplyTo":"nycvar.QRO.7.76.6.1908281327180.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v5 2/2] am: reload .gitattributes after patching it","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-08-29T23:09:28Z","receivedAt":"2019-08-29T23:09:37Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-08-28 at 11:30:53, Johannes Schindelin wrote:\n> > diff --git a/convert.c b/convert.c\n> > index 94ff837649..0e6e9d2d75 100644\n> > --- a/convert.c\n> > +++ b/convert.c\n> > @@ -8,6 +8,7 @@\n> >  #include \"pkt-line.h\"\n> >  #include \"sub-process.h\"\n> >  #include \"utf8.h\"\n> > +#include \"ll-merge.h\"\n> >\n> >  /*\n> >   * convert.c - convert a file when checking it out and checking it in.\n> > @@ -1293,10 +1294,11 @@ struct conv_attrs {\n> >  \tconst char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n> >  };\n> >\n> > +static struct attr_check *check;\n> > +\n> >  static void convert_attrs(const struct index_state *istate,\n> >  \t\t\t  struct conv_attrs *ca, const char *path)\n> >  {\n> > -\tstatic struct attr_check *check;\n> >  \tstruct attr_check_item *ccheck = NULL;\n> >\n> >  \tif (!check) {\n> \n> After this line:\n> \n>                 check = attr_check_initl(\"crlf\", \"ident\", \"filter\",\n> \t\t\t\t\"eol\", \"text\", \"working-tree-encoding\",\n> \t\t\t\tNULL);\n> \t\tuser_convert_tail = &user_convert;\n> \t\tgit_config(read_convert_config, NULL);\n> \t}\n> \n> I am a bit worried about `user_convert`: it seems never to be re-set.\n\nYeah, it looks like I'll need to reset that as well.  The only\nconsequence is that we leak a small amount of memory if there are filter\nattributes, but it's better to avoid that leak if we can.\n\n> Also, how thread-safe do we need `reset_parsed_attributes()` to be?\n\nSince patch application isn't thread safe, it doesn't need to be\nthread safe at all.  The original wasn't thread safe, either, since it\nused a static variable without a mutex.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"381604","messageId":"nycvar.QRO.7.76.6.1908302145270.46@tvgsbejvaqbjf.bet","threadId":"51734","inReplyTo":"20190829230928.GF11334@genre.crustytoothpaste.net","subject":"Re: [PATCH v5 2/2] am: reload .gitattributes after patching it","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-08-30T19:46:04Z","receivedAt":"2019-08-30T19:46:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi brian,\n\nOn Thu, 29 Aug 2019, brian m. carlson wrote:\n\n> On 2019-08-28 at 11:30:53, Johannes Schindelin wrote:\n> > > diff --git a/convert.c b/convert.c\n> > > index 94ff837649..0e6e9d2d75 100644\n> > > --- a/convert.c\n> > > +++ b/convert.c\n> > > @@ -8,6 +8,7 @@\n> > >  #include \"pkt-line.h\"\n> > >  #include \"sub-process.h\"\n> > >  #include \"utf8.h\"\n> > > +#include \"ll-merge.h\"\n> > >\n> > >  /*\n> > >   * convert.c - convert a file when checking it out and checking it in.\n> > > @@ -1293,10 +1294,11 @@ struct conv_attrs {\n> > >  \tconst char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n> > >  };\n> > >\n> > > +static struct attr_check *check;\n> > > +\n> > >  static void convert_attrs(const struct index_state *istate,\n> > >  \t\t\t  struct conv_attrs *ca, const char *path)\n> > >  {\n> > > -\tstatic struct attr_check *check;\n> > >  \tstruct attr_check_item *ccheck = NULL;\n> > >\n> > >  \tif (!check) {\n> >\n> > After this line:\n> >\n> >                 check = attr_check_initl(\"crlf\", \"ident\", \"filter\",\n> > \t\t\t\t\"eol\", \"text\", \"working-tree-encoding\",\n> > \t\t\t\tNULL);\n> > \t\tuser_convert_tail = &user_convert;\n> > \t\tgit_config(read_convert_config, NULL);\n> > \t}\n> >\n> > I am a bit worried about `user_convert`: it seems never to be re-set.\n>\n> Yeah, it looks like I'll need to reset that as well.  The only\n> consequence is that we leak a small amount of memory if there are filter\n> attributes, but it's better to avoid that leak if we can.\n\nOkay, good, so my suggestion was not so completely off the mark.\n>\n> > Also, how thread-safe do we need `reset_parsed_attributes()` to be?\n>\n> Since patch application isn't thread safe, it doesn't need to be\n> thread safe at all.  The original wasn't thread safe, either, since it\n> used a static variable without a mutex.\n\nThank you for clarifying! Much appreciated.\n\nCiao,\nDscho\n"},{"id":"381693","messageId":"20190902223944.897504-1-sandals@crustytoothpaste.net","threadId":"51734","inReplyTo":"20190825233340.10894-1-sandals@crustytoothpaste.net","subject":"[PATCH v6 0/2] Honor .gitattributes with rebase --am","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-09-02T22:39:42Z","receivedAt":"2019-09-02T22:39:54Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This series makes rebase --am honor the .gitattributes file for\nsubsequent patches when a patch changes it.\n\nChanges from v5:\n* Avoid leaking memory from already parsed filter rules.\n\nChanges from v4:\n* Wrap lines in apply.c.\n* Handle merge and conflict-marker-size attributes.\n* Add tests for am and am -3 in addition to rebase.\n\nChanges from v3:\n* Check for both addition and removal of .gitattributes files.\n* Switch from \"test_config\" to \"git config\".\n\nChanges from v2:\n* Rename has_path_suffix to ends_with_path_components.\n\nChanges from v1:\n* Add has_path_suffix in a separate commit.\n\nbrian m. carlson (2):\n  path: add a function to check for path suffix\n  am: reload .gitattributes after patching it\n\n apply.c           | 11 ++++++++++\n convert.c         | 21 ++++++++++++++++++-\n convert.h         |  6 ++++++\n ll-merge.c        | 19 +++++++++++++----\n ll-merge.h        |  1 +\n path.c            | 39 +++++++++++++++++++++++++++--------\n path.h            |  3 +++\n t/t3400-rebase.sh | 36 ++++++++++++++++++++++++++++++++\n t/t4150-am.sh     | 52 +++++++++++++++++++++++++++++++++++++++++++++++\n 9 files changed, 174 insertions(+), 14 deletions(-)\n\nRange-diff against v5:\n1:  2077a0829e ! 1:  1573fbd82d am: reload .gitattributes after patching it\n    @@ convert.c: static void convert_attrs(const struct index_state *istate,\n      \n     +void reset_parsed_attributes(void)\n     +{\n    ++\tstruct convert_driver *drv, *next;\n    ++\n     +\tattr_check_free(check);\n     +\tcheck = NULL;\n     +\treset_merge_attributes();\n    ++\n    ++\tfor (drv = user_convert; drv; drv = next) {\n    ++\t\tnext = drv->next;\n    ++\t\tfree((void *)drv->name);\n    ++\t\tfree(drv);\n    ++\t}\n    ++\tuser_convert = NULL;\n    ++\tuser_convert_tail = NULL;\n     +}\n     +\n      int would_convert_to_git_filter_fd(const struct index_state *istate, const char *path)\n"},{"id":"381694","messageId":"20190902223944.897504-2-sandals@crustytoothpaste.net","threadId":"51734","inReplyTo":"20190902223944.897504-1-sandals@crustytoothpaste.net","subject":"[PATCH v6 1/2] path: add a function to check for path suffix","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-09-02T22:39:43Z","receivedAt":"2019-09-02T22:39:57Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We have a function to strip the path suffix from a commit, but we don't\nhave one to check for a path suffix. For a plain filename, we can use\nbasename, but that requires an allocation, since POSIX allows it to\nmodify its argument. Refactor strip_path_suffix into a helper function\nand a new function, ends_with_path_components, to meet this need.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n path.c | 39 ++++++++++++++++++++++++++++++---------\n path.h |  3 +++\n 2 files changed, 33 insertions(+), 9 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex 25e97b8c3f..e3da1f3c4e 100644\n--- a/path.c\n+++ b/path.c\n@@ -1221,31 +1221,52 @@ static inline int chomp_trailing_dir_sep(const char *path, int len)\n }\n \n /*\n- * If path ends with suffix (complete path components), returns the\n- * part before suffix (sans trailing directory separators).\n- * Otherwise returns NULL.\n+ * If path ends with suffix (complete path components), returns the offset of\n+ * the last character in the path before the suffix (sans trailing directory\n+ * separators), and -1 otherwise.\n  */\n-char *strip_path_suffix(const char *path, const char *suffix)\n+static ssize_t stripped_path_suffix_offset(const char *path, const char *suffix)\n {\n \tint path_len = strlen(path), suffix_len = strlen(suffix);\n \n \twhile (suffix_len) {\n \t\tif (!path_len)\n-\t\t\treturn NULL;\n+\t\t\treturn -1;\n \n \t\tif (is_dir_sep(path[path_len - 1])) {\n \t\t\tif (!is_dir_sep(suffix[suffix_len - 1]))\n-\t\t\t\treturn NULL;\n+\t\t\t\treturn -1;\n \t\t\tpath_len = chomp_trailing_dir_sep(path, path_len);\n \t\t\tsuffix_len = chomp_trailing_dir_sep(suffix, suffix_len);\n \t\t}\n \t\telse if (path[--path_len] != suffix[--suffix_len])\n-\t\t\treturn NULL;\n+\t\t\treturn -1;\n \t}\n \n \tif (path_len && !is_dir_sep(path[path_len - 1]))\n-\t\treturn NULL;\n-\treturn xstrndup(path, chomp_trailing_dir_sep(path, path_len));\n+\t\treturn -1;\n+\treturn chomp_trailing_dir_sep(path, path_len);\n+}\n+\n+/*\n+ * Returns true if the path ends with components, considering only complete path\n+ * components, and false otherwise.\n+ */\n+int ends_with_path_components(const char *path, const char *components)\n+{\n+\treturn stripped_path_suffix_offset(path, components) != -1;\n+}\n+\n+/*\n+ * If path ends with suffix (complete path components), returns the\n+ * part before suffix (sans trailing directory separators).\n+ * Otherwise returns NULL.\n+ */\n+char *strip_path_suffix(const char *path, const char *suffix)\n+{\n+\tssize_t offset = stripped_path_suffix_offset(path, suffix);\n+\n+\treturn offset == -1 ? NULL : xstrndup(path, offset);\n }\n \n int daemon_avoid_alias(const char *p)\ndiff --git a/path.h b/path.h\nindex 2ba6ca58c8..14d6dcad16 100644\n--- a/path.h\n+++ b/path.h\n@@ -193,4 +193,7 @@ const char *git_path_merge_head(struct repository *r);\n const char *git_path_fetch_head(struct repository *r);\n const char *git_path_shallow(struct repository *r);\n \n+\n+int ends_with_path_components(const char *path, const char *components);\n+\n #endif /* PATH_H */\n"},{"id":"381695","messageId":"20190902223944.897504-3-sandals@crustytoothpaste.net","threadId":"51734","inReplyTo":"20190902223944.897504-1-sandals@crustytoothpaste.net","subject":"[PATCH v6 2/2] am: reload .gitattributes after patching it","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-09-02T22:39:44Z","receivedAt":"2019-09-02T22:39:58Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When applying multiple patches with git am, or when rebasing using the\nam backend, it's possible that one of our patches has updated a\ngitattributes file. Currently, we cache this information, so if a\nfile in a subsequent patch has attributes applied, the file will be\nwritten out with the attributes in place as of the time we started the\nrebase or am operation, not with the attributes applied by the previous\npatch. This problem does not occur when using the -m or -i flags to\nrebase.\n\nTo ensure we write the correct data into the working tree, expire the\ncache after each patch that touches a path ending in \".gitattributes\".\nSince we load these attributes in multiple separate files, we must\nexpire them accordingly.\n\nVerify that both the am and rebase code paths work correctly, including\nthe conflict marker size with am -3.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n apply.c           | 11 ++++++++++\n convert.c         | 21 ++++++++++++++++++-\n convert.h         |  6 ++++++\n ll-merge.c        | 19 +++++++++++++----\n ll-merge.h        |  1 +\n t/t3400-rebase.sh | 36 ++++++++++++++++++++++++++++++++\n t/t4150-am.sh     | 52 +++++++++++++++++++++++++++++++++++++++++++++++\n 7 files changed, 141 insertions(+), 5 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex cde95369bb..57a61f2881 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4643,6 +4643,7 @@ static int apply_patch(struct apply_state *state,\n \tstruct patch *list = NULL, **listp = &list;\n \tint skipped_patch = 0;\n \tint res = 0;\n+\tint flush_attributes = 0;\n \n \tstate->patch_input_file = filename;\n \tif (read_patch_file(&buf, fd) < 0)\n@@ -4670,6 +4671,14 @@ static int apply_patch(struct apply_state *state,\n \t\t\tpatch_stats(state, patch);\n \t\t\t*listp = patch;\n \t\t\tlistp = &patch->next;\n+\n+\t\t\tif ((patch->new_name &&\n+\t\t\t     ends_with_path_components(patch->new_name,\n+\t\t\t\t\t\t       GITATTRIBUTES_FILE)) ||\n+\t\t\t    (patch->old_name &&\n+\t\t\t     ends_with_path_components(patch->old_name,\n+\t\t\t\t\t\t       GITATTRIBUTES_FILE)))\n+\t\t\t\tflush_attributes = 1;\n \t\t}\n \t\telse {\n \t\t\tif (state->apply_verbosity > verbosity_normal)\n@@ -4746,6 +4755,8 @@ static int apply_patch(struct apply_state *state,\n \tif (state->summary && state->apply_verbosity > verbosity_silent)\n \t\tsummary_patch_list(list);\n \n+\tif (flush_attributes)\n+\t\treset_parsed_attributes();\n end:\n \tfree_patch_list(list);\n \tstrbuf_release(&buf);\ndiff --git a/convert.c b/convert.c\nindex 94ff837649..deb6f71b2d 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -8,6 +8,7 @@\n #include \"pkt-line.h\"\n #include \"sub-process.h\"\n #include \"utf8.h\"\n+#include \"ll-merge.h\"\n \n /*\n  * convert.c - convert a file when checking it out and checking it in.\n@@ -1293,10 +1294,11 @@ struct conv_attrs {\n \tconst char *working_tree_encoding; /* Supported encoding or default encoding if NULL */\n };\n \n+static struct attr_check *check;\n+\n static void convert_attrs(const struct index_state *istate,\n \t\t\t  struct conv_attrs *ca, const char *path)\n {\n-\tstatic struct attr_check *check;\n \tstruct attr_check_item *ccheck = NULL;\n \n \tif (!check) {\n@@ -1339,6 +1341,23 @@ static void convert_attrs(const struct index_state *istate,\n \t\tca->crlf_action = CRLF_AUTO_INPUT;\n }\n \n+void reset_parsed_attributes(void)\n+{\n+\tstruct convert_driver *drv, *next;\n+\n+\tattr_check_free(check);\n+\tcheck = NULL;\n+\treset_merge_attributes();\n+\n+\tfor (drv = user_convert; drv; drv = next) {\n+\t\tnext = drv->next;\n+\t\tfree((void *)drv->name);\n+\t\tfree(drv);\n+\t}\n+\tuser_convert = NULL;\n+\tuser_convert_tail = NULL;\n+}\n+\n int would_convert_to_git_filter_fd(const struct index_state *istate, const char *path)\n {\n \tstruct conv_attrs ca;\ndiff --git a/convert.h b/convert.h\nindex 831559f10d..3710969d43 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -94,6 +94,12 @@ void convert_to_git_filter_fd(const struct index_state *istate,\n int would_convert_to_git_filter_fd(const struct index_state *istate,\n \t\t\t\t   const char *path);\n \n+/*\n+ * Reset the internal list of attributes used by convert_to_git and\n+ * convert_to_working_tree.\n+ */\n+void reset_parsed_attributes(void);\n+\n /*****************************************************************\n  *\n  * Streaming conversion support\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 5b8d46aede..d65a8971db 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -32,6 +32,20 @@ struct ll_merge_driver {\n \tchar *cmdline;\n };\n \n+static struct attr_check *merge_attributes;\n+static struct attr_check *load_merge_attributes(void)\n+{\n+\tif (!merge_attributes)\n+\t\tmerge_attributes = attr_check_initl(\"merge\", \"conflict-marker-size\", NULL);\n+\treturn merge_attributes;\n+}\n+\n+void reset_merge_attributes(void)\n+{\n+\tattr_check_free(merge_attributes);\n+\tmerge_attributes = NULL;\n+}\n+\n /*\n  * Built-in low-levels\n  */\n@@ -354,7 +368,7 @@ int ll_merge(mmbuffer_t *result_buf,\n \t     struct index_state *istate,\n \t     const struct ll_merge_options *opts)\n {\n-\tstatic struct attr_check *check;\n+\tstruct attr_check *check = load_merge_attributes();\n \tstatic const struct ll_merge_options default_opts;\n \tconst char *ll_driver_name = NULL;\n \tint marker_size = DEFAULT_CONFLICT_MARKER_SIZE;\n@@ -369,9 +383,6 @@ int ll_merge(mmbuffer_t *result_buf,\n \t\tnormalize_file(theirs, path, istate);\n \t}\n \n-\tif (!check)\n-\t\tcheck = attr_check_initl(\"merge\", \"conflict-marker-size\", NULL);\n-\n \tgit_check_attr(istate, path, check);\n \tll_driver_name = check->items[0].value;\n \tif (check->items[1].value) {\ndiff --git a/ll-merge.h b/ll-merge.h\nindex b9e2af1c88..e78973dd55 100644\n--- a/ll-merge.h\n+++ b/ll-merge.h\n@@ -26,5 +26,6 @@ int ll_merge(mmbuffer_t *result_buf,\n \t     const struct ll_merge_options *opts);\n \n int ll_merge_marker_size(struct index_state *istate, const char *path);\n+void reset_merge_attributes(void);\n \n #endif\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 80b23fd326..23469cc789 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -301,6 +301,42 @@ test_expect_success 'rebase --am and --show-current-patch' '\n \t)\n '\n \n+test_expect_success 'rebase --am and .gitattributes' '\n+\ttest_create_repo attributes &&\n+\t(\n+\t\tcd attributes &&\n+\t\ttest_commit init &&\n+\t\tgit config filter.test.clean \"sed -e '\\''s/smudged/clean/g'\\''\" &&\n+\t\tgit config filter.test.smudge \"sed -e '\\''s/clean/smudged/g'\\''\" &&\n+\n+\t\ttest_commit second &&\n+\t\tgit checkout -b test HEAD^ &&\n+\n+\t\techo \"*.txt filter=test\" >.gitattributes &&\n+\t\tgit add .gitattributes &&\n+\t\ttest_commit third &&\n+\n+\t\techo \"This text is smudged.\" >a.txt &&\n+\t\tgit add a.txt &&\n+\t\ttest_commit fourth &&\n+\n+\t\tgit checkout -b removal HEAD^ &&\n+\t\tgit rm .gitattributes &&\n+\t\tgit add -u &&\n+\t\ttest_commit fifth &&\n+\t\tgit cherry-pick test &&\n+\n+\t\tgit checkout test &&\n+\t\tgit rebase master &&\n+\t\tgrep \"smudged\" a.txt &&\n+\n+\t\tgit checkout removal &&\n+\t\tgit reset --hard &&\n+\t\tgit rebase master &&\n+\t\tgrep \"clean\" a.txt\n+\t)\n+'\n+\n test_expect_success 'rebase--merge.sh and --show-current-patch' '\n \ttest_create_repo conflict-merge &&\n \t(\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 3f7f750cc8..4f1e24ecbe 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -1061,4 +1061,56 @@ test_expect_success 'am --quit keeps HEAD where it is' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'am and .gitattibutes' '\n+\ttest_create_repo attributes &&\n+\t(\n+\t\tcd attributes &&\n+\t\ttest_commit init &&\n+\t\tgit config filter.test.clean \"sed -e '\\''s/smudged/clean/g'\\''\" &&\n+\t\tgit config filter.test.smudge \"sed -e '\\''s/clean/smudged/g'\\''\" &&\n+\n+\t\ttest_commit second &&\n+\t\tgit checkout -b test HEAD^ &&\n+\n+\t\techo \"*.txt filter=test conflict-marker-size=10\" >.gitattributes &&\n+\t\tgit add .gitattributes &&\n+\t\ttest_commit third &&\n+\n+\t\techo \"This text is smudged.\" >a.txt &&\n+\t\tgit add a.txt &&\n+\t\ttest_commit fourth &&\n+\n+\t\tgit checkout -b removal HEAD^ &&\n+\t\tgit rm .gitattributes &&\n+\t\tgit add -u &&\n+\t\ttest_commit fifth &&\n+\t\tgit cherry-pick test &&\n+\n+\t\tgit checkout -b conflict third &&\n+\t\techo \"This text is different.\" >a.txt &&\n+\t\tgit add a.txt &&\n+\t\ttest_commit sixth &&\n+\n+\t\tgit checkout test &&\n+\t\tgit format-patch --stdout master..HEAD >patches &&\n+\t\tgit reset --hard master &&\n+\t\tgit am patches &&\n+\t\tgrep \"smudged\" a.txt &&\n+\n+\t\tgit checkout removal &&\n+\t\tgit reset --hard &&\n+\t\tgit format-patch --stdout master..HEAD >patches &&\n+\t\tgit reset --hard master &&\n+\t\tgit am patches &&\n+\t\tgrep \"clean\" a.txt &&\n+\n+\t\tgit checkout conflict &&\n+\t\tgit reset --hard &&\n+\t\tgit format-patch --stdout master..HEAD >patches &&\n+\t\tgit reset --hard fourth &&\n+\t\ttest_must_fail git am -3 patches &&\n+\t\tgrep \"<<<<<<<<<<\" a.txt\n+\t)\n+'\n+\n test_done\n"}]}