git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin Johannes.Schindelin@gmx.de,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/3] Dir rename fixes
Date: Tue, 22 Oct 2019 21:22:48 +0000	[thread overview]
Message-ID: <pull.390.v2.git.1571779371.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.390.git.gitgitgadget@gmail.com>

This series improves a couple things found after looking into things Dscho
flagged:

 * clarify and slightly restructure code in the get_renamed_dir_portion()
   function
 * extend support of detecting renaming/merging of one directory into
   another to support the root directory as a target directory

First patch best viewed with a --histogram diff (sorry, gitgitgadget does
not yet know how to generate those).

Changes since v1:

 * Incorporated code cleanups suggested by Dscho
 * Fixed to work with an alternate rename-to-root-directory case (end_of_new
   == NULL), with new testcase
 * Added a new patch to the end of the series to stop making setup tests be
   part of a separate test_expect_success block.

Elijah Newren (3):
  merge-recursive: clean up get_renamed_dir_portion()
  merge-recursive: fix merging a subdirectory into the root directory
  t604[236]: do not run setup in separate tests

 merge-recursive.c                      | 104 ++++-
 t/t6042-merge-rename-corner-cases.sh   | 111 +++--
 t/t6043-merge-rename-directories.sh    | 568 ++++++++++++++++---------
 t/t6046-merge-skip-unneeded-updates.sh | 135 +++---
 4 files changed, 582 insertions(+), 336 deletions(-)


base-commit: 08da6496b61341ec45eac36afcc8f94242763468
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-390%2Fnewren%2Fdir-rename-fixes-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-390/newren/dir-rename-fixes-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/390

