threads / discuss / 416

Should git-prune-script warn about dircache?

Subject: Should git-prune-script warn about dircache?

## tl;dr

6 messages between May 1, 2005 and May 1, 2005.

replies: 5people: 2as markdown or json

Junio C Hamano· May 1, 2005, 10:50 UTC · lore

When the user has a change that is recorded in the dircache but the change has not been committed, git-fsck-cache --unreachable would report blobs and trees (if you have a subdirectory) that are involved.

    $ git-init-db
    defaulting to local storage area
    $ date >foo
    $ git-update-cache --add foo
    $ date | git-commit-tree $(git-write-tree) >.git/HEAD
    Committing initial tree 9061076c3ec10fef339c84f4e0c9e06576d3b7db
    $ date >foo
    $ git-update-cache foo
    $ git-fsck-cache --unreachable $(cat .git/HEAD)
    unreachable blob d8c61e975a591ad5905c7defc567a77fca58f092
    $ git-ls-files --stage
    100644 d8c61e975a591ad5905c7defc567a77fca58f092 0 foo

At this point, if we run git-prune-script with the current head, we would remove the blob, and after that, code that assumes the objects pointed by SHA1s recorded in the dircache are locally available start failing.

    $ git-prune-script
    $ touch foo
    $ git-diff-files -p
    .git/objects/d8/c61e975a591....: No such file or directory
    fatal: unable to read blob object for foo (d8c61e975a591....)

I have a feeling somewhere in the chain from git-prune-script we should have a mechanism to prevent us from losing such blob and trees. I know it should not be in git-fsck-cache, but I have not decided what my suggestions would be for us to do yet.

Maybe a big warning in red ugly bold blinking typeface somewhere in the doc?

Linus Torvalds· May 1, 2005, 16:18 UTC · re: Junio C Hamano · lore

Re: Should git-prune-script warn about dircache?

On Sun, 1 May 2005, Junio C Hamano wrote:

> 
> Maybe a big warning in red ugly bold blinking typeface somewhere
> in the doc?

