All of lore.kernel.org
 help / color / mirror / Atom feed
* index rebuild for cloned repo
@ 2010-10-31 17:44 Pete Wyckoff
  2010-10-31 18:07 ` Jonathan Nieder
  2010-10-31 19:59 ` [PATCH] cherry-pick/revert: transparently refresh index Jonathan Nieder
  0 siblings, 2 replies; 16+ messages in thread
From: Pete Wyckoff @ 2010-10-31 17:44 UTC (permalink / raw)
  To: git

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: index rebuild for cloned repo
  2010-10-31 17:44 index rebuild for cloned repo Pete Wyckoff
@ 2010-10-31 18:07 ` Jonathan Nieder
  2010-10-31 19:59 ` [PATCH] cherry-pick/revert: transparently refresh index Jonathan Nieder
  1 sibling, 0 replies; 16+ messages in thread
From: Jonathan Nieder @ 2010-10-31 18:07 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] cherry-pick/revert: transparently refresh index
  2010-10-31 17:44 index rebuild for cloned repo Pete Wyckoff
  2010-10-31 18:07 ` Jonathan Nieder
@ 2010-10-31 19:59 ` Jonathan Nieder
  2010-10-31 20:26   ` Andreas Schwab
                     ` (3 more replies)
  1 sibling, 4 replies; 16+ messages in thread
From: Jonathan Nieder @ 2010-10-31 19:59 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git, Christian Couder, Junio C Hamano

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 <pw@padd.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] cherry-pick/revert: transparently refresh index
  2010-10-31 19:59 ` [PATCH] cherry-pick/revert: transparently refresh index Jonathan Nieder
@ 2010-10-31 20:26   ` Andreas Schwab
  2010-10-31 20:32     ` Jonathan Nieder
  2010-10-31 22:26   ` Pete Wyckoff
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Andreas Schwab @ 2010-10-31 20:26 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Pete Wyckoff, git, Christian Couder, Junio C Hamano

Jonathan Nieder <jrnieder@gmail.com> writes:

> 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' '

ITYM s/revert/cherry-pick/

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] cherry-pick/revert: transparently refresh index
  2010-10-31 20:26   ` Andreas Schwab
@ 2010-10-31 20:32     ` Jonathan Nieder
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Nieder @ 2010-10-31 20:32 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Pete Wyckoff, git, Christian Couder, Junio C Hamano

Andreas Schwab wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> --- a/t/t3501-revert-cherry-pick.sh
>> +++ b/t/t3501-revert-cherry-pick.sh
>> @@ -83,4 +83,17 @@ test_expect_success 'revert after renaming branch' '
>>
>> +test_expect_success 'revert on stat-dirty working tree' '
>
> ITYM s/revert/cherry-pick/

Good eyes.  Yes, I do.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] cherry-pick/revert: transparently refresh index
  2010-10-31 19:59 ` [PATCH] cherry-pick/revert: transparently refresh index Jonathan Nieder
  2010-10-31 20:26   ` Andreas Schwab
@ 2010-10-31 22:26   ` Pete Wyckoff
  2010-10-31 22:28     ` [PATCH] teach update-index --refresh about --data-unchanged Pete Wyckoff
  2010-11-01  8:09   ` [PATCH] cherry-pick/revert: transparently refresh index Johannes Sixt
  2010-11-01  8:44   ` Nguyen Thai Ngoc Duy
  3 siblings, 1 reply; 16+ messages in thread
From: Pete Wyckoff @ 2010-10-31 22:26 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Christian Couder, Junio C Hamano

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 <pw@padd.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] teach update-index --refresh about --data-unchanged
  2010-10-31 22:26   ` Pete Wyckoff
@ 2010-10-31 22:28     ` Pete Wyckoff
  2010-11-03 17:37       ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Pete Wyckoff @ 2010-10-31 22:28 UTC (permalink / raw)
  To: git; +Cc: Christian Couder, Junio C Hamano, Jonathan Nieder

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 <pw@padd.com>
---
 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] cherry-pick/revert: transparently refresh index
  2010-10-31 19:59 ` [PATCH] cherry-pick/revert: transparently refresh index Jonathan Nieder
  2010-10-31 20:26   ` Andreas Schwab
  2010-10-31 22:26   ` Pete Wyckoff
@ 2010-11-01  8:09   ` Johannes Sixt
  2010-11-03 17:30     ` Junio C Hamano
  2010-11-01  8:44   ` Nguyen Thai Ngoc Duy
  3 siblings, 1 reply; 16+ messages in thread
