git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 3/4] sha1_file: consolidate storage-agnostic object fns

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Jun 13, 2017, 21:05 UTC
Message-ID
<33a75a60b1d4298ec0af21c0df19e12bb0e43e2d.1497387714.git.jonathantanmy@google.com>
In-Reply-To
<cover.1497387713.git.jonathantanmy@google.com>
In sha1_file.c, there are a few functions that provide information on an
object regardless of its storage (cached, loose, or packed). Looking
through all non-static functions in sha1_file.c that take in an unsigned
char * pointer, the relevant ones are:
 - sha1_object_info_extended
 - sha1_object_info (auto-fixed by sha1_object_info_extended)
 - read_sha1_file_extended (uses read_object)
 - read_object_with_reference (auto-fixed by read_sha1_file_extended)
 - has_sha1_file_with_flags
 - assert_sha1_type (auto-fixed by sha1_object_info)

Looking at the 3 primary functions (sha1_object_info_extended, read_object, has_sha1_file_with_flags), they independently implement mechanisms such as object replacement, retrying the packed store after failing to find the object in the packed store then the loose store, and being able to mark a packed object as bad and then retrying the whole process. Consolidating these mechanisms would be a great help to maintainability.

Therefore, consolidate all 3 functions by extending sha1_object_info_extended() to support the functionality needed by all 3 functions, and then modifying the other 2 to use sha1_object_info_extended().

Note that has_sha1_file_with_flags() does not try cached storage, whereas the other 2 functions do - this functionality is preserved.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 cache.h     |   7 +++
 sha1_file.c | 143 +++++++++++++++++++++++++++++++-----------------------------
 2 files changed, 81 insertions(+), 69 deletions(-)
diff --git a/cache.h b/cache.h
index 4d92aae0e..3c85867c3 100644
--- a/cache.h
+++ b/cache.h
@@ -1835,6 +1835,7 @@ struct object_info {
 	off_t *disk_sizep;
 	unsigned char *delta_base_sha1;
 	struct strbuf *typename;
+	void **contentp;
 
 	/* Response */
 	enum {
@@ -1866,6 +1867,12 @@ struct object_info {
  */
 #define OBJECT_INFO_INIT {NULL}
 
+/*
+ * sha1_object_info_extended() supports the LOOKUP_ flags and the OBJECT_INFO_
+ * flags.
+ */
+#define OBJECT_INFO_QUICK 4
+#define OBJECT_INFO_SKIP_CACHED 8
 extern int sha1_object_info_extended(const unsigned char *, struct object_info *, unsigned flags);
 extern int packed_object_info(struct packed_git *pack, off_t offset, struct object_info *);
 
diff --git a/sha1_file.c b/sha1_file.c
index a158907d1..98086e21e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2005,19 +2005,6 @@ int parse_sha1_header(const char *hdr, unsigned long *sizep)
 	return parse_sha1_header_extended(hdr, &oi, LOOKUP_REPLACE_OBJECT);
 }
 
-static void *unpack_sha1_file(void *map, unsigned long mapsize, enum object_type *type, unsigned long *size, const unsigned char *sha1)
-{
-	int ret;
-	git_zstream stream;
-	char hdr[8192];
-
-	ret = unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr));
-	if (ret < Z_OK || (*type = parse_sha1_header(hdr, size)) < 0)
-		return NULL;
-
-	return unpack_sha1_rest(&stream, hdr, *size, sha1);
-}
-
 unsigned long get_size_from_delta(struct packed_git *p,
 				  struct pack_window **w_curs,
 			          off_t curpos)
@@ -2326,8 +2313,10 @@ static void *cache_or_unpack_entry(struct packed_git *p, off_t base_offset,
 	if (!ent)
 		return unpack_entry(p, base_offset, type, base_size);
 
-	*type = ent->type;
-	*base_size = ent->size;
+	if (type)
+		*type = ent->type;
+	if (base_size)
+		*base_size = ent->size;
 	return xmemdupz(ent->data, ent->size);
 }
 
