Merge branch 'bc/maint-keep-pack' into maint

* bc/maint-keep-pack:
  repack: only unpack-unreachable if we are deleting redundant packs
  t7700: test that 'repack -a' packs alternate packed objects
  pack-objects: extend --local to mean ignore non-local loose objects too
  sha1_file.c: split has_loose_object() into local and non-local counterparts
  t7700: demonstrate mishandling of loose objects in an alternate ODB
  builtin-gc.c: use new pack_keep bitfield to detect .keep file existence
  repack: do not fall back to incremental repacking with [-a|-A]
  repack: don't repack local objects in packs with .keep file
  pack-objects: new option --honor-pack-keep
  packed_git: convert pack_local flag into a bitfield and add pack_keep
  t7700: demonstrate mishandling of objects in packs with a .keep file

Documentation/git-pack-objects.txt

@@ -109,6 +109,11 @@ base-name::
 	The default is unlimited, unless the config variable
 	`pack.packSizeLimit` is set.
 
+--honor-pack-keep::
+	This flag causes an object already in a local pack that
+	has a .keep file to be ignored, even if it appears in the
+	standard input.
+
 --incremental::
 	This flag causes an object already in a pack ignored
 	even if it appears in the standard input.
@@ -116,7 +121,7 @@ base-name::
 --local::
 	This flag is similar to `--incremental`; instead of
 	ignoring all packed objects, it only ignores objects
-	that are packed and not in the local object store
+	that are packed and/or not in the local object store
 	(i.e. borrowed from an alternate).
 
 --non-empty::

Documentation/git-repack.txt

@@ -38,12 +38,11 @@ OPTIONS
 	dangling.
 
 -A::
-	Same as `-a`, but any unreachable objects in a previous
-	pack become loose, unpacked objects, instead of being
-	left in the old pack.  Unreachable objects are never
-	intentionally added to a pack, even when repacking.
-	When used with '-d', this option
-	prevents unreachable objects from being immediately
+	Same as `-a`, unless '-d' is used.  Then any unreachable
+	objects in a previous pack become loose, unpacked objects,
+	instead of being left in the old pack.  Unreachable objects
+	are never intentionally added to a pack, even when repacking.
+	This option prevents unreachable objects from being immediately
 	deleted by way of being left in the old pack and then
 	removed.  Instead, the loose unreachable objects
 	will be pruned according to normal expiry rules

builtin-gc.c

