threads / discuss / 23889

'git pull --dry-run' accepted, but moves HEAD and changes working tree

Subject: 'git pull --dry-run' accepted, but moves HEAD and changes working tree

## tl;dr

3 messages between May 24, 2010 and May 26, 2010.

replies: 2people: 3as markdown or json

Jeff Epler· May 24, 2010, 13:58 UTC · lore

I don't recall why I initially typed it, I was surprised to discover that 'git pull --dry-run' moves HEAD and modifies the working tree.

Here's an example:
$ git reset --hard HEAD^  # so there's something on origin to merge
HEAD is now at c26a08d fix clearing of mesa components
$ git rev-parse HEAD; git pull --dry-run; git rev-parse HEAD
c26a08d1535a02ec044efd0d1fa50205d2da03fa
Updating c26a08d..41c8ee3
Fast-forward
 docs/man/man9/hostmot2.9                |    7 +++++--
 src/hal/drivers/mesa-hostmot2/stepgen.c |    5 +++++
 2 files changed, 10 insertions(+), 2 deletions(-)
41c8ee3e19ffb13cc357375d87940cac4769e029
$ git --version
git version 1.7.1

On IRC, jast points out that 'git pull --dry-run' is not explicitly documented, but unfortunately it is accepted and then does something that is really counter to the user's expectations. (I assume it's passing --dry-run to fetch, which does accept it, but that doesn't ensure that there's nothing at all to merge)

Maybe 'git pull --dry-run' should just be forbidden, or maybe it could tell the user whether the result of the pull would be a merge, FF, or no change.

Jeff
Jeff King· May 25, 2010, 06:07 UTC · re: Jeff Epler · lore

Re: 'git pull --dry-run' accepted, but moves HEAD and changes working tree

On Mon, May 24, 2010 at 08:58:23AM -0500, Jeff Epler wrote:
> I don't recall why I initially typed it, I was surprised to discover
> that 'git pull --dry-run' moves HEAD and modifies the working tree.
Yeah, that's bad.
> (I assume it's passing --dry-run to fetch, which does accept it, but
> that doesn't ensure that there's nothing at all to merge)
That is exactly what is happening.
> Maybe 'git pull --dry-run' should just be forbidden, or maybe it could
> tell the user whether the result of the pull would be a merge, FF, or no
> change.

We can't tell what would have happened, because the dry-run fetch doesn't write anything into FETCH_HEAD (which is where we would look to see what was merge-able or not). But we can at least stop at the fetch dry-run to prevent any further damage.

I have no problem with simply reporting an error, but it is easy enough to also just have it stop after doing the fetch dry-run, as below.

-- >8 --
Subject: [PATCH] pull: do nothing on --dry-run

Pull was never meant to take --dry-run at all. However, it passes unknown arguments to git-fetch, which does do a dry-run. Unfortunately, pull then attempts to merge whatever cruft was in FETCH_HEAD (which the dry-run fetch will not have written to).

Even though we never advertise --dry-run as something that should work, it is still worth being defensive because:

  1. Other commands (including fetch) take --dry-run, so a
     user might try it.
  2. Rather than simply producing an error, it actually
     changes the repository in totally unexpected ways.

This patch makes "pull --dry-run" equivalent to "fetch --dry-run".

Signed-off-by: Jeff King <peff@peff.net>
---
 git-pull.sh |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index 1a4729f..a09a44e 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -43,6 +43,7 @@ merge_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
+dry_run=
 while :
 do
 	case "$1" in
@@ -104,6 +105,9 @@ do
 	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
 		rebase=false
 		;;
+	--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
+		dry_run=--dry-run
+		;;
 	-h|--h|--he|--hel|--help)
 		usage
 		;;
@@ -216,7 +220,8 @@ test true = "$rebase" && {
 	done
 }
 orig_head=$(git rev-parse -q --verify HEAD)
-git fetch $verbosity $progress --update-head-ok "$@" || exit 1
+git fetch $verbosity $progress $dry_run --update-head-ok "$@" || exit 1
+test -z "$dry_run" || exit 0
 
 curr_head=$(git rev-parse -q --verify HEAD)
 if test -n "$orig_head" && test "$curr_head" != "$orig_head"
-- 
1.7.1.226.g770c5.dirty

← back to recent threads