git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/9] object store: oid_object_info is the next contender
@ 2018-04-23 23:43 Stefan Beller
  2018-04-23 23:43 ` [PATCH 1/9] cache.h: add repository argument to oid_object_info_extended Stefan Beller
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Stefan Beller @ 2018-04-23 23:43 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

This applies on top of origin/sb/object-store-replace and is available as
https://github.com/stefanbeller/git/tree/oid_object_info

This continues the work of sb/packfiles-in-repository,
extending the layer at which we have to pass in an explicit
repository object to oid_object_info.

A test merge to next shows only a minor merge conflicit (adding
different #include lines in one c file), so this might be a good next
step for the object store series.

Notes on further object store series:
I plan on converting the "parsed object store" next,
which would be {alloc, object, tree, commit, tag}.c as that is a prerequisite
for migrating shallow (which is intermingled with grafts) information to the
object store.

There is currently work going on in allocation (mempool - Jameson Miller)
and grafts (deprecate grafts - DScho), which is why I am sending this
series first. I think it can go in parallel to the "parsed object store"
that is coming next.

Thanks,
Stefan

Jonathan Nieder (1):
  packfile: add repository argument to packed_object_info

Stefan Beller (8):
  cache.h: add repository argument to oid_object_info_extended
  cache.h: add repository argument to oid_object_info
  packfile: add repository argument to retry_bad_packed_offset
  packfile: add repository argument to packed_to_object_type
  packfile: add repository argument to read_object
  packfile: add repository argument to unpack_entry
  packfile: add repository argument to cache_or_unpack_entry
  cache.h: allow sha1_object_info to handle arbitrary repositories

 archive-tar.c            |  2 +-
 archive-zip.c            |  3 ++-
 blame.c                  |  4 ++--
 builtin/blame.c          |  2 +-
 builtin/cat-file.c       | 12 ++++++------
 builtin/describe.c       |  2 +-
 builtin/fast-export.c    |  2 +-
 builtin/fetch.c          |  2 +-
 builtin/fsck.c           |  3 ++-
 builtin/index-pack.c     |  4 ++--
 builtin/ls-tree.c        |  2 +-
 builtin/mktree.c         |  2 +-
 builtin/pack-objects.c   | 11 +++++++----
 builtin/prune.c          |  3 ++-
 builtin/replace.c        | 11 ++++++-----
 builtin/tag.c            |  4 ++--
 builtin/unpack-objects.c |  2 +-
 cache.h                  |  7 +++++--
 diff.c                   |  3 ++-
 fast-import.c            | 16 ++++++++++------
 list-objects-filter.c    |  2 +-
 object.c                 |  2 +-
 pack-bitmap-write.c      |  3 ++-
 pack-check.c             |  3 ++-
 packfile.c               | 40 +++++++++++++++++++++++-----------------
 packfile.h               |  6 ++++--
 reachable.c              |  2 +-
 refs.c                   |  2 +-
 remote.c                 |  2 +-
 sequencer.c              |  3 ++-
 sha1_file.c              | 32 ++++++++++++++++++--------------
 sha1_name.c              | 12 ++++++------
 streaming.c              |  2 +-
 submodule.c              |  2 +-
 tag.c                    |  2 +-
 35 files changed, 121 insertions(+), 91 deletions(-)

-- 
2.17.0.484.g0c8726318c-goog


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

* [PATCH 1/9] cache.h: add repository argument to oid_object_info_extended
  2018-04-23 23:43 [PATCH 0/9] object store: oid_object_info is the next contender Stefan Beller
@ 2018-04-23 23:43 ` Stefan Beller
  2018-04-23 23:43 ` [PATCH 2/9] cache.h: add repository argument to oid_object_info Stefan Beller
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Stefan Beller @ 2018-04-23 23:43 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

Add a repository argument to allow oid_object_info_extended callers
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/cat-file.c |  6 +++---
 cache.h            |  5 ++++-
 packfile.c         |  2 +-
 sha1_file.c        | 10 +++++-----
 streaming.c        |  2 +-
 5 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2c46d257cd..4ecdb9ff54 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -77,7 +77,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	switch (opt) {
 	case 't':
 		oi.type_name = &sb;
-		if (oid_object_info_extended(&oid, &oi, flags) < 0)
+		if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
 			die("git cat-file: could not get object info");
 		if (sb.len) {
 			printf("%s\n", sb.buf);
@@ -88,7 +88,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 
 	case 's':
 		oi.sizep = &size;
-		if (oid_object_info_extended(&oid, &oi, flags) < 0)
+		if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
 			die("git cat-file: could not get object info");
 		printf("%lu\n", size);
 		return 0;
@@ -342,7 +342,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
 	struct strbuf buf = STRBUF_INIT;
 
 	if (!data->skip_object_info &&
-	    oid_object_info_extended(&data->oid, &data->info,
+	    oid_object_info_extended(the_repository, &data->oid, &data->info,
 				     OBJECT_INFO_LOOKUP_REPLACE) < 0) {
 		printf("%s missing\n",
 		       obj_name ? obj_name : oid_to_hex(&data->oid));
diff --git a/cache.h b/cache.h
index 027bd7ffc8..588c4fff9a 100644
--- a/cache.h
+++ b/cache.h
@@ -1673,7 +1673,10 @@ struct object_info {
 #define OBJECT_INFO_QUICK 8
 /* Do not check loose object */
 #define OBJECT_INFO_IGNORE_LOOSE 16
-extern int oid_object_info_extended(const struct object_id *, struct object_info *, unsigned flags);
+
+#define oid_object_info_extended(r, oid, oi, flags) \
+	oid_object_info_extended_##r(oid, oi, flags)
+int oid_object_info_extended_the_repository(const struct object_id *, struct object_info *, unsigned flags);
 
 /*
  * Set this to 0 to prevent sha1_object_info_extended() from fetching missing
diff --git a/packfile.c b/packfile.c
index 0bc67d0e00..d9914ba723 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1474,7 +1474,7 @@ static void *read_object(const struct object_id *oid, enum object_type *type,
 	oi.sizep = size;
 	oi.contentp = &content;
 
-	if (oid_object_info_extended(oid, &oi, 0) < 0)
+	if (oid_object_info_extended(the_repository, oid, &oi, 0) < 0)
 		return NULL;
 	return content;
 }
diff --git a/sha1_file.c b/sha1_file.c
index 64a5bd7d87..50a2dc5f0a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1231,7 +1231,7 @@ static int sha1_loose_object_info(struct repository *r,
 
 int fetch_if_missing = 1;
 
-int oid_object_info_extended(const struct object_id *oid, struct object_info *oi, unsigned flags)
+int oid_object_info_extended_the_repository(const struct object_id *oid, struct object_info *oi, unsigned flags)
 {
 	static struct object_info blank_oi = OBJECT_INFO_INIT;
 	struct pack_entry e;
@@ -1310,7 +1310,7 @@ int oid_object_info_extended(const struct object_id *oid, struct object_info *oi
 	rtype = packed_object_info(e.p, e.offset, oi);
 	if (rtype < 0) {
 		mark_bad_packed_object(e.p, real->hash);
-		return oid_object_info_extended(real, oi, 0);
+		return oid_object_info_extended(the_repository, real, oi, 0);
 	} else if (oi->whence == OI_PACKED) {
 		oi->u.packed.offset = e.offset;
 		oi->u.packed.pack = e.p;
@@ -1329,7 +1329,7 @@ int oid_object_info(const struct object_id *oid, unsigned long *sizep)
 
 	oi.typep = &type;
 	oi.sizep = sizep;
-	if (oid_object_info_extended(oid, &oi,
+	if (oid_object_info_extended(the_repository, oid, &oi,
 				     OBJECT_INFO_LOOKUP_REPLACE) < 0)
 		return -1;
 	return type;
@@ -1347,7 +1347,7 @@ static void *read_object(const unsigned char *sha1, enum object_type *type,
 
 	hashcpy(oid.hash, sha1);
 
-	if (oid_object_info_extended(&oid, &oi, 0) < 0)
+	if (oid_object_info_extended(the_repository, &oid, &oi, 0) < 0)
 		return NULL;
 	return content;
 }
@@ -1745,7 +1745,7 @@ int has_sha1_file_with_flags(const unsigned char *sha1, int flags)
 	if (!startup_info->have_repository)
 		return 0;
 	hashcpy(oid.hash, sha1);
-	return oid_object_info_extended(&oid, NULL,
+	return oid_object_info_extended(the_repository, &oid, NULL,
 					flags | OBJECT_INFO_SKIP_CACHED) >= 0;
 }
 
diff --git a/streaming.c b/streaming.c
index cce7b17ea7..d1e6b2dce6 100644
--- a/streaming.c
+++ b/streaming.c
@@ -117,7 +117,7 @@ static enum input_source istream_source(const struct object_id *oid,
 
 	oi->typep = type;
 	oi->sizep = &size;
-	status = oid_object_info_extended(oid, oi, 0);
+	status = oid_object_info_extended(the_repository, oid, oi, 0);
 	if (status < 0)
 		return stream_error;
 
-- 
2.17.0.484.g0c8726318c-goog


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

* [PATCH 2/9] cache.h: add repository argument to oid_object_info
  2018-04-23 23:43 [PATCH 0/9] object store: oid_object_info is the next contender Stefan Beller
  2018-04-23 23:43 ` [PATCH 1/9] cache.h: add repository argument to oid_object_info_extended Stefan Beller
@ 2018-04-23 23:43 ` Stefan Beller
  2018-04-24  0:31   ` brian m. carlson
  2018-04-24 18:15   ` Jonathan Tan
  2018-04-23 23:43 ` [PATCH 3/9] packfile: add repository argument to retry_bad_packed_offset Stefan Beller
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 19+ messages in thread
From: Stefan Beller @ 2018-04-23 23:43 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