@@ -134,19 +134,9 @@ static int too_many_packs(void)
 
 	prepare_packed_git();
 	for (cnt = 0, p = packed_git; p; p = p->next) {
-		char path[PATH_MAX];
-		size_t len;
-		int keep;
-
 		if (!p->pack_local)
 			continue;
-		len = strlen(p->pack_name);
-		if (PATH_MAX <= len + 1)
-			continue; /* oops, give up */
-		memcpy(path, p->pack_name, len-5);
-		memcpy(path + len - 5, ".keep", 6);
-		keep = access(p->pack_name, F_OK) && (errno == ENOENT);
-		if (keep)
+		if (p->pack_keep)
 			continue;
 		/*
 		 * Perhaps check the size of the pack and count only

builtin-pack-objects.c

@@ -71,6 +71,7 @@ static int reuse_delta = 1, reuse_object = 1;
 static int keep_unreachable, unpack_unreachable, include_tag;
 static int local;
 static int incremental;
+static int ignore_packed_keep;
 static int allow_ofs_delta;
 static const char *base_name;
 static int progress = 1;
@@ -698,6 +699,9 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
 		return 0;
 	}
 
+	if (!exclude && local && has_loose_object_nonlocal(sha1))
+		return 0;
+
 	for (p = packed_git; p; p = p->next) {
 		off_t offset = find_pack_entry_one(sha1, p);
 		if (offset) {
@@ -711,6 +715,8 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
 				return 0;
 			if (local && !p->pack_local)
 				return 0;
+			if (ignore_packed_keep && p->pack_local && p->pack_keep)
+				return 0;
 		}
 	}
 
@@ -2050,6 +2056,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			incremental = 1;
 			continue;
 		}
+		if (!strcmp("--honor-pack-keep", arg)) {
+			ignore_packed_keep = 1;
+			continue;
+		}
 		if (!prefixcmp(arg, "--compression=")) {
 			char *end;
 			int level = strtoul(arg+14, &end, 0);

cache.h

@@ -567,6 +567,7 @@ extern int move_temp_to_file(const char *tmpfile, const char *filename);
 
 extern int has_sha1_pack(const unsigned char *sha1, const char **ignore);
 extern int has_sha1_file(const unsigned char *sha1);
+extern int has_loose_object_nonlocal(const unsigned char *sha1);
 
 extern int has_pack_file(const unsigned char *sha1);
 extern int has_pack_index(const unsigned char *sha1);
@@ -673,7 +674,8 @@ extern struct packed_git {
 	int index_version;
 	time_t mtime;
 	int pack_fd;
-	int pack_local;
+	unsigned pack_local:1,
+		 pack_keep:1;
 	unsigned char sha1[20];
 	/* something like ".git/objects/pack/xxxxx.pack" */
 	char pack_name[FLEX_ARRAY]; /* more */

git-repack.sh

@@ -71,19 +71,17 @@ case ",$all_into_one," in
 				existing="$existing $e"
 			fi
 		done
-	fi
-	if test -z "$args"
-	then
-		args='--unpacked --incremental'
-	elif test -n "$unpack_unreachable"
-	then
-		args="$args $unpack_unreachable"
+		if test -n "$args" -a -n "$unpack_unreachable" -a \
+			-n "$remove_redundant"
+		then
+			args="$args $unpack_unreachable"
+		fi
 	fi
 	;;
 esac
 
 args="$args $local $quiet $no_reuse$extra"
-names=$(git pack-objects --non-empty --all --reflog $args </dev/null "$PACKTMP") ||
+names=$(git pack-objects --honor-pack-keep --non-empty --all --reflog $args </dev/null "$PACKTMP") ||
 	exit 1
 if [ -z "$names" ]; then
 	if test -z "$quiet"; then

sha1_file.c

@@ -410,23 +410,30 @@ void prepare_alt_odb(void)
 	read_info_alternates(get_object_directory(), 0);
 }
 
-static int has_loose_object(const unsigned char *sha1)
+static int has_loose_object_local(const unsigned char *sha1)
 {
 	char *name = sha1_file_name(sha1);
-	struct alternate_object_database *alt;
+	return !access(name, F_OK);
+}
 
-	if (!access(name, F_OK))
-		return 1;
+int has_loose_object_nonlocal(const unsigned char *sha1)
+{
+	struct alternate_object_database *alt;
 	prepare_alt_odb();
 	for (alt = alt_odb_list; alt; alt = alt->next) {
-		name = alt->name;
-		fill_sha1_path(name, sha1);
+		fill_sha1_path(alt->name, sha1);
 		if (!access(alt->base, F_OK))
 			return 1;
 	}
 	return 0;
 }
 
+static int has_loose_object(const unsigned char *sha1)
+{
+	return has_loose_object_local(sha1) ||
+	       has_loose_object_nonlocal(sha1);
+}
+
 static unsigned int pack_used_ctr;
 static unsigned int pack_mmap_calls;
 static unsigned int peak_pack_open_windows;
@@ -828,6 +835,11 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)
 		return NULL;
 	}
 	memcpy(p->pack_name, path, path_len);
