index rebuild for cloned repo

I'm trying to share pre-built working trees using NFS-server-side
volume cloning (not "git clone").  This makes a new volume that
shares all the data with the source volume, including build
products that would otherwise take hours to regenerate.  While
the data is identical, much of the inode information changes:
uid, gid, ino, dev, mtime, ctime.

What is the best way to rewrite .git/index in the clone?  Options
that work but are slow:

    git reset --hard HEAD
	write all files, breaking data sharing, 2 min 45 sec

    git update-index --refresh
	stat and read all files, 5 min 55 sec

I hacked out the file data comparison in ce_modified_check_fs()
to measure:

    HACKED git update-index --refresh
	Just the stat, 13 sec

The lstat() is required to look up the new inode number.

The rest of the clone operation takes around 3 min, so
I'd like to avoid this additional 5+ min of read()s if
possible.  Is there a way to do so using existing commands?
Should I add a new option to update-index, or maybe write
a stand-alone tool to manipulate the index file directly?

Thanks,

		-- Pete


P.S.  The user-observable problem that occurs if I do not
rebuild the index is, e.g.:

    $ git cherry-pick build/top
    error: Your local changes to 'file.h' would be overwritten by merge.  Aborting.
    Please, commit your changes or stash them before you can merge.

Hi Pete,

Pete Wyckoff wrote:

                                                         While
the data is identical, much of the inode information changes:
uid, gid, ino, dev, mtime, ctime.

What is the best way to rewrite .git/index in the clone?  Options
that work but are slow:

    git reset --hard HEAD

Not very good for this use case, as you noticed.

    git update-index --refresh
	stat and read all files, 5 min 55 sec

That's better (better yet with -q).  The user-facing version is
"git add --refresh .".

[...]

    HACKED git update-index --refresh
	Just the stat, 13 sec

The lstat() is required to look up the new inode number.

The rest of the clone operation takes around 3 min, so
I'd like to avoid this additional 5+ min of read()s if
possible.  Is there a way to do so using existing commands?
Should I add a new option to update-index

Yes, a new option sounds sane.

P.S.  The user-observable problem that occurs if I do not
rebuild the index is, e.g.:

    $ git cherry-pick build/top
    error: Your local changes to 'file.h' would be overwritten by merge.  Aborting.
    Please, commit your changes or stash them before you can merge.

That's a bug.  In general, the "high-level commands (porcelain)"
listed in the git manpage (other than gitk) are supposed to hide an
un-refreshed index from the user, generally by transparently
refreshing the index.

Thanks for reporting.

Hope that helps,
Jonathan

A stat-dirty index is not a detail that ought to concern the operator
of porcelain such as "git cherry-pick".

Without this change, a cherry-pick after copying a worktree with rsync
errors out with a misleading message.

	$ git cherry-pick build/top
	error: Your local changes to 'file.h' would be overwritten by merge.  Aborting.
	Please, commit your changes or stash them before you can merge.

Noticed-by: Pete Wyckoff [off-list ref]
Signed-off-by: Jonathan Nieder <redacted>
---
Pete Wyckoff wrote:

P.S.  The user-observable problem that occurs if I do not
rebuild the index is, e.g.:

    $ git cherry-pick build/top
    error: Your local changes to 'file.h' would be overwritten by merge.  Aborting.
    Please, commit your changes or stash them before you can merge.

Maybe this would help?

 builtin/revert.c              |   18 ++++++++++++++++--
 t/t3501-revert-cherry-pick.sh |   13 +++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 57b51e4..bb6e9e8 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c

@@ -547,6 +547,21 @@ static void prepare_revs(struct rev_info *revs)
 		die("empty commit set passed");
 }
 