How about making git-prune-script first run "git-update-cache --refresh", and checking the return value of it (this, of course, assumes that git-update-cache --refresh would return non-zero if it can't refresh a file, which is currently not true, but should be easily fixable).

		Linus

Junio C Hamano· May 1, 2005, 17:20 UTC · re: Linus Torvalds · lore

Re: Should git-prune-script warn about dircache?

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> How about making git-prune-script first run "git-update-cache --refresh", LT> and checking the return value of it (this, of course, assumes that LT> git-update-cache --refresh would return non-zero if it can't refresh a LT> file, which is currently not true, but should be easily fixable).

Or just check if it sees anything in the output, especially "needs update" line.

I do not think it is such a big deal [*1*] but I should point out that, "git-update-cache --refresh" needs to be run on all of them if the user (or the porcelain layer) is using more than one GIT_INDEX_FILEs [*2*].

[Footnotes]

*1* Because git-prune-script is just an example and it already assumes it knows where the valid heads are; right now it looks only at .git/HEAD and not .git/refs/*/*. Each Porcelain layer implementation should provide its own prune script anyway.

*2* I do not do this anymore but an earlier incarnation of my little SCM on GIT [*3*] allowed a user to keep snapshots of work tree state and switch between them by juggling multiple GIT_INDEX_FILE. I just create commits off of the current state when making a snapshot in the latest version so it is not a problem anymore for me.

*3* (PLUG) found in http://members.cox.net/junkio/.

Junio C Hamano· May 1, 2005, 20:41 UTC · re: Junio C Hamano · lore

Re: Should git-prune-script warn about dircache?

>>>>> "JCH" == Junio C Hamano <junkio@cox.net> writes:

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

JCH> Or just check if it sees anything in the output, especially JCH> "needs update" line.

Well, we were both wrong. The problem is not about the work tree changes since the last git-update-cache, but about the blobs recorded in the cache but still not committed.

I think we should do something like this.

    git-ls-files --cached | "sed to SHA1 only" | sort >,,1
    git-fsck-cache --unreachable | "sed to SHA1 only" | sort >,,2
    comm -13 ,,1 ,,2 | "sed to .git/object/ path" | xargs -r rm -f

Junio C Hamano· May 1, 2005, 21:27 UTC · re: Junio C Hamano · lore

[PATCH] Make git-prune-script a bit more careful.

As discussed on the git list, this patch makes the sample script git-prune-script a bit more careful by keeping the objects referenced from the current cache from being reclaimed.

At the same time it also fixes and enhances the following:

 - Optional command line parameters can specify which commit
   heads to start reachablity test from.  Earlier we had a
   hardcoded .git/HEAD.  It now defaults to the contents of
   .git/HEAD and .git/refs/*/* files.

 - It does not assume SHA1_FILE_DIRECTORY is .git/objects/ but
   uses the value from the environment if there is one.

 - It runs xargs with "-r" option to prevent it from making "rm"
   command barf if there is nothing to remove.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

git-prune-script | 32 +++++++++++++++++++++++++++++++- 1 files changed, 31 insertions(+), 1 deletion(-)

# - date handling: handle "AM"/"PM" on time # + [PATCH] Make git-prune-script a bit more careful. --- k/git-prune-script +++ l/git-prune-script

@@ -1,2 +1,32 @@
 #!/bin/sh
-git-fsck-cache --unreachable $(cat .git/HEAD ) | grep unreachable | cut -d' ' -f3 | sed 's:^\(..\):.git/objects/\1/:' | xargs rm
+
+tmp=.git-prune-script-$$
+trap 'rm -f $tmp-*' 0 1 2 3 15 
+
+# Defaulting to include .git/refs/*/* may be debatable from the
+# purist POV but power users can always give explicit parameters
+# to the script anyway.
+case "$#" in
+0) set x $(cat .git/HEAD .git/refs/*/*); shift ;;
+esac
+
+git-fsck-cache --unreachable "$@" |
+sed -ne 's/unreachable [^ ][^ ]* //p' |
+sort >$tmp-unreachable
+
+# This makes extra objects to be kept if the cache has an entry
+# with an unusual name like "this\n0 0123...abcdef 0 file", but
+# we are trying not to discard information and keeping extra in
+# an unusual situation would be OK.
+git-ls-files --stage |
+sed -ne 's|^[0-7][0-7]* \([0-9a-f][0-9a-f]*\) [0-3] .*|\1|p' |
+sort >$tmp-keep
+
+comm -23 $tmp-unreachable $tmp-keep |
+sed -e 's|\(..\)|\1/|' | {
+	case "$SHA1_FILE_DIRECTORY" in
+	'') cd .git/objects/ ;;
+	*) cd "$SHA1_FILE_DIRECTORY" ;;
+	esac || exit
+	xargs -r echo rm -f
+}

Junio C Hamano· May 1, 2005, 21:29 UTC · re: Linus Torvalds · lore

[PATCH] Make git-update-cache --refresh fail if update/merge needed.

Scripts may find it useful if they do not have to parse the output from the command but just can rely on its exit status.

Earlier both Linus and myself thought this would be necessary to make git-prune-script safer but it turns out that the issue was somewhere else and not related to what this patch addresses.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

update-cache.c | 15 ++++++++++----- 1 files changed, 10 insertions(+), 5 deletions(-)

# - date handling: handle "AM"/"PM" on time # + [PATCH] Make git-prune-script a bit more careful. --- k/update-cache.c +++ l/update-cache.c

@@ -212,15 +212,17 @@ static struct cache_entry *refresh_entry
 	return updated;
 }
 
-static void refresh_cache(void)
+static int refresh_cache(void)
 {
 	int i;
+	int has_errors = 0;
 
 	for (i = 0; i < active_nr; i++) {
 		struct cache_entry *ce, *new;
 		ce = active_cache[i];
 		if (ce_stage(ce)) {
 			printf("%s: needs merge\n", ce->name);
+			has_errors = 1;
 			while ((i < active_nr) &&
 			       ! strcmp(active_cache[i]->name, ce->name))
 				i++;
@@ -230,12 +232,15 @@ static void refresh_cache(void)
 
 		new = refresh_entry(ce);
 		if (IS_ERR(new)) {
-			if (!(not_new && PTR_ERR(new) == -ENOENT))
+			if (!(not_new && PTR_ERR(new) == -ENOENT)) {
 				printf("%s: needs update\n", ce->name);
+				has_errors = 1;
+			}
 			continue;
 		}
 		active_cache[i] = new;
 	}
+	return has_errors;
 }
 
 /*
@@ -307,7 +312,7 @@ static void remove_lock_file_on_signal(i
 
 int main(int argc, char **argv)
 {
-	int i, newfd, entries;
+	int i, newfd, entries, has_errors = 0;
 	int allow_options = 1;
 	static char lockfile[MAXPATHLEN+1];
 	const char *indexfile = get_index_file();
@@ -343,7 +348,7 @@ int main(int argc, char **argv)
 				continue;
 			}
 			if (!strcmp(path, "--refresh")) {
-				refresh_cache();
+				has_errors |= refresh_cache();
 				continue;
 			}
 			if (!strcmp(path, "--cacheinfo")) {
@@ -369,5 +374,5 @@ int main(int argc, char **argv)
 		die("Unable to write new cachefile");
 
 	lockfile_name = NULL;
-	return 0;
+	return has_errors;
 }