@@ -2388,9 +2377,16 @@ int packed_object_info(struct packed_git *p, off_t obj_offset,
 	 * We always get the representation type, but only convert it to
 	 * a "real" type later if the caller is interested.
 	 */
-	type = unpack_object_header(p, &w_curs, &curpos, &size);
+	if (oi->contentp) {
+		*oi->contentp = cache_or_unpack_entry(p, obj_offset, oi->sizep,
+						      &type);
+		if (!*oi->contentp)
+			type = OBJ_BAD;
+	} else {
+		type = unpack_object_header(p, &w_curs, &curpos, &size);
+	}
 
-	if (oi->sizep) {
+	if (!oi->contentp && oi->sizep) {
 		if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
 			off_t tmp_pos = curpos;
 			off_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,
@@ -2685,8 +2681,10 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
 		free(external_base);
 	}
 
-	*final_type = type;
-	*final_size = size;
+	if (final_type)
+		*final_type = type;
+	if (final_size)
+		*final_size = size;
 
 	unuse_pack(&w_curs);
 
@@ -2920,6 +2918,7 @@ static int sha1_loose_object_info(const unsigned char *sha1,
 	git_zstream stream;
 	char hdr[32];
 	struct strbuf hdrbuf = STRBUF_INIT;
+	unsigned long size_scratch;
 
 	if (oi->delta_base_sha1)
 		hashclr(oi->delta_base_sha1);
@@ -2932,7 +2931,7 @@ static int sha1_loose_object_info(const unsigned char *sha1,
 	 * return value implicitly indicates whether the
 	 * object even exists.
 	 */
-	if (!oi->typep && !oi->typename && !oi->sizep) {
+	if (!oi->typep && !oi->typename && !oi->sizep && !oi->contentp) {
 		const char *path;
 		struct stat st;
 		if (stat_sha1_file(sha1, &st, &path) < 0)
@@ -2945,6 +2944,10 @@ static int sha1_loose_object_info(const unsigned char *sha1,
 	map = map_sha1_file(sha1, &mapsize);
 	if (!map)
 		return -1;
+
+	if (!oi->sizep)
+		oi->sizep = &size_scratch;
+
 	if (oi->disk_sizep)
 		*oi->disk_sizep = mapsize;
 	if ((flags & LOOKUP_UNKNOWN_OBJECT)) {
@@ -2962,50 +2965,71 @@ static int sha1_loose_object_info(const unsigned char *sha1,
 				       sha1_to_hex(sha1));
 	} else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)
 		status = error("unable to parse %s header", sha1_to_hex(sha1));
-	git_inflate_end(&stream);
+
+	if (status >= 0 && oi->contentp)
+		*oi->contentp = unpack_sha1_rest(&stream, hdr,
+						 *oi->sizep, sha1);
+	else
+		git_inflate_end(&stream);
+
 	munmap(map, mapsize);
 	if (status && oi->typep)
 		*oi->typep = status;
+	if (oi->sizep == &size_scratch)
+		oi->sizep = NULL;
 	strbuf_release(&hdrbuf);
 	return (status < 0) ? status : 0;
 }
 
 int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi, unsigned flags)
 {
-	struct cached_object *co;
 	struct pack_entry e;
 	int rtype;
 	const unsigned char *real = lookup_replace_object_extended(sha1, flags);
 
-	co = find_cached_object(real);
-	if (co) {
-		if (oi->typep)
-			*(oi->typep) = co->type;
-		if (oi->sizep)
-			*(oi->sizep) = co->size;
-		if (oi->disk_sizep)
-			*(oi->disk_sizep) = 0;
-		if (oi->delta_base_sha1)
-			hashclr(oi->delta_base_sha1);
-		if (oi->typename)
-			strbuf_addstr(oi->typename, typename(co->type));
-		oi->whence = OI_CACHED;
-		return 0;
+	if (!(flags & OBJECT_INFO_SKIP_CACHED)) {
+		struct cached_object *co = find_cached_object(real);
+		if (co) {
+			if (!oi)
+				return 0;
+			if (oi->typep)
+				*(oi->typep) = co->type;
+			if (oi->sizep)
+				*(oi->sizep) = co->size;
+			if (oi->disk_sizep)
+				*(oi->disk_sizep) = 0;
+			if (oi->delta_base_sha1)
+				hashclr(oi->delta_base_sha1);
+			if (oi->typename)
+				strbuf_addstr(oi->typename, typename(co->type));
+			if (oi->contentp)
+				*oi->contentp = xmemdupz(co->buf, co->size);
+			oi->whence = OI_CACHED;
+			return 0;
+		}
 	}
 
 	if (!find_pack_entry(real, &e)) {
 		/* Most likely it's a loose object. */
-		if (!sha1_loose_object_info(real, oi, flags)) {
+		if (oi && !sha1_loose_object_info(real, oi, flags)) {
 			oi->whence = OI_LOOSE;
 			return 0;
 		}
+		if (!oi && has_loose_object(real))
+			return 0;
 
 		/* Not a loose object; someone else may have just packed it. */
-		reprepare_packed_git();
-		if (!find_pack_entry(real, &e))
+		if (flags & OBJECT_INFO_QUICK) {
 			return -1;
+		} else {
+			reprepare_packed_git();
+			if (!find_pack_entry(real, &e))
+				return -1;
+		}
 	}
 
+	if (!oi)
+		return 0;
 	rtype = packed_object_info(e.p, e.offset, oi);
 	if (rtype < 0) {
 		mark_bad_packed_object(e.p, real);
@@ -3081,28 +3105,15 @@ int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
 static void *read_object(const unsigned char *sha1, enum object_type *type,
 			 unsigned long *size)
 {
-	unsigned long mapsize;
-	void *map, *buf;
-	struct cached_object *co;
-
-	co = find_cached_object(sha1);
-	if (co) {
-		*type = co->type;
-		*size = co->size;
-		return xmemdupz(co->buf, co->size);
-	}
+	struct object_info oi = OBJECT_INFO_INIT;
+	void *content;
+	oi.typep = type;
+	oi.sizep = size;
+	oi.contentp = &content;
 
-	buf = read_packed_sha1(sha1, type, size);
-	if (buf)
-		return buf;
-	map = map_sha1_file(sha1, &mapsize);
-	if (map) {
-		buf = unpack_sha1_file(map, mapsize, type, size, sha1);
-		munmap(map, mapsize);
-		return buf;
-	}
-	reprepare_packed_git();
-	return read_packed_sha1(sha1, type, size);
+	if (sha1_object_info_extended(sha1, &oi, 0))
+		return NULL;
+	return content;
 }
 
 /*
@@ -3480,18 +3491,12 @@ int has_sha1_pack(const unsigned char *sha1)
 
 int has_sha1_file_with_flags(const unsigned char *sha1, int flags)
 {
-	struct pack_entry e;
+	int f = OBJECT_INFO_SKIP_CACHED |
+		((flags & HAS_SHA1_QUICK) ? OBJECT_INFO_QUICK : 0);
 
 	if (!startup_info->have_repository)
 		return 0;
-	if (find_pack_entry(sha1, &e))
-		return 1;
-	if (has_loose_object(sha1))
-		return 1;
-	if (flags & HAS_SHA1_QUICK)
-		return 0;
-	reprepare_packed_git();
-	return find_pack_entry(sha1, &e);
+	return !sha1_object_info_extended(sha1, NULL, f);
 }
 
 int has_object_file(const struct object_id *oid)

-- 
2.13.1.518.g3df882009-goog
Previous: Junio C HamanoNext: Junio C Hamano
Message 14 of 65 in “Improvements to sha1_file”
  1. 0/4 Improvements to sha1_fileJonathan Tan, Jun 9, 2017
  2. 1/4 sha1_file: teach packed_object_info about typenameJonathan Tan, Jun 9, 2017
  3. Junio C HamanoJun 12, 2017
  4. 2/4 sha1_file: extract type and size from object_infoJonathan Tan, Jun 9, 2017
  5. Jeff KingJun 10, 2017
  6. Jonathan TanJun 12, 2017
  7. Jeff KingJun 12, 2017
  8. 4/4 sha1_file, fsck: add missing blob supportJonathan Tan, Jun 9, 2017
  9. 3/4 sha1_file: consolidate storage-agnostic object fnsJonathan Tan, Jun 9, 2017
  10. 0/4 Improvements to sha1_fileJonathan Tan, Jun 13, 2017
  11. 1/4 sha1_file: teach packed_object_info about typenameJonathan Tan, Jun 13, 2017
  12. 2/4 sha1_file: move delta base cache code upJonathan Tan, Jun 13, 2017
  13. Junio C HamanoJun 15, 2017
  14. 3/4 sha1_file: consolidate storage-agnostic object fnsJonathan Tan, Jun 13, 2017
  15. Junio C HamanoJun 15, 2017
  16. Jonathan TanJun 15, 2017
  17. Jeff KingJun 17, 2017
  18. Junio C HamanoJun 19, 2017
  19. 4/4 sha1_file, fsck: add missing blob supportJonathan Tan, Jun 13, 2017
  20. Junio C HamanoJun 15, 2017
  21. Jonathan TanJun 15, 2017
  22. Junio C HamanoJun 15, 2017
  23. 0/8 Improvements to sha1_fileJonathan Tan, Jun 20, 2017
  24. 1/8 sha1_file: teach packed_object_info about typenameJonathan Tan, Jun 20, 2017
  25. 2/8 sha1_file: rename LOOKUP_UNKNOWN_OBJECTJonathan Tan, Jun 20, 2017
  26. Junio C HamanoJun 21, 2017
  27. Jonathan TanJun 21, 2017
  28. 3/8 sha1_file: rename LOOKUP_REPLACE_OBJECTJonathan Tan, Jun 20, 2017
  29. Junio C HamanoJun 21, 2017
  30. 4/8 sha1_file: move delta base cache code upJonathan Tan, Jun 20, 2017
  31. 5/8 sha1_file: refactor read_objectJonathan Tan, Jun 20, 2017
  32. Junio C HamanoJun 21, 2017
  33. 6/8 sha1_file: improve sha1_object_info_extendedJonathan Tan, Jun 20, 2017
  34. Jeff KingJun 24, 2017
  35. Jonathan TanJun 26, 2017
  36. Junio C HamanoJun 26, 2017
  37. Jonathan TanJun 26, 2017
  38. Junio C HamanoJun 26, 2017
  39. 8/8 sha1_file: refactor has_sha1_file_with_flagsJonathan Tan, Jun 20, 2017
  40. 7/8 sha1_file: do not access pack if unneededJonathan Tan, Jun 20, 2017
  41. Junio C HamanoJun 21, 2017
  42. Jeff KingJun 24, 2017
  43. Junio C HamanoJun 24, 2017
  44. Jeff KingJun 24, 2017
  45. Jonathan TanJun 26, 2017
  46. Junio C HamanoJun 21, 2017
  47. Jeff KingJun 24, 2017
  48. 0/8 Improvements to sha1_fileJonathan Tan, Jun 22, 2017
  49. 1/8 sha1_file: teach packed_object_info about typenameJonathan Tan, Jun 22, 2017
  50. 2/8 sha1_file: rename LOOKUP_UNKNOWN_OBJECTJonathan Tan, Jun 22, 2017
  51. 3/8 sha1_file: rename LOOKUP_REPLACE_OBJECTJonathan Tan, Jun 22, 2017
  52. 8/8 sha1_file: refactor has_sha1_file_with_flagsJonathan Tan, Jun 22, 2017
  53. Christian CouderJul 18, 2017
  54. Jonathan TanJul 18, 2017
  55. Johannes SchindelinJul 19, 2017
  56. sha1_file: use access(), not lstat(), if possibleJonathan Tan, Jul 19, 2017
  57. Junio C HamanoJul 20, 2017
  58. Johannes SchindelinJul 22, 2017
  59. Junio C HamanoJul 22, 2017
  60. Johannes SchindelinJul 25, 2017
  61. 6/8 sha1_file: improve sha1_object_info_extendedJonathan Tan, Jun 22, 2017
  62. 5/8 sha1_file: refactor read_objectJonathan Tan, Jun 22, 2017
  63. 4/8 sha1_file: move delta base cache code upJonathan Tan, Jun 22, 2017
  64. 7/8 sha1_file: do not access pack if unneededJonathan Tan, Jun 22, 2017
  65. Junio C HamanoJun 22, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.