From: Johannes Sixt @ 2010-11-01  8:09 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Pete Wyckoff, git, Christian Couder, Junio C Hamano

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] cherry-pick/revert: transparently refresh index
  2010-10-31 19:59 ` [PATCH] cherry-pick/revert: transparently refresh index Jonathan Nieder
                     ` (2 preceding siblings ...)
  2010-11-01  8:09   ` [PATCH] cherry-pick/revert: transparently refresh index Johannes Sixt
@ 2010-11-01  8:44   ` Nguyen Thai Ngoc Duy
  3 siblings, 0 replies; 16+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2010-11-01  8:44 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Pete Wyckoff, git, Christian Couder, Junio C Hamano

> +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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] cherry-pick/revert: transparently refresh index
  2010-11-01  8:09   ` [PATCH] cherry-pick/revert: transparently refresh index Johannes Sixt
@ 2010-11-03 17:30     ` Junio C Hamano
  2010-11-03 20:33       ` Johannes Sixt
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2010-11-03 17:30 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jonathan Nieder, Pete Wyckoff, git, Christian Couder

Johannes Sixt <j6t@kdbg.org> writes:

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

Like this, perhaps?

diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index b210188..a5b963a 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -81,15 +81,12 @@ 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 &&
+test_expect_success 'cherry-pick on stat-dirty working tree' '
+	git clone . copy
 	(
 		cd copy &&
+		git checkout initial &&
+		test-chmtime +40 oops &&
 		git cherry-pick added
 	)
 '

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] teach update-index --refresh about --data-unchanged
  2010-10-31 22:28     ` [PATCH] teach update-index --refresh about --data-unchanged Pete Wyckoff
@ 2010-11-03 17:37       ` Junio C Hamano
  2010-11-03 18:36         ` Jonathan Nieder
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2010-11-03 17:37 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: git, Christian Couder, Jonathan Nieder

Pete Wyckoff <pw@padd.com> writes:

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

I know our traditional attitude towards the plumbing commands have been
"give them long enough rope and let users hang themselves", but this
particular rope feels a bit too long for my taste.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] teach update-index --refresh about --data-unchanged
  2010-11-03 17:37       ` Junio C Hamano
@ 2010-11-03 18:36         ` Jonathan Nieder
  2010-11-03 22:02           ` Pete Wyckoff
  0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Nieder @ 2010-11-03 18:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pete Wyckoff, git, Christian Couder

On Wed, Nov 03, 2010 at 10:37:40AM -0700, Junio C Hamano wrote:
> Pete Wyckoff <pw@padd.com> writes:

>> 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.
>
> I know our traditional attitude towards the plumbing commands have been
> "give them long enough rope and let users hang themselves", but this
> particular rope feels a bit too long for my taste.

Pete, I think you mentioned the possibility of a special-case tool for
contrib/ that just updates the inode, device number, and ctime fields?
That sounds a little less worrying to use, as plumbing.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] cherry-pick/revert: transparently refresh index
  2010-11-03 17:30     ` Junio C Hamano
@ 2010-11-03 20:33       ` Johannes Sixt
  0 siblings, 0 replies; 16+ messages in thread
From: Johannes Sixt @ 2010-11-03 20:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jonathan Nieder, Pete Wyckoff, git, Christian Couder

On Mittwoch, 3. November 2010, Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
> > 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.
>
> Like this, perhaps?
>
> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
> index b210188..a5b963a 100755
> --- a/t/t3501-revert-cherry-pick.sh
> +++ b/t/t3501-revert-cherry-pick.sh
> @@ -81,15 +81,12 @@ 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 &&
> +test_expect_success 'cherry-pick on stat-dirty working tree' '
> +	git clone . copy
>  	(
>  		cd copy &&
> +		git checkout initial &&
> +		test-chmtime +40 oops &&
>  		git cherry-pick added
>  	)
>  '

Yes, makes sense.

-- Hannes

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] teach update-index --refresh about --data-unchanged
  2010-11-03 18:36         ` Jonathan Nieder
@ 2010-11-03 22:02           ` Pete Wyckoff
  2010-11-14 16:58             ` Pete Wyckoff
  0 siblings, 1 reply; 16+ messages in thread
From: Pete Wyckoff @ 2010-11-03 22:02 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git, Christian Couder

