unpack-trees: add the dry_run flag to unpack_trees_options

RFC PATCH Superseded Jens Lehmann 2 participants 7 messages Jun 15, 2016 Documentation

Share & tools

Jens Lehmann

author

2016-06-15 22:51 UTC #0 lore raw

RFC PATCH 1/1 unpack-trees: add the dry_run flag to unpack_trees_options

Until now there was no way to test if unpack_trees() with update=1 would succeed without really updating the work tree. The reason for that is that setting update to 0 does skip the tests for new files and deactivates the sparse handling, thereby making that unsuitable as a dry run.

Add the new dry_run flag to struct unpack_trees_options unpack_trees(). Setting that together with the update flag will check if the work tree update would be successful without doing it for real.

The only class of problems that is not detected at the moment are file system conditions like ENOSPC or missing permissions. Also the index entries of updated files are not as they would be after a real checkout because lstat() isn't run as the files aren't updated for real.

Signed-off-by Jens Lehmann <[email protected]>

---

AFAICS this patch enables the user to achieve a dry run by calling unpack_trees() with both update and dry_run set.

My "Teach read-tree the -n|--dry-run option" adapted to this new flag runs all tests successfully ...

 unpack-trees.c |    4 ++--
 unpack-trees.h |    3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

unpack-trees.c +2 -2

index 500ebcf..e26a710 100644
@@ -203,7 +203,7 @@ static int check_updates(struct unpack_trees_options *o)
 
 		if (ce->ce_flags & CE_WT_REMOVE) {
 			display_progress(progress, ++cnt);
-			if (o->update)
+			if (o->update && !o->dry_run)
 				unlink_entry(ce);
 			continue;
 		}

