[PATCH 1/2] do not overwrite files marked "assume unchanged"

[PATCH 1/2] do not overwrite files marked "assume unchanged"

[Clemens Buchacher]

A merge will fail gracefully if it needs to update files marked
"assume unchanged", but other similar commands will not. In
particular, checkout and rebase will silently overwrite changes to
such files.

This is a regression introduced in commit 1dcafcc0 (verify_uptodate():
add ce_uptodate(ce) test), which avoids lstat's during a merge, if the
index entry is up-to-date. If the CE_VALID flag is set, however, we
cannot trust CE_UPTODATE.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
 t/t2106-update-index-assume-unchanged.sh |   24 ++++++++++++++++++++++++
 unpack-trees.c                           |    2 +-
 2 files changed, 25 insertions(+), 1 deletions(-)
 create mode 100755 t/t2106-update-index-assume-unchanged.sh

diff --git a/t/t2106-update-index-assume-unchanged.sh b/t/t2106-update-index-assume-unchanged.sh
new file mode 100755
index 0000000..99d858c
--- /dev/null
+++ b/t/t2106-update-index-assume-unchanged.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+test_description='git update-index --assume-unchanged test.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' \
+	': >file &&
+	 git add file &&
+	 git commit -m initial &&
+	 git branch other &&
+	 echo upstream >file &&
+	 git add file &&
+	 git commit -m upstream'
+
+test_expect_success 'do not switch branches with dirty file' \
+	'git reset --hard &&
+	 git checkout other &&
+	 echo dirt >file &&
+	 git update-index --assume-unchanged file &&
+	 test_must_fail git checkout master'
+
+test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 75f54ca..1a8030c 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -862,7 +862,7 @@ static int verify_uptodate_1(struct cache_entry *ce,
 {
 	struct stat st;
 
-	if (o->index_only || (!ce_skip_worktree(ce) && (o->reset || ce_uptodate(ce))))
+	if (o->index_only || (!((ce->ce_flags & CE_VALID) || ce_skip_worktree(ce)) && (o->reset || ce_uptodate(ce))))
 		return 0;
 
 	if (!lstat(ce->name, &st)) {
-- 
1.7.0.5.3.ga76e

[PATCH 2/2] Documentation: git-add does not update files marked "assume unchanged"

[Clemens Buchacher]

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---

Behavior of git-add also changed since commit 1dcafcc0, but I actually
prefer it this way.

 Documentation/git-update-index.txt |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index 68dc187..2d45774 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -93,8 +93,6 @@ OPTIONS
 This option can be also used as a coarse file-level mechanism
 to ignore uncommitted changes in tracked files (akin to what
 `.gitignore` does for untracked files).
-You should remember that an explicit 'git add' operation will
-still cause the file to be refreshed from the working tree.
 Git will fail (gracefully) in case it needs to modify this file
 in the index e.g. when merging in a commit;
 thus, in case the assumed-untracked file is changed upstream,
@@ -102,7 +100,8 @@ you will need to handle the situation manually.
 
 --really-refresh::
 	Like '--refresh', but checks stat information unconditionally,
-	without regard to the "assume unchanged" setting.
+	without regard to the "assume unchanged" setting. The "assume
+	unchanged" bit is unset for all paths.
 
 --skip-worktree::
 --no-skip-worktree::
-- 
1.7.0.5.3.ga76e

update-index --really-refresh unsets assume-unchanged bit

[Clemens Buchacher]

On Sat, May 01, 2010 at 11:27:20AM +0200, Clemens Buchacher wrote:

>  --really-refresh::
>  	Like '--refresh', but checks stat information unconditionally,
> -	without regard to the "assume unchanged" setting.
> +	without regard to the "assume unchanged" setting. The "assume
> +	unchanged" bit is unset for all paths.

Scratch that latter part, please. I just noticed the bit is unset only for
modified files. If the file matches the index, or even if it has been
deleted in the work tree, the bit is _not_ unset.

So the current behavior is quite strange. I see several possible
interpretations of --really-refresh:

 Update index for specified paths, disregarding the assume-unchanged bit,

 a) ... and do not ignore work tree changes for tracked files any more.
    I.e., unset the assume-unchanged bit for all files.

 b) ... and do not ignore work tree changes for the specified paths any more.
    I.e., unset assume-unchanged bit only for specified paths.

 c) ... but continue ignoring work tree changes as before.
    I.e., do not unset the assume-unchanged bit for any paths.

(and the current behavior)

 d) ... and do not ignore work tree changes for modified files any more.
    I.e., unset assume-unchanged bit for modified paths.

I believe c) is the most useful, since it allows the user to deactivate the
assume-unchanged bit temporarily. All other options potentially change the
assume-unchanged bit, which the user may or may not want. In my opinion, it
makes no sense to bundle the two operations "update index disregarding the
assume-unchanged bit" and "(conditionally) unset assume-unchanged bit."

Clemens

Re: update-index --really-refresh unsets assume-unchanged bit

[Avery Pennarun]

On Tue, May 4, 2010 at 4:57 AM, Clemens Buchacher <drizzd@aon.at> wrote:
> On Sat, May 01, 2010 at 11:27:20AM +0200, Clemens Buchacher wrote:
>
>>  --really-refresh::
>>       Like '--refresh', but checks stat information unconditionally,
>> -     without regard to the "assume unchanged" setting.
>> +     without regard to the "assume unchanged" setting. The "assume
>> +     unchanged" bit is unset for all paths.
>
> Scratch that latter part, please. I just noticed the bit is unset only for
> modified files. If the file matches the index, or even if it has been
> deleted in the work tree, the bit is _not_ unset.
>
> So the current behavior is quite strange. I see several possible
> interpretations of --really-refresh:

