Skip to content

Commit

Permalink
am: reload .gitattributes after patching it
Browse files Browse the repository at this point in the history
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 <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
bk2204 authored and gitster committed Sep 3, 2019
1 parent ce17feb commit 2c65d90
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 5 deletions.
11 changes: 11 additions & 0 deletions apply.c
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
21 changes: 20 additions & 1 deletion convert.c
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions convert.h
Expand Up @@ -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
Expand Down
19 changes: 15 additions & 4 deletions ll-merge.c
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions ll-merge.h
Expand Up @@ -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
36 changes: 36 additions & 0 deletions t/t3400-rebase.sh
Expand Up @@ -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 &&
(
Expand Down
52 changes: 52 additions & 0 deletions t/t4150-am.sh
Expand Up @@ -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

0 comments on commit 2c65d90

Please sign in to comment.