jrnieder@gmail.com wrote on Wed, 03 Nov 2010 13:36 -0500:
> On Wed, Nov 03, 2010 at 10:37:40AM -0700, Junio C Hamano wrote:
> > Pete Wyckoff <pw@padd.com> writes:
> 
> >> 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.
> >
> > I know our traditional attitude towards the plumbing commands have been
> > "give them long enough rope and let users hang themselves", but this
> > particular rope feels a bit too long for my taste.
> 
> Pete, I think you mentioned the possibility of a special-case tool for
> contrib/ that just updates the inode, device number, and ctime fields?
> That sounds a little less worrying to use, as plumbing.

Yes, I'd been discussing with Jonathan offlist that I too agree
this is a bit bizarre for general use.  Especially when thinking
about how to explain the new flag in the manpage, and how
interactions with other command-line options should work.

I wrote a short C program to use existing functions from cache.h
to read and write the index, updating the entries by hand.  Once
I figure out how to build it nicely, I'll submit for contrib/.  I
haven't figured out how to include the useful bits of the
top-level Makefile (like SSL setting, which SHA1, -lpthread,
etc.).

		-- Pete

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] teach update-index --refresh about --data-unchanged
  2010-11-03 22:02           ` Pete Wyckoff
@ 2010-11-14 16:58             ` Pete Wyckoff
  2010-11-14 17:34               ` Jonathan Nieder
  0 siblings, 1 reply; 16+ messages in thread
From: Pete Wyckoff @ 2010-11-14 16:58 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git, Christian Couder

pw@padd.com wrote on Wed, 03 Nov 2010 18:02 -0400:
> jrnieder@gmail.com wrote on Wed, 03 Nov 2010 13:36 -0500:
> > On Wed, Nov 03, 2010 at 10:37:40AM -0700, Junio C Hamano wrote:
> > > Pete Wyckoff <pw@padd.com> writes:
> > 
> > >> 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.
> > >
> > > I know our traditional attitude towards the plumbing commands have been
> > > "give them long enough rope and let users hang themselves", but this
> > > particular rope feels a bit too long for my taste.
> > 
> > Pete, I think you mentioned the possibility of a special-case tool for
> > contrib/ that just updates the inode, device number, and ctime fields?
> > That sounds a little less worrying to use, as plumbing.
> 
> Yes, I'd been discussing with Jonathan offlist that I too agree
> this is a bit bizarre for general use.  Especially when thinking
> about how to explain the new flag in the manpage, and how
> interactions with other command-line options should work.
> 
> I wrote a short C program to use existing functions from cache.h
> to read and write the index, updating the entries by hand.  Once
> I figure out how to build it nicely, I'll submit for contrib/.  I
> haven't figured out how to include the useful bits of the
> top-level Makefile (like SSL setting, which SHA1, -lpthread,
> etc.).

Turns out this was dead simple with dulwich, a python interface
to git internals:  http://samba.org/~jelmer/dulwich .  I got rid
of the C version.

The script uses dulwich to generate a list of index entries,
stats each file and updates the entries, then writes it back
out.  I won't submit it to contrib, since it is so trivial and
because it depends on dulwich.

		-- Pete

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] teach update-index --refresh about --data-unchanged
  2010-11-14 16:58             ` Pete Wyckoff
@ 2010-11-14 17:34               ` Jonathan Nieder
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Nieder @ 2010-11-14 17:34 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: Junio C Hamano, git, Christian Couder

Pete Wyckoff wrote:

> The script uses dulwich to generate a list of index entries,
> stats each file and updates the entries, then writes it back
> out.  I won't submit it to contrib, since it is so trivial and
> because it depends on dulwich.

Maybe we can add Dulwich to contrib? :)

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2010-11-14 17:35 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-31 17:44 index rebuild for cloned repo Pete Wyckoff
2010-10-31 18:07 ` Jonathan Nieder
2010-10-31 19:59 ` [PATCH] cherry-pick/revert: transparently refresh index Jonathan Nieder
2010-10-31 20:26   ` Andreas Schwab
2010-10-31 20:32     ` Jonathan Nieder
2010-10-31 22:26   ` Pete Wyckoff
2010-10-31 22:28     ` [PATCH] teach update-index --refresh about --data-unchanged Pete Wyckoff
2010-11-03 17:37       ` Junio C Hamano
2010-11-03 18:36         ` Jonathan Nieder
2010-11-03 22:02           ` Pete Wyckoff
2010-11-14 16:58             ` Pete Wyckoff
2010-11-14 17:34               ` Jonathan Nieder
2010-11-01  8:09   ` [PATCH] cherry-pick/revert: transparently refresh index Johannes Sixt
2010-11-03 17:30     ` Junio C Hamano
2010-11-03 20:33       ` Johannes Sixt
2010-11-01  8:44   ` Nguyen Thai Ngoc Duy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.