I don't know, the current behaviour sounds consistent with the name:
"assume unchanged."

If the index says the file is unmodified, then assume it's unchanged;
don't check for changes.

If the index says the file is modified, then clearly it's changed; it
would be pointless to assume otherwise, so the "assume unchanged" bit
should probably not be set.  (Plus it's quite possibly dangerous to
assume the file permissions are the same as they were when you first
noticed they were changed; imagine if the file has changed twice and
now has a different length or mode.)

Since most of the time most of your files won't have changed,
assume-unchanged gets you the optimization without too much danger.
The cost of not assuming a changed file is unchanged is pretty low.

...now if only there was a global "just never check for changes or new
files or anything, even if I run git status" bit, someone could write
a useful git inotify daemon :)

Have fun,

Avery

Re: update-index --really-refresh unsets assume-unchanged bit

[Clemens Buchacher]

On Tue, May 04, 2010 at 12:41:22PM -0400, Avery Pennarun wrote:

> On Tue, May 4, 2010 at 4:57 AM, Clemens Buchacher <drizzd@aon.at> wrote:
>
> > Scratch that latter part, please. I just noticed the bit is unset only for
> > modified files. If the file matches the index, or even if it has been
> > deleted in the work tree, the bit is _not_ unset.
> >
> > So the current behavior is quite strange. I see several possible
> > interpretations of --really-refresh:
> 
> I don't know, the current behaviour sounds consistent with the name:
> "assume unchanged."
> 
> If the index says the file is unmodified, then assume it's unchanged;
> don't check for changes.

This is not about changes with respect to HEAD.  The "assume unchanged" flag
is unset if the work tree differs from the index.  Whether or not the file
is modified in the index is irrelevant.

> If the index says the file is modified, then clearly it's changed; it
> would be pointless to assume otherwise, so the "assume unchanged" bit
> should probably not be set.

On the contrary. I _want_ git to assume the file is unchanged, even though I
know it changed. This is an intended and documented use case (from the
git-update-index manpage):

 This option can be also used as a coarse file-level mechanism
 to ignore uncommitted changes in tracked files (akin to what
 `.gitignore` does for untracked files).

But even if you look at it from the performance optimization perspective,
the behavior is still inconsistent, because it does not unset the bit for
deleted files.

Furthermore, after update-index --really-refresh --add, the file is in fact
unchanged again and it would make much more sense to keep the
assume-unchanged flag.

> (Plus it's quite possibly dangerous to
> assume the file permissions are the same as they were when you first
> noticed they were changed; imagine if the file has changed twice and
> now has a different length or mode.)

What do you mean, "dangerous"? I tell git to update the index, and then
continue to ignore changes to the work tree, until I explicitly update
again. I can already achieve this behavior using

 git ls-files -v | grep ^h | cut -f2 -d' ' >my-ignored-files
 git update-index --really-refresh
 cat my-ignored-files | xargs git update-index --assume-unchanged

Clemens

Re: update-index --really-refresh unsets assume-unchanged bit

[Junio C Hamano]

Clemens Buchacher <drizzd@aon.at> writes:

> On the contrary. I _want_ git to assume the file is unchanged, even though I
> know it changed. This is an intended and documented use case (from the
> git-update-index manpage):

The intended use of *really* one is to ignore the assume unchanged bit and
make the bit reflect reality.  The bit was originally invented back when
the machinery was too lstat(2) heavy so that people on slow filesystems
can say "I haven't changed these paths, and I promise I won't, so you can
trust this and assume that the working tree file exactly matches what is
recorded in the index."  Obviously we needed to give such users an way to
tell git that they broke their promise, and that is what *really* refresh
meant to give.

Note that the promise is _different_ from telling "I may or may not have
changed the work tree files, but don't bother telling me that I did".  The
promise you make by "assume unchanged" bit allows git to read from the
working tree file when it needs the contents of blob recorded in the index
and the bit is set, as you promised that you will never change it.

Re: update-index --really-refresh unsets assume-unchanged bit

[Clemens Buchacher]

On Tue, May 04, 2010 at 03:45:20PM -0700, Junio C Hamano wrote:
> Clemens Buchacher <drizzd@aon.at> writes:
> 
> > On the contrary. I _want_ git to assume the file is unchanged, even though I
> > know it changed. This is an intended and documented use case (from the
> > git-update-index manpage):
> 
> The intended use of *really* one is to ignore the assume unchanged bit and
> make the bit reflect reality.

Yes, and right below the part you are quoting I explained why this is not
what it does either.

> The bit was originally invented back when
> the machinery was too lstat(2) heavy so that people on slow filesystems
> can say "I haven't changed these paths, and I promise I won't, so you can
> trust this and assume that the working tree file exactly matches what is
> recorded in the index."  Obviously we needed to give such users an way to
> tell git that they broke their promise, and that is what *really* refresh
> meant to give.

Yes, but after the index is refreshed, the file is now, again, unchanged.
Yet, the assume-unchanged bit is unset.

But there is no reason presume that the user will break their promise again
in the future. If they plan to do so, they can always use git update-index
--no-assume-unchanged.

> Note that the promise is _different_ from telling "I may or may not have
> changed the work tree files, but don't bother telling me that I did".  The
> promise you make by "assume unchanged" bit allows git to read from the
> working tree file when it needs the contents of blob recorded in the index
> and the bit is set, as you promised that you will never change it.

And in what case does git do that? I can only find commands which read from
the object store and not from the work tree. This would also break the
"ignore changes to tracked files" use case.

Clemens