What exactly does 'needs update' mean?

What exactly does 'needs update' mean?

[Joshua Jensen]

  I've come to accept the term 'needs update' when I've forgotten to 
stash or commit before certain Git operations.  However, I got cornered 
today and was asked to explain what it means.  I had to admit I don't know.

What does "you can't do this operation without having a clean working 
tree and these are the modified files currently preventing the operation 
from proceeding" have to do with "needs update"?

Thanks.

Josh

Re: What exactly does 'needs update' mean?

[Junio C Hamano]

Joshua Jensen <jjensen@workspacewhiz.com> writes:

>  I've come to accept the term 'needs update' when I've forgotten to
> stash or commit before certain Git operations.  However, I got
> cornered today and was asked to explain what it means.  I had to admit
> I don't know.

It came from "you need to run update-index on that path, as you have local
modification in the working tree".

Re: What exactly does 'needs update' mean?

[Joshua Jensen]

  ----- Original Message -----
From: Junio C Hamano
Date: 9/25/2010 12:06 AM
> Joshua Jensen<jjensen@workspacewhiz.com>  writes:
>>   I've come to accept the term 'needs update' when I've forgotten to
>> stash or commit before certain Git operations.  However, I got
>> cornered today and was asked to explain what it means.  I had to admit
>> I don't know.
> It came from "you need to run update-index on that path, as you have local
> modification in the working tree".
Okay, your description makes sense to me, and I'll be able to explain 
what it means.

I did a Google search before I posted here.  It turns out this phrase is 
*very* confusing to others.  Casual Joes don't use the plumbing commands 
(which I assume git update-index is).  Is there opposition to 
modernizing this turn to make it more clear based on the porcelain 
commands being run?

Thanks.

Josh

Re: What exactly does 'needs update' mean?

[Joshua Jensen]

  ----- Original Message -----
From: Joshua Jensen
Date: 9/25/2010 8:16 AM
>  ----- Original Message -----
> From: Junio C Hamano
> Date: 9/25/2010 12:06 AM
>> Joshua Jensen<jjensen@workspacewhiz.com>  writes:
>>>   I've come to accept the term 'needs update' when I've forgotten to
>>> stash or commit before certain Git operations.  However, I got
>>> cornered today and was asked to explain what it means.  I had to admit
>>> I don't know.
>> It came from "you need to run update-index on that path, as you have 
>> local
>> modification in the working tree".
> Okay, your description makes sense to me, and I'll be able to explain 
> what it means.
>
> I did a Google search before I posted here.  It turns out this phrase 
> is *very* confusing to others.  Casual Joes don't use the plumbing 
> commands (which I assume git update-index is).  Is there opposition to 
> modernizing this turn to make it more clear based on the porcelain 
> commands being run?
<sigh> Just waking up for the day.

Is there opposition to modernizing this *term* to make it more clear 
based on the porcelain commands being run?

-Josh

[PATCH 0/2] Eliminate cryptic "needs update" error message

[Ramkumar Ramachandra]

Hi Joshua,

Joshua Jensen writes:
> Is there opposition to modernizing this *term* to make it more clear
> based on the porcelain commands being run?

Most porcelain commands already contain checks to specifically prevent
it from displaying this message. See:

cache.h:506:#define REFRESH_IN_PORCELAIN        0x0020  /* user friendly output, not "needs update" */
git-add--interactive.perl:215:          ;# ignore 'needs update'
read-cache.c:1109:      needs_update_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
builtin/add.c:190:      refresh_index(&the_index, verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET,
builtin/commit.c:284:   if (refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN))
builtin/reset.c:326:                            quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
builtin/reset.c:377:                            quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
cache.h:506:#define REFRESH_IN_PORCELAIN        0x0020  /* user friendly output, not "needs update" */
read-cache.c:1104:      int in_porcelain = (flags & REFRESH_IN_PORCELAIN);

The real problem is with the shell scripts that invoke `git
update-index --refresh` without `-q` and forget to redirect output to
/dev/null. The Git infrastructure thinks update-index is a
non-porcelain: little does it know that update-index being run from a
porcelain-level shell script.

Thank you for reporting this bug.

-- Ram

Ramkumar Ramachandra (2):
  sh-setup: Write a new require_clean_work_tree function
  Porcelain scripts: Rewrite cryptic "needs update" error message

 git-pull.sh                |    5 +----
 git-rebase--interactive.sh |   16 ++++------------
 git-rebase.sh              |   14 +-------------
 git-sh-setup.sh            |   23 +++++++++++++++++++++++
 4 files changed, 29 insertions(+), 29 deletions(-)

-- 
1.7.2.2.409.gdbb11.dirty

[PATCH 1/2] sh-setup: Write a new require_clean_work_tree function

[Ramkumar Ramachandra]