Range-diff vs v1:

 1:  8ae78679c9 = 1:  8ae78679c9 merge-recursive: clean up get_renamed_dir_portion()
 2:  37aee862e1 ! 2:  a1e80e8fbb merge-recursive: fix merging a subdirectory into the root directory
     @@ -34,9 +34,41 @@
       	strbuf_grow(&new_path, newlen);
       	strbuf_addbuf(&new_path, &entry->new_dir);
      @@
     - 	    *end_of_old == *end_of_new)
     - 		return; /* We haven't modified *old_dir or *new_dir yet. */
     + 	 */
     + 	end_of_old = strrchr(old_path, '/');
     + 	end_of_new = strrchr(new_path, '/');
     +-	if (end_of_old == NULL || end_of_new == NULL)
     +-		return; /* We haven't modified *old_dir or *new_dir yet. */
     ++
     ++	/*
     ++	 * If end_of_old is NULL, old_path wasn't in a directory, so there
     ++	 * could not be a directory rename (our rule elsewhere that a
     ++	 * directory which still exists is not considered to have been
     ++	 * renamed means the root directory can never be renamed -- because
     ++	 * the root directory always exists).
     ++	 */
     ++	if (end_of_old == NULL)
     ++		return; /* Note: *old_dir and *new_dir are still NULL */
     ++
     ++	/*
     ++	 * If new_path contains no directory (end_of_new is NULL), then we
     ++	 * have a rename of old_path's directory to the root directory.
     ++	 */
     ++	if (end_of_new == NULL) {
     ++		*old_dir = xstrndup(old_path, end_of_old - old_path);
     ++		*new_dir = xstrdup("");
     ++		return;
     ++	}
       
     + 	/* Find the first non-matching character traversing backwards */
     + 	while (*--end_of_new == *--end_of_old &&
     +@@
     + 	 */
     + 	if (end_of_old == old_path && end_of_new == new_path &&
     + 	    *end_of_old == *end_of_new)
     +-		return; /* We haven't modified *old_dir or *new_dir yet. */
     ++		return; /* Note: *old_dir and *new_dir are still NULL */
     ++
      +	/*
      +	 * If end_of_new got back to the beginning of its string, and
      +	 * end_of_old got back to the beginning of some subdirectory, then
     @@ -44,21 +76,19 @@
      +	 * needs slightly special handling.
      +	 *
      +	 * Note: There is no need to consider the opposite case, with a
     -+	 * rename/merge of the root directory into some subdirectory.
     -+	 * Our rule elsewhere that a directory which still exists is not
     -+	 * considered to have been renamed means the root directory can
     -+	 * never be renamed (because the root directory always exists).
     ++	 * rename/merge of the root directory into some subdirectory
     ++	 * because as noted above the root directory always exists so it
     ++	 * cannot be considered to be renamed.
      +	 */
      +	if (end_of_new == new_path &&
      +	    end_of_old != old_path && end_of_old[-1] == '/') {
     -+		*old_dir = xstrndup(old_path, end_of_old-1 - old_path);
     -+		*new_dir = xstrndup(new_path, end_of_new - new_path);
     ++		*old_dir = xstrndup(old_path, --end_of_old - old_path);
     ++		*new_dir = xstrdup("");
      +		return;
      +	}
     -+
     + 
       	/*
       	 * We've found the first non-matching character in the directory
     - 	 * paths.  That means the current characters we were looking at
      
       diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
       --- a/t/t6043-merge-rename-directories.sh
     @@ -68,18 +98,18 @@
       '
       
      +# Testcase 12d, Rename/merge of subdirectory into the root
     -+#   Commit O: a/b/{foo.c}
     -+#   Commit A: foo.c
     -+#   Commit B: a/b/{foo.c,bar.c}
     -+#   Expected: a/b/{foo.c,bar.c}
     ++#   Commit O: a/b/subdir/foo
     ++#   Commit A: subdir/foo
     ++#   Commit B: a/b/subdir/foo, a/b/bar
     ++#   Expected: subdir/foo, bar
      +
     -+test_expect_success '12d-setup: Rename (merge) of subdirectory into the root' '
     ++test_expect_success '12d-setup: Rename/merge subdir into the root, variant 1' '
      +	test_create_repo 12d &&
      +	(
      +		cd 12d &&
      +
      +		mkdir -p a/b/subdir &&
     -+		test_commit a/b/subdir/foo.c &&
     ++		test_commit a/b/subdir/foo &&
      +
      +		git branch O &&
      +		git branch A &&
     @@ -87,16 +117,16 @@
      +
      +		git checkout A &&
      +		mkdir subdir &&
     -+		git mv a/b/subdir/foo.c.t subdir/foo.c.t &&
     ++		git mv a/b/subdir/foo.t subdir/foo.t &&
      +		test_tick &&
      +		git commit -m "A" &&
      +
      +		git checkout B &&
     -+		test_commit a/b/bar.c
     ++		test_commit a/b/bar
      +	)
      +'
      +
     -+test_expect_success '12d-check: Rename (merge) of subdirectory into the root' '
     ++test_expect_success '12d-check: Rename/merge subdir into the root, variant 1' '
      +	(
      +		cd 12d &&
      +
     @@ -108,18 +138,76 @@
      +		test_line_count = 2 out &&
      +
      +		git rev-parse >actual \
     -+			HEAD:subdir/foo.c.t   HEAD:bar.c.t &&
     ++			HEAD:subdir/foo.t   HEAD:bar.t &&
     ++		git rev-parse >expect \
     ++			O:a/b/subdir/foo.t  B:a/b/bar.t &&
     ++		test_cmp expect actual &&
     ++
     ++		git hash-object bar.t >actual &&
     ++		git rev-parse B:a/b/bar.t >expect &&
     ++		test_cmp expect actual &&
     ++
     ++		test_must_fail git rev-parse HEAD:a/b/subdir/foo.t &&
     ++		test_must_fail git rev-parse HEAD:a/b/bar.t &&
     ++		test_path_is_missing a/ &&
     ++		test_path_is_file bar.t
     ++	)
     ++'
     ++
     ++# Testcase 12e, Rename/merge of subdirectory into the root
     ++#   Commit O: a/b/foo
     ++#   Commit A: foo
     ++#   Commit B: a/b/foo, a/b/bar
     ++#   Expected: foo, bar
     ++
     ++test_expect_success '12e-setup: Rename/merge subdir into the root, variant 2' '
     ++	test_create_repo 12e &&
     ++	(
     ++		cd 12e &&
     ++
     ++		mkdir -p a/b &&
     ++		test_commit a/b/foo &&
     ++
     ++		git branch O &&
     ++		git branch A &&
     ++		git branch B &&
     ++
     ++		git checkout A &&
     ++		mkdir subdir &&
     ++		git mv a/b/foo.t foo.t &&
     ++		test_tick &&
     ++		git commit -m "A" &&
     ++
     ++		git checkout B &&
     ++		test_commit a/b/bar
     ++	)
     ++'
     ++
     ++test_expect_success '12e-check: Rename/merge subdir into the root, variant 2' '
     ++	(
     ++		cd 12e &&
     ++
     ++		git checkout A^0 &&
     ++
     ++		git -c merge.directoryRenames=true merge -s recursive B^0 &&
     ++
     ++		git ls-files -s >out &&
     ++		test_line_count = 2 out &&
     ++
     ++		git rev-parse >actual \
     ++			HEAD:foo.t   HEAD:bar.t &&
      +		git rev-parse >expect \
     -+			O:a/b/subdir/foo.c.t  B:a/b/bar.c.t &&
     ++			O:a/b/foo.t  B:a/b/bar.t &&
      +		test_cmp expect actual &&
      +
     -+		git hash-object bar.c.t >actual &&
     -+		git rev-parse B:a/b/bar.c.t >expect &&
     ++		git hash-object bar.t >actual &&
     ++		git rev-parse B:a/b/bar.t >expect &&
      +		test_cmp expect actual &&
      +
     -+		test_must_fail git rev-parse HEAD:a/b/subdir/foo.c.t &&
     -+		test_must_fail git rev-parse HEAD:a/b/bar.c.t &&
     -+		test_path_is_missing a/
     ++		test_must_fail git rev-parse HEAD:a/b/foo.t &&
     ++		test_must_fail git rev-parse HEAD:a/b/bar.t &&
     ++		test_path_is_missing a/ &&
     ++		test_path_is_file bar.t
      +	)
      +'
      +
 -:  ---------- > 3:  0dd61a3580 t604[236]: do not run setup in separate tests

-- 
gitgitgadget

  parent reply	other threads:[~2019-10-22 21:22 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-11 20:42 [PATCH 0/2] " Elijah Newren via GitGitGadget
2019-10-11 20:42 ` [PATCH 1/2] merge-recursive: clean up get_renamed_dir_portion() Elijah Newren via GitGitGadget
2019-10-12 19:47   ` Johannes Schindelin
2019-10-11 20:42 ` [PATCH 2/2] merge-recursive: fix merging a subdirectory into the root directory Elijah Newren via GitGitGadget
2019-10-12 20:37   ` Johannes Schindelin
2019-10-13  0:40     ` Elijah Newren
2019-10-14 10:41       ` Johannes Schindelin
2019-10-22 19:15         ` Elijah Newren
2019-10-24 22:22           ` Johannes Schindelin
2019-10-25  0:12             ` Elijah Newren
2019-10-25 13:30               ` Johannes Schindelin
2019-10-29  1:20                 ` Junio C Hamano
2019-10-30  7:01                   ` Elijah Newren
2019-10-30 22:16                     ` Johannes Schindelin
2019-10-30 22:31                       ` Elijah Newren
2019-10-31  8:28                         ` Johannes Schindelin
2019-10-12 18:41 ` [PATCH 0/2] Dir rename fixes Johannes Schindelin
2019-10-22 21:22 ` Elijah Newren via GitGitGadget [this message]
2019-10-22 21:22   ` [PATCH v2 1/3] merge-recursive: clean up get_renamed_dir_portion() Elijah Newren via GitGitGadget
2019-10-22 21:22   ` [PATCH v2 2/3] merge-recursive: fix merging a subdirectory into the root directory Elijah Newren via GitGitGadget
2019-10-22 21:22   ` [PATCH v2 3/3] t604[236]: do not run setup in separate tests Elijah Newren via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.390.v2.git.1571779371.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --subject='Re: [PATCH v2 0/3] Dir rename fixes' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).