Teach checkout the -n|--dry-run option

This option can be used to determine if checking out a branch or commit would touch files which are modified in the work tree. It doesn't change the work tree contents or the index, so it can only tell if the checkout would succeed using trivial merges.

It can neither be used when checking out only certain paths nor together with the '-m' option.

---

This is a repost of a RFC patch from Sep 04, 2010 I did not receive any responses upon:

I need this new option for the recursive submodule checkout. Denying a checkout inside a submodule just because it has modifications is a too limiting condition (and it is way too expensive to check the whole tree, just looking at those paths going to be changed by the checkout should be much faster, especially for large submodules).

IMO the same behavior that applies for a checkout in the superproject should apply for the checkout inside the submodule: no local changes may be overwritten by the checkout (and the HEAD must match the commit recorded in the superproject, but that is handled elsewhere).

To be able to test that, I added the -n|--dry-run option to checkout. I think I found all relevant places, but a few more eyeballs are highly appreciated as I am not very familiar with this part of the code.

A thing I'm not so sure about is the output of show_local_changes(), with -n it doesn't show the changes as they would have been *after* the
- not executed - checkout but *without* having done it.
Is that a problem here?

 Documentation/git-checkout.txt |   15 ++++++++++++---
 builtin/checkout.c             |   20 +++++++++++++++-----
 t/t2018-checkout-branch.sh     |   23 +++++++++++++++++++++++
 3 files changed, 50 insertions(+), 8 deletions(-)

index 1063f69..ae58762 100644
@@ -8,9 +8,9 @@ git-checkout - Checkout a branch or paths to the working tree
 SYNOPSIS
 --------
 [verse]
-'git checkout' [-q] [-f] [-m] [<branch>]
-'git checkout' [-q] [-f] [-m] [--detach] [<commit>]
-'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]
+'git checkout' [-q] [-f] [-m|-n] [<branch>]
+'git checkout' [-q] [-f] [-m|-n] [--detach] [<commit>]
+'git checkout' [-q] [-f] [-m|-n] [[-b|-B|--orphan] <new_branch>] [<start_point>]
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
 'git checkout' --patch [<tree-ish>] [--] [<paths>...]
 

@@ -79,6 +79,13 @@ OPTIONS
 When checking out paths from the index, do not fail upon unmerged
 entries; instead, unmerged entries are ignored.
 
+-n::
+--dry-run::
+	Don't really checkout the file(s), just check if it would succeed
+	using only trivial merges. Using this option will not touch the work
+	tree or the index. The command will fail when the checkout would change
+	locally modified files. This option can not be used together with `-m`.
+
 --ours::
 --theirs::
 	When checking out paths from the index, check out stage #2

@@ -167,6 +174,8 @@ should result in deletion of the path).
 +
 When checking out paths from the index, this option lets you recreate
 the conflicted merge in the specified paths.
++
+This option does not work together with '-n'.
 
 --conflict=<style>::
 	The same as --merge option above, but changes the way the

index 38632fc..01175d1 100644
@@ -33,6 +33,7 @@ struct checkout_opts {
 	int force_detach;
 	int writeout_stage;
 	int writeout_error;
+	int dry_run;
 
 	/* not set by parse_options */
 	int branch_exists;

@@ -373,6 +374,8 @@ static int merge_working_tree(struct checkout_opts *opts,
 
 	resolve_undo_clear();
 	if (opts->force) {
+		if (opts->dry_run)
+			return 0;
 		ret = reset_tree(new->commit->tree, opts, 1);
 		if (ret)
 			return ret;

@@ -397,7 +400,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 
 		/* 2-way merge to the new branch */
 		topts.initial_checkout = is_cache_unborn();
-		topts.update = 1;
+		topts.update = !opts->dry_run;
 		topts.merge = 1;
 		topts.gently = opts->merge && old->commit;
 		topts.verbose_update = !opts->quiet;

@@ -471,8 +474,8 @@ static int merge_working_tree(struct checkout_opts *opts,
 		}
 	}
 
-	if (write_cache(newfd, active_cache, active_nr) ||
-	    commit_locked_index(lock_file))
+	if (!opts->dry_run && (write_cache(newfd, active_cache, active_nr) ||
+			       commit_locked_index(lock_file)))
 		die(_("unable to write new index file"));
 
 	if (!opts->force && !opts->quiet)

@@ -732,7 +735,7 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 	}
 
 	ret = merge_working_tree(opts, &old, new);
-	if (ret)
+	if (ret || opts->dry_run)
 		return ret;
 
 	if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)

@@ -941,6 +944,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN('m', "merge", &opts.merge, "perform a 3-way merge with the new branch"),
 		OPT_STRING(0, "conflict", &conflict_style, "style",
 			   "conflict style (merge or diff3)"),