Write a new require_clean_work_tree function to error out when working
tree contains unstaged changes or index contains uncommitted changes.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-sh-setup.sh |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 6131670..3a337da 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -145,6 +145,29 @@ require_work_tree () {
 	die "fatal: $0 cannot be used without a working tree."
 }
 
+require_clean_work_tree () {
+	# Update the index
+	git update-index -q --ignore-submodules --refresh
+
+	# Disallow unstaged changes in the working tree
+	if ! git diff-files --quiet --ignore-submodules --
+	then
+		echo >&2 "cannot $1: you have unstaged changes."
+		echo >&2 "Please commit or stash them."
+		git diff-files --name-status -r --ignore-submodules -- >&2
+		exit 1
+	fi
+
+	# Disallow uncommitted changes in the index
+	if ! git diff-index --cached --quiet --ignore-submodules HEAD --
+	then
+		echo >&2 "cannot $1: your index contains uncommitted changes."
+		echo >&2 "Please commit or stash them."
+		git diff-index --cached --name-status -r --ignore-submodules HEAD -- >&2
+		exit 1
+	fi
+}
+
 get_author_ident_from_commit () {
 	pick_author_script='
 	/^author /{
-- 
1.7.2.2.409.gdbb11.dirty

Re: [PATCH 1/2] sh-setup: Write a new require_clean_work_tree function

[Matthieu Moy]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> +	then
> +		echo >&2 "cannot $1: you have unstaged changes."
> +		echo >&2 "Please commit or stash them."
> +		git diff-files --name-status -r --ignore-submodules -- >&2

I totally agree on the idea, and the implementation is OK. On the
format of the message, you can try to make it more consistent with
other error messages, like:

$ git merge branch
error: The following untracked working tree files would be overwritten by merge:
        one
	two
Please move or remove them before you can merge.

That would give stg like:

echo >&2 "error: The following files have unstaged changes:"
git diff-files --name-status -r --ignore-submodules -- >&2
echo >&2 "Please commit or stash them to proceed."

(same for the second case)

Also, you probably want to give all the error before you "exit 1",
hence stg like:

error=f
...
if
then
	...
	error=t
fi

if
then
	...
	error=t
fi

if [ "$error" = "t" ]; then
	exit 1
fi

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 1/2] sh-setup: Write a new require_clean_work_tree function

[Ramkumar Ramachandra]

Hi Matthieu,

Matthieu Moy writes:
> I totally agree on the idea, and the implementation is OK. On the
> format of the message, you can try to make it more consistent with
> other error messages, like:

Thanks for the review.

> $ git merge branch
> error: The following untracked working tree files would be overwritten by merge:
>         one
> 	two
> Please move or remove them before you can merge.
> 
> That would give stg like:
> 
> echo >&2 "error: The following files have unstaged changes:"
> git diff-files --name-status -r --ignore-submodules -- >&2
> echo >&2 "Please commit or stash them to proceed."

Ok, sounds good.

> Also, you probably want to give all the error before you "exit 1",
> hence stg like:

Hm, is that a good idea? We want the output to be functional and
indicative: it should tell the user what to do immediately. I'm afraid
that displaying both errors will make the output very verbose. We can
just tell the user about the unstaged changes, and wait for them to
commit or stash it. Either way, both commit and stash will affect the
index by default :)

-- Ram

Re: [PATCH 1/2] sh-setup: Write a new require_clean_work_tree function

[Matthieu Moy]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

>> Also, you probably want to give all the error before you "exit 1",
>> hence stg like:
>
> Hm, is that a good idea? We want the output to be functional and
> indicative: it should tell the user what to do immediately.

Yes, but I find this very painfull when you

$ git do-something
error: you need X before you can do-something
$ do X
$ git do-something
error: Ah, you also need Y before you can do-something

> I'm afraid that displaying both errors will make the output very
> verbose. We can just tell the user about the unstaged changes, and
> wait for them to commit or stash it. Either way, both commit and
> stash will affect the index by default :)

A plain commit will get rid of staged changes, not of unstaged ones.

Your patch shows unstaged changes first. If the only problem was
unstaged changes, then "git stash --keep-index" would be a good
solution. As a user, I prefer knowing both problems to find the right
solution (and avoid trying to solve only unstaged changes before
noticing I need to solve the other one too).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 1/2] sh-setup: Write a new require_clean_work_tree function

[Ramkumar Ramachandra]

Hi Matthieu,

Matthieu Moy writes:
> Yes, but I find this very painfull when you
> 
> $ git do-something
> error: you need X before you can do-something
> $ do X
> $ git do-something
> error: Ah, you also need Y before you can do-something

Ah, yes. This would definitely be annoying.