@@ -217,7 +217,7 @@ static int check_updates(struct unpack_trees_options *o)
 		if (ce->ce_flags & CE_UPDATE) {
 			display_progress(progress, ++cnt);
 			ce->ce_flags &= ~CE_UPDATE;
-			if (o->update) {
+			if (o->update && !o->dry_run) {
 				errs |= checkout_entry(ce, &state, NULL);
 			}
 		}

unpack-trees.h +2 -1

index cd11a08..64f02cb 100644
@@ -46,7 +46,8 @@ struct unpack_trees_options {
 		     debug_unpack,
 		     skip_sparse_checkout,
 		     gently,
-		     show_all_errors;
+		     show_all_errors,
+		     dry_run;
 	const char *prefix;
 	int cache_bottom;
 	struct dir_struct *dir;

Signature · 1 line

1.7.5.1.340.g7f395.dirty

Junio C Hamano

2016-06-15 22:51 UTC #1 lore raw

Quoted 9 lines · Jens Lehmann

Until now there was no way to test if unpack_trees() with update=1 would succeed without really updating the work tree. The reason for that is that setting update to 0 does skip the tests for new files and deactivates the sparse handling, thereby making that unsuitable as a dry run.
Add the new dry_run flag to struct unpack_trees_options unpack_trees(). Setting that together with the update flag will check if the work tree update would be successful without doing it for real.

Quoted 7 lines

The only class of problems that is not detected at the moment are file system conditions like ENOSPC or missing permissions. Also the index entries of updated files are not as they would be after a real checkout because lstat() isn't run as the files aren't updated for real.
Signed-off-by Jens Lehmann <[email protected]>
---

A very sane and clear description. Good.

Quoted 10 lines

AFAICS this patch enables the user to achieve a dry run by calling unpack_trees() with both update and dry_run set.
My "Teach read-tree the -n|--dry-run option" adapted to this new flag runs all tests successfully ...
 unpack-trees.c |    4 ++--
 unpack-trees.h |    3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

But on which change does this come?

Jens Lehmann

author

2016-06-15 22:51 UTC #2 lore raw

v2 cover Teach read-tree the -n|--dry-run option

Am 25.05.2011 01:18, schrieb Junio C Hamano:

Quoted 10 lines

Quoted 7 lines · Jens Lehmann
My "Teach read-tree the -n|--dry-run option" adapted to this new flag runs all tests successfully ...
 unpack-trees.c |    4 ++--
 unpack-trees.h |    3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)
But on which change does this come?

As a preparing step for "read-tree the -n|--dry-run" (and adding a dry-run option to other users of unpack_trees() should be pretty straightforward now too).

The first patch in this series enables unpack_trees() to do a dry-run. That enables users of unpack_trees() to see if running this function with the update flag set would succeed.

The second patch uses this new dry_run flag to teach read-tree the -n|--dry-run option.

Jens Lehmann (2):
unpack-trees: add the dry_run flag to unpack_trees_options Teach read-tree the -n|--dry-run option

 Documentation/git-read-tree.txt      |    5 ++
 builtin/read-tree.c                  |    3 +-
 t/lib-read-tree.sh                   |   43 ++++++++++++++++++
 t/t1000-read-tree-m-3way.sh          |   81 +++++++++++++++++-----------------
 t/t1001-read-tree-m-2way.sh          |   45 ++++++++++---------
 t/t1002-read-tree-m-u-2way.sh        |   81 +++++++++++++++++-----------------
 t/t1004-read-tree-m-u-wf.sh          |   23 +++++-----
 t/t1005-read-tree-reset.sh           |   13 +++---
 t/t1008-read-tree-overlay.sh         |    3 +-
 t/t1011-read-tree-sparse-checkout.sh |   27 ++++++-----
 t/t1012-read-tree-df.sh              |    9 ++--
 t/t1020-subdirectory.sh              |    5 +-
 unpack-trees.c                       |    4 +-
 unpack-trees.h                       |    3 +-
 14 files changed, 202 insertions(+), 143 deletions(-)
 create mode 100644 t/lib-read-tree.sh

Signature · 1 line

1.7.5.2.355.gdceb0

Jens Lehmann

author

2016-06-15 22:51 UTC #3 lore raw

PATCH v2 1/2 unpack-trees: add the dry_run flag to unpack_trees_options

Signed-off-by Jens Lehmann <[email protected]>

---
 unpack-trees.c |    4 ++--
 unpack-trees.h |    3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

unpack-trees.c +2 -2

index 0bc4b2d..07f8364 100644
@@ -203,7 +203,7 @@ static int check_updates(struct unpack_trees_options *o)
 
 		if (ce->ce_flags & CE_WT_REMOVE) {
 			display_progress(progress, ++cnt);
-			if (o->update)
+			if (o->update && !o->dry_run)
 				unlink_entry(ce);
 			continue;
 		}

@@ -217,7 +217,7 @@ static int check_updates(struct unpack_trees_options *o)
 		if (ce->ce_flags & CE_UPDATE) {
 			display_progress(progress, ++cnt);
 			ce->ce_flags &= ~CE_UPDATE;
-			if (o->update) {
+			if (o->update && !o->dry_run) {
 				errs |= checkout_entry(ce, &state, NULL);
 			}
 		}

unpack-trees.h +2 -1

index cd11a08..64f02cb 100644
@@ -46,7 +46,8 @@ struct unpack_trees_options {
 		     debug_unpack,
 		     skip_sparse_checkout,
 		     gently,
-		     show_all_errors;
+		     show_all_errors,
+		     dry_run;
 	const char *prefix;
 	int cache_bottom;
 	struct dir_struct *dir;

Signature · 1 line

1.7.5.2.355.gdceb0

Junio C Hamano

2016-06-15 22:51 UTC #4 lore raw

Quoted 9 lines · Jens Lehmann

Until now there was no way to test if unpack_trees() with update=1 would succeed without really updating the work tree. The reason for that is that setting update to 0 does skip the tests for new files and deactivates the sparse handling, thereby making that unsuitable as a dry run.
Add the new dry_run flag to struct unpack_trees_options unpack_trees(). Setting that together with the update flag will check if the work tree update would be successful without doing it for real.

Quoted 7 lines

The only class of problems that is not detected at the moment are file system conditions like ENOSPC or missing permissions. Also the index entries of updated files are not as they would be after a real checkout because lstat() isn't run as the files aren't updated for real.
Signed-off-by Jens Lehmann <[email protected]>
---

Looks good. remove_marked_cache_entries() does not touch the working tree, and because you are not calling unlink_entry(), you won't trigger schedule_dir_for_removal() hence remove_scheduled_dirs() won't cause us any trouble either.

Will queue. Thanks.

Jens Lehmann

author

2016-06-15 22:51 UTC #5 lore raw

PATCH v2 2/2 Teach read-tree the -n|--dry-run option

In reply to Jens Lehmann

Using this option tells read-tree to not update the index. That makes it possible to check if updating the index would be successful without changing anything. Using this option will silently ignore -u but still test if updating the work tree would succeed.

The relevant tests in the t10?? range were extended to do a read-tree -n before the real read-tree to make sure neither the index nor any local files were changed with -n and the same exit code as without -n is returned. The helper functions added for that purpose reside in the new t/lib-read-tree.sh file.

The only exception is #13 in t1004 ("unlinking an un-unlink-able symlink"). As this is an issue of wrong directory permissions it is not detected with -n.

Signed-off-by Jens Lehmann <[email protected]>

---
 Documentation/git-read-tree.txt      |    5 ++
 builtin/read-tree.c                  |    3 +-
 t/lib-read-tree.sh                   |   43 ++++++++++++++++++
 t/t1000-read-tree-m-3way.sh          |   81 +++++++++++++++++-----------------
 t/t1001-read-tree-m-2way.sh          |   45 ++++++++++---------
 t/t1002-read-tree-m-u-2way.sh        |   81 +++++++++++++++++-----------------
 t/t1004-read-tree-m-u-wf.sh          |   23 +++++-----
 t/t1005-read-tree-reset.sh           |   13 +++---
 t/t1008-read-tree-overlay.sh         |    3 +-
 t/t1011-read-tree-sparse-checkout.sh |   27 ++++++-----
 t/t1012-read-tree-df.sh              |    9 ++--
 t/t1020-subdirectory.sh              |    5 +-
 12 files changed, 198 insertions(+), 140 deletions(-)
 create mode 100644 t/lib-read-tree.sh

Documentation/git-read-tree.txt +5 -0

index 26fdadc..46a96f2 100644
@@ -53,6 +53,11 @@ OPTIONS
 	trees that are not directly related to the current
 	working tree status into a temporary index file.
 
+-n::
+--dry-run::
+	Check if the command would error out, without updating the index
+	nor the files in the working tree for real.
+
 -v::
 	Show the progress of checking files out.

builtin/read-tree.c +2 -1

index 93c9281..df6c4c8 100644
@@ -130,6 +130,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 		  PARSE_OPT_NONEG, exclude_per_directory_cb },
 		OPT_SET_INT('i', NULL, &opts.index_only,
 			    "don't check the working tree after merging", 1),
+		OPT__DRY_RUN(&opts.dry_run, "don't update the index or the work tree"),
 		OPT_SET_INT(0, "no-sparse-checkout", &opts.skip_sparse_checkout,
 			    "skip applying sparse checkout filter", 1),
 		OPT_SET_INT(0, "debug-unpack", &opts.debug_unpack,

@@ -219,7 +220,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 	if (unpack_trees(nr_trees, t, &opts))
 		return 128;
 
-	if (opts.debug_unpack)
+	if (opts.debug_unpack || opts.dry_run)
 		return 0; /* do not write the index out */
 
 	/*

t/lib-read-tree.sh new file +43 -0

43 lines not on this page · raw message

t/t1000-read-tree-m-3way.sh +41 ~~-40~~

322 lines not on this page · raw message

t/t1001-read-tree-m-2way.sh +23 ~~-22~~

183 lines not on this page · raw message

t/t1002-read-tree-m-u-2way.sh +41 ~~-40~~

241 lines not on this page · raw message

t/t1004-read-tree-m-u-wf.sh +12 ~~-11~~

index eb8e3d4..b3ae7d5 100755
@@ -3,6 +3,7 @@
 test_description='read-tree -m -u checks working tree files'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-read-tree.sh
 
 # two-tree test

@@ -29,7 +30,7 @@ test_expect_success 'two-way not clobbering' '
 
 	echo >file2 master creates untracked file2 &&
 	echo >subdir/file2 master creates untracked subdir/file2 &&
-	if err=`git read-tree -m -u master side 2>&1`
+	if err=`read_tree_u_must_succeed -m -u master side 2>&1`
 	then
 		echo should have complained
 		false

@@ -42,7 +43,7 @@ echo file2 >.gitignore
 
 test_expect_success 'two-way with incorrect --exclude-per-directory (1)' '
 
-	if err=`git read-tree -m --exclude-per-directory=.gitignore master side 2>&1`
+	if err=`read_tree_u_must_succeed -m --exclude-per-directory=.gitignore master side 2>&1`
 	then
 		echo should have complained
 		false

@@ -53,7 +54,7 @@ test_expect_success 'two-way with incorrect --exclude-per-directory (1)' '
 
 test_expect_success 'two-way with incorrect --exclude-per-directory (2)' '
 
-	if err=`git read-tree -m -u --exclude-per-directory=foo --exclude-per-directory=.gitignore master side 2>&1`
+	if err=`read_tree_u_must_succeed -m -u --exclude-per-directory=foo --exclude-per-directory=.gitignore master side 2>&1`
 	then
 		echo should have complained
 		false

@@ -64,7 +65,7 @@ test_expect_success 'two-way with incorrect --exclude-per-directory (2)' '
 
 test_expect_success 'two-way clobbering a ignored file' '
 
-	git read-tree -m -u --exclude-per-directory=.gitignore master side
+	read_tree_u_must_succeed -m -u --exclude-per-directory=.gitignore master side
 '
 
 rm -f .gitignore

@@ -84,7 +85,7 @@ test_expect_success 'three-way not complaining on an untracked path in both' '
 	echo >file2 file two is untracked on the master side &&
 	echo >subdir/file2 file two is untracked on the master side &&
 
-	git read-tree -m -u branch-point master side
+	read_tree_u_must_succeed -m -u branch-point master side
 '
 
 test_expect_success 'three-way not clobbering a working tree file' '

@@ -94,7 +95,7 @@ test_expect_success 'three-way not clobbering a working tree file' '
 	git checkout master &&
 	echo >file3 file three created in master, untracked &&
 	echo >subdir/file3 file three created in master, untracked &&
-	if err=`git read-tree -m -u branch-point master side 2>&1`
+	if err=`read_tree_u_must_succeed -m -u branch-point master side 2>&1`
 	then
 		echo should have complained
 		false

@@ -113,7 +114,7 @@ test_expect_success 'three-way not complaining on an untracked file' '
 	echo >file3 file three created in master, untracked &&
 	echo >subdir/file3 file three created in master, untracked &&
 
-	git read-tree -m -u --exclude-per-directory=.gitignore branch-point master side
+	read_tree_u_must_succeed -m -u --exclude-per-directory=.gitignore branch-point master side
 '
 
 test_expect_success '3-way not overwriting local changes (setup)' '

@@ -137,7 +138,7 @@ test_expect_success '3-way not overwriting local changes (our side)' '
 	git reset --hard &&
 
 	echo >>file1 "local changes" &&
-	git read-tree -m -u branch-point side-a side-b &&
+	read_tree_u_must_succeed -m -u branch-point side-a side-b &&
 	grep "new line to be kept" file1 &&
 	grep "local changes" file1

@@ -151,7 +152,7 @@ test_expect_success '3-way not overwriting local changes (their side)' '
 	git reset --hard &&
 
 	echo >>file2 "local changes" &&
-	test_must_fail git read-tree -m -u branch-point side-a side-b &&
+	read_tree_u_must_fail -m -u branch-point side-a side-b &&
 	! grep "new line to be kept" file2 &&
 	grep "local changes" file2

@@ -173,7 +174,7 @@ test_expect_success SYMLINKS 'funny symlink in work tree' '
 	git add a/b &&
 	git commit -m "we add a/b" &&
 
-	git read-tree -m -u sym-a sym-a sym-b
+	read_tree_u_must_succeed -m -u sym-a sym-a sym-b
 
 '

@@ -209,7 +210,7 @@ test_expect_success 'D/F setup' '
 test_expect_success 'D/F' '
 
 	git checkout side-b &&
-	git read-tree -m -u branch-point side-b side-a &&
+	read_tree_u_must_succeed -m -u branch-point side-b side-a &&
 	git ls-files -u >actual &&
 	(
 		a=$(git rev-parse branch-point:subdir/file2)

t/t1005-read-tree-reset.sh +7 -6

index 8499116..f53de79 100755
@@ -3,6 +3,7 @@
 test_description='read-tree -u --reset'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-read-tree.sh
 
 # two-tree test

@@ -22,13 +23,13 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'reset should work' '
-  git read-tree -u --reset HEAD^ &&
+  read_tree_u_must_succeed -u --reset HEAD^ &&
   git ls-files >actual &&
   test_cmp expect actual
 '
 
 test_expect_success 'reset should remove remnants from a failed merge' '
-  git read-tree --reset -u HEAD &&
+  read_tree_u_must_succeed --reset -u HEAD &&
   git ls-files -s >expect &&
   sha1=$(git rev-parse :new) &&
   (

@@ -37,13 +38,13 @@ test_expect_success 'reset should remove remnants from a failed merge' '
   ) | git update-index --index-info &&
   >old &&
   git ls-files -s &&
-  git read-tree --reset -u HEAD &&
+  read_tree_u_must_succeed --reset -u HEAD &&
   git ls-files -s >actual &&
   ! test -f old
 '
 
 test_expect_success 'Porcelain reset should remove remnants too' '
-  git read-tree --reset -u HEAD &&
+  read_tree_u_must_succeed --reset -u HEAD &&
   git ls-files -s >expect &&
   sha1=$(git rev-parse :new) &&
   (

@@ -58,7 +59,7 @@ test_expect_success 'Porcelain reset should remove remnants too' '
 '
 
 test_expect_success 'Porcelain checkout -f should remove remnants too' '
-  git read-tree --reset -u HEAD &&
+  read_tree_u_must_succeed --reset -u HEAD &&
   git ls-files -s >expect &&
   sha1=$(git rev-parse :new) &&
   (

@@ -73,7 +74,7 @@ test_expect_success 'Porcelain checkout -f should remove remnants too' '
 '
 
 test_expect_success 'Porcelain checkout -f HEAD should remove remnants too' '
-  git read-tree --reset -u HEAD &&
+  read_tree_u_must_succeed --reset -u HEAD &&
   git ls-files -s >expect &&
   sha1=$(git rev-parse :new) &&
   (

t/t1008-read-tree-overlay.sh +2 -1

index f9e0028..4c50ed9 100755
@@ -3,6 +3,7 @@
 test_description='test multi-tree read-tree without merging'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-read-tree.sh
 
 test_expect_success setup '
 	echo one >a &&

@@ -21,7 +22,7 @@ test_expect_success setup '
 '
 
 test_expect_success 'multi-read' '
-	git read-tree initial master side &&
+	read_tree_must_succeed initial master side &&
 	(echo a; echo b/c) >expect &&
 	git ls-files >actual &&
 	test_cmp expect actual

t/t1011-read-tree-sparse-checkout.sh +14 ~~-13~~

index 20a50eb..018c354 100755
@@ -12,6 +12,7 @@ test_description='sparse checkout tests
 '
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-read-tree.sh
 
 test_expect_success 'setup' '
 	cat >expected <<-\EOF &&

@@ -43,7 +44,7 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'read-tree without .git/info/sparse-checkout' '
-	git read-tree -m -u HEAD &&
+	read_tree_u_must_succeed -m -u HEAD &&
 	git ls-files --stage >result &&
 	test_cmp expected result &&
 	git ls-files -t >result &&

@@ -52,7 +53,7 @@ test_expect_success 'read-tree without .git/info/sparse-checkout' '
 
 test_expect_success 'read-tree with .git/info/sparse-checkout but disabled' '
 	echo >.git/info/sparse-checkout &&
-	git read-tree -m -u HEAD &&
+	read_tree_u_must_succeed -m -u HEAD &&
 	git ls-files -t >result &&
 	test_cmp expected.swt result &&
 	test -f init.t &&

@@ -62,7 +63,7 @@ test_expect_success 'read-tree with .git/info/sparse-checkout but disabled' '
 test_expect_success 'read-tree --no-sparse-checkout with empty .git/info/sparse-checkout and enabled' '
 	git config core.sparsecheckout true &&
 	echo >.git/info/sparse-checkout &&
-	git read-tree --no-sparse-checkout -m -u HEAD &&
+	read_tree_u_must_succeed --no-sparse-checkout -m -u HEAD &&
 	git ls-files -t >result &&
 	test_cmp expected.swt result &&
 	test -f init.t &&

@@ -72,7 +73,7 @@ test_expect_success 'read-tree --no-sparse-checkout with empty .git/info/sparse-
 test_expect_success 'read-tree with empty .git/info/sparse-checkout' '
 	git config core.sparsecheckout true &&
 	echo >.git/info/sparse-checkout &&
-	test_must_fail git read-tree -m -u HEAD &&
+	read_tree_u_must_fail -m -u HEAD &&
 	git ls-files --stage >result &&
 	test_cmp expected result &&
 	git ls-files -t >result &&

@@ -90,7 +91,7 @@ test_expect_success 'match directories with trailing slash' '
 	EOF
 
 	echo sub/ > .git/info/sparse-checkout &&
-	git read-tree -m -u HEAD &&
+	read_tree_u_must_succeed -m -u HEAD &&
 	git ls-files -t > result &&
 	test_cmp expected.swt-noinit result &&
 	test ! -f init.t &&

@@ -99,7 +100,7 @@ test_expect_success 'match directories with trailing slash' '
 
 test_expect_success 'match directories without trailing slash' '
 	echo sub >.git/info/sparse-checkout &&
-	git read-tree -m -u HEAD &&
+	read_tree_u_must_succeed -m -u HEAD &&
 	git ls-files -t >result &&
 	test_cmp expected.swt-noinit result &&
 	test ! -f init.t &&

@@ -149,7 +150,7 @@ EOF
 
 test_expect_success 'match directory pattern' '
 	echo "s?b" >.git/info/sparse-checkout &&
-	git read-tree -m -u HEAD &&
+	read_tree_u_must_succeed -m -u HEAD &&
 	git ls-files -t >result &&
 	test_cmp expected.swt-noinit result &&
 	test ! -f init.t &&

@@ -165,7 +166,7 @@ test_expect_success 'checkout area changes' '
 	EOF
 
 	echo init.t >.git/info/sparse-checkout &&
-	git read-tree -m -u HEAD &&
+	read_tree_u_must_succeed -m -u HEAD &&
 	git ls-files -t >result &&
 	test_cmp expected.swt-nosub result &&
 	test -f init.t &&

@@ -175,7 +176,7 @@ test_expect_success 'checkout area changes' '
 test_expect_success 'read-tree updates worktree, absent case' '
 	echo sub/added >.git/info/sparse-checkout &&
 	git checkout -f top &&
-	git read-tree -m -u HEAD^ &&
+	read_tree_u_must_succeed -m -u HEAD^ &&
 	test ! -f init.t
 '

@@ -183,7 +184,7 @@ test_expect_success 'read-tree updates worktree, dirty case' '
 	echo sub/added >.git/info/sparse-checkout &&
 	git checkout -f top &&
 	echo dirty >init.t &&
-	git read-tree -m -u HEAD^ &&
+	read_tree_u_must_succeed -m -u HEAD^ &&
 	grep -q dirty init.t &&
 	rm init.t
 '

@@ -192,14 +193,14 @@ test_expect_success 'read-tree removes worktree, dirty case' '
 	echo init.t >.git/info/sparse-checkout &&
 	git checkout -f top &&
 	echo dirty >added &&
-	git read-tree -m -u HEAD^ &&
+	read_tree_u_must_succeed -m -u HEAD^ &&
 	grep -q dirty added
 '
 
 test_expect_success 'read-tree adds to worktree, absent case' '
 	echo init.t >.git/info/sparse-checkout &&
 	git checkout -f removed &&
-	git read-tree -u -m HEAD^ &&
+	read_tree_u_must_succeed -u -m HEAD^ &&
 	test ! -f sub/added
 '

@@ -208,7 +209,7 @@ test_expect_success 'read-tree adds to worktree, dirty case' '
 	git checkout -f removed &&
 	mkdir sub &&
 	echo dirty >sub/added &&
-	git read-tree -u -m HEAD^ &&
+	read_tree_u_must_succeed -u -m HEAD^ &&
 	grep -q dirty sub/added
 '

t/t1012-read-tree-df.sh +5 -4

index 9811d46..a6a04b6 100755
@@ -3,6 +3,7 @@
 test_description='read-tree D/F conflict corner cases'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-read-tree.sh
 
 maketree () {
 	(

@@ -53,7 +54,7 @@ test_expect_success setup '
 
 test_expect_success '3-way (1)' '
 	settree A-000 &&
-	git read-tree -m -u O-000 A-000 B-000 &&
+	read_tree_u_must_succeed -m -u O-000 A-000 B-000 &&
 	checkindex <<-EOF
 	3 a/b
 	0 a/b-2/c/d

@@ -65,7 +66,7 @@ test_expect_success '3-way (1)' '
 
 test_expect_success '3-way (2)' '
 	settree A-001 &&
-	git read-tree -m -u O-000 A-001 B-000 &&
+	read_tree_u_must_succeed -m -u O-000 A-001 B-000 &&
 	checkindex <<-EOF
 	3 a/b
 	0 a/b-2/c/d

@@ -78,7 +79,7 @@ test_expect_success '3-way (2)' '
 
 test_expect_success '3-way (3)' '
 	settree A-010 &&
-	git read-tree -m -u O-010 A-010 B-010 &&
+	read_tree_u_must_succeed -m -u O-010 A-010 B-010 &&
 	checkindex <<-EOF
 	2 t
 	1 t-0

@@ -92,7 +93,7 @@ test_expect_success '3-way (3)' '
 
 test_expect_success '2-way (1)' '
 	settree O-020 &&
-	git read-tree -m -u O-020 A-020 &&
+	read_tree_u_must_succeed -m -u O-020 A-020 &&
 	checkindex <<-EOF
 	0 ds/dma/ioat/Makefile
 	0 ds/dma/ioat/registers.h

t/t1020-subdirectory.sh +3 -2

index ddc3921..f6a44c9 100755
@@ -7,6 +7,7 @@ test_description='Try various core-level commands in subdirectory.
 '
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-read-tree.sh
 
 test_expect_success setup '
 	long="a b c d e f g h i j k l m n o p q r s t u v w x y z" &&

@@ -98,13 +99,13 @@ test_expect_success 'checkout-index' '
 test_expect_success 'read-tree' '
 	rm -f one dir/two &&
 	tree=`git write-tree` &&
-	git read-tree --reset -u "$tree" &&
+	read_tree_u_must_succeed --reset -u "$tree" &&
 	cmp one original.one &&
 	cmp dir/two original.two &&
 	(
 		cd dir &&
 		rm -f two &&
-		git read-tree --reset -u "$tree" &&
+		read_tree_u_must_succeed --reset -u "$tree" &&
 		cmp two ../original.two &&
 		cmp ../one ../original.one
 	)

Signature · 1 line

1.7.5.2.355.gdceb0

Junio C Hamano

2016-06-15 22:51 UTC #6 lore raw

Re: [PATCH v2 2/2] Teach read-tree the -n|--dry-run option

Quoted 4 lines · Jens Lehmann

Using this option tells read-tree to not update the index. That makes it possible to check if updating the index would be successful without changing anything. Using this option will silently ignore -u but still test if updating the work tree would succeed.

I'll reword the above to de-stress "the index" when I queue this patch, as this is not about the index anymore. Also I do not think you "silently ignore" the "-u" option. Don't you check what would happen when "-u" is given and checking out the merge result would overwrite the file in the working tree? In that scenario, "-u" actively participates in the dry-run check you implemented.

Quoted 11 lines

t/lib-read-tree.sh new file +5 -0

new file mode 100644
index 0000000..435d19c
@@ -0,0 +1,43 @@
+#!/bin/sh
+#
+# Helper functions to check if read-tree would succeed/fail as expected with
+# and without the dry-run option. They also test that the dry-run does not
+# write the index and that together with -u it doesn't touch the work tree.

t/lib-read-tree.sh

Quoted 10 lines

t/lib-read-tree.sh new file +9 -0

+#
...
+read_tree_u_must_succeed () {
+    git ls-files -s >pre-dry-run &&
+    git diff-files >pre-dry-run-wt &&
+    git read-tree -n "$@" &&
+    git ls-files -s >post-dry-run &&
+    git diff-files >post-dry-run-wt &&
+    test_cmp pre-dry-run post-dry-run &&
+    test_cmp pre-dry-run-wt post-dry-run-wt &&

As the RHS of diff-files output that represent the working tree status indicates "contents indeterminate" with 0{40}, this comparison is the same as comparing what is in the index before and after the dry-run.

t/lib-read-tree.sh

Quoted 11 lines

...

t/lib-read-tree.sh new file +10 -0

+read_tree_u_must_fail () {
+    git ls-files -s >pre-dry-run &&
+    git diff-files >pre-dry-run-wt &&
+    test_must_fail git read-tree -n "$@" &&
+    git ls-files -s >post-dry-run &&
+    git diff-files >post-dry-run-wt &&
+    test_cmp pre-dry-run post-dry-run &&
+    test_cmp pre-dry-run-wt post-dry-run-wt &&
+    test_must_fail git read-tree "$@"
+}

Same here. A few additions of -p will trivially fix them, though.

t/lib-read-tree.sh +4 -4

index 435d19c..abc2c6f 100644
@@ -22,10 +22,10 @@ read_tree_must_fail () {
 
 read_tree_u_must_succeed () {
     git ls-files -s >pre-dry-run &&
-    git diff-files >pre-dry-run-wt &&
+    git diff-files -p >pre-dry-run-wt &&
     git read-tree -n "$@" &&
     git ls-files -s >post-dry-run &&
-    git diff-files >post-dry-run-wt &&
+    git diff-files -p >post-dry-run-wt &&
     test_cmp pre-dry-run post-dry-run &&
     test_cmp pre-dry-run-wt post-dry-run-wt &&
     git read-tree "$@"

@@ -33,10 +33,10 @@ read_tree_u_must_succeed () {
 
 read_tree_u_must_fail () {
     git ls-files -s >pre-dry-run &&
-    git diff-files >pre-dry-run-wt &&
+    git diff-files -p >pre-dry-run-wt &&
     test_must_fail git read-tree -n "$@" &&
     git ls-files -s >post-dry-run &&
-    git diff-files >post-dry-run-wt &&
+    git diff-files -p >post-dry-run-wt &&
     test_cmp pre-dry-run post-dry-run &&
     test_cmp pre-dry-run-wt post-dry-run-wt &&
     test_must_fail git read-tree "$@"