+static void read_and_refresh_cache(const char *me)
+{
+	static struct lock_file index_lock;
+	int index_fd = hold_locked_index(&index_lock, 0);
+	if (read_index_preload(&the_index, NULL) < 0)
+		die("git %s: failed to read the index", me);
+	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
+	if (the_index.cache_changed) {
+		if (write_index(&the_index, index_fd) ||
+		    commit_locked_index(&index_lock))
+			die("git %s: failed to refresh the index", me);
+	}
+	rollback_lock_file(&index_lock);
+}
+
 static int revert_or_cherry_pick(int argc, const char **argv)
 {
 	struct rev_info revs;

@@ -567,8 +582,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 			die("cherry-pick --ff cannot be used with --edit");
 	}
 
-	if (read_cache() < 0)
-		die("git %s: failed to read the index", me);
+	read_and_refresh_cache(me);
 
 	prepare_revs(&revs);
 

diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index bc7aedd..b210188 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh

@@ -81,6 +81,19 @@ test_expect_success 'revert after renaming branch' '
 
 '
 
+test_expect_success 'revert on stat-dirty working tree' '
+	git clone . repo &&
+	(
+		cd repo &&
+		git checkout initial
+	) &&
+	cp -R repo copy &&
+	(
+		cd copy &&
+		git cherry-pick added
+	)
+'
+
 test_expect_success 'revert forbidden on dirty working tree' '
 
 	echo content >extra_file &&

-- 
1.7.2.3.557.gab647.dirty

jrnieder@gmail.com wrote on Sun, 31 Oct 2010 14:59 -0500:

A stat-dirty index is not a detail that ought to concern the operator
of porcelain such as "git cherry-pick".

Without this change, a cherry-pick after copying a worktree with rsync
errors out with a misleading message.

	$ git cherry-pick build/top
	error: Your local changes to 'file.h' would be overwritten by merge.  Aborting.
	Please, commit your changes or stash them before you can merge.

Noticed-by: Pete Wyckoff [off-list ref]
Signed-off-by: Jonathan Nieder <redacted>

Thanks, this works well.  I tested it and am happy with the
change.  It is needed for correctness.

Since I know that I just copied the repo, I'd prefer not to
make people wait to refresh the index.  A new flag to
update-index improves performance by avoiding the initial
re-read of all files in the repository.  Patch follows in
next mail.

		-- Pete

When a repository has been copied with rsync, or cloned using
a volume manager, the index can be incorrect even though the
data is unchanged.  This new flag tells update-index --refresh
that it is not necessary to reread the data contents.  Without
this flag, everything works, but it is much slower due to the
need to read all files.

Signed-off-by: Pete Wyckoff <redacted>
---
 Documentation/git-update-index.txt |    9 +++++++++
 builtin/update-index.c             |    6 +++++-
 cache.h                            |    9 ++++++---
 read-cache.c                       |   16 ++++++++++------
 4 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index 765d4b3..69aaaee 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt

@@ -14,6 +14,7 @@ SYNOPSIS
 	     [--refresh] [-q] [--unmerged] [--ignore-missing]
 	     [--cacheinfo <mode> <object> <file>]\*
 	     [--chmod=(+|-)x]
+	     [--data-unchanged]
 	     [--assume-unchanged | --no-assume-unchanged]
 	     [--skip-worktree | --no-skip-worktree]
 	     [--ignore-submodules]

@@ -65,6 +66,14 @@ OPTIONS
 	behavior is to error out.  This option makes 'git update-index'
         continue anyway.
 
+--data-unchanged::
+	When refreshing the cache, do not re-read all data
+	files to verify that they are unchanged.  This flag
+	attests that even though the index may be invalid, file content
+	has not been changed.  This situation can happen when
+	a repository is copied with `rsync` or with a volume manager
+	clone command.
+
 --ignore-missing::
 	Ignores missing files during a --refresh
 

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 3ab214d..dc609cc 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c

@@ -398,7 +398,7 @@ static void read_index_info(int line_termination)
 }
 
 static const char update_index_usage[] =
