From fea33a1ef37a5891ebd1fcf6018849150e7b91cb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 15 May 2011 12:54:50 -0700 Subject: [PATCH 1/5] Declare lookup_replace_object() in cache.h, not in commit.h The declaration is misplaced as the replace API is supposed to affect not just commits, but all types of objects. Signed-off-by: Junio C Hamano --- cache.h | 1 + commit.h | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/cache.h b/cache.h index 2b34116624..e09cf75013 100644 --- a/cache.h +++ b/cache.h @@ -763,6 +763,7 @@ static inline void *read_sha1_file(const unsigned char *sha1, enum object_type * { return read_sha1_file_repl(sha1, type, size, NULL); } +extern const unsigned char *lookup_replace_object(const unsigned char *sha1); extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1); extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1); extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *); diff --git a/commit.h b/commit.h index b3c3bb70c5..f251e75a5b 100644 --- a/commit.h +++ b/commit.h @@ -145,8 +145,6 @@ struct commit_graft *read_graft_line(char *buf, int len); int register_commit_graft(struct commit_graft *, int); struct commit_graft *lookup_commit_graft(const unsigned char *sha1); -const unsigned char *lookup_replace_object(const unsigned char *sha1); - extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2, int cleanup); extern struct commit_list *get_merge_bases_many(struct commit *one, int n, struct commit **twos, int cleanup); extern struct commit_list *get_octopus_merge_bases(struct commit_list *in); From abb25ac365791e16563dfd09e4ecd3e7e4dcf6b1 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 15 May 2011 12:54:51 -0700 Subject: [PATCH 2/5] t6050: make sure we test not just commit replacement The replacement mechanism should affect all types of objects not just commits, so make sure it deals with at least a blob. Signed-off-by: Junio C Hamano --- t/t6050-replace.sh | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index ae2194e07d..5c87f28e4e 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -236,6 +236,20 @@ test_expect_success 'index-pack and replacements' ' git index-pack test-*.pack ' -# -# +test_expect_success 'not just commits' ' + echo replaced >file && + git add file && + REPLACED=$(git rev-parse :file) && + mv file file.replaced && + + echo original >file && + git add file && + ORIGINAL=$(git rev-parse :file) && + git update-ref refs/replace/$ORIGINAL $REPLACED && + mv file file.original && + + git checkout file && + test_cmp file.replaced file +' + test_done From 4bbf5a2615420ac50c696b72dc303727e6218562 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 15 May 2011 12:54:52 -0700 Subject: [PATCH 3/5] read_sha1_file(): get rid of read_sha1_file_repl() madness Most callers want to silently get a replacement object, and they do not care what the real name of the replacement object is. Worse yet, no sane interface to return the underlying object without replacement is provided. Remove the function and make only the few callers that want the name of the replacement object find it themselves. Signed-off-by: Junio C Hamano --- builtin/mktag.c | 4 ++-- cache.h | 6 +----- object.c | 4 ++-- sha1_file.c | 12 ++++-------- 4 files changed, 9 insertions(+), 17 deletions(-) diff --git a/builtin/mktag.c b/builtin/mktag.c index 324a267163..640ab64f41 100644 --- a/builtin/mktag.c +++ b/builtin/mktag.c @@ -23,8 +23,8 @@ static int verify_object(const unsigned char *sha1, const char *expected_type) int ret = -1; enum object_type type; unsigned long size; - const unsigned char *repl; - void *buffer = read_sha1_file_repl(sha1, &type, &size, &repl); + void *buffer = read_sha1_file(sha1, &type, &size); + const unsigned char *repl = lookup_replace_object(sha1); if (buffer) { if (type == type_from_string(expected_type)) diff --git a/cache.h b/cache.h index e09cf75013..a9ae100542 100644 --- a/cache.h +++ b/cache.h @@ -758,11 +758,7 @@ int offset_1st_component(const char *path); /* Read and unpack a sha1 file into memory, write memory to a sha1 file */ extern int sha1_object_info(const unsigned char *, unsigned long *); -extern void *read_sha1_file_repl(const unsigned char *sha1, enum object_type *type, unsigned long *size, const unsigned char **replacement); -static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size) -{ - return read_sha1_file_repl(sha1, type, size, NULL); -} +extern void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size); extern const unsigned char *lookup_replace_object(const unsigned char *sha1); extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1); extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1); diff --git a/object.c b/object.c index 7e1f2bbed2..31976b5d70 100644 --- a/object.c +++ b/object.c @@ -188,8 +188,8 @@ struct object *parse_object(const unsigned char *sha1) unsigned long size; enum object_type type; int eaten; - const unsigned char *repl; - void *buffer = read_sha1_file_repl(sha1, &type, &size, &repl); + const unsigned char *repl = lookup_replace_object(sha1); + void *buffer = read_sha1_file(sha1, &type, &size); if (buffer) { struct object *obj; diff --git a/sha1_file.c b/sha1_file.c index 889fe71830..5d80febde2 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2206,10 +2206,9 @@ static void *read_object(const unsigned char *sha1, enum object_type *type, * deal with them should arrange to call read_object() and give error * messages themselves. */ -void *read_sha1_file_repl(const unsigned char *sha1, - enum object_type *type, - unsigned long *size, - const unsigned char **replacement) +void *read_sha1_file(const unsigned char *sha1, + enum object_type *type, + unsigned long *size) { const unsigned char *repl = lookup_replace_object(sha1); void *data; @@ -2218,11 +2217,8 @@ void *read_sha1_file_repl(const unsigned char *sha1, errno = 0; data = read_object(repl, type, size); - if (data) { - if (replacement) - *replacement = repl; + if (data) return data; - } if (errno && errno != ENOENT) die_errno("failed to read object %s", sha1_to_hex(sha1)); From e1111cef23cef1d48e9e7f222db87d58c1d51ece Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 15 May 2011 12:54:53 -0700 Subject: [PATCH 4/5] inline lookup_replace_object() calls In a repository without object replacement, lookup_replace_object() should be a no-op. Check the flag "read_replace_refs" on the side of the caller, and bypess a function call when we know we are not dealing with replacement. Also, even when we are set up to replace objects, if we do not find any replacement defined, flip that flag off to avoid function call overhead for all the later object accesses. As this change the semantics of the flag from "do we need read the replacement definition?" to "do we need to check with the lookup table?" the flag needs to be renamed later to something saner, e.g. "use_replace", when the codebase is calmer, but not now. Signed-off-by: Junio C Hamano --- cache.h | 12 ++++++++++-- environment.c | 2 +- replace_object.c | 4 +++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/cache.h b/cache.h index a9ae100542..c10a91d90a 100644 --- a/cache.h +++ b/cache.h @@ -756,10 +756,18 @@ char *strip_path_suffix(const char *path, const char *suffix); int daemon_avoid_alias(const char *path); int offset_1st_component(const char *path); +/* object replacement */ +extern void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size); +extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1); +static inline const unsigned char *lookup_replace_object(const unsigned char *sha1) +{ + if (!read_replace_refs) + return sha1; + return do_lookup_replace_object(sha1); +} + /* Read and unpack a sha1 file into memory, write memory to a sha1 file */ extern int sha1_object_info(const unsigned char *, unsigned long *); -extern void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size); -extern const unsigned char *lookup_replace_object(const unsigned char *sha1); extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1); extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1); extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *); diff --git a/environment.c b/environment.c index 40185bc854..91828201d8 100644 --- a/environment.c +++ b/environment.c @@ -42,7 +42,7 @@ const char *editor_program; const char *askpass_program; const char *excludes_file; enum auto_crlf auto_crlf = AUTO_CRLF_FALSE; -int read_replace_refs = 1; +int read_replace_refs = 1; /* NEEDSWORK: rename to use_replace_refs */ enum eol eol = EOL_UNSET; enum safe_crlf safe_crlf = SAFE_CRLF_WARN; unsigned whitespace_rule_cfg = WS_DEFAULT_RULE; diff --git a/replace_object.c b/replace_object.c index 7c6c7544ad..d0b1548726 100644 --- a/replace_object.c +++ b/replace_object.c @@ -85,12 +85,14 @@ static void prepare_replace_object(void) for_each_replace_ref(register_replace_ref, NULL); replace_object_prepared = 1; + if (!replace_object_nr) + read_replace_refs = 0; } /* We allow "recursive" replacement. Only within reason, though */ #define MAXREPLACEDEPTH 5 -const unsigned char *lookup_replace_object(const unsigned char *sha1) +const unsigned char *do_lookup_replace_object(const unsigned char *sha1) { int pos, depth = MAXREPLACEDEPTH; const unsigned char *cur = sha1; From 5bf29b950063c8fa2f3666cb6cf2ca20be61f3d1 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 15 May 2011 12:54:54 -0700 Subject: [PATCH 5/5] read_sha1_file(): allow selective bypassing of replacement mechanism The way "object replacement" mechanism was tucked to the read_sha1_file() interface was suboptimal in a couple of ways: - Callers that want it to die with useful diagnosis upon seeing a corrupt object does not have a way to say that they do not want any object replacement. - Callers who do not want it to die but want to handle the errors themselves are told to arrange to call read_object(), but the function does not use the replacement mechanism, and also it is a file scope static function that not many callers can call to begin with. This adds a read_sha1_file_extended() that takes a set of flags; the callers of read_sha1_file() passes a flag READ_SHA1_FILE_REPLACE to ask for object replacement mechanism to kick in. Later, we could add another flag bit to tell the function to return an error instead of dying and then remove the misguided "call read_object() yourself". Signed-off-by: Junio C Hamano --- cache.h | 7 ++++++- sha1_file.c | 10 ++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cache.h b/cache.h index c10a91d90a..5f1f5c3395 100644 --- a/cache.h +++ b/cache.h @@ -757,7 +757,12 @@ int daemon_avoid_alias(const char *path); int offset_1st_component(const char *path); /* object replacement */ -extern void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size); +#define READ_SHA1_FILE_REPLACE 1 +extern void *read_sha1_file_extended(const unsigned char *sha1, enum object_type *type, unsigned long *size, unsigned flag); +static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size) +{ + return read_sha1_file_extended(sha1, type, size, READ_SHA1_FILE_REPLACE); +} extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1); static inline const unsigned char *lookup_replace_object(const unsigned char *sha1) { diff --git a/sha1_file.c b/sha1_file.c index 5d80febde2..7e6e976c23 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2206,14 +2206,16 @@ static void *read_object(const unsigned char *sha1, enum object_type *type, * deal with them should arrange to call read_object() and give error * messages themselves. */ -void *read_sha1_file(const unsigned char *sha1, - enum object_type *type, - unsigned long *size) +void *read_sha1_file_extended(const unsigned char *sha1, + enum object_type *type, + unsigned long *size, + unsigned flag) { - const unsigned char *repl = lookup_replace_object(sha1); void *data; char *path; const struct packed_git *p; + const unsigned char *repl = (flag & READ_SHA1_FILE_REPLACE) + ? lookup_replace_object(sha1) : sha1; errno = 0; data = read_object(repl, type, size);