+		OPT_BOOLEAN('n', "dry-run", &opts.dry_run, "show if the checkout would fail because it touches locally modified files"),
 		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
 		{ OPTION_BOOLEAN, 0, "guess", &dwim_new_local_branch, NULL,
 		  "second guess 'git checkout no-such-branch'",

@@ -967,7 +971,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	if (opts.new_branch_force)
 		opts.new_branch = opts.new_branch_force;
 
-	if (patch_mode && (opts.track > 0 || opts.new_branch
+	if (patch_mode && (opts.track > 0 || opts.new_branch || opts.dry_run
 			   || opts.new_branch_log || opts.merge || opts.force
 			   || opts.force_detach))
 		die (_("--patch is incompatible with all other options"));

@@ -1008,6 +1012,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	if (opts.force && opts.merge)
 		die(_("git checkout: -f and -m are incompatible"));
 
+	if (opts.merge && opts.dry_run)
+		die("git checkout: -m and -n are incompatible");
+
 	/*
 	 * Extract branch name from command line arguments, so
 	 * all that is left is pathspecs.

@@ -1060,6 +1067,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		if (1 < !!opts.writeout_stage + !!opts.force + !!opts.merge)
 			die(_("git checkout: --ours/--theirs, --force and --merge are incompatible when\nchecking out of the index."));
 
+		if (opts.dry_run)
+			die("git checkout: updating paths is incompatible with -n.");
+
 		return checkout_paths(source_tree, pathspec, &opts);
 	}
 

index fa69016..61c82ef 100755
@@ -169,4 +169,27 @@ test_expect_success 'checkout -f -B to an existing branch with mergeable changes
 	test_must_fail test_dirty_mergeable
 '
 
+test_expect_success 'checkout -n/--dry-run does not change work tree or index' '
+	echo precious >expect &&
+	echo precious >file1 &&
+	test_must_fail git checkout -n branch1 &&
+	test_cmp expect file1 &&
+	git checkout --dry-run -f branch1 &&
+	test_cmp expect file1 &&
+	test_must_fail git checkout -b new_branch --dry-run branch1 &&
+	test_cmp expect file1 &&
+	git checkout -b new_branch -n -f branch1 &&
+	test_cmp expect file1 &&
+	git checkout -f branch1 &&
+	git status -s -uno > actual &&
+	! test -s actual &&
+	git checkout -n HEAD^ &&
+	git status -s -uno > actual &&
+	! test -s actual
+'
+
+test_expect_success 'checkout -n/--dry-run is not allowed when checking out only certain paths' '
+	test_must_fail git checkout -n branch1 file1 &&
+	test_must_fail git checkout -n HEAD file1
+'
 test_done

1.7.5.1.174.g799a9

I hated this patch when I saw it the first time. I thought you were joking and let it pass. But now you brought it again, I hate it even more now.

I do not see a merit in contaminating the checkout Porcelain with a half baked [*1*] "dry-run" code, when your ultimate motive to use it in submodule script and exercise only a small portion of checkout codepaths. You are only interested in branch switching, and not other things the Porcelain can do, like checking out a path out of the index or a tree, or re-checking out a conflicted path out of the index.

In other words, can't the check you need in submodule be scripted around the specific plumbing responsible for the branch switching, which is:

    $ git read-tree -m HEAD $other_branch

That is, this would succeed:

	$ git reset --hard master
        $ git read-tree --index-output=rubbish -m master next

and these would fail:

	$ git reset --hard master
	$ echo >>Makefile
        $ git read-tree --index-output=rubbish -m master next
        error: Entry 'Makefile' not uptodate. Cannot merge.

	$ git reset --hard master
	$ echo >>Makefile
	$ git add Makefile
        $ git read-tree --index-output=rubbish -m master next
        error: Entry 'Makefile' would be overwritten by merge. Cannot merge.

Having said that, please do not discard your patch. After sleeping on this, I started to think that "checkout -n" might be a better interface than using the plumbing read-tree in the longer term, especially if you can enhance it to handle "checkout -m -n" to check if the local change can be merged without conflicts.

But if the only question you are interested in is "can I switch to that branch from the current state of the index and the working tree?", I would prefer to see if the script can use "read-tree -m" approach first.

We may also want to add "read-tree -n" so that you do not have to specify a dummy index output only to prevent from writing the real index over, though.

Am 07.05.2011 21:24, schrieb Junio C Hamano:

Thanks for pointing me to "git read-tree -m". When I saw that a "git checkout -n" would do exactly what I needed, I stopped looking for alternatives (especially as I saw that adding -n there would help other non-submodule use cases as well).

I'll see if I can come up with a solution for the "-m -n" case (I stopped after implementing the checkout branch case I needed to get some feedback if this thing went into the right direction). And I assume the "git checkout <pathspec>" case should learn -n too?

Yup, I will try that.

Hmm, wouldn't using "read-tree --index-output=/dev/null" be equivalent to "read-tree -n"? But nonetheless it might make sense to add the -n option.

Am 08.05.2011 13:22, schrieb Jens Lehmann:

No idea how I could manage to test "read-tree --index-output=/dev/null" successfully without getting an "unable to write new index file" error.

But using read-tree works for me, so what about something like this, maybe with some more tests?

-------------8<---------------
Subject: [PATCH] Teach read-tree the -n|--dry-run option

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 it.

As using --dry-run is expected to have no side effects, this option makes no sense together with "-u".

---
 Documentation/git-read-tree.txt |    5 +++++
 builtin/read-tree.c             |    7 +++++--
 t/t1000-read-tree-m-3way.sh     |    2 ++
 3 files changed, 12 insertions(+), 2 deletions(-)

index 26fdadc..a35849f 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::
+	Don't write the index file. This option isn't allowed together
+	with -u.
+
 -v::
 	Show the progress of checking files out.
 

index 93c9281..693576f 100644
@@ -98,7 +98,7 @@ static struct lock_file lock_file;
 
 int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 {
-	int i, newfd, stage = 0;
+	int i, newfd, stage = 0, dry_run = 0;
 	unsigned char sha1[20];
 	struct tree_desc t[MAX_UNPACK_TREES];
 	struct unpack_trees_options opts;

@@ -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(&dry_run, "don't update the index"),
 		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,

@@ -183,6 +184,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 		die("--exclude-per-directory is meaningless unless -u");
 	if (opts.merge && !opts.index_only)
 		setup_work_tree();
+	if (opts.update && dry_run)
+		die("--dry-run contradicts -u");
 
 	if (opts.merge) {
 		if (stage < 2)

@@ -219,7 +222,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 || dry_run)
 		return 0; /* do not write the index out */
 
 	/*

index ca8a409..bcfb5e6 100755
@@ -259,6 +259,8 @@ test_expect_success \
     "rm -f .git/index AA &&
      cp .orig-A/AA AA &&
      git update-index --add AA &&
+     git read-tree -n -m $tree_O $tree_A $tree_B &&
+     test_must_fail check_result &&
      git read-tree -m $tree_O $tree_A $tree_B &&
      check_result"
 

1.7.5.1.218.g8af57

Am 08.05.2011 23:30, schrieb Junio C Hamano:

Yeah, that is much better than my first sketch.

Thanks, will send an updated patch with better test cases soon ...

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 it.

As using --dry-run is expected to have no side effects, this option makes no sense together with "-u".

---

Now with updated tests.

 Documentation/git-read-tree.txt |    5 ++
 builtin/read-tree.c             |    7 ++-
 t/t1000-read-tree-m-3way.sh     |   96 +++++++++++++++++++++++----------------
 t/t1001-read-tree-m-2way.sh     |   48 +++++++++++--------
 t/t1002-read-tree-m-u-2way.sh   |    4 ++
 5 files changed, 98 insertions(+), 62 deletions(-)

index 26fdadc..a35849f 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::
+	Don't write the index file. This option isn't allowed together
+	with -u.
+
 -v::
 	Show the progress of checking files out.
 

index 93c9281..693576f 100644
@@ -98,7 +98,7 @@ static struct lock_file lock_file;
 
 int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 {
-	int i, newfd, stage = 0;
+	int i, newfd, stage = 0, dry_run = 0;
 	unsigned char sha1[20];
 	struct tree_desc t[MAX_UNPACK_TREES];
 	struct unpack_trees_options opts;

@@ -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(&dry_run, "don't update the index"),
 		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,

@@ -183,6 +184,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 		die("--exclude-per-directory is meaningless unless -u");
 	if (opts.merge && !opts.index_only)
 		setup_work_tree();
+	if (opts.update && dry_run)
+		die("--dry-run contradicts -u");
 
 	if (opts.merge) {
 		if (stage < 2)

@@ -219,7 +222,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 || dry_run)
 		return 0; /* do not write the index out */
 
 	/*

index a4a17e0..ad9a4e8 100755
@@ -341,4 +341,8 @@ test_expect_success \
      test_cmp DFDF.out DFDFcheck.out &&
      check_cache_at DF/DF clean'
 
+test_expect_success \
+    '-u is not allowed with -n' \
+    'test_must_fail git read-tree -n -u -m $treeDF $treeDFDF'
+
 test_done

1.7.5.1.218.g2da34

Thanks for verifying the 3-way case as well.

I wondered if there be cases where "read-tree -m <1 to 3 trees>" will succeed but the same command with "-u" can fail. If there were such cases, we would need more than this patch does.

An obvious case is when you cannot write to your working tree, perhaps due to ENOSPC or incorrect permission settings in the working tree, but I am not worried about that. I am only worried about situations related to version control (i.e. you may lose local changes).

I _think_ the only difference "-u" makes is that check_updates() makes calls to checkout_entry(), and the only errors checkout_entry() would catch are filesystem related ones. Even though there is one conditional that says "if the cached stat does not match, you cannot checkout unless you set .force to the checkout state", but unpack_trees() does set that flag, so we should be safe.

Thanks.