{"thread":{"id":"27364","subject":"[PATCH 0/5] Updates to replace-object API","startedAt":"2011-05-15T19:54:49Z","lastAt":"2011-05-15T19:54:54Z","messageCount":6,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"167907","messageId":"1305489294-14341-1-git-send-email-gitster@pobox.com","threadId":"27364","inReplyTo":null,"subject":"[PATCH 0/5] Updates to replace-object API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-15T19:54:49Z","receivedAt":"2011-05-15T19:54:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"While I was looking at gaining a low-level access to read from the object\nstore, I found a handful poorly-designed part in the internal API to deal\nwith object replacement.\n\nJunio C Hamano (5):\n  Declare lookup_replace_object() in cache.h, not in commit.h\n  t6050: make sure we test not just commit replacement\n  read_sha1_file(): get rid of read_sha1_file_repl() madness\n  inline lookup_replace_object() calls\n  read_sha1_file(): allow selective bypassing of replacement mechanism\n\n builtin/mktag.c    |    4 ++--\n cache.h            |   18 ++++++++++++++----\n commit.h           |    2 --\n environment.c      |    2 +-\n object.c           |    4 ++--\n replace_object.c   |    4 +++-\n sha1_file.c        |   16 +++++++---------\n t/t6050-replace.sh |   18 ++++++++++++++++--\n 8 files changed, 45 insertions(+), 23 deletions(-)\n\n-- \n1.7.5.1.334.gdfd07\n"},{"id":"167908","messageId":"1305489294-14341-2-git-send-email-gitster@pobox.com","threadId":"27364","inReplyTo":"1305489294-14341-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/5] Declare lookup_replace_object() in cache.h, not in commit.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-15T19:54:50Z","receivedAt":"2011-05-15T19:54:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The declaration is misplaced as the replace API is supposed to affect\nnot just commits, but all types of objects.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n cache.h  |    1 +\n commit.h |    2 --\n 2 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 2b34116..e09cf75 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -763,6 +763,7 @@ static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *\n {\n \treturn read_sha1_file_repl(sha1, type, size, NULL);\n }\n+extern const unsigned char *lookup_replace_object(const unsigned char *sha1);\n extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1);\n extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1);\n extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);\ndiff --git a/commit.h b/commit.h\nindex b3c3bb7..f251e75 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -145,8 +145,6 @@ struct commit_graft *read_graft_line(char *buf, int len);\n int register_commit_graft(struct commit_graft *, int);\n struct commit_graft *lookup_commit_graft(const unsigned char *sha1);\n \n-const unsigned char *lookup_replace_object(const unsigned char *sha1);\n-\n extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2, int cleanup);\n extern struct commit_list *get_merge_bases_many(struct commit *one, int n, struct commit **twos, int cleanup);\n extern struct commit_list *get_octopus_merge_bases(struct commit_list *in);\n-- \n1.7.5.1.334.gdfd07\n"},{"id":"167912","messageId":"1305489294-14341-3-git-send-email-gitster@pobox.com","threadId":"27364","inReplyTo":"1305489294-14341-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/5] t6050: make sure we test not just commit replacement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-15T19:54:51Z","receivedAt":"2011-05-15T19:54:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The replacement mechanism should affect all types of objects not\njust commits, so make sure it deals with at least a blob.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t6050-replace.sh |   18 ++++++++++++++++--\n 1 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6050-replace.sh b/t/t6050-replace.sh\nindex ae2194e..5c87f28 100755\n--- a/t/t6050-replace.sh\n+++ b/t/t6050-replace.sh\n@@ -236,6 +236,20 @@ test_expect_success 'index-pack and replacements' '\n \tgit index-pack test-*.pack\n '\n \n-#\n-#\n+test_expect_success 'not just commits' '\n+\techo replaced >file &&\n+\tgit add file &&\n+\tREPLACED=$(git rev-parse :file) &&\n+\tmv file file.replaced &&\n+\n+\techo original >file &&\n+\tgit add file &&\n+\tORIGINAL=$(git rev-parse :file) &&\n+\tgit update-ref refs/replace/$ORIGINAL $REPLACED &&\n+\tmv file file.original &&\n+\n+\tgit checkout file &&\n+\ttest_cmp file.replaced file\n+'\n+\n test_done\n-- \n1.7.5.1.334.gdfd07\n"},{"id":"167910","messageId":"1305489294-14341-4-git-send-email-gitster@pobox.com","threadId":"27364","inReplyTo":"1305489294-14341-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/5] read_sha1_file(): get rid of read_sha1_file_repl() madness","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-15T19:54:52Z","receivedAt":"2011-05-15T19:54:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Most callers want to silently get a replacement object, and they do not\ncare what the real name of the replacement object is.  Worse yet, no sane\ninterface to return the underlying object without replacement is provided.\n\nRemove the function and make only the few callers that want the name of\nthe replacement object find it themselves.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/mktag.c |    4 ++--\n cache.h         |    6 +-----\n object.c        |    4 ++--\n sha1_file.c     |   12 ++++--------\n 4 files changed, 9 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/mktag.c b/builtin/mktag.c\nindex 324a267..640ab64 100644\n--- a/builtin/mktag.c\n+++ b/builtin/mktag.c\n@@ -23,8 +23,8 @@ static int verify_object(const unsigned char *sha1, const char *expected_type)\n \tint ret = -1;\n \tenum object_type type;\n \tunsigned long size;\n-\tconst unsigned char *repl;\n-\tvoid *buffer = read_sha1_file_repl(sha1, &type, &size, &repl);\n+\tvoid *buffer = read_sha1_file(sha1, &type, &size);\n+\tconst unsigned char *repl = lookup_replace_object(sha1);\n \n \tif (buffer) {\n \t\tif (type == type_from_string(expected_type))\ndiff --git a/cache.h b/cache.h\nindex e09cf75..a9ae100 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -758,11 +758,7 @@ int offset_1st_component(const char *path);\n \n /* Read and unpack a sha1 file into memory, write memory to a sha1 file */\n extern int sha1_object_info(const unsigned char *, unsigned long *);\n-extern void *read_sha1_file_repl(const unsigned char *sha1, enum object_type *type, unsigned long *size, const unsigned char **replacement);\n-static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)\n-{\n-\treturn read_sha1_file_repl(sha1, type, size, NULL);\n-}\n+extern void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size);\n extern const unsigned char *lookup_replace_object(const unsigned char *sha1);\n extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1);\n extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1);\ndiff --git a/object.c b/object.c\nindex 7e1f2bb..31976b5 100644\n--- a/object.c\n+++ b/object.c\n@@ -188,8 +188,8 @@ struct object *parse_object(const unsigned char *sha1)\n \tunsigned long size;\n \tenum object_type type;\n \tint eaten;\n-\tconst unsigned char *repl;\n-\tvoid *buffer = read_sha1_file_repl(sha1, &type, &size, &repl);\n+\tconst unsigned char *repl = lookup_replace_object(sha1);\n+\tvoid *buffer = read_sha1_file(sha1, &type, &size);\n \n \tif (buffer) {\n \t\tstruct object *obj;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 889fe71..5d80feb 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2206,10 +2206,9 @@ static void *read_object(const unsigned char *sha1, enum object_type *type,\n  * deal with them should arrange to call read_object() and give error\n  * messages themselves.\n  */\n-void *read_sha1_file_repl(const unsigned char *sha1,\n-\t\t\t  enum object_type *type,\n-\t\t\t  unsigned long *size,\n-\t\t\t  const unsigned char **replacement)\n+void *read_sha1_file(const unsigned char *sha1,\n+\t\t     enum object_type *type,\n+\t\t     unsigned long *size)\n {\n \tconst unsigned char *repl = lookup_replace_object(sha1);\n \tvoid *data;\n@@ -2218,11 +2217,8 @@ void *read_sha1_file_repl(const unsigned char *sha1,\n \n \terrno = 0;\n \tdata = read_object(repl, type, size);\n-\tif (data) {\n-\t\tif (replacement)\n-\t\t\t*replacement = repl;\n+\tif (data)\n \t\treturn data;\n-\t}\n \n \tif (errno && errno != ENOENT)\n \t\tdie_errno(\"failed to read object %s\", sha1_to_hex(sha1));\n-- \n1.7.5.1.334.gdfd07\n"},{"id":"167911","messageId":"1305489294-14341-5-git-send-email-gitster@pobox.com","threadId":"27364","inReplyTo":"1305489294-14341-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/5] inline lookup_replace_object() calls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-15T19:54:53Z","receivedAt":"2011-05-15T19:54:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In a repository without object replacement, lookup_replace_object() should\nbe a no-op. Check the flag \"read_replace_refs\" on the side of the caller,\nand bypess a function call when we know we are not dealing with replacement.\n\nAlso, even when we are set up to replace objects, if we do not find any\nreplacement defined, flip that flag off to avoid function call overhead\nfor all the later object accesses.\n\nAs this change the semantics of the flag from \"do we need read the\nreplacement definition?\" to \"do we need to check with the lookup table?\"\nthe flag needs to be renamed later to something saner, e.g. \"use_replace\",\nwhen the codebase is calmer, but not now.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n cache.h          |   12 ++++++++++--\n environment.c    |    2 +-\n replace_object.c |    4 +++-\n 3 files changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex a9ae100..c10a91d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -756,10 +756,18 @@ char *strip_path_suffix(const char *path, const char *suffix);\n int daemon_avoid_alias(const char *path);\n int offset_1st_component(const char *path);\n \n+/* object replacement */\n+extern void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size);\n+extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1);\n+static inline const unsigned char *lookup_replace_object(const unsigned char *sha1)\n+{\n+\tif (!read_replace_refs)\n+\t\treturn sha1;\n+\treturn do_lookup_replace_object(sha1);\n+}\n+\n /* Read and unpack a sha1 file into memory, write memory to a sha1 file */\n extern int sha1_object_info(const unsigned char *, unsigned long *);\n-extern void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size);\n-extern const unsigned char *lookup_replace_object(const unsigned char *sha1);\n extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1);\n extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1);\n extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);\ndiff --git a/environment.c b/environment.c\nindex 40185bc..9182820 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -42,7 +42,7 @@ const char *editor_program;\n const char *askpass_program;\n const char *excludes_file;\n enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;\n-int read_replace_refs = 1;\n+int read_replace_refs = 1; /* NEEDSWORK: rename to use_replace_refs */\n enum eol eol = EOL_UNSET;\n enum safe_crlf safe_crlf = SAFE_CRLF_WARN;\n unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\ndiff --git a/replace_object.c b/replace_object.c\nindex 7c6c754..d0b1548 100644\n--- a/replace_object.c\n+++ b/replace_object.c\n@@ -85,12 +85,14 @@ static void prepare_replace_object(void)\n \n \tfor_each_replace_ref(register_replace_ref, NULL);\n \treplace_object_prepared = 1;\n+\tif (!replace_object_nr)\n+\t\tread_replace_refs = 0;\n }\n \n /* We allow \"recursive\" replacement. Only within reason, though */\n #define MAXREPLACEDEPTH 5\n \n-const unsigned char *lookup_replace_object(const unsigned char *sha1)\n+const unsigned char *do_lookup_replace_object(const unsigned char *sha1)\n {\n \tint pos, depth = MAXREPLACEDEPTH;\n \tconst unsigned char *cur = sha1;\n-- \n1.7.5.1.334.gdfd07\n"},{"id":"167909","messageId":"1305489294-14341-6-git-send-email-gitster@pobox.com","threadId":"27364","inReplyTo":"1305489294-14341-1-git-send-email-gitster@pobox.com","subject":"[PATCH 5/5] read_sha1_file(): allow selective bypassing of replacement mechanism","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-15T19:54:54Z","receivedAt":"2011-05-15T19:54:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The way \"object replacement\" mechanism was tucked to the read_sha1_file()\ninterface was suboptimal in a couple of ways:\n\n - Callers that want it to die with useful diagnosis upon seeing a corrupt\n   object does not have a way to say that they do not want any object\n   replacement.\n\n - Callers who do not want it to die but want to handle the errors\n   themselves are told to arrange to call read_object(), but the function\n   does not use the replacement mechanism, and also it is a file scope\n   static function that not many callers can call to begin with.\n\nThis adds a read_sha1_file_extended() that takes a set of flags; the\ncallers of read_sha1_file() passes a flag READ_SHA1_FILE_REPLACE to ask\nfor object replacement mechanism to kick in.\n\nLater, we could add another flag bit to tell the function to return an\nerror instead of dying and then remove the misguided \"call read_object()\nyourself\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n cache.h     |    7 ++++++-\n sha1_file.c |   10 ++++++----\n 2 files changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex c10a91d..5f1f5c3 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -757,7 +757,12 @@ int daemon_avoid_alias(const char *path);\n int offset_1st_component(const char *path);\n \n /* object replacement */\n-extern void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size);\n+#define READ_SHA1_FILE_REPLACE 1\n+extern void *read_sha1_file_extended(const unsigned char *sha1, enum object_type *type, unsigned long *size, unsigned flag);\n+static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)\n+{\n+\treturn read_sha1_file_extended(sha1, type, size, READ_SHA1_FILE_REPLACE);\n+}\n extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1);\n static inline const unsigned char *lookup_replace_object(const unsigned char *sha1)\n {\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 5d80feb..7e6e976 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2206,14 +2206,16 @@ static void *read_object(const unsigned char *sha1, enum object_type *type,\n  * deal with them should arrange to call read_object() and give error\n  * messages themselves.\n  */\n-void *read_sha1_file(const unsigned char *sha1,\n-\t\t     enum object_type *type,\n-\t\t     unsigned long *size)\n+void *read_sha1_file_extended(const unsigned char *sha1,\n+\t\t\t      enum object_type *type,\n+\t\t\t      unsigned long *size,\n+\t\t\t      unsigned flag)\n {\n-\tconst unsigned char *repl = lookup_replace_object(sha1);\n \tvoid *data;\n \tchar *path;\n \tconst struct packed_git *p;\n+\tconst unsigned char *repl = (flag & READ_SHA1_FILE_REPLACE)\n+\t\t? lookup_replace_object(sha1) : sha1;\n \n \terrno = 0;\n \tdata = read_object(repl, type, size);\n-- \n1.7.5.1.334.gdfd07\n"}]}