-"git update-index [-q] [--add] [--replace] [--remove] [--unmerged] [--refresh] [--really-refresh] [--cacheinfo] [--chmod=(+|-)x] [--assume-unchanged] [--skip-worktree|--no-skip-worktree] [--info-only] [--force-remove] [--stdin] [--index-info] [--unresolve] [--again | -g] [--ignore-missing] [-z] [--verbose] [--] <file>...";
+"git update-index [-q] [--add] [--replace] [--remove] [--unmerged] [--data-unchanged] [--refresh] [--really-refresh] [--cacheinfo] [--chmod=(+|-)x] [--assume-unchanged] [--skip-worktree|--no-skip-worktree] [--info-only] [--force-remove] [--stdin] [--index-info] [--unresolve] [--again | -g] [--ignore-missing] [-z] [--verbose] [--] <file>...";
 
 static unsigned char head_sha1[20];
 static unsigned char merge_head_sha1[20];

@@ -635,6 +635,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 				refresh_flags |= REFRESH_UNMERGED;
 				continue;
 			}
+			if (!strcmp(path, "--data-unchanged")) {
+				refresh_flags |= REFRESH_DATA_UNCHANGED;
+				continue;
+			}
 			if (!strcmp(path, "--refresh")) {
 				setup_work_tree();
 				has_errors |= refresh_cache(refresh_flags);

diff --git a/cache.h b/cache.h
index 1e690d1..b2e692c 100644
--- a/cache.h
+++ b/cache.h

@@ -485,11 +485,13 @@ extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
 extern int index_name_is_other(const struct index_state *, const char *, int);
 
 /* do stat comparison even if CE_VALID is true */
-#define CE_MATCH_IGNORE_VALID		01
+#define CE_MATCH_IGNORE_VALID		0x1
 /* do not check the contents but report dirty on racily-clean entries */
-#define CE_MATCH_RACY_IS_DIRTY		02
+#define CE_MATCH_RACY_IS_DIRTY		0x2
 /* do stat comparison even if CE_SKIP_WORKTREE is true */
-#define CE_MATCH_IGNORE_SKIP_WORKTREE	04
+#define CE_MATCH_IGNORE_SKIP_WORKTREE	0x4
+/* trust that the contents of files have not been modified */
+#define CE_MATCH_DATA_UNCHANGED		0x8
 extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 

@@ -504,6 +506,7 @@ extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
 #define REFRESH_IGNORE_MISSING	0x0008	/* ignore non-existent */
 #define REFRESH_IGNORE_SUBMODULES	0x0010	/* ignore submodules */
 #define REFRESH_IN_PORCELAIN	0x0020	/* user friendly output, not "needs update" */
+#define REFRESH_DATA_UNCHANGED	0x0040	/* no need to read all files, trust unmodified */
 extern int refresh_index(struct index_state *, unsigned int flags, const char **pathspec, char *seen, char *header_msg);
 
 struct lock_file {

diff --git a/read-cache.c b/read-cache.c
index 1f42473..26d5d5f 100644
--- a/read-cache.c
+++ b/read-cache.c

@@ -137,12 +137,14 @@ static int ce_compare_gitlink(struct cache_entry *ce)
 	return hashcmp(sha1, ce->sha1);
 }
 
-static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)
+static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st,
+				int options)
 {
 	switch (st->st_mode & S_IFMT) {
 	case S_IFREG:
-		if (ce_compare_data(ce, st))
-			return DATA_CHANGED;
+		if (!(options & CE_MATCH_DATA_UNCHANGED))
+			if (ce_compare_data(ce, st))
+				return DATA_CHANGED;
 		break;
 	case S_IFLNK:
 		if (ce_compare_link(ce, xsize_t(st->st_size)))

@@ -304,7 +306,7 @@ int ie_match_stat(const struct index_state *istate,
 		if (assume_racy_is_modified)
 			changed |= DATA_CHANGED;
 		else
-			changed |= ce_modified_check_fs(ce, st);
+			changed |= ce_modified_check_fs(ce, st, 0);
 	}
 
 	return changed;

@@ -343,7 +345,7 @@ int ie_modified(const struct index_state *istate,
 	    (S_ISGITLINK(ce->ce_mode) || ce->ce_size != 0))
 		return changed;
 
-	changed_fs = ce_modified_check_fs(ce, st);
+	changed_fs = ce_modified_check_fs(ce, st, options);
 	if (changed_fs)
 		return changed | changed_fs;
 	return 0;

@@ -1108,6 +1110,8 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
 
 	needs_update_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
 	needs_merge_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
+	if (flags & REFRESH_DATA_UNCHANGED)
+	    options |= CE_MATCH_DATA_UNCHANGED;
 	for (i = 0; i < istate->cache_nr; i++) {
 		struct cache_entry *ce, *new;
 		int cache_errno = 0;

@@ -1481,7 +1485,7 @@ static void ce_smudge_racily_clean_entry(struct cache_entry *ce)
 		return;
 	if (ce_match_stat_basic(ce, &st))
 		return;
-	if (ce_modified_check_fs(ce, &st)) {
+	if (ce_modified_check_fs(ce, &st, 0)) {
 		/* This is "racily clean"; smudge it.  Note that this
 		 * is a tricky code.  At first glance, it may appear
 		 * that it can break with this sequence:

-- 
1.7.2.3

On Sonntag, 31. Oktober 2010, Jonathan Nieder wrote:

+test_expect_success 'revert on stat-dirty working tree' '
+	git clone . repo &&
+	(
+		cd repo &&
+		git checkout initial
+	) &&
+	cp -R repo copy &&
+	(
+		cd copy &&
+		git cherry-pick added
+	)

On Windows, this doesn't test what it should test because we do not look at 
the inode number (currently). Please use test-chmtime to change stat 
information.

-- Hannes

+static void read_and_refresh_cache(const char *me)
+{
+       static struct lock_file index_lock;
+       int index_fd = hold_locked_index(&index_lock, 0);
+       if (read_index_preload(&the_index, NULL) < 0)
+               die("git %s: failed to read the index", me);
+       refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);

Can we refresh only index entries that are cherry-picked/reverted?
Full refresh can be expensive on large repos while these operations
don't usually touch the whole repo.

I'm thinking of adding ce_really_uptodate() and converting most of
ce_uptodate() call sites  to the new one.

diff --git a/cache.h b/cache.h
index 123dd4b..81dc5cf 100644
--- a/cache.h
+++ b/cache.h

@@ -240,6 +240,7 @@ static inline size_t ce_namelen(const struct

cache_entry *ce)
 			    ondisk_cache_entry_size(ce_namelen(ce)))
 #define ce_stage(ce) ((CE_STAGEMASK & (ce)->ce_flags) >> CE_STAGESHIFT)
 #define ce_uptodate(ce) ((ce)->ce_flags & CE_UPTODATE)
+#define ce_really_uptodate(ce) (ce_uptodate(ce) ||
(refresh_cache_entry(ce), ce_uptodate(ce)))
 #define ce_skip_worktree(ce) ((ce)->ce_flags & CE_SKIP_WORKTREE)
 #define ce_mark_uptodate(ce) ((ce)->ce_flags |= CE_UPTODATE)

@@ -512,6 +513,7 @@ extern void fill_stat_cache_info(struct

cache_entry *ce, struct stat *st);
 #define REFRESH_IGNORE_SUBMODULES	0x0010	/* ignore submodules */
 #define REFRESH_IN_PORCELAIN	0x0020	/* user friendly output, not
"needs update" */
 extern int refresh_index(struct index_state *, unsigned int flags,
const char **pathspec, char *seen, char *header_msg);
+extern struct cache_entry *refresh_cache_entry(struct cache_entry
*ce, int really);

 struct lock_file {
 	struct lock_file *next;

diff --git a/read-cache.c b/read-cache.c
index 1f42473..76525a9 100644
--- a/read-cache.c
+++ b/read-cache.c

@@ -1156,7 +1156,7 @@ int refresh_index(struct index_state *istate,

unsigned int flags, const char **p
 	return has_errors;
 }

-static struct cache_entry *refresh_cache_entry(struct cache_entry
*ce, int really)
+struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really)
 {
 	return refresh_cache_ent(&the_index, ce, really, NULL);
 }
-- 
Duy