Add a repository argument to allow the callers of oid_object_info
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

In the expanded macro the identifier `the_repository` is not actually used,
so the compiler does not catch if the repository.h header is not included
at the call site. call sites needing that #include were identified by
changing the macro to definition to

      #define sha1_object_info(r, sha1, size) \
          (r, sha1_object_info_##r(sha1, size)).

This produces a compiler warning about the left hand side of the comma
operator being unused, which can be suppressed using -Wno-unused-value.

To avoid breaking bisection, do not include this trick in the patch.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 archive-tar.c            |  2 +-
 archive-zip.c            |  3 ++-
 blame.c                  |  4 ++--
 builtin/blame.c          |  2 +-
 builtin/cat-file.c       |  6 +++---
 builtin/describe.c       |  2 +-
 builtin/fast-export.c    |  2 +-
 builtin/fetch.c          |  2 +-
 builtin/fsck.c           |  3 ++-
 builtin/index-pack.c     |  4 ++--
 builtin/ls-tree.c        |  2 +-
 builtin/mktree.c         |  2 +-
 builtin/pack-objects.c   |  8 +++++---
 builtin/prune.c          |  3 ++-
 builtin/replace.c        | 11 ++++++-----
 builtin/tag.c            |  4 ++--
 builtin/unpack-objects.c |  2 +-
 cache.h                  |  3 ++-
 diff.c                   |  3 ++-
 fast-import.c            | 14 +++++++++-----
 list-objects-filter.c    |  2 +-
 object.c                 |  2 +-
 pack-bitmap-write.c      |  3 ++-
 packfile.c               |  2 +-
 reachable.c              |  2 +-
 refs.c                   |  2 +-
 remote.c                 |  2 +-
 sequencer.c              |  3 ++-
 sha1_file.c              |  4 ++--
 sha1_name.c              | 12 ++++++------
 submodule.c              |  2 +-
 tag.c                    |  2 +-
 32 files changed, 67 insertions(+), 53 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index 3563bcb9f2..f93409324f 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -276,7 +276,7 @@ static int write_tar_entry(struct archiver_args *args,
 		memcpy(header.name, path, pathlen);
 
 	if (S_ISREG(mode) && !args->convert &&
-	    oid_object_info(oid, &size) == OBJ_BLOB &&
+	    oid_object_info(the_repository, oid, &size) == OBJ_BLOB &&
 	    size > big_file_threshold)
 		buffer = NULL;
 	else if (S_ISLNK(mode) || S_ISREG(mode)) {
diff --git a/archive-zip.c b/archive-zip.c
index 6b20bce4d1..74f3fe9103 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -325,7 +325,8 @@ static int write_zip_entry(struct archiver_args *args,
 		compressed_size = 0;
 		buffer = NULL;
 	} else if (S_ISREG(mode) || S_ISLNK(mode)) {
-		enum object_type type = oid_object_info(oid, &size);
+		enum object_type type = oid_object_info(the_repository, oid,
+							&size);
 
 		method = 0;
 		attr2 = S_ISLNK(mode) ? ((mode | 0777) << 16) :
diff --git a/blame.c b/blame.c
index 78c9808bd1..dfa24473dc 100644
--- a/blame.c
+++ b/blame.c
@@ -81,7 +81,7 @@ static void verify_working_tree_path(struct commit *work_tree, const char *path)
 		unsigned mode;
 
 		if (!get_tree_entry(commit_oid, path, &blob_oid, &mode) &&
-		    oid_object_info(&blob_oid, NULL) == OBJ_BLOB)
+		    oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB)
 			return;
 	}
 
@@ -504,7 +504,7 @@ static int fill_blob_sha1_and_mode(struct blame_origin *origin)
 		return 0;
 	if (get_tree_entry(&origin->commit->object.oid, origin->path, &origin->blob_oid, &origin->mode))
 		goto error_out;
-	if (oid_object_info(&origin->blob_oid, NULL) != OBJ_BLOB)
+	if (oid_object_info(the_repository, &origin->blob_oid, NULL) != OBJ_BLOB)
 		goto error_out;
 	return 0;
  error_out:
diff --git a/builtin/blame.c b/builtin/blame.c
index db38c0b307..bfdf7cc132 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -655,7 +655,7 @@ static int is_a_rev(const char *name)
 
 	if (get_oid(name, &oid))
 		return 0;
-	return OBJ_NONE < oid_object_info(&oid, NULL);
+	return OBJ_NONE < oid_object_info(the_repository, &oid, NULL);
 }
 
 int cmd_blame(int argc, const char **argv, const char *prefix)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 4ecdb9ff54..b8ecbea98e 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -116,7 +116,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		/* else fallthrough */
 
 	case 'p':
-		type = oid_object_info(&oid, NULL);
+		type = oid_object_info(the_repository, &oid, NULL);
 		if (type < 0)
 			die("Not a valid object name %s", obj_name);
 
@@ -140,7 +140,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	case 0:
 		if (type_from_string(exp_type) == OBJ_BLOB) {
 			struct object_id blob_oid;
-			if (oid_object_info(&oid, NULL) == OBJ_TAG) {
+			if (oid_object_info(the_repository, &oid, NULL) == OBJ_TAG) {
 				char *buffer = read_object_file(&oid, &type,
 								&size);
 				const char *target;
@@ -151,7 +151,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 			} else
 				oidcpy(&blob_oid, &oid);
 
-			if (oid_object_info(&blob_oid, NULL) == OBJ_BLOB)
+			if (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB)
 				return stream_blob_to_fd(1, &blob_oid, NULL, 0);
 			/*
 			 * we attempted to dereference a tag to a blob
diff --git a/builtin/describe.c b/builtin/describe.c
index de840f96a4..66c497f789 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -502,7 +502,7 @@ static void describe(const char *arg, int last_one)
 
 	if (cmit)
 		describe_commit(&oid, &sb);
-	else if (oid_object_info(&oid, NULL) == OBJ_BLOB)
+	else if (oid_object_info(the_repository, &oid, NULL) == OBJ_BLOB)
 		describe_blob(oid, &sb);
 	else
 		die(_("%s is neither a commit nor blob"), arg);
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index a15898d641..373c794873 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -947,7 +947,7 @@ static void import_marks(char *input_file)
 		if (last_idnum < mark)
 			last_idnum = mark;
 
-		type = oid_object_info(&oid, NULL);
+		type = oid_object_info(the_repository, &oid, NULL);
 		if (type < 0)
 			die("object not found: %s", oid_to_hex(&oid));
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index dcdfc66f09..73be393b2e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -637,7 +637,7 @@ static int update_local_ref(struct ref *ref,
 	struct branch *current_branch = branch_get(NULL);
 	const char *pretty_ref = prettify_refname(ref->name);
 
-	type = oid_object_info(&ref->new_oid, NULL);
+	type = oid_object_info(the_repository, &ref->new_oid, NULL);
 	if (type < 0)
 		die(_("object %s not found"), oid_to_hex(&ref->new_oid));
 
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 087360a675..9d59d7d5a2 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -67,7 +67,8 @@ static const char *printable_type(struct object *obj)
 	const char *ret;
 
 	if (obj->type == OBJ_NONE) {
-		enum object_type type = oid_object_info(&obj->oid, NULL);
+		enum object_type type = oid_object_info(the_repository,
+							&obj->oid, NULL);
 		if (type > 0)
 			object_as_type(obj, type, 0);
 	}
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index d81473e722..2d04a596f5 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -223,7 +223,7 @@ static unsigned check_object(struct object *obj)
 
 	if (!(obj->flags & FLAG_CHECKED)) {
 		unsigned long size;
-		int type = oid_object_info(&obj->oid, &size);
+		int type = oid_object_info(the_repository, &obj->oid, &size);
 		if (type <= 0)
 			die(_("did not receive expected object %s"),
 			      oid_to_hex(&obj->oid));
@@ -812,7 +812,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
 		enum object_type has_type;
 		unsigned long has_size;
 		read_lock();
-		has_type = oid_object_info(oid, &has_size);
+		has_type = oid_object_info(the_repository, oid, &has_size);
 		if (has_type < 0)
 			die(_("cannot read existing object info %s"), oid_to_hex(oid));
 		if (has_type != type || has_size != size)
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index d44b4f9c27..409da4e835 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -94,7 +94,7 @@ static int show_tree(const struct object_id *oid, struct strbuf *base,
 			char size_text[24];
 			if (!strcmp(type, blob_type)) {
 				unsigned long size;
-				if (oid_object_info(oid, &size) == OBJ_BAD)
+				if (oid_object_info(the_repository, oid, &size) == OBJ_BAD)
 					xsnprintf(size_text, sizeof(size_text),
 						  "BAD");
 				else
diff --git a/builtin/mktree.c b/builtin/mktree.c
index 263c530315..bb76b469fd 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -116,7 +116,7 @@ static void mktree_line(char *buf, size_t len, int nul_term_line, int allow_miss
 	}
 
 	/* Check the type of object identified by sha1 */
-	obj_type = oid_object_info(&oid, NULL);
+	obj_type = oid_object_info(the_repository, &oid, NULL);
 	if (obj_type < 0) {
 		if (allow_missing) {
 			; /* no problem - missing objects are presumed to be of the right type */
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 4bdae5a1d8..8d4111f748 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1516,7 +1516,8 @@ static void check_object(struct object_entry *entry)
 		unuse_pack(&w_curs);
 	}
 
-	entry->type = oid_object_info(&entry->idx.oid, &entry->size);
+	entry->type = oid_object_info(the_repository, &entry->idx.oid,
+				      &entry->size);
 	/*
 	 * The error condition is checked in prepare_pack().  This is
 	 * to permit a missing preferred base object to be ignored
@@ -1578,7 +1579,8 @@ static void drop_reused_delta(struct object_entry *entry)
 		 * And if that fails, the error will be recorded in entry->type
 		 * and dealt with in prepare_pack().
 		 */
-		entry->type = oid_object_info(&entry->idx.oid, &entry->size);
+		entry->type = oid_object_info(the_repository, &entry->idx.oid,
+					      &entry->size);
 	}
 }
 
@@ -2706,7 +2708,7 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs)
 static int add_loose_object(const struct object_id *oid, const char *path,
 			    void *data)
 {
-	enum object_type type = oid_object_info(oid, NULL);
+	enum object_type type = oid_object_info(the_repository, oid, NULL);
 
 	if (type < 0) {
 		warning("loose object at %s could not be examined", path);
diff --git a/builtin/prune.c b/builtin/prune.c
index 38ced18dad..518ffbea13 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -50,7 +50,8 @@ static int prune_object(const struct object_id *oid, const char *fullpath,
 	if (st.st_mtime > expire)
 		return 0;
 	if (show_only || verbose) {
-		enum object_type type = oid_object_info(oid, NULL);
+		enum object_type type = oid_object_info(the_repository, oid,
+							NULL);
 		printf("%s %s\n", oid_to_hex(oid),
 		       (type > 0) ? type_name(type) : "unknown");
 	}
diff --git a/builtin/replace.c b/builtin/replace.c
index 237ea656cf..14e142d5a8 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -55,8 +55,9 @@ static int show_reference(const char *refname, const struct object_id *oid,
 			if (get_oid(refname, &object))
 				return error("Failed to resolve '%s' as a valid ref.", refname);
 
-			obj_type = oid_object_info(&object, NULL);
-			repl_type = oid_object_info(oid, NULL);
+			obj_type = oid_object_info(the_repository, &object,
+						   NULL);
+			repl_type = oid_object_info(the_repository, oid, NULL);
 
 			printf("%s (%s) -> %s (%s)\n", refname, type_name(obj_type),
 			       oid_to_hex(oid), type_name(repl_type));
@@ -164,8 +165,8 @@ static int replace_object_oid(const char *object_ref,
 	struct ref_transaction *transaction;
 	struct strbuf err = STRBUF_INIT;
 
-	obj_type = oid_object_info(object, NULL);
-	repl_type = oid_object_info(repl, NULL);
+	obj_type = oid_object_info(the_repository, object, NULL);
+	repl_type = oid_object_info(the_repository, repl, NULL);
 	if (!force && obj_type != repl_type)
 		die("Objects must be of the same type.\n"
 		    "'%s' points to a replaced object of type '%s'\n"
@@ -292,7 +293,7 @@ static int edit_and_replace(const char *object_ref, int force, int raw)
 	if (get_oid(object_ref, &old_oid) < 0)
 		die("Not a valid object name: '%s'", object_ref);
 
-	type = oid_object_info(&old_oid, NULL);
+	type = oid_object_info(the_repository, &old_oid, NULL);
 	if (type < 0)
 		die("unable to get object type for %s", oid_to_hex(&old_oid));
 
diff --git a/builtin/tag.c b/builtin/tag.c
index 8cff6d0b72..26d7729f57 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -212,7 +212,7 @@ static void create_tag(const struct object_id *object, const char *tag,
 	struct strbuf header = STRBUF_INIT;
 	char *path = NULL;
 
-	type = oid_object_info(object, NULL);
+	type = oid_object_info(the_repository, object, NULL);
 	if (type <= OBJ_NONE)
 	    die(_("bad object type."));
 
@@ -298,7 +298,7 @@ static void create_reflog_msg(const struct object_id *oid, struct strbuf *sb)
 	}
 
 	strbuf_addstr(sb, " (");
-	type = oid_object_info(oid, NULL);
+	type = oid_object_info(the_repository, oid, NULL);
 	switch (type) {
 	default:
 		strbuf_addstr(sb, "object of unknown type");
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index b7755c6cc5..cfe9019f80 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -199,7 +199,7 @@ static int check_object(struct object *obj, int type, void *data, struct fsck_op
 
 	if (!(obj->flags & FLAG_OPEN)) {
 		unsigned long size;
-		int type = oid_object_info(&obj->oid, &size);
+		int type = oid_object_info(the_repository, &obj->oid, &size);
 		if (type != obj->type || type <= 0)
 			die("object of unexpected type");
 		obj->flags |= FLAG_WRITTEN;
diff --git a/cache.h b/cache.h
index 588c4fff9a..6340b2c572 100644
--- a/cache.h
+++ b/cache.h
@@ -1192,7 +1192,8 @@ static inline void *read_object_file(const struct object_id *oid, enum object_ty
 }
 
 /* Read and unpack an object file into memory, write memory to an object file */
-extern int oid_object_info(const struct object_id *, unsigned long *);
+#define oid_object_info(r, o, f) oid_object_info_##r(o, f)
+int oid_object_info_the_repository(const struct object_id *, unsigned long *);
 
 extern int hash_object_file(const void *buf, unsigned long len,
 			    const char *type, struct object_id *oid);
diff --git a/diff.c b/diff.c
index 1289df4b1f..4753170fe1 100644
--- a/diff.c
+++ b/diff.c
@@ -3638,7 +3638,8 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
 	else {
 		enum object_type type;
 		if (size_only || (flags & CHECK_BINARY)) {
-			type = oid_object_info(&s->oid, &s->size);
+			type = oid_object_info(the_repository, &s->oid,
+					       &s->size);
 			if (type < 0)
 				die("unable to read %s",
 				    oid_to_hex(&s->oid));
diff --git a/fast-import.c b/fast-import.c
index 99f8f56e8c..afe06bd7c1 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1917,7 +1917,8 @@ static void read_marks(void)
 			die("corrupt mark line: %s", line);
 		e = find_object(&oid);
 		if (!e) {
-			enum object_type type = oid_object_info(&oid, NULL);
+			enum object_type type = oid_object_info(the_repository,
+								&oid, NULL);
 			if (type < 0)
 				die("object not found: %s", oid_to_hex(&oid));
 			e = insert_object(&oid);
@@ -2447,7 +2448,8 @@ static void file_change_m(const char *p, struct branch *b)
 		enum object_type expected = S_ISDIR(mode) ?
 						OBJ_TREE: OBJ_BLOB;
 		enum object_type type = oe ? oe->type :
-					oid_object_info(&oid, NULL);
+					oid_object_info(the_repository, &oid,
+							NULL);
 		if (type < 0)
 			die("%s not found: %s",
 					S_ISDIR(mode) ?  "Tree" : "Blob",
@@ -2608,7 +2610,8 @@ static void note_change_n(const char *p, struct branch *b, unsigned char *old_fa
 			die("Not a blob (actually a %s): %s",
 				type_name(oe->type), command_buf.buf);
 	} else if (!is_null_oid(&oid)) {
-		enum object_type type = oid_object_info(&oid, NULL);
+		enum object_type type = oid_object_info(the_repository, &oid,
+							NULL);
 		if (type < 0)
 			die("Blob not found: %s", command_buf.buf);
 		if (type != OBJ_BLOB)
@@ -2895,7 +2898,7 @@ static void parse_new_tag(const char *arg)
 	} else if (!get_oid(from, &oid)) {
 		struct object_entry *oe = find_object(&oid);
 		if (!oe) {
-			type = oid_object_info(&oid, NULL);
+			type = oid_object_info(the_repository, &oid, NULL);
 			if (type < 0)
 				die("Not a valid object: %s", from);
 		} else
@@ -3053,7 +3056,8 @@ static struct object_entry *dereference(struct object_entry *oe,
 	unsigned long size;
 	char *buf = NULL;
 	if (!oe) {
-		enum object_type type = oid_object_info(oid, NULL);
+		enum object_type type = oid_object_info(the_repository, oid,
+							NULL);
 		if (type < 0)
 			die("object not found: %s", oid_to_hex(oid));
 		/* cache it! */
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 0ec83aaf18..ea94fe8af2 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -117,7 +117,7 @@ static enum list_objects_filter_result filter_blobs_limit(
 		assert(obj->type == OBJ_BLOB);
 		assert((obj->flags & SEEN) == 0);
 
-		t = oid_object_info(&obj->oid, &object_length);
+		t = oid_object_info(the_repository, &obj->oid, &object_length);
 		if (t != OBJ_BLOB) { /* probably OBJ_NONE */
 			/*
 			 * We DO NOT have the blob locally, so we cannot
diff --git a/object.c b/object.c
index 66cffaf6e5..5044d08e96 100644
--- a/object.c
+++ b/object.c
@@ -257,7 +257,7 @@ struct object *parse_object(const struct object_id *oid)
 
 	if ((obj && obj->type == OBJ_BLOB && has_object_file(oid)) ||
 	    (!obj && has_object_file(oid) &&
-	     oid_object_info(oid, NULL) == OBJ_BLOB)) {
+	     oid_object_info(the_repository, oid, NULL) == OBJ_BLOB)) {
 		if (check_object_signature(repl, NULL, 0, NULL) < 0) {
 			error("sha1 mismatch %s", oid_to_hex(oid));
 			return NULL;
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 41ae27fb19..cd1903e717 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -73,7 +73,8 @@ void bitmap_writer_build_type_index(struct pack_idx_entry **index,
 			break;
 
 		default:
-			real_type = oid_object_info(&entry->idx.oid, NULL);
+			real_type = oid_object_info(the_repository,
+						    &entry->idx.oid, NULL);
 			break;
 		}
 
diff --git a/packfile.c b/packfile.c
index d9914ba723..80c7fa734f 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1114,7 +1114,7 @@ static int retry_bad_packed_offset(struct packed_git *p, off_t obj_offset)
 		return OBJ_BAD;
 	nth_packed_object_oid(&oid, p, revidx->nr);
 	mark_bad_packed_object(p, oid.hash);
-	type = oid_object_info(&oid, NULL);
+	type = oid_object_info(the_repository, &oid, NULL);
 	if (type <= OBJ_NONE)
 		return OBJ_BAD;
 	return type;
diff --git a/reachable.c b/reachable.c
index a6ea33a5db..ffb976c33c 100644
--- a/reachable.c
+++ b/reachable.c
@@ -78,7 +78,7 @@ static void add_recent_object(const struct object_id *oid,
 	 * later processing, and the revision machinery expects
 	 * commits and tags to have been parsed.
 	 */
-	type = oid_object_info(oid, NULL);
+	type = oid_object_info(the_repository, oid, NULL);
 	if (type < 0)
 		die("unable to get object info for %s", oid_to_hex(oid));
 
diff --git a/refs.c b/refs.c
index 9b56fa9b81..27c88ba768 100644
--- a/refs.c
+++ b/refs.c
@@ -302,7 +302,7 @@ enum peel_status peel_object(const struct object_id *name, struct object_id *oid
 	struct object *o = lookup_unknown_object(name->hash);
 
 	if (o->type == OBJ_NONE) {
-		int type = oid_object_info(name, NULL);
+		int type = oid_object_info(the_repository, name, NULL);
 		if (type < 0 || !object_as_type(o, type, 0))
 			return PEEL_INVALID;
 	}
diff --git a/remote.c b/remote.c
index 91eb010ca9..481bf933f3 100644
--- a/remote.c
+++ b/remote.c
@@ -1376,7 +1376,7 @@ static void add_missing_tags(struct ref *src, struct ref **dst, struct ref ***ds
 			continue; /* not a tag */
 		if (string_list_has_string(&dst_tag, ref->name))
 			continue; /* they already have it */
-		if (oid_object_info(&ref->new_oid, NULL) != OBJ_TAG)
+		if (oid_object_info(the_repository, &ref->new_oid, NULL) != OBJ_TAG)
 			continue; /* be conservative */
 		item = string_list_append(&src_tag, ref->name);
 		item->util = ref;
diff --git a/sequencer.c b/sequencer.c
index 667f35ebdf..44f0518b9c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2876,7 +2876,8 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 
 		if (!get_oid(name, &oid)) {
 			if (!lookup_commit_reference_gently(&oid, 1)) {
-				enum object_type type = oid_object_info(&oid,
+				enum object_type type = oid_object_info(the_repository,
+									&oid,
 									NULL);
 				return error(_("%s: can't cherry-pick a %s"),
 					name, type_name(type));
diff --git a/sha1_file.c b/sha1_file.c
index 50a2dc5f0a..93f25c6c6a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1322,7 +1322,7 @@ int oid_object_info_extended_the_repository(const struct object_id *oid, struct
 }
 
 /* returns enum object_type or negative */
-int oid_object_info(const struct object_id *oid, unsigned long *sizep)
+int oid_object_info_the_repository(const struct object_id *oid, unsigned long *sizep)
 {
 	enum object_type type;
 	struct object_info oi = OBJECT_INFO_INIT;
@@ -1988,7 +1988,7 @@ int read_pack_header(int fd, struct pack_header *header)
 
 void assert_oid_type(const struct object_id *oid, enum object_type expect)
 {
-	enum object_type type = oid_object_info(oid, NULL);
+	enum object_type type = oid_object_info(the_repository, oid, NULL);
 	if (type < 0)
 		die("%s is not a valid object", oid_to_hex(oid));
 	if (type != expect)
diff --git a/sha1_name.c b/sha1_name.c
index 5b93bf8da3..b5406b6eb2 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -223,7 +223,7 @@ static int finish_object_disambiguation(struct disambiguate_state *ds,
 
 static int disambiguate_commit_only(const struct object_id *oid, void *cb_data_unused)
 {
-	int kind = oid_object_info(oid, NULL);
+	int kind = oid_object_info(the_repository, oid, NULL);
 	return kind == OBJ_COMMIT;
 }
 
@@ -232,7 +232,7 @@ static int disambiguate_committish_only(const struct object_id *oid, void *cb_da
 	struct object *obj;
 	int kind;
 
-	kind = oid_object_info(oid, NULL);
+	kind = oid_object_info(the_repository, oid, NULL);
 	if (kind == OBJ_COMMIT)
 		return 1;
 	if (kind != OBJ_TAG)
@@ -247,7 +247,7 @@ static int disambiguate_committish_only(const struct object_id *oid, void *cb_da
 
 static int disambiguate_tree_only(const struct object_id *oid, void *cb_data_unused)
 {
-	int kind = oid_object_info(oid, NULL);
+	int kind = oid_object_info(the_repository, oid, NULL);
 	return kind == OBJ_TREE;
 }
 
@@ -256,7 +256,7 @@ static int disambiguate_treeish_only(const struct object_id *oid, void *cb_data_
 	struct object *obj;
 	int kind;
 
-	kind = oid_object_info(oid, NULL);
+	kind = oid_object_info(the_repository, oid, NULL);
 	if (kind == OBJ_TREE || kind == OBJ_COMMIT)
 		return 1;
 	if (kind != OBJ_TAG)
@@ -271,7 +271,7 @@ static int disambiguate_treeish_only(const struct object_id *oid, void *cb_data_
 
 static int disambiguate_blob_only(const struct object_id *oid, void *cb_data_unused)
 {
-	int kind = oid_object_info(oid, NULL);
+	int kind = oid_object_info(the_repository, oid, NULL);
 	return kind == OBJ_BLOB;
 }
 
@@ -350,7 +350,7 @@ static int show_ambiguous_object(const struct object_id *oid, void *data)
 	if (ds->fn && !ds->fn(oid, ds->cb_data))
 		return 0;
 
-	type = oid_object_info(oid, NULL);
+	type = oid_object_info(the_repository, oid, NULL);
 	if (type == OBJ_COMMIT) {
 		struct commit *commit = lookup_commit(oid);
 		if (commit) {
diff --git a/submodule.c b/submodule.c
index 9a50168b23..bb133e9b93 100644
--- a/submodule.c
+++ b/submodule.c
@@ -818,7 +818,7 @@ static int check_has_commit(const struct object_id *oid, void *data)
 {
 	struct has_commit_data *cb = data;
 
-	enum object_type type = oid_object_info(oid, NULL);
+	enum object_type type = oid_object_info(the_repository, oid, NULL);
 
 	switch (type) {
 	case OBJ_COMMIT:
diff --git a/tag.c b/tag.c
index 86b1dcbb82..3d37c1bd25 100644
--- a/tag.c
+++ b/tag.c
@@ -41,7 +41,7 @@ int gpg_verify_tag(const struct object_id *oid, const char *name_to_report,
 	unsigned long size;
 	int ret;
 
-	type = oid_object_info(oid, NULL);
+	type = oid_object_info(the_repository, oid, NULL);
 	if (type != OBJ_TAG)
 		return error("%s: cannot verify a non-tag object of type %s.",
 				name_to_report ?
-- 
2.17.0.484.g0c8726318c-goog


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

* [PATCH 3/9] packfile: add repository argument to retry_bad_packed_offset
  2018-04-23 23:43 [PATCH 0/9] object store: oid_object_info is the next contender Stefan Beller
  2018-04-23 23:43 ` [PATCH 1/9] cache.h: add repository argument to oid_object_info_extended Stefan Beller
  2018-04-23 23:43 ` [PATCH 2/9] cache.h: add repository argument to oid_object_info Stefan Beller
@ 2018-04-23 23:43 ` Stefan Beller
  2018-04-23 23:43 ` [PATCH 4/9] packfile: add repository argument to packed_to_object_type Stefan Beller
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Stefan Beller @ 2018-04-23 23:43 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller, Jonathan Nieder

Add a repository argument to allow the callers of retry_bad_packed_offset
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 packfile.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/packfile.c b/packfile.c
index 80c7fa734f..d2b3f3503b 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1104,7 +1104,9 @@ static const unsigned char *get_delta_base_sha1(struct packed_git *p,
 		return NULL;
 }
 
-static int retry_bad_packed_offset(struct packed_git *p, off_t obj_offset)
+#define retry_bad_packed_offset(r, p, o) \
+	retry_bad_packed_offset_##r(p, o)
+static int retry_bad_packed_offset_the_repository(struct packed_git *p, off_t obj_offset)
 {
 	int type;
 	struct revindex_entry *revidx;
@@ -1153,7 +1155,7 @@ static enum object_type packed_to_object_type(struct packed_git *p,
 		if (type <= OBJ_NONE) {
 			/* If getting the base itself fails, we first
 			 * retry the base, otherwise unwind */
-			type = retry_bad_packed_offset(p, base_offset);
+			type = retry_bad_packed_offset(the_repository, p, base_offset);
 			if (type > OBJ_NONE)
 				goto out;
 			goto unwind;
@@ -1181,7 +1183,7 @@ static enum object_type packed_to_object_type(struct packed_git *p,
 unwind:
 	while (poi_stack_nr) {
 		obj_offset = poi_stack[--poi_stack_nr];
-		type = retry_bad_packed_offset(p, obj_offset);
+		type = retry_bad_packed_offset(the_repository, p, obj_offset);
 		if (type > OBJ_NONE)
 			goto out;
 	}
-- 
2.17.0.484.g0c8726318c-goog


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

* [PATCH 4/9] packfile: add repository argument to packed_to_object_type
  2018-04-23 23:43 [PATCH 0/9] object store: oid_object_info is the next contender Stefan Beller
                   ` (2 preceding siblings ...)
  2018-04-23 23:43 ` [PATCH 3/9] packfile: add repository argument to retry_bad_packed_offset Stefan Beller
@ 2018-04-23 23:43 ` Stefan Beller
  2018-04-23 23:43 ` [PATCH 5/9] packfile: add repository argument to packed_object_info Stefan Beller
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Stefan Beller @ 2018-04-23 23:43 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller, Jonathan Nieder

Add a repository argument to allow the callers of packed_to_object_type
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 packfile.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/packfile.c b/packfile.c
index d2b3f3503b..3ecfba66af 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1124,11 +1124,13 @@ static int retry_bad_packed_offset_the_repository(struct packed_git *p, off_t ob
 
 #define POI_STACK_PREALLOC 64
 
-static enum object_type packed_to_object_type(struct packed_git *p,
-					      off_t obj_offset,
-					      enum object_type type,
-					      struct pack_window **w_curs,
-					      off_t curpos)
+#define packed_to_object_type(r, p, o, t, w, c) \
+	packed_to_object_type_##r(p, o, t, w, c)
+static enum object_type packed_to_object_type_the_repository(struct packed_git *p,
+							     off_t obj_offset,
+							     enum object_type type,
+							     struct pack_window **w_curs,
+							     off_t curpos)
 {
 	off_t small_poi_stack[POI_STACK_PREALLOC];
 	off_t *poi_stack = small_poi_stack;
@@ -1378,8 +1380,8 @@ int packed_object_info(struct packed_git *p, off_t obj_offset,
 
 	if (oi->typep || oi->type_name) {
 		enum object_type ptot;
-		ptot = packed_to_object_type(p, obj_offset, type, &w_curs,
-					     curpos);
+		ptot = packed_to_object_type(the_repository, p, obj_offset,
+					     type, &w_curs, curpos);
 		if (oi->typep)
 			*oi->typep = ptot;
 		if (oi->type_name) {
-- 
2.17.0.484.g0c8726318c-goog


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

* [PATCH 5/9] packfile: add repository argument to packed_object_info
  2018-04-23 23:43 [PATCH 0/9] object store: oid_object_info is the next contender Stefan Beller
                   ` (3 preceding siblings ...)
  2018-04-23 23:43 ` [PATCH 4/9] packfile: add repository argument to packed_to_object_type Stefan Beller
@ 2018-04-23 23:43 ` Stefan Beller
  2018-04-24 18:16   ` Jonathan Tan
  2018-04-23 23:43 ` [PATCH 6/9] packfile: add repository argument to read_object Stefan Beller
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Stefan Beller @ 2018-04-23 23:43 UTC (permalink / raw)
  To: git; +Cc: Jonathan Nieder, Stefan Beller

From: Jonathan Nieder <jrnieder@gmail.com>

Add a repository argument to allow callers of packed_object_info to be
more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/pack-objects.c | 3 ++-
 packfile.c             | 4 ++--
 packfile.h             | 3 ++-
 sha1_file.c            | 3 ++-
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8d4111f748..d65eb4a947 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1572,7 +1572,8 @@ static void drop_reused_delta(struct object_entry *entry)
 
 	oi.sizep = &entry->size;
 	oi.typep = &entry->type;
-	if (packed_object_info(entry->in_pack, entry->in_pack_offset, &oi) < 0) {
+	if (packed_object_info(the_repository, entry->in_pack,
+			       entry->in_pack_offset, &oi) < 0) {
 		/*
 		 * We failed to get the info from this pack for some reason;
 		 * fall back to sha1_object_info, which may find another copy.
diff --git a/packfile.c b/packfile.c
index 3ecfba66af..5fa7d27d3b 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1333,8 +1333,8 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
 	hashmap_add(&delta_base_cache, ent);
 }
 
-int packed_object_info(struct packed_git *p, off_t obj_offset,
-		       struct object_info *oi)
+int packed_object_info_the_repository(struct packed_git *p, off_t obj_offset,
+				      struct object_info *oi)
 {
 	struct pack_window *w_curs = NULL;
 	unsigned long size;
diff --git a/packfile.h b/packfile.h
index a92c0b241c..bc8d840b1b 100644
--- a/packfile.h
+++ b/packfile.h
@@ -125,7 +125,8 @@ extern void release_pack_memory(size_t);
 /* global flag to enable extra checks when accessing packed objects */
 extern int do_check_packed_object_crc;
 
-extern int packed_object_info(struct packed_git *pack, off_t offset, struct object_info *);
+#define packed_object_info(r, p, o, oi) packed_object_info_##r(p, o, oi)
+extern int packed_object_info_the_repository(struct packed_git *pack, off_t offset, struct object_info *);
 
 extern void mark_bad_packed_object(struct packed_git *p, const unsigned char *sha1);
 extern const struct packed_git *has_packed_and_bad(const unsigned char *sha1);
diff --git a/sha1_file.c b/sha1_file.c
index 93f25c6c6a..b292e04fd3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1307,7 +1307,8 @@ int oid_object_info_extended_the_repository(const struct object_id *oid, struct
 		 * information below, so return early.
 		 */
 		return 0;
-	rtype = packed_object_info(e.p, e.offset, oi);
+
+	rtype = packed_object_info(the_repository, e.p, e.offset, oi);
 	if (rtype < 0) {
 		mark_bad_packed_object(e.p, real->hash);
 		return oid_object_info_extended(the_repository, real, oi, 0);
-- 
2.17.0.484.g0c8726318c-goog


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

* [PATCH 6/9] packfile: add repository argument to read_object
  2018-04-23 23:43 [PATCH 0/9] object store: oid_object_info is the next contender Stefan Beller
                   ` (4 preceding siblings ...)
  2018-04-23 23:43 ` [PATCH 5/9] packfile: add repository argument to packed_object_info Stefan Beller
@ 2018-04-23 23:43 ` Stefan Beller
  2018-04-23 23:43 ` [PATCH 7/9] packfile: add repository argument to unpack_entry Stefan Beller
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Stefan Beller @ 2018-04-23 23:43 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

Add a repository argument to allow the callers of read_object
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 packfile.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/packfile.c b/packfile.c
index 5fa7d27d3b..2876e04bb1 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1469,8 +1469,10 @@ struct unpack_entry_stack_ent {
 	unsigned long size;
 };
 
-static void *read_object(const struct object_id *oid, enum object_type *type,
-			 unsigned long *size)
+#define read_object(r, o, t, s) read_object_##r(o, t, s)
+static void *read_object_the_repository(const struct object_id *oid,
+					enum object_type *type,
+					unsigned long *size)
 {
 	struct object_info oi = OBJECT_INFO_INIT;
 	void *content;
@@ -1614,7 +1616,7 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
 				      oid_to_hex(&base_oid), (uintmax_t)obj_offset,
 				      p->pack_name);
 				mark_bad_packed_object(p, base_oid.hash);
-				base = read_object(&base_oid, &type, &base_size);
+				base = read_object(the_repository, &base_oid, &type, &base_size);
 				external_base = base;
 			}
 		}
-- 
2.17.0.484.g0c8726318c-goog


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

* [PATCH 7/9] packfile: add repository argument to unpack_entry
  2018-04-23 23:43 [PATCH 0/9] object store: oid_object_info is the next contender Stefan Beller
                   ` (5 preceding siblings ...)
  2018-04-23 23:43 ` [PATCH 6/9] packfile: add repository argument to read_object Stefan Beller
@ 2018-04-23 23:43 ` Stefan Beller
  2018-04-23 23:43 ` [PATCH 8/9] packfile: add repository argument to cache_or_unpack_entry Stefan Beller
  2018-04-23 23:43 ` [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories Stefan Beller
  8 siblings, 0 replies; 19+ messages in thread
From: Stefan Beller @ 2018-04-23 23:43 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller, Jonathan Nieder

Add a repository argument to allow the callers of unpack_entry
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 fast-import.c | 2 +-
 pack-check.c  | 3 ++-
 packfile.c    | 7 ++++---
 packfile.h    | 3 ++-
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index afe06bd7c1..b009353e93 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1376,7 +1376,7 @@ static void *gfi_unpack_entry(
 		 */
 		p->pack_size = pack_size + the_hash_algo->rawsz;
 	}
-	return unpack_entry(p, oe->idx.offset, &type, sizep);
+	return unpack_entry(the_repository, p, oe->idx.offset, &type, sizep);
 }
 
 static const char *get_mode(const char *str, uint16_t *modep)
diff --git a/pack-check.c b/pack-check.c
index 385d964bdd..d3a57df34f 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "repository.h"
 #include "pack.h"
 #include "pack-revindex.h"
 #include "progress.h"
@@ -134,7 +135,7 @@ static int verify_packfile(struct packed_git *p,
 			data = NULL;
 			data_valid = 0;
 		} else {
-			data = unpack_entry(p, entries[i].offset, &type, &size);
+			data = unpack_entry(the_repository, p, entries[i].offset, &type, &size);
 			data_valid = 1;
 		}
 
diff --git a/packfile.c b/packfile.c
index 2876e04bb1..d5ac48ef18 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1279,7 +1279,7 @@ static void *cache_or_unpack_entry(struct packed_git *p, off_t base_offset,
 
 	ent = get_delta_base_cache_entry(p, base_offset);
 	if (!ent)
-		return unpack_entry(p, base_offset, type, base_size);
+		return unpack_entry(the_repository, p, base_offset, type, base_size);
 
 	if (type)
 		*type = ent->type;
@@ -1485,8 +1485,9 @@ static void *read_object_the_repository(const struct object_id *oid,
 	return content;
 }
 
-void *unpack_entry(struct packed_git *p, off_t obj_offset,
-		   enum object_type *final_type, unsigned long *final_size)
+void *unpack_entry_the_repository(struct packed_git *p, off_t obj_offset,
+				  enum object_type *final_type,
+				  unsigned long *final_size)
 {
 	struct pack_window *w_curs = NULL;
 	off_t curpos = obj_offset;
diff --git a/packfile.h b/packfile.h
index bc8d840b1b..1efa57a90e 100644
--- a/packfile.h
+++ b/packfile.h
@@ -115,7 +115,8 @@ extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t n);
 extern off_t find_pack_entry_one(const unsigned char *sha1, struct packed_git *);
 
 extern int is_pack_valid(struct packed_git *);
-extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsigned long *);
+#define unpack_entry(r, p, of, ot, s) unpack_entry_##r(p, of, ot, s)
+extern void *unpack_entry_the_repository(struct packed_git *, off_t, enum object_type *, unsigned long *);
 extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
 extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
 extern int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
-- 
2.17.0.484.g0c8726318c-goog


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

* [PATCH 8/9] packfile: add repository argument to cache_or_unpack_entry
  2018-04-23 23:43 [PATCH 0/9] object store: oid_object_info is the next contender Stefan Beller
                   ` (6 preceding siblings ...)
  2018-04-23 23:43 ` [PATCH 7/9] packfile: add repository argument to unpack_entry Stefan Beller
@ 2018-04-23 23:43 ` Stefan Beller
  2018-04-23 23:43 ` [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories Stefan Beller
  8 siblings, 0 replies; 19+ messages in thread
From: Stefan Beller @ 2018-04-23 23:43 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller, Jonathan Nieder

Add a repository argument to allow the callers of cache_or_unpack_entry
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 packfile.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/packfile.c b/packfile.c
index d5ac48ef18..8de87f904b 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1272,7 +1272,8 @@ static void detach_delta_base_cache_entry(struct delta_base_cache_entry *ent)
 	free(ent);
 }
 
-static void *cache_or_unpack_entry(struct packed_git *p, off_t base_offset,
+#define cache_or_unpack_entry(r, p, bo, bs, t) cache_or_unpack_entry_##r(p, bo, bs, t)
+static void *cache_or_unpack_entry_the_repository(struct packed_git *p, off_t base_offset,
 	unsigned long *base_size, enum object_type *type)
 {
 	struct delta_base_cache_entry *ent;
@@ -1346,7 +1347,7 @@ int packed_object_info_the_repository(struct packed_git *p, off_t obj_offset,
 	 * a "real" type later if the caller is interested.
 	 */
 	if (oi->contentp) {
-		*oi->contentp = cache_or_unpack_entry(p, obj_offset, oi->sizep,
+		*oi->contentp = cache_or_unpack_entry(the_repository, p, obj_offset, oi->sizep,
 						      &type);
 		if (!*oi->contentp)
 			type = OBJ_BAD;
-- 
2.17.0.484.g0c8726318c-goog


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

* [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories
  2018-04-23 23:43 [PATCH 0/9] object store: oid_object_info is the next contender Stefan Beller
                   ` (7 preceding siblings ...)
  2018-04-23 23:43 ` [PATCH 8/9] packfile: add repository argument to cache_or_unpack_entry Stefan Beller
@ 2018-04-23 23:43 ` Stefan Beller
  2018-04-24  0:34   ` brian m. carlson
  2018-04-24 18:23   ` Jonathan Tan
  8 siblings, 2 replies; 19+ messages in thread
From: Stefan Beller @ 2018-04-23 23:43 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller, Jonathan Nieder

This involves also adapting sha1_object_info_extended and a some
internal functions that are used to implement these. It all has to
happen in one patch, because of a single recursive chain of calls visits
all these functions.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 cache.h     |  9 ++++-----
 packfile.c  | 58 ++++++++++++++++++++++++++---------------------------
 packfile.h  |  8 ++++----
 sha1_file.c | 25 +++++++++++++----------
 4 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/cache.h b/cache.h
index 6340b2c572..3a4d80e92b 100644
--- a/cache.h
+++ b/cache.h
@@ -1192,8 +1192,7 @@ static inline void *read_object_file(const struct object_id *oid, enum object_ty
 }
 
 /* Read and unpack an object file into memory, write memory to an object file */
-#define oid_object_info(r, o, f) oid_object_info_##r(o, f)
-int oid_object_info_the_repository(const struct object_id *, unsigned long *);
+int oid_object_info(struct repository *r, const struct object_id *, unsigned long *);
 
 extern int hash_object_file(const void *buf, unsigned long len,
 			    const char *type, struct object_id *oid);
@@ -1675,9 +1674,9 @@ struct object_info {
 /* Do not check loose object */
 #define OBJECT_INFO_IGNORE_LOOSE 16
 
-#define oid_object_info_extended(r, oid, oi, flags) \
-	oid_object_info_extended_##r(oid, oi, flags)
-int oid_object_info_extended_the_repository(const struct object_id *, struct object_info *, unsigned flags);
+int oid_object_info_extended(struct repository *r,
+			     const struct object_id *,
+			     struct object_info *, unsigned flags);
 
 /*
  * Set this to 0 to prevent sha1_object_info_extended() from fetching missing
diff --git a/packfile.c b/packfile.c
index 8de87f904b..55d383ed0a 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1104,9 +1104,9 @@ static const unsigned char *get_delta_base_sha1(struct packed_git *p,
 		return NULL;
 }
 
-#define retry_bad_packed_offset(r, p, o) \
-	retry_bad_packed_offset_##r(p, o)
-static int retry_bad_packed_offset_the_repository(struct packed_git *p, off_t obj_offset)
+static int retry_bad_packed_offset(struct repository *r,
+				   struct packed_git *p,
+				   off_t obj_offset)
 {
 	int type;
 	struct revindex_entry *revidx;
@@ -1116,7 +1116,7 @@ static int retry_bad_packed_offset_the_repository(struct packed_git *p, off_t ob
 		return OBJ_BAD;
 	nth_packed_object_oid(&oid, p, revidx->nr);
 	mark_bad_packed_object(p, oid.hash);
-	type = oid_object_info(the_repository, &oid, NULL);
+	type = oid_object_info(r, &oid, NULL);
 	if (type <= OBJ_NONE)
 		return OBJ_BAD;
 	return type;
@@ -1124,13 +1124,12 @@ static int retry_bad_packed_offset_the_repository(struct packed_git *p, off_t ob
 
 #define POI_STACK_PREALLOC 64
 
-#define packed_to_object_type(r, p, o, t, w, c) \
-	packed_to_object_type_##r(p, o, t, w, c)
-static enum object_type packed_to_object_type_the_repository(struct packed_git *p,
-							     off_t obj_offset,
-							     enum object_type type,
-							     struct pack_window **w_curs,
-							     off_t curpos)
+static enum object_type packed_to_object_type(struct repository *r,
+					      struct packed_git *p,
+					      off_t obj_offset,
+					      enum object_type type,
+					      struct pack_window **w_curs,
+					      off_t curpos)
 {
 	off_t small_poi_stack[POI_STACK_PREALLOC];
 	off_t *poi_stack = small_poi_stack;
@@ -1157,7 +1156,7 @@ static enum object_type packed_to_object_type_the_repository(struct packed_git *
 		if (type <= OBJ_NONE) {
 			/* If getting the base itself fails, we first
 			 * retry the base, otherwise unwind */
-			type = retry_bad_packed_offset(the_repository, p, base_offset);
+			type = retry_bad_packed_offset(r, p, base_offset);
 			if (type > OBJ_NONE)
 				goto out;
 			goto unwind;
@@ -1185,7 +1184,7 @@ static enum object_type packed_to_object_type_the_repository(struct packed_git *
 unwind:
 	while (poi_stack_nr) {
 		obj_offset = poi_stack[--poi_stack_nr];
-		type = retry_bad_packed_offset(the_repository, p, obj_offset);
+		type = retry_bad_packed_offset(r, p, obj_offset);
 		if (type > OBJ_NONE)
 			goto out;
 	}
@@ -1272,15 +1271,15 @@ static void detach_delta_base_cache_entry(struct delta_base_cache_entry *ent)
 	free(ent);
 }
 
-#define cache_or_unpack_entry(r, p, bo, bs, t) cache_or_unpack_entry_##r(p, bo, bs, t)
-static void *cache_or_unpack_entry_the_repository(struct packed_git *p, off_t base_offset,
-	unsigned long *base_size, enum object_type *type)
+static void *cache_or_unpack_entry(struct repository *r, struct packed_git *p,
+				   off_t base_offset, unsigned long *base_size,
+				   enum object_type *type)
 {
 	struct delta_base_cache_entry *ent;
 
 	ent = get_delta_base_cache_entry(p, base_offset);
 	if (!ent)
-		return unpack_entry(the_repository, p, base_offset, type, base_size);
+		return unpack_entry(r, p, base_offset, type, base_size);
 
 	if (type)
 		*type = ent->type;
@@ -1334,8 +1333,8 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
 	hashmap_add(&delta_base_cache, ent);
 }
 
-int packed_object_info_the_repository(struct packed_git *p, off_t obj_offset,
-				      struct object_info *oi)
+int packed_object_info(struct repository *r, struct packed_git *p,
+		       off_t obj_offset, struct object_info *oi)
 {
 	struct pack_window *w_curs = NULL;
 	unsigned long size;
@@ -1347,7 +1346,7 @@ int packed_object_info_the_repository(struct packed_git *p, off_t obj_offset,
 	 * a "real" type later if the caller is interested.
 	 */
 	if (oi->contentp) {
-		*oi->contentp = cache_or_unpack_entry(the_repository, p, obj_offset, oi->sizep,
+		*oi->contentp = cache_or_unpack_entry(r, p, obj_offset, oi->sizep,
 						      &type);
 		if (!*oi->contentp)
 			type = OBJ_BAD;
@@ -1381,7 +1380,7 @@ int packed_object_info_the_repository(struct packed_git *p, off_t obj_offset,
 
 	if (oi->typep || oi->type_name) {
 		enum object_type ptot;
-		ptot = packed_to_object_type(the_repository, p, obj_offset,
+		ptot = packed_to_object_type(r, p, obj_offset,
 					     type, &w_curs, curpos);
 		if (oi->typep)
 			*oi->typep = ptot;
@@ -1470,10 +1469,10 @@ struct unpack_entry_stack_ent {
 	unsigned long size;
 };
 
-#define read_object(r, o, t, s) read_object_##r(o, t, s)
-static void *read_object_the_repository(const struct object_id *oid,
-					enum object_type *type,
-					unsigned long *size)
+static void *read_object(struct repository *r,
+			 const struct object_id *oid,
+			 enum object_type *type,
+			 unsigned long *size)
 {
 	struct object_info oi = OBJECT_INFO_INIT;
 	void *content;
@@ -1481,14 +1480,13 @@ static void *read_object_the_repository(const struct object_id *oid,
 	oi.sizep = size;
 	oi.contentp = &content;
 
-	if (oid_object_info_extended(the_repository, oid, &oi, 0) < 0)
+	if (oid_object_info_extended(r, oid, &oi, 0) < 0)
 		return NULL;
 	return content;
 }
 
-void *unpack_entry_the_repository(struct packed_git *p, off_t obj_offset,
-				  enum object_type *final_type,
-				  unsigned long *final_size)
+void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
+		   enum object_type *final_type, unsigned long *final_size)
 {
 	struct pack_window *w_curs = NULL;
 	off_t curpos = obj_offset;
@@ -1618,7 +1616,7 @@ void *unpack_entry_the_repository(struct packed_git *p, off_t obj_offset,
 				      oid_to_hex(&base_oid), (uintmax_t)obj_offset,
 				      p->pack_name);
 				mark_bad_packed_object(p, base_oid.hash);
-				base = read_object(the_repository, &base_oid, &type, &base_size);
+				base = read_object(r, &base_oid, &type, &base_size);
 				external_base = base;
 			}
 		}
diff --git a/packfile.h b/packfile.h
index 1efa57a90e..fdfddb89b5 100644
--- a/packfile.h
+++ b/packfile.h
@@ -115,8 +115,7 @@ extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t n);
 extern off_t find_pack_entry_one(const unsigned char *sha1, struct packed_git *);
 
 extern int is_pack_valid(struct packed_git *);
-#define unpack_entry(r, p, of, ot, s) unpack_entry_##r(p, of, ot, s)
-extern void *unpack_entry_the_repository(struct packed_git *, off_t, enum object_type *, unsigned long *);
+extern void *unpack_entry(struct repository *r, struct packed_git *, off_t, enum object_type *, unsigned long *);
 extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
 extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
 extern int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);
@@ -126,8 +125,9 @@ extern void release_pack_memory(size_t);
 /* global flag to enable extra checks when accessing packed objects */
 extern int do_check_packed_object_crc;
 
-#define packed_object_info(r, p, o, oi) packed_object_info_##r(p, o, oi)
-extern int packed_object_info_the_repository(struct packed_git *pack, off_t offset, struct object_info *);
+extern int packed_object_info(struct repository *r,
+			      struct packed_git *pack,
+			      off_t offset, struct object_info *);
 
 extern void mark_bad_packed_object(struct packed_git *p, const unsigned char *sha1);
 extern const struct packed_git *has_packed_and_bad(const unsigned char *sha1);
diff --git a/sha1_file.c b/sha1_file.c
index b292e04fd3..9e0b81eadb 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1231,7 +1231,8 @@ static int sha1_loose_object_info(struct repository *r,
 
 int fetch_if_missing = 1;
 
-int oid_object_info_extended_the_repository(const struct object_id *oid, struct object_info *oi, unsigned flags)
+int oid_object_info_extended(struct repository *r, const struct object_id *oid,
+			     struct object_info *oi, unsigned flags)
 {
 	static struct object_info blank_oi = OBJECT_INFO_INIT;
 	struct pack_entry e;
@@ -1240,7 +1241,7 @@ int oid_object_info_extended_the_repository(const struct object_id *oid, struct
 	int already_retried = 0;
 
 	if (flags & OBJECT_INFO_LOOKUP_REPLACE)
-		real = lookup_replace_object(the_repository, oid);
+		real = lookup_replace_object(r, oid);
 
 	if (is_null_oid(real))
 		return -1;
@@ -1269,20 +1270,20 @@ int oid_object_info_extended_the_repository(const struct object_id *oid, struct
 	}
 
 	while (1) {
-		if (find_pack_entry(the_repository, real->hash, &e))
+		if (find_pack_entry(r, real->hash, &e))
 			break;
 
 		if (flags & OBJECT_INFO_IGNORE_LOOSE)
 			return -1;
 
 		/* Most likely it's a loose object. */
-		if (!sha1_loose_object_info(the_repository, real->hash, oi, flags))
+		if (!sha1_loose_object_info(r, real->hash, oi, flags))
 			return 0;
 
 		/* Not a loose object; someone else may have just packed it. */
 		if (!(flags & OBJECT_INFO_QUICK)) {
-			reprepare_packed_git(the_repository);
-			if (find_pack_entry(the_repository, real->hash, &e))
+			reprepare_packed_git(r);
+			if (find_pack_entry(r, real->hash, &e))
 				break;
 		}
 
@@ -1308,10 +1309,10 @@ int oid_object_info_extended_the_repository(const struct object_id *oid, struct
 		 */
 		return 0;
 
-	rtype = packed_object_info(the_repository, e.p, e.offset, oi);
+	rtype = packed_object_info(r, e.p, e.offset, oi);
 	if (rtype < 0) {
 		mark_bad_packed_object(e.p, real->hash);
-		return oid_object_info_extended(the_repository, real, oi, 0);
+		return oid_object_info_extended(r, real, oi, 0);
 	} else if (oi->whence == OI_PACKED) {
 		oi->u.packed.offset = e.offset;
 		oi->u.packed.pack = e.p;
@@ -1323,15 +1324,17 @@ int oid_object_info_extended_the_repository(const struct object_id *oid, struct
 }
 
 /* returns enum object_type or negative */
-int oid_object_info_the_repository(const struct object_id *oid, unsigned long *sizep)
+int oid_object_info(struct repository *r,
+		    const struct object_id *oid,
+		    unsigned long *sizep)
 {
 	enum object_type type;
 	struct object_info oi = OBJECT_INFO_INIT;
 
 	oi.typep = &type;
 	oi.sizep = sizep;
-	if (oid_object_info_extended(the_repository, oid, &oi,
-				     OBJECT_INFO_LOOKUP_REPLACE) < 0)
+	if (oid_object_info_extended(r, oid, &oi,
+				      OBJECT_INFO_LOOKUP_REPLACE) < 0)
 		return -1;
 	return type;
 }
-- 
2.17.0.484.g0c8726318c-goog


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

* Re: [PATCH 2/9] cache.h: add repository argument to oid_object_info
  2018-04-23 23:43 ` [PATCH 2/9] cache.h: add repository argument to oid_object_info Stefan Beller
@ 2018-04-24  0:31   ` brian m. carlson
  2018-04-24 18:15   ` Jonathan Tan
  1 sibling, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-04-24  0:31 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

On Mon, Apr 23, 2018 at 04:43:20PM -0700, Stefan Beller wrote:
> Add a repository argument to allow the callers of oid_object_info
> to be more specific about which repository to handle. This is a small
> mechanical change; it doesn't change the implementation to handle
> repositories other than the_repository yet.
> 
> In the expanded macro the identifier `the_repository` is not actually used,
> so the compiler does not catch if the repository.h header is not included
> at the call site. call sites needing that #include were identified by
> changing the macro to definition to
> 
>       #define sha1_object_info(r, sha1, size) \
>           (r, sha1_object_info_##r(sha1, size)).

I think you may have wanted to write "oid_object_info" here.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

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

* Re: [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories
  2018-04-23 23:43 ` [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories Stefan Beller
@ 2018-04-24  0:34   ` brian m. carlson
  2018-04-24  0:38     ` Stefan Beller
  2018-04-24 18:23   ` Jonathan Tan
  1 sibling, 1 reply; 19+ messages in thread
From: brian m. carlson @ 2018-04-24  0:34 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, Jonathan Nieder

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

On Mon, Apr 23, 2018 at 04:43:27PM -0700, Stefan Beller wrote:
> This involves also adapting sha1_object_info_extended and a some
> internal functions that are used to implement these. It all has to
> happen in one patch, because of a single recursive chain of calls visits
> all these functions.

Ah, yes, I remember that recursive call chain.

Anyway, other than the one item I mentioned earlier, this series looked
good to me.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

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

* Re: [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories
  2018-04-24  0:34   ` brian m. carlson
@ 2018-04-24  0:38     ` Stefan Beller
  0 siblings, 0 replies; 19+ messages in thread
From: Stefan Beller @ 2018-04-24  0:38 UTC (permalink / raw)
  To: brian m. carlson, Stefan Beller, git, Jonathan Nieder

On Mon, Apr 23, 2018 at 5:34 PM, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On Mon, Apr 23, 2018 at 04:43:27PM -0700, Stefan Beller wrote:
>> This involves also adapting sha1_object_info_extended and a some
>> internal functions that are used to implement these. It all has to
>> happen in one patch, because of a single recursive chain of calls visits
>> all these functions.
>
> Ah, yes, I remember that recursive call chain.
>
> Anyway, other than the one item I mentioned earlier, this series looked
> good to me.

Thanks for the quick review!

Well, this very paragraph that you quote also mentions
*sha1*_object_info_extended, so I'll fix that, too

I'll wait until tomorrow for a resend to give others
the opportunity to weigh in as well.

Thanks,
Stefan

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

* Re: [PATCH 2/9] cache.h: add repository argument to oid_object_info
  2018-04-23 23:43 ` [PATCH 2/9] cache.h: add repository argument to oid_object_info Stefan Beller
  2018-04-24  0:31   ` brian m. carlson
@ 2018-04-24 18:15   ` Jonathan Tan
  1 sibling, 0 replies; 19+ messages in thread
From: Jonathan Tan @ 2018-04-24 18:15 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git

On Mon, 23 Apr 2018 16:43:20 -0700
Stefan Beller <sbeller@google.com> wrote:

> Add a repository argument to allow the callers of oid_object_info
> to be more specific about which repository to handle. This is a small
> mechanical change; it doesn't change the implementation to handle
> repositories other than the_repository yet.

From here...

> In the expanded macro the identifier `the_repository` is not actually used,
> so the compiler does not catch if the repository.h header is not included
> at the call site. call sites needing that #include were identified by
> changing the macro to definition to
> 
>       #define sha1_object_info(r, sha1, size) \
>           (r, sha1_object_info_##r(sha1, size)).
> 
> This produces a compiler warning about the left hand side of the comma
> operator being unused, which can be suppressed using -Wno-unused-value.
> 
> To avoid breaking bisection, do not include this trick in the patch.

...until here: I don't think this explanation is necessary - this macro
trick is temporary anyway in all our patch sets. Also, wouldn't
repository.h be needed anyway if we reference "struct repository"?

I would replace this section with the "As with the previous commits"
explanation you have in PATCH 3/9 and others.

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

* Re: [PATCH 5/9] packfile: add repository argument to packed_object_info
  2018-04-23 23:43 ` [PATCH 5/9] packfile: add repository argument to packed_object_info Stefan Beller
@ 2018-04-24 18:16   ` Jonathan Tan
  2018-04-25  0:21     ` Junio C Hamano
  0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Tan @ 2018-04-24 18:16 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, Jonathan Nieder

On Mon, 23 Apr 2018 16:43:23 -0700
Stefan Beller <sbeller@google.com> wrote:

> diff --git a/sha1_file.c b/sha1_file.c
> index 93f25c6c6a..b292e04fd3 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1307,7 +1307,8 @@ int oid_object_info_extended_the_repository(const struct object_id *oid, struct
>  		 * information below, so return early.
>  		 */
>  		return 0;
> -	rtype = packed_object_info(e.p, e.offset, oi);
> +
> +	rtype = packed_object_info(the_repository, e.p, e.offset, oi);

Extra blank line introduced.

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

* Re: [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories
  2018-04-23 23:43 ` [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories Stefan Beller
  2018-04-24  0:34   ` brian m. carlson
@ 2018-04-24 18:23   ` Jonathan Tan
  2018-04-24 18:42     ` Brandon Williams
  1 sibling, 1 reply; 19+ messages in thread
From: Jonathan Tan @ 2018-04-24 18:23 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, Jonathan Nieder

On Mon, 23 Apr 2018 16:43:27 -0700
Stefan Beller <sbeller@google.com> wrote:

> This involves also adapting sha1_object_info_extended and a some
> internal functions that are used to implement these. It all has to
> happen in one patch, because of a single recursive chain of calls visits
> all these functions.

In packfile.c, unpack_entry() invokes get_delta_base_cache_entry(),
which references a global (delta_base_cache). Does delta_base_cache need
to be moved to the repo object (or object store object) first, or is
this safe?

Also, in sha1_file.c, oid_object_info_extended() invokes fetch_object(),
which attempts to fetch a missing object. For this, I think that it's
best to guard with a "r == the_repository" check, or if there's a better
way to distinguish between the "default" repository and any repository
that we newly create (I vaguely remember some distinction when parsing
environment variables when determining repo paths - the envvars were
only used for the "default" repository, but not for the others).

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

* Re: [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories
  2018-04-24 18:23   ` Jonathan Tan
@ 2018-04-24 18:42     ` Brandon Williams
  2018-04-24 18:55       ` Jonathan Tan
  0 siblings, 1 reply; 19+ messages in thread
From: Brandon Williams @ 2018-04-24 18:42 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: Stefan Beller, git, Jonathan Nieder

On 04/24, Jonathan Tan wrote:
> On Mon, 23 Apr 2018 16:43:27 -0700
> Stefan Beller <sbeller@google.com> wrote:
> 
> > This involves also adapting sha1_object_info_extended and a some
> > internal functions that are used to implement these. It all has to
> > happen in one patch, because of a single recursive chain of calls visits
> > all these functions.
> 
> In packfile.c, unpack_entry() invokes get_delta_base_cache_entry(),
> which references a global (delta_base_cache). Does delta_base_cache need
> to be moved to the repo object (or object store object) first, or is
> this safe?

After looking at this, I think it should be safe for now since its a
cache that requires a packed_git pointer to access (and those would be
per repository).  We may want to move it in to the repository at some
point though.

> 
> Also, in sha1_file.c, oid_object_info_extended() invokes fetch_object(),
> which attempts to fetch a missing object. For this, I think that it's
> best to guard with a "r == the_repository" check, or if there's a better
> way to distinguish between the "default" repository and any repository
> that we newly create (I vaguely remember some distinction when parsing
> environment variables when determining repo paths - the envvars were
> only used for the "default" repository, but not for the others).

This is a little more difficult and I'm not sure I know what the best
course of action would be for this.  Mostly because then this puts a big
recursive dependency on the whole fetch mechanism to handle arbitrary
repositories at the same time these functions are converted.  So maybe
throwing in the runtime check would be the best way to break the
dependencies for now.

-- 
Brandon Williams

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

* Re: [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories
  2018-04-24 18:42     ` Brandon Williams
@ 2018-04-24 18:55       ` Jonathan Tan
  0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Tan @ 2018-04-24 18:55 UTC (permalink / raw)
  To: Brandon Williams; +Cc: Stefan Beller, git, Jonathan Nieder

On Tue, 24 Apr 2018 11:42:33 -0700
Brandon Williams <bmwill@google.com> wrote:

> On 04/24, Jonathan Tan wrote:
> > On Mon, 23 Apr 2018 16:43:27 -0700
> > Stefan Beller <sbeller@google.com> wrote:
> > 
> > > This involves also adapting sha1_object_info_extended and a some
> > > internal functions that are used to implement these. It all has to
> > > happen in one patch, because of a single recursive chain of calls visits
> > > all these functions.
> > 
> > In packfile.c, unpack_entry() invokes get_delta_base_cache_entry(),
> > which references a global (delta_base_cache). Does delta_base_cache need
> > to be moved to the repo object (or object store object) first, or is
> > this safe?
> 
> After looking at this, I think it should be safe for now since its a
> cache that requires a packed_git pointer to access (and those would be
> per repository).  We may want to move it in to the repository at some
> point though.

Thanks for the pointer. I see that a packed_git pointer is part of the
key to that hashmap, so this is indeed safe. (Probably worth discussing
this in the commit message.)

There is another global, delta_base_cached, but it just limits the total
size of the hashmap, so it's safe to use that too.

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

* Re: [PATCH 5/9] packfile: add repository argument to packed_object_info
  2018-04-24 18:16   ` Jonathan Tan
@ 2018-04-25  0:21     ` Junio C Hamano
  0 siblings, 0 replies; 19+ messages in thread
From: Junio C Hamano @ 2018-04-25  0:21 UTC (permalink / raw)
  To: Jonathan Tan; +Cc: Stefan Beller, git, Jonathan Nieder

Jonathan Tan <jonathantanmy@google.com> writes:

> On Mon, 23 Apr 2018 16:43:23 -0700
> Stefan Beller <sbeller@google.com> wrote:
>
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 93f25c6c6a..b292e04fd3 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -1307,7 +1307,8 @@ int oid_object_info_extended_the_repository(const struct object_id *oid, struct
>>  		 * information below, so return early.
>>  		 */
>>  		return 0;
>> -	rtype = packed_object_info(e.p, e.offset, oi);
>> +
>> +	rtype = packed_object_info(the_repository, e.p, e.offset, oi);
>
> Extra blank line introduced.

Yes, but from the way this function is formatted, I think the
new paragraph break there makes sort of sense.

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

end of thread, other threads:[~2018-04-25  0:23 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-23 23:43 [PATCH 0/9] object store: oid_object_info is the next contender Stefan Beller
2018-04-23 23:43 ` [PATCH 1/9] cache.h: add repository argument to oid_object_info_extended Stefan Beller
2018-04-23 23:43 ` [PATCH 2/9] cache.h: add repository argument to oid_object_info Stefan Beller
2018-04-24  0:31   ` brian m. carlson
2018-04-24 18:15   ` Jonathan Tan
2018-04-23 23:43 ` [PATCH 3/9] packfile: add repository argument to retry_bad_packed_offset Stefan Beller
2018-04-23 23:43 ` [PATCH 4/9] packfile: add repository argument to packed_to_object_type Stefan Beller
2018-04-23 23:43 ` [PATCH 5/9] packfile: add repository argument to packed_object_info Stefan Beller
2018-04-24 18:16   ` Jonathan Tan
2018-04-25  0:21     ` Junio C Hamano
2018-04-23 23:43 ` [PATCH 6/9] packfile: add repository argument to read_object Stefan Beller
2018-04-23 23:43 ` [PATCH 7/9] packfile: add repository argument to unpack_entry Stefan Beller
2018-04-23 23:43 ` [PATCH 8/9] packfile: add repository argument to cache_or_unpack_entry Stefan Beller
2018-04-23 23:43 ` [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories Stefan Beller
2018-04-24  0:34   ` brian m. carlson
2018-04-24  0:38     ` Stefan Beller
2018-04-24 18:23   ` Jonathan Tan
2018-04-24 18:42     ` Brandon Williams
2018-04-24 18:55       ` Jonathan Tan

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).