From ce17feb1b3ecfb0344af9b9111a4b4d313d51d7a Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Sun, 25 Aug 2019 23:33:39 +0000 Subject: [PATCH 1/2] path: add a function to check for path suffix We have a function to strip the path suffix from a commit, but we don't have one to check for a path suffix. For a plain filename, we can use basename, but that requires an allocation, since POSIX allows it to modify its argument. Refactor strip_path_suffix into a helper function and a new function, ends_with_path_components, to meet this need. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- path.c | 57 +++++++++++++++++++++++++++++++++++++++------------------ path.h | 3 +++ 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/path.c b/path.c index 25e97b8c3f..e3da1f3c4e 100644 --- a/path.c +++ b/path.c @@ -1220,6 +1220,43 @@ static inline int chomp_trailing_dir_sep(const char *path, int len) return len; } +/* + * If path ends with suffix (complete path components), returns the offset of + * the last character in the path before the suffix (sans trailing directory + * separators), and -1 otherwise. + */ +static ssize_t stripped_path_suffix_offset(const char *path, const char *suffix) +{ + int path_len = strlen(path), suffix_len = strlen(suffix); + + while (suffix_len) { + if (!path_len) + return -1; + + if (is_dir_sep(path[path_len - 1])) { + if (!is_dir_sep(suffix[suffix_len - 1])) + return -1; + path_len = chomp_trailing_dir_sep(path, path_len); + suffix_len = chomp_trailing_dir_sep(suffix, suffix_len); + } + else if (path[--path_len] != suffix[--suffix_len]) + return -1; + } + + if (path_len && !is_dir_sep(path[path_len - 1])) + return -1; + return chomp_trailing_dir_sep(path, path_len); +} + +/* + * Returns true if the path ends with components, considering only complete path + * components, and false otherwise. + */ +int ends_with_path_components(const char *path, const char *components) +{ + return stripped_path_suffix_offset(path, components) != -1; +} + /* * If path ends with suffix (complete path components), returns the * part before suffix (sans trailing directory separators). @@ -1227,25 +1264,9 @@ static inline int chomp_trailing_dir_sep(const char *path, int len) */ char *strip_path_suffix(const char *path, const char *suffix) { - int path_len = strlen(path), suffix_len = strlen(suffix); + ssize_t offset = stripped_path_suffix_offset(path, suffix); - while (suffix_len) { - if (!path_len) - return NULL; - - if (is_dir_sep(path[path_len - 1])) { - if (!is_dir_sep(suffix[suffix_len - 1])) - return NULL; - path_len = chomp_trailing_dir_sep(path, path_len); - suffix_len = chomp_trailing_dir_sep(suffix, suffix_len); - } - else if (path[--path_len] != suffix[--suffix_len]) - return NULL; - } - - if (path_len && !is_dir_sep(path[path_len - 1])) - return NULL; - return xstrndup(path, chomp_trailing_dir_sep(path, path_len)); + return offset == -1 ? NULL : xstrndup(path, offset); } int daemon_avoid_alias(const char *p) diff --git a/path.h b/path.h index 2ba6ca58c8..14d6dcad16 100644 --- a/path.h +++ b/path.h @@ -193,4 +193,7 @@ const char *git_path_merge_head(struct repository *r); const char *git_path_fetch_head(struct repository *r); const char *git_path_shallow(struct repository *r); + +int ends_with_path_components(const char *path, const char *components); + #endif /* PATH_H */ From 2c65d90f7579a0e2a6460eebce44795587e87043 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Mon, 2 Sep 2019 22:39:44 +0000 Subject: [PATCH 2/2] am: reload .gitattributes after patching it When applying multiple patches with git am, or when rebasing using the am backend, it's possible that one of our patches has updated a gitattributes file. Currently, we cache this information, so if a file in a subsequent patch has attributes applied, the file will be written out with the attributes in place as of the time we started the rebase or am operation, not with the attributes applied by the previous patch. This problem does not occur when using the -m or -i flags to rebase. To ensure we write the correct data into the working tree, expire the cache after each patch that touches a path ending in ".gitattributes". Since we load these attributes in multiple separate files, we must expire them accordingly. Verify that both the am and rebase code paths work correctly, including the conflict marker size with am -3. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- apply.c | 11 ++++++++++ convert.c | 21 ++++++++++++++++++- convert.h | 6 ++++++ ll-merge.c | 19 +++++++++++++---- ll-merge.h | 1 + t/t3400-rebase.sh | 36 ++++++++++++++++++++++++++++++++ t/t4150-am.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 141 insertions(+), 5 deletions(-) diff --git a/apply.c b/apply.c index 4992eca416..2e11438667 100644 --- a/apply.c +++ b/apply.c @@ -4663,6 +4663,7 @@ static int apply_patch(struct apply_state *state, struct patch *list = NULL, **listp = &list; int skipped_patch = 0; int res = 0; + int flush_attributes = 0; state->patch_input_file = filename; if (read_patch_file(&buf, fd) < 0) @@ -4690,6 +4691,14 @@ static int apply_patch(struct apply_state *state, patch_stats(state, patch); *listp = patch; listp = &patch->next; + + if ((patch->new_name && + ends_with_path_components(patch->new_name, + GITATTRIBUTES_FILE)) || + (patch->old_name && + ends_with_path_components(patch->old_name, + GITATTRIBUTES_FILE))) + flush_attributes = 1; } else { if (state->apply_verbosity > verbosity_normal) @@ -4766,6 +4775,8 @@ static int apply_patch(struct apply_state *state, if (state->summary && state->apply_verbosity > verbosity_silent) summary_patch_list(list); + if (flush_attributes) + reset_parsed_attributes(); end: free_patch_list(list); strbuf_release(&buf); diff --git a/convert.c b/convert.c index 94ff837649..deb6f71b2d 100644 --- a/convert.c +++ b/convert.c @@ -8,6 +8,7 @@ #include "pkt-line.h" #include "sub-process.h" #include "utf8.h" +#include "ll-merge.h" /* * convert.c - convert a file when checking it out and checking it in. @@ -1293,10 +1294,11 @@ struct conv_attrs { const char *working_tree_encoding; /* Supported encoding or default encoding if NULL */ }; +static struct attr_check *check; + static void convert_attrs(const struct index_state *istate, struct conv_attrs *ca, const char *path) { - static struct attr_check *check; struct attr_check_item *ccheck = NULL; if (!check) { @@ -1339,6 +1341,23 @@ static void convert_attrs(const struct index_state *istate, ca->crlf_action = CRLF_AUTO_INPUT; } +void reset_parsed_attributes(void) +{ + struct convert_driver *drv, *next; + + attr_check_free(check); + check = NULL; + reset_merge_attributes(); + + for (drv = user_convert; drv; drv = next) { + next = drv->next; + free((void *)drv->name); + free(drv); + } + user_convert = NULL; + user_convert_tail = NULL; +} + int would_convert_to_git_filter_fd(const struct index_state *istate, const char *path) { struct conv_attrs ca; diff --git a/convert.h b/convert.h index 831559f10d..3710969d43 100644 --- a/convert.h +++ b/convert.h @@ -94,6 +94,12 @@ void convert_to_git_filter_fd(const struct index_state *istate, int would_convert_to_git_filter_fd(const struct index_state *istate, const char *path); +/* + * Reset the internal list of attributes used by convert_to_git and + * convert_to_working_tree. + */ +void reset_parsed_attributes(void); + /***************************************************************** * * Streaming conversion support diff --git a/ll-merge.c b/ll-merge.c index 5b8d46aede..d65a8971db 100644 --- a/ll-merge.c +++ b/ll-merge.c @@ -32,6 +32,20 @@ struct ll_merge_driver { char *cmdline; }; +static struct attr_check *merge_attributes; +static struct attr_check *load_merge_attributes(void) +{ + if (!merge_attributes) + merge_attributes = attr_check_initl("merge", "conflict-marker-size", NULL); + return merge_attributes; +} + +void reset_merge_attributes(void) +{ + attr_check_free(merge_attributes); + merge_attributes = NULL; +} + /* * Built-in low-levels */ @@ -354,7 +368,7 @@ int ll_merge(mmbuffer_t *result_buf, struct index_state *istate, const struct ll_merge_options *opts) { - static struct attr_check *check; + struct attr_check *check = load_merge_attributes(); static const struct ll_merge_options default_opts; const char *ll_driver_name = NULL; int marker_size = DEFAULT_CONFLICT_MARKER_SIZE; @@ -369,9 +383,6 @@ int ll_merge(mmbuffer_t *result_buf, normalize_file(theirs, path, istate); } - if (!check) - check = attr_check_initl("merge", "conflict-marker-size", NULL); - git_check_attr(istate, path, check); ll_driver_name = check->items[0].value; if (check->items[1].value) { diff --git a/ll-merge.h b/ll-merge.h index b9e2af1c88..e78973dd55 100644 --- a/ll-merge.h +++ b/ll-merge.h @@ -26,5 +26,6 @@ int ll_merge(mmbuffer_t *result_buf, const struct ll_merge_options *opts); int ll_merge_marker_size(struct index_state *istate, const char *path); +void reset_merge_attributes(void); #endif diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index 42f147858d..4f22e6d3d8 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -301,6 +301,42 @@ test_expect_success 'rebase--am.sh and --show-current-patch' ' ) ' +test_expect_success 'rebase --am and .gitattributes' ' + test_create_repo attributes && + ( + cd attributes && + test_commit init && + git config filter.test.clean "sed -e '\''s/smudged/clean/g'\''" && + git config filter.test.smudge "sed -e '\''s/clean/smudged/g'\''" && + + test_commit second && + git checkout -b test HEAD^ && + + echo "*.txt filter=test" >.gitattributes && + git add .gitattributes && + test_commit third && + + echo "This text is smudged." >a.txt && + git add a.txt && + test_commit fourth && + + git checkout -b removal HEAD^ && + git rm .gitattributes && + git add -u && + test_commit fifth && + git cherry-pick test && + + git checkout test && + git rebase master && + grep "smudged" a.txt && + + git checkout removal && + git reset --hard && + git rebase master && + grep "clean" a.txt + ) +' + test_expect_success 'rebase--merge.sh and --show-current-patch' ' test_create_repo conflict-merge && ( diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 3f7f750cc8..4f1e24ecbe 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -1061,4 +1061,56 @@ test_expect_success 'am --quit keeps HEAD where it is' ' test_cmp expected actual ' +test_expect_success 'am and .gitattibutes' ' + test_create_repo attributes && + ( + cd attributes && + test_commit init && + git config filter.test.clean "sed -e '\''s/smudged/clean/g'\''" && + git config filter.test.smudge "sed -e '\''s/clean/smudged/g'\''" && + + test_commit second && + git checkout -b test HEAD^ && + + echo "*.txt filter=test conflict-marker-size=10" >.gitattributes && + git add .gitattributes && + test_commit third && + + echo "This text is smudged." >a.txt && + git add a.txt && + test_commit fourth && + + git checkout -b removal HEAD^ && + git rm .gitattributes && + git add -u && + test_commit fifth && + git cherry-pick test && + + git checkout -b conflict third && + echo "This text is different." >a.txt && + git add a.txt && + test_commit sixth && + + git checkout test && + git format-patch --stdout master..HEAD >patches && + git reset --hard master && + git am patches && + grep "smudged" a.txt && + + git checkout removal && + git reset --hard && + git format-patch --stdout master..HEAD >patches && + git reset --hard master && + git am patches && + grep "clean" a.txt && + + git checkout conflict && + git reset --hard && + git format-patch --stdout master..HEAD >patches && + git reset --hard fourth && + test_must_fail git am -3 patches && + grep "<<<<<<<<<<" a.txt + ) +' + test_done