Re: [PATCH 1/2] pack-objects: Teach --use-bitmap-index codepath to respect --local, --honor-pack-keep and --incremental

Jeff King [off-list ref] writes:

On Mon, Aug 08, 2016 at 03:37:35PM +0300, Kirill Smelkov wrote:
...
  static int want_object_in_pack(...)
  {
	...
	if (!exclude && local && has_loose_object_nonlocal(sha1))
		return 0;

	if (*found_pack) {
		int ret = want_found_object(exclude, *found_pack);
		if (ret != -1)
			return ret;
	}

	for (p = packed_git; p; p = p->next) {
		off_t offset;

		if (p == *found_pack)
			offset = *found_offset;
		else
			offset = find_pack_entry(sha1, p);
		if (offset) {
			... fill in *found_pack ...
			int ret = want_found_object(exclude, p);
			if (ret != -1)
				return ret;
		}
	}
	return 1;
  }

That's a little more verbose, but IMHO the flow is a lot easier to
follow (especially as the later re-rolls of that series actually muck
with the loop order more, but with this approach there's no conflict).

I agree; Kirill's version was so confusing that I couldn't see what
it was trying to do with "pack1_seen" flag that is reset every time
loop repeats (at least, before got my coffee ;-).  A helper function
like the above makes the logic a lot easier to grasp.

quoted

 static int add_object_entry(const unsigned char *sha1, enum object_type type,
 			    const char *name, int exclude)
 {
-	struct packed_git *found_pack;
-	off_t found_offset;
+	struct packed_git *found_pack = NULL;
+	off_t found_offset = 0;
 	uint32_t index_pos;
 
 	if (have_duplicate_entry(sha1, exclude, &index_pos))

@@ -1073,6 +1088,9 @@ static int add_object_entry_from_bitmap(const unsigned char *sha1,
 	if (have_duplicate_entry(sha1, 0, &index_pos))
 		return 0;
 
+	if (!want_object_in_pack(sha1, 0, &pack, &offset))
+		return 0;
+

This part looks correct and easy to understand.

Yes.