+
+	strcpy(p->pack_name + path_len, ".keep");
+	if (!access(p->pack_name, F_OK))
+		p->pack_keep = 1;
+
 	strcpy(p->pack_name + path_len, ".pack");
 	if (stat(p->pack_name, &st) || !S_ISREG(st.st_mode)) {
 		free(p);

t/t7700-repack.sh

@@ -0,0 +1,73 @@
+#!/bin/sh
+
+test_description='git repack works correctly'
+
+. ./test-lib.sh
+
+test_expect_success 'objects in packs marked .keep are not repacked' '
+	echo content1 > file1 &&
+	echo content2 > file2 &&
+	git add . &&
+	git commit -m initial_commit &&
+	# Create two packs
+	# The first pack will contain all of the objects except one
+	git rev-list --objects --all | grep -v file2 |
+		git pack-objects pack > /dev/null &&
+	# The second pack will contain the excluded object
+	packsha1=$(git rev-list --objects --all | grep file2 |
+		git pack-objects pack) &&
+	touch -r pack-$packsha1.pack pack-$packsha1.keep &&
+	objsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 |
+		sed -e "s/^\([0-9a-f]\{40\}\).*/\1/") &&
+	mv pack-* .git/objects/pack/ &&
+	git repack -A -d -l &&
+	git prune-packed &&
+	for p in .git/objects/pack/*.idx; do
+		idx=$(basename $p)
+		test "pack-$packsha1.idx" = "$idx" && continue
+		if git verify-pack -v $p | egrep "^$objsha1"; then
+			found_duplicate_object=1
+			echo "DUPLICATE OBJECT FOUND"
+			break
+		fi
+	done &&
+	test -z "$found_duplicate_object"
+'
+
+test_expect_success 'loose objects in alternate ODB are not repacked' '
+	mkdir alt_objects &&
+	echo `pwd`/alt_objects > .git/objects/info/alternates &&
+	echo content3 > file3 &&
+	objsha1=$(GIT_OBJECT_DIRECTORY=alt_objects git hash-object -w file3) &&
+	git add file3 &&
+	git commit -m commit_file3 &&
+	git repack -a -d -l &&
+	git prune-packed &&
+	for p in .git/objects/pack/*.idx; do
+		if git verify-pack -v $p | egrep "^$objsha1"; then
+			found_duplicate_object=1
+			echo "DUPLICATE OBJECT FOUND"
+			break
+		fi
+	done &&
+	test -z "$found_duplicate_object"
+'
+
+test_expect_success 'packed obs in alt ODB are repacked even when local repo is packless' '
+	mkdir alt_objects/pack
+	mv .git/objects/pack/* alt_objects/pack &&
+	git repack -a &&
+	myidx=$(ls -1 .git/objects/pack/*.idx) &&
+	test -f "$myidx" &&
+	for p in alt_objects/pack/*.idx; do
+		git verify-pack -v $p | sed -n -e "/^[0-9a-f]\{40\}/p"
+	done | while read sha1 rest; do
+		if ! ( git verify-pack -v $myidx | grep "^$sha1" ); then
+			echo "Missing object in local pack: $sha1"
+			return 1
+		fi
+	done
+'
+
+test_done
+

t/t7701-repack-unpack-unreachable.sh

@@ -8,7 +8,7 @@ fsha1=
 csha1=
 tsha1=
 
-test_expect_success '-A option leaves unreachable objects unpacked' '
+test_expect_success '-A with -d option leaves unreachable objects unpacked' '
 	echo content > file1 &&
 	git add . &&
 	git commit -m initial_commit &&
@@ -58,7 +58,7 @@ compare_mtimes ()
 		' -- "$@"
 }
 
-test_expect_success 'unpacked objects receive timestamp of pack file' '
+test_expect_success '-A without -d option leaves unreachable objects packed' '
 	fsha1path=$(echo "$fsha1" | sed -e "s|\(..\)|\1/|") &&
 	fsha1path=".git/objects/$fsha1path" &&
 	csha1path=$(echo "$csha1" | sed -e "s|\(..\)|\1/|") &&
@@ -75,7 +75,19 @@ test_expect_success 'unpacked objects receive timestamp of pack file' '
 	git branch -D transient_branch &&
 	sleep 1 &&
 	git repack -A -l &&
-	compare_mtimes "$packfile" "$fsha1path" "$csha1path" "$tsha1path"
+	test ! -f "$fsha1path" &&
+	test ! -f "$csha1path" &&
+	test ! -f "$tsha1path" &&
+	git show $fsha1 &&
+	git show $csha1 &&
+	git show $tsha1
+'
+
+test_expect_success 'unpacked objects receive timestamp of pack file' '
+	tmppack=".git/objects/pack/tmp_pack" &&
+	ln "$packfile" "$tmppack" &&
+	git repack -A -l -d &&
+	compare_mtimes "$tmppack" "$fsha1path" "$csha1path" "$tsha1path"
 '
 
 test_done