Output fixes for --remerge-diff
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v3 Latest3 patches 0 repliesNew
- v23 patches 1 replySuperseded
- v13 patches 10 repliesSuperseded
Changes in v3 from the cover letter
* Added a comment describing the rationale for the new diff_filepair_is_phoney() function.
Patches in v3 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/3 | diff: have submodule_format logic avoid additional diff headersdiff.c t/ | +31 | 0 replies | |
| 2/3 | diff: fix filtering of additional headers under --remerge-diffdiff.c t/ | +18 | 0 replies | |
| 3/3 | diff: fix filtering of merge commits under --remerge-diffdiff.c t/ | +6 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v3 yet.
Cover letter [PATCH v3 0/3] Output fixes for --remerge-diff
Philippe Blain found and reported a couple issues with the output of --remerge-diff[1]. After digging in, I think one of them actually counts as two separate issues, so here's a series with three patches to fix these issues. Each includes testcases to keep us from regressing.
Changes since v2:
* Added a comment describing the rationale for the new diff_filepair_is_phoney() function.
Changes since v1:
* Added a new diff_filepair_is_phoney() function to make the code more self-documenting, as suggested by Junio.
* Note: Patch 2, as called out in its commit message, disables TEST_PASSES_SANITIZE_LEAK for t4069. This is not because I introduce any memory leaks, but because I add new testcases invoking additional parts of the code (pickaxe stuff) which already had pre-existing leaks. This is not a change since v1, but this somehow accidentally got munged out of Junio's application of my v1.
Note: The issue fixed by the third commit for --remerge-diff is also an issue exhibited by 'git log --cc $FILTER_RULES $COMMIT' (or by -c instead of --cc). However, as far as I can tell the causes are different and come from separate codepaths; this series focuses on --remerge-diff and hence makes no attempt to fix independent (even if similar) --cc or -c issues.
[1]
https://lore.kernel.org/git/[email protected]/
Elijah Newren (3):
diff: have submodule_format logic avoid additional diff headers
diff: fix filtering of additional headers under --remerge-diff
diff: fix filtering of merge commits under --remerge-diff
diff.c | 32 ++++++++++++++++++++++++++------ t/t4069-remerge-diff.sh | 30 +++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 7 deletions(-)
base-commit: 6c8e4ee870332d11e4bba84901654b355a9ff016
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1342%2Fnewren%2Fremerge-diff-output-fixes-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1342/newren/remerge-diff-output-fixes-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1342
Range-diff vs v2:
1: aea0bbc1c6a ! 1: ed28acaed97 diff: have submodule_format logic avoid additional diff headers
@@ Commit message
The submodule_format handling is another codepath with the same issue; it would operate on these additional headers and attempt to display them
- as submodule changes. Prevent that by explicitly checking for both
- modes being 0.
+ as submodule changes. Prevent that by explicitly checking for "phoney"
+ filepairs (i.e. filepairs with both modes being 0).
@@ diff.c: static void add_formatted_headers(struct strbuf *msg,
+static int diff_filepair_is_phoney(struct diff_filespec *one,
+ struct diff_filespec *two)
+{
++ /*
++ * This function specifically looks for pairs injected by
++ * create_filepairs_for_header_only_notifications(). Such
++ * pairs are "phoney" in that they do not represent any
++ * content or even mode difference, but were inserted because
++ * diff_queued_diff previously had no pair associated with
++ * that path but we needed some pair to avoid losing the
++ * "remerge CONFLICT" header associated with the path.
++ */
+ return !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);
+}
+
@@ diff.c: static void builtin_diff(const char *name_a,
- if (!DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two)) {
+ if (diff_filepair_is_phoney(one, two)) {
/*
- * We should only reach this point for pairs from
+- * We should only reach this point for pairs from
++ * We should only reach this point for pairs generated from
* create_filepairs_for_header_only_notifications(). For
+- * these, we should avoid the "/dev/null" special casing
+- * above, meaning we avoid showing such pairs as either
++ * these, we want to avoid the "/dev/null" special casing
++ * above, because we do not want such pairs shown as either
+ * "new file" or "deleted file" below.
+ */
+ lbl[0] = a_one;
## t/t4069-remerge-diff.sh ##
@@ t/t4069-remerge-diff.sh: test_expect_success 'remerge-diff w/ diff-filter=U: all conflict headers, no dif
2: 3bd622d5b45 = 2: f91bea2bbc3 diff: fix filtering of additional headers under --remerge-diff 3: 7903f8380dc = 3: 084a037756d diff: fix filtering of merge commits under --remerge-diff
Signature · 1 line
gitgitgadget