> A plain commit will get rid of staged changes, not of unstaged ones.
> 
> Your patch shows unstaged changes first. If the only problem was
> unstaged changes, then "git stash --keep-index" would be a good
> solution. As a user, I prefer knowing both problems to find the right
> solution (and avoid trying to solve only unstaged changes before
> noticing I need to solve the other one too).

Good point. I'll make the error messages a little more comprehensive
and focused in the next iteration.

-- Ram

[PATCH 2/2] Porcelain scripts: Rewrite cryptic "needs update" error message

[Ramkumar Ramachandra]

Although Git interally has the facility to differentiate between
porcelain and plubmbing commands and appropriately print errors,
several shell scripts invoke plubming commands triggering cryptic
plumbing errors to be displayed on a porcelain interface. This patch
replaces the "needs update" message in git-pull and git-rebase, when
`git update-index` is run, with a more friendly message.

Reported-by: Joshua Jensen <jjensen@workspacewhiz.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-pull.sh                |    5 +----
 git-rebase--interactive.sh |   16 ++++------------
 git-rebase.sh              |   14 +-------------
 3 files changed, 6 insertions(+), 29 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 8eb74d4..5da0f76 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -201,10 +201,7 @@ test true = "$rebase" && {
 			die "updating an unborn branch with changes added to the index"
 		fi
 	else
-		git update-index --ignore-submodules --refresh &&
-		git diff-files --ignore-submodules --quiet &&
-		git diff-index --ignore-submodules --cached --quiet HEAD -- ||
-		die "refusing to pull with rebase: your working tree is not up-to-date"
+		require_clean_work_tree "pull with rebase"
 	fi
 	oldremoteref= &&
 	. git-parse-remote &&
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a27952d..8722baf 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -153,14 +153,6 @@ run_pre_rebase_hook () {
 	fi
 }
 
-require_clean_work_tree () {
-	# test if working tree is dirty
-	git rev-parse --verify HEAD > /dev/null &&
-	git update-index --ignore-submodules --refresh &&
-	git diff-files --quiet --ignore-submodules &&
-	git diff-index --cached --quiet HEAD --ignore-submodules -- ||
-	die "Working tree is dirty"
-}
 
 ORIG_REFLOG_ACTION="$GIT_REFLOG_ACTION"
 
@@ -557,7 +549,7 @@ do_next () {
 			exit "$status"
 		fi
 		# Run in subshell because require_clean_work_tree can die.
-		if ! (require_clean_work_tree)
+		if ! (require_clean_work_tree "rebase")
 		then
 			warn "Commit or stash your changes, and then run"
 			warn
@@ -740,7 +732,7 @@ do
 			die "Cannot read HEAD"
 		git update-index --ignore-submodules --refresh &&
 			git diff-files --quiet --ignore-submodules ||
-			die "Working tree is dirty"
+			die "Working tree is dirty. Please commit or stash your changes to proceed."
 
 		# do we have anything to commit?
 		if git diff-index --cached --quiet --ignore-submodules HEAD --
@@ -768,7 +760,7 @@ first and then run 'git rebase --continue' again."
 
 		record_in_rewritten "$(cat "$DOTEST"/stopped-sha)"
 
-		require_clean_work_tree
+		require_clean_work_tree "rebase"
 		do_rest
 		;;
 	--abort)
@@ -866,7 +858,7 @@ first and then run 'git rebase --continue' again."
 
 		comment_for_reflog start
 
-		require_clean_work_tree
+		require_clean_work_tree "rebase"
 
 		if test ! -z "$1"
 		then
diff --git a/git-rebase.sh b/git-rebase.sh
index 3335cee..c3ca8d5 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -416,19 +416,7 @@ else
 	fi
 fi
 
-# The tree must be really really clean.
-if ! git update-index --ignore-submodules --refresh > /dev/null; then
-	echo >&2 "cannot rebase: you have unstaged changes"
-	git diff-files --name-status -r --ignore-submodules -- >&2
-	exit 1
-fi
-diff=$(git diff-index --cached --name-status -r --ignore-submodules HEAD --)
-case "$diff" in
-?*)	echo >&2 "cannot rebase: your index contains uncommitted changes"
-	echo >&2 "$diff"
-	exit 1
-	;;
-esac
+require_clean_work_tree "rebase"
 
 if test -z "$rebase_root"
 then
-- 
1.7.2.2.409.gdbb11.dirty

Re: [PATCH 2/2] Porcelain scripts: Rewrite cryptic "needs update" error message

[Matthieu Moy]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

>  		# Run in subshell because require_clean_work_tree can die.
> -		if ! (require_clean_work_tree)
> +		if ! (require_clean_work_tree "rebase")
>  		then
>  			warn "Commit or stash your changes, and then run"

That will make a duplicate "commit or stash your changes" message.

Otherwise, the patch seems OK.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/