From 89df9c84e478b7e6055a93cf45cf37027d25b3e4 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 14 Apr 2013 14:54:16 +0200 Subject: refs: document flags constants REF_* Document the bits that can appear in the "flags" parameter passed to an each_ref_function and/or in the ref_entry::flag field. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 6770e962a9..6b3511ee0b 100644 --- a/refs.c +++ b/refs.c @@ -157,7 +157,17 @@ struct ref_dir { struct ref_entry **entries; }; -/* ISSYMREF=0x01, ISPACKED=0x02, and ISBROKEN=0x04 are public interfaces */ +/* + * Bit values for ref_entry::flag. REF_ISSYMREF=0x01, + * REF_ISPACKED=0x02, and REF_ISBROKEN=0x04 are public values; see + * refs.h. + */ + +/* + * The field ref_entry->u.value.peeled of this value entry contains + * the correct peeled value for the reference, which might be + * null_sha1 if the reference is not a tag or if it is broken. + */ #define REF_KNOWS_PEELED 0x08 /* ref_entry represents a directory of references */ -- cgit v1.2.3 From 6c6f58dfd26c0643e38d7ea754b6d4173573a1f6 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 14 Apr 2013 14:54:17 +0200 Subject: refs: document the fields of struct ref_value Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 6b3511ee0b..ed1b4cf9b6 100644 --- a/refs.c +++ b/refs.c @@ -108,7 +108,19 @@ struct ref_entry; * (ref_entry->flag & REF_DIR) is zero. */ struct ref_value { + /* + * The name of the object to which this reference resolves + * (which may be a tag object). If REF_ISBROKEN, this is + * null. If REF_ISSYMREF, then this is the name of the object + * referred to by the last reference in the symlink chain. + */ unsigned char sha1[20]; + + /* + * If REF_KNOWS_PEELED, then this field holds the peeled value + * of this reference, or null if the reference is known not to + * be peelable. + */ unsigned char peeled[20]; }; -- cgit v1.2.3 From fcce17039cb94e6490a65a846b2375081785ce9b Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:11 +0200 Subject: refs: document do_for_each_ref() and do_one_ref() Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index ed1b4cf9b6..6c8fe18581 100644 --- a/refs.c +++ b/refs.c @@ -526,10 +526,14 @@ static void sort_ref_dir(struct ref_dir *dir) dir->sorted = dir->nr = i; } -#define DO_FOR_EACH_INCLUDE_BROKEN 01 +/* Include broken references in a do_for_each_ref*() iteration: */ +#define DO_FOR_EACH_INCLUDE_BROKEN 0x01 static struct ref_entry *current_ref; +/* + * Handle one reference in a do_for_each_ref*()-style iteration. + */ static int do_one_ref(const char *base, each_ref_fn fn, int trim, int flags, void *cb_data, struct ref_entry *entry) { @@ -1339,6 +1343,15 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname) for_each_rawref(warn_if_dangling_symref, &data); } +/* + * Call fn for each reference in the specified submodule for which the + * refname begins with base. If trim is non-zero, then trim that many + * characters off the beginning of each refname before passing the + * refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to include + * broken references in the iteration. If fn ever returns a non-zero + * value, stop the iteration and return that value; otherwise, return + * 0. + */ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn, int trim, int flags, void *cb_data) { -- cgit v1.2.3 From 7d76fdc8299639096ace153aef0f0b96dcc5b308 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:12 +0200 Subject: refs: document how current_ref is used Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 6c8fe18581..ccbd04a035 100644 --- a/refs.c +++ b/refs.c @@ -529,6 +529,15 @@ static void sort_ref_dir(struct ref_dir *dir) /* Include broken references in a do_for_each_ref*() iteration: */ #define DO_FOR_EACH_INCLUDE_BROKEN 0x01 +/* + * current_ref is a performance hack: when iterating over references + * using the for_each_ref*() functions, current_ref is set to the + * current reference's entry before calling the callback function. If + * the callback function calls peel_ref(), then peel_ref() first + * checks whether the reference to be peeled is the current reference + * (it usually is) and if so, returns that reference's peeled version + * if it is available. This avoids a refname lookup in a common case. + */ static struct ref_entry *current_ref; /* -- cgit v1.2.3 From 3feb4f0cfb7597a98d6e650abc8e9a1a9a0d65b5 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:13 +0200 Subject: refs: define constant PEELED_LINE_LENGTH Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index ccbd04a035..390251476c 100644 --- a/refs.c +++ b/refs.c @@ -806,6 +806,9 @@ void invalidate_ref_cache(const char *submodule) clear_loose_ref_cache(refs); } +/* The length of a peeled reference line in packed-refs, including EOL: */ +#define PEELED_LINE_LENGTH 42 + /* * Parse one line from a packed-refs file. Write the SHA1 to sha1. * Return a pointer to the refname within the line (null-terminated), @@ -898,8 +901,8 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir) } if (last && refline[0] == '^' && - strlen(refline) == 42 && - refline[41] == '\n' && + strlen(refline) == PEELED_LINE_LENGTH && + refline[PEELED_LINE_LENGTH - 1] == '\n' && !get_sha1_hex(refline + 1, sha1)) { hashcpy(last->u.value.peeled, sha1); /* -- cgit v1.2.3 From b830f6c66b2fd635d9cfa318d047917ea987a719 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:14 +0200 Subject: do_for_each_ref_in_dirs(): remove dead code There is no way to drop out of the while loop. This code has been dead since 432ad41e. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 390251476c..ea209e4907 100644 --- a/refs.c +++ b/refs.c @@ -667,13 +667,6 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1, if (retval) return retval; } - if (i1 < dir1->nr) - return do_for_each_ref_in_dir(dir1, i1, - base, fn, trim, flags, cb_data); - if (i2 < dir2->nr) - return do_for_each_ref_in_dir(dir2, i2, - base, fn, trim, flags, cb_data); - return 0; } /* -- cgit v1.2.3 From 63331581aba9537a83d2e9fe21ffab305d9f8ad6 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:15 +0200 Subject: get_packed_ref(): return a ref_entry Instead of copying the reference's SHA1 into a caller-supplied variable, just return the ref_entry itself (or NULL if there is no such entry). This change will allow the function to be used from elsewhere. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index ea209e4907..fbcc044eec 100644 --- a/refs.c +++ b/refs.c @@ -1101,18 +1101,12 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sh } /* - * Try to read ref from the packed references. On success, set sha1 - * and return 0; otherwise, return -1. + * Return the ref_entry for the given refname from the packed + * references. If it does not exist, return NULL. */ -static int get_packed_ref(const char *refname, unsigned char *sha1) +static struct ref_entry *get_packed_ref(const char *refname) { - struct ref_dir *packed = get_packed_refs(get_ref_cache(NULL)); - struct ref_entry *entry = find_ref(packed, refname); - if (entry) { - hashcpy(sha1, entry->u.value.sha1); - return 0; - } - return -1; + return find_ref(get_packed_refs(get_ref_cache(NULL)), refname); } const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int reading, int *flag) @@ -1140,13 +1134,17 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea git_snpath(path, sizeof(path), "%s", refname); if (lstat(path, &st) < 0) { + struct ref_entry *entry; + if (errno != ENOENT) return NULL; /* * The loose reference file does not exist; * check for a packed reference. */ - if (!get_packed_ref(refname, sha1)) { + entry = get_packed_ref(refname); + if (entry) { + hashcpy(sha1, entry->u.value.sha1); if (flag) *flag |= REF_ISPACKED; return refname; -- cgit v1.2.3 From f361baeb71fbe9e6fe02f53243673e194265c4cc Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:16 +0200 Subject: peel_ref(): use function get_packed_ref() Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index fbcc044eec..d2458275a9 100644 --- a/refs.c +++ b/refs.c @@ -1283,10 +1283,9 @@ int peel_ref(const char *refname, unsigned char *sha1) return -1; if ((flag & REF_ISPACKED)) { - struct ref_dir *dir = get_packed_refs(get_ref_cache(NULL)); - struct ref_entry *r = find_ref(dir, refname); + struct ref_entry *r = get_packed_ref(refname); - if (r != NULL && r->flag & REF_KNOWS_PEELED) { + if (r && (r->flag & REF_KNOWS_PEELED)) { hashcpy(sha1, r->u.value.peeled); return 0; } -- cgit v1.2.3 From 7618fd808aab2b7232abea04f1e7d8aa0ca2a476 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:17 +0200 Subject: repack_without_ref(): use function get_packed_ref() Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index d2458275a9..03c19be2c1 100644 --- a/refs.c +++ b/refs.c @@ -1821,9 +1821,11 @@ static int repack_without_ref(const char *refname) { struct repack_without_ref_sb data; struct ref_cache *refs = get_ref_cache(NULL); - struct ref_dir *packed = get_packed_refs(refs); - if (find_ref(packed, refname) == NULL) - return 0; + struct ref_dir *packed; + + if (!get_packed_ref(refname)) + return 0; /* refname does not exist in packed refs */ + data.refname = refname; data.fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0); if (data.fd < 0) { -- cgit v1.2.3 From 662428f4e98a740800659081910ef8b1ef3940fa Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:18 +0200 Subject: refs: extract a function ref_resolves_to_object() It is a nice unit of work and soon will be needed from multiple locations. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 03c19be2c1..5d760b4f05 100644 --- a/refs.c +++ b/refs.c @@ -529,6 +529,22 @@ static void sort_ref_dir(struct ref_dir *dir) /* Include broken references in a do_for_each_ref*() iteration: */ #define DO_FOR_EACH_INCLUDE_BROKEN 0x01 +/* + * Return true iff the reference described by entry can be resolved to + * an object in the database. Emit a warning if the referred-to + * object does not exist. + */ +static int ref_resolves_to_object(struct ref_entry *entry) +{ + if (entry->flag & REF_ISBROKEN) + return 0; + if (!has_sha1_file(entry->u.value.sha1)) { + error("%s does not point to a valid object!", entry->name); + return 0; + } + return 1; +} + /* * current_ref is a performance hack: when iterating over references * using the for_each_ref*() functions, current_ref is set to the @@ -550,14 +566,10 @@ static int do_one_ref(const char *base, each_ref_fn fn, int trim, if (prefixcmp(entry->name, base)) return 0; - if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) { - if (entry->flag & REF_ISBROKEN) - return 0; /* ignore broken refs e.g. dangling symref */ - if (!has_sha1_file(entry->u.value.sha1)) { - error("%s does not point to a valid object!", entry->name); - return 0; - } - } + if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN) && + !ref_resolves_to_object(entry)) + return 0; + current_ref = entry; retval = fn(entry->name + trim, entry->u.value.sha1, entry->flag, cb_data); current_ref = NULL; -- cgit v1.2.3 From cb2ae1c418eae2cf2c632ca78a14861ff5f3da68 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:19 +0200 Subject: refs: extract function peel_object() It is a nice, logical unit of work, and putting it in a function removes the need to use a goto in peel_ref(). Soon it will also have other uses. The algorithm is unchanged. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 50 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 20 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 5d760b4f05..9cd5582d38 100644 --- a/refs.c +++ b/refs.c @@ -1273,11 +1273,38 @@ static int filter_refs(const char *refname, const unsigned char *sha1, int flags return filter->fn(refname, sha1, flags, filter->cb_data); } +/* + * Peel the named object; i.e., if the object is a tag, resolve the + * tag recursively until a non-tag is found. Store the result to sha1 + * and return 0 iff successful. If the object is not a tag or is not + * valid, return -1 and leave sha1 unchanged. + */ +static int peel_object(const unsigned char *name, unsigned char *sha1) +{ + struct object *o = lookup_unknown_object(name); + + if (o->type == OBJ_NONE) { + int type = sha1_object_info(name, NULL); + if (type < 0) + return -1; + o->type = type; + } + + if (o->type != OBJ_TAG) + return -1; + + o = deref_tag_noverify(o); + if (!o) + return -1; + + hashcpy(sha1, o->sha1); + return 0; +} + int peel_ref(const char *refname, unsigned char *sha1) { int flag; unsigned char base[20]; - struct object *o; if (current_ref && (current_ref->name == refname || !strcmp(current_ref->name, refname))) { @@ -1287,8 +1314,7 @@ int peel_ref(const char *refname, unsigned char *sha1) hashcpy(sha1, current_ref->u.value.peeled); return 0; } - hashcpy(base, current_ref->u.value.sha1); - goto fallback; + return peel_object(current_ref->u.value.sha1, sha1); } if (read_ref_full(refname, base, 1, &flag)) @@ -1303,23 +1329,7 @@ int peel_ref(const char *refname, unsigned char *sha1) } } -fallback: - o = lookup_unknown_object(base); - if (o->type == OBJ_NONE) { - int type = sha1_object_info(base, NULL); - if (type < 0) - return -1; - o->type = type; - } - - if (o->type == OBJ_TAG) { - o = deref_tag_noverify(o); - if (o) { - hashcpy(sha1, o->sha1); - return 0; - } - } - return -1; + return peel_object(base, sha1); } struct warn_if_dangling_data { -- cgit v1.2.3 From 68cf87034402500f0c5dfb8d618b98b4e09895a7 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:20 +0200 Subject: peel_object(): give more specific information in return value Instead of just returning a success/failure bit, return an enumeration value that explains the reason for any failure. This will come in handy shortly. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 9cd5582d38..7405d1c962 100644 --- a/refs.c +++ b/refs.c @@ -1273,32 +1273,48 @@ static int filter_refs(const char *refname, const unsigned char *sha1, int flags return filter->fn(refname, sha1, flags, filter->cb_data); } +enum peel_status { + /* object was peeled successfully: */ + PEEL_PEELED = 0, + + /* + * object cannot be peeled because the named object (or an + * object referred to by a tag in the peel chain), does not + * exist. + */ + PEEL_INVALID = -1, + + /* object cannot be peeled because it is not a tag: */ + PEEL_NON_TAG = -2 +}; + /* * Peel the named object; i.e., if the object is a tag, resolve the - * tag recursively until a non-tag is found. Store the result to sha1 - * and return 0 iff successful. If the object is not a tag or is not - * valid, return -1 and leave sha1 unchanged. + * tag recursively until a non-tag is found. If successful, store the + * result to sha1 and return PEEL_PEELED. If the object is not a tag + * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively, + * and leave sha1 unchanged. */ -static int peel_object(const unsigned char *name, unsigned char *sha1) +static enum peel_status peel_object(const unsigned char *name, unsigned char *sha1) { struct object *o = lookup_unknown_object(name); if (o->type == OBJ_NONE) { int type = sha1_object_info(name, NULL); if (type < 0) - return -1; + return PEEL_INVALID; o->type = type; } if (o->type != OBJ_TAG) - return -1; + return PEEL_NON_TAG; o = deref_tag_noverify(o); if (!o) - return -1; + return PEEL_INVALID; hashcpy(sha1, o->sha1); - return 0; + return PEEL_PEELED; } int peel_ref(const char *refname, unsigned char *sha1) -- cgit v1.2.3 From 2312a7932080f17c2847ec3ce5dddbc65c2e0b41 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:21 +0200 Subject: peel_ref(): fix return value for non-peelable, not-current reference The old version was inconsistent: when a reference was REF_KNOWS_PEELED but with a null peeled value, it returned non-zero for the current reference but zero for other references. Change the behavior for non-current references to match that of current_ref, which is what callers expect. Document the behavior. Current callers only call peel_ref() from within a for_each_ref-style iteration and only for the current ref; therefore, the buggy code path was never reached. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 7405d1c962..787db21dfc 100644 --- a/refs.c +++ b/refs.c @@ -119,7 +119,8 @@ struct ref_value { /* * If REF_KNOWS_PEELED, then this field holds the peeled value * of this reference, or null if the reference is known not to - * be peelable. + * be peelable. See the documentation for peel_ref() for an + * exact definition of "peelable". */ unsigned char peeled[20]; }; @@ -1340,6 +1341,8 @@ int peel_ref(const char *refname, unsigned char *sha1) struct ref_entry *r = get_packed_ref(refname); if (r && (r->flag & REF_KNOWS_PEELED)) { + if (is_null_sha1(r->u.value.peeled)) + return -1; hashcpy(sha1, r->u.value.peeled); return 0; } -- cgit v1.2.3 From 9a489f3c17d6c974b18c47cf406404ca2a721c87 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:22 +0200 Subject: refs: extract a function peel_entry() Peel the entry, and as a side effect store the peeled value in the entry. Use this function from two places in peel_ref(); a third caller will be added soon. Please note that this change can lead to ref_entries for unpacked refs being peeled. This has no practical benefit but is harmless. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 14 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 787db21dfc..689b55d061 100644 --- a/refs.c +++ b/refs.c @@ -1286,7 +1286,17 @@ enum peel_status { PEEL_INVALID = -1, /* object cannot be peeled because it is not a tag: */ - PEEL_NON_TAG = -2 + PEEL_NON_TAG = -2, + + /* ref_entry contains no peeled value because it is a symref: */ + PEEL_IS_SYMREF = -3, + + /* + * ref_entry cannot be peeled because it is broken (i.e., the + * symbolic reference cannot even be resolved to an object + * name): + */ + PEEL_BROKEN = -4 }; /* @@ -1318,31 +1328,56 @@ static enum peel_status peel_object(const unsigned char *name, unsigned char *sh return PEEL_PEELED; } +/* + * Peel the entry (if possible) and return its new peel_status. + */ +static enum peel_status peel_entry(struct ref_entry *entry) +{ + enum peel_status status; + + if (entry->flag & REF_KNOWS_PEELED) + return is_null_sha1(entry->u.value.peeled) ? + PEEL_NON_TAG : PEEL_PEELED; + if (entry->flag & REF_ISBROKEN) + return PEEL_BROKEN; + if (entry->flag & REF_ISSYMREF) + return PEEL_IS_SYMREF; + + status = peel_object(entry->u.value.sha1, entry->u.value.peeled); + if (status == PEEL_PEELED || status == PEEL_NON_TAG) + entry->flag |= REF_KNOWS_PEELED; + return status; +} + int peel_ref(const char *refname, unsigned char *sha1) { int flag; unsigned char base[20]; if (current_ref && (current_ref->name == refname - || !strcmp(current_ref->name, refname))) { - if (current_ref->flag & REF_KNOWS_PEELED) { - if (is_null_sha1(current_ref->u.value.peeled)) - return -1; - hashcpy(sha1, current_ref->u.value.peeled); - return 0; - } - return peel_object(current_ref->u.value.sha1, sha1); + || !strcmp(current_ref->name, refname))) { + if (peel_entry(current_ref)) + return -1; + hashcpy(sha1, current_ref->u.value.peeled); + return 0; } if (read_ref_full(refname, base, 1, &flag)) return -1; - if ((flag & REF_ISPACKED)) { + /* + * If the reference is packed, read its ref_entry from the + * cache in the hope that we already know its peeled value. + * We only try this optimization on packed references because + * (a) forcing the filling of the loose reference cache could + * be expensive and (b) loose references anyway usually do not + * have REF_KNOWS_PEELED. + */ + if (flag & REF_ISPACKED) { struct ref_entry *r = get_packed_ref(refname); - - if (r && (r->flag & REF_KNOWS_PEELED)) { - if (is_null_sha1(r->u.value.peeled)) - return -1; + if (r) { + if (peel_entry(r)) + return -1; hashcpy(sha1, r->u.value.peeled); return 0; } -- cgit v1.2.3 From 624cac351478be90bf02a753f9de3e7dc80ca300 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:23 +0200 Subject: refs: change the internal reference-iteration API Establish an internal API for iterating over references, which gives the callback functions direct access to the ref_entry structure describing the reference. (Do not change the iteration API that is exposed outside of the module.) Define a new internal callback signature int each_ref_entry_fn(struct ref_entry *entry, void *cb_data) Change do_for_each_ref_in_dir() and do_for_each_ref_in_dirs() to accept each_ref_entry_fn callbacks, and rename them to do_for_each_entry_in_dir() and do_for_each_entry_in_dirs(), respectively. Adapt their callers accordingly. Add a new function do_for_each_entry() analogous to do_for_each_ref() but using the new callback style. Change do_one_ref() into an each_ref_entry_fn that does some bookkeeping and then calls a wrapped each_ref_fn. Reimplement do_for_each_ref() in terms of do_for_each_entry(), using do_one_ref() as an adapter. Please note that the responsibility for setting current_ref remains in do_one_ref(), which means that current_ref is *not* set when iterating over references via the new internal API. This is not a disadvantage, because current_ref is not needed by callers of the internal API (they receive a pointer to the current ref_entry anyway). But more importantly, this change prevents peel_ref() from returning invalid results in the following scenario: When iterating via the external API, the iteration always includes both packed and loose references, and in particular never presents a packed ref if there is a loose ref with the same name. The internal API, on the other hand, gives the option to iterate over only the packed references. During such an iteration, there is no check whether the packed ref might be hidden by a loose ref of the same name. But until now the packed ref was recorded in current_ref during the iteration. So if peel_ref() were called with the reference name corresponding to current ref, it would return the peeled version of the packed ref even though there might be a loose ref that peels to a different value. This scenario doesn't currently occur in the code, but fix it to prevent things from breaking in a very confusing way in the future. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 144 +++++++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 83 insertions(+), 61 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 689b55d061..d884f7d78f 100644 --- a/refs.c +++ b/refs.c @@ -557,22 +557,34 @@ static int ref_resolves_to_object(struct ref_entry *entry) */ static struct ref_entry *current_ref; +typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data); + +struct ref_entry_cb { + const char *base; + int trim; + int flags; + each_ref_fn *fn; + void *cb_data; +}; + /* - * Handle one reference in a do_for_each_ref*()-style iteration. + * Handle one reference in a do_for_each_ref*()-style iteration, + * calling an each_ref_fn for each entry. */ -static int do_one_ref(const char *base, each_ref_fn fn, int trim, - int flags, void *cb_data, struct ref_entry *entry) +static int do_one_ref(struct ref_entry *entry, void *cb_data) { + struct ref_entry_cb *data = cb_data; int retval; - if (prefixcmp(entry->name, base)) + if (prefixcmp(entry->name, data->base)) return 0; - if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN) && + if (!(data->flags & DO_FOR_EACH_INCLUDE_BROKEN) && !ref_resolves_to_object(entry)) return 0; current_ref = entry; - retval = fn(entry->name + trim, entry->u.value.sha1, entry->flag, cb_data); + retval = data->fn(entry->name + data->trim, entry->u.value.sha1, + entry->flag, data->cb_data); current_ref = NULL; return retval; } @@ -581,11 +593,11 @@ static int do_one_ref(const char *base, each_ref_fn fn, int trim, * Call fn for each reference in dir that has index in the range * offset <= index < dir->nr. Recurse into subdirectories that are in * that index range, sorting them before iterating. This function - * does not sort dir itself; it should be sorted beforehand. + * does not sort dir itself; it should be sorted beforehand. fn is + * called for all references, including broken ones. */ -static int do_for_each_ref_in_dir(struct ref_dir *dir, int offset, - const char *base, - each_ref_fn fn, int trim, int flags, void *cb_data) +static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset, + each_ref_entry_fn fn, void *cb_data) { int i; assert(dir->sorted == dir->nr); @@ -595,10 +607,9 @@ static int do_for_each_ref_in_dir(struct ref_dir *dir, int offset, if (entry->flag & REF_DIR) { struct ref_dir *subdir = get_ref_dir(entry); sort_ref_dir(subdir); - retval = do_for_each_ref_in_dir(subdir, 0, - base, fn, trim, flags, cb_data); + retval = do_for_each_entry_in_dir(subdir, 0, fn, cb_data); } else { - retval = do_one_ref(base, fn, trim, flags, cb_data, entry); + retval = fn(entry, cb_data); } if (retval) return retval; @@ -611,12 +622,12 @@ static int do_for_each_ref_in_dir(struct ref_dir *dir, int offset, * by refname. Recurse into subdirectories. If a value entry appears * in both dir1 and dir2, then only process the version that is in * dir2. The input dirs must already be sorted, but subdirs will be - * sorted as needed. + * sorted as needed. fn is called for all references, including + * broken ones. */ -static int do_for_each_ref_in_dirs(struct ref_dir *dir1, - struct ref_dir *dir2, - const char *base, each_ref_fn fn, int trim, - int flags, void *cb_data) +static int do_for_each_entry_in_dirs(struct ref_dir *dir1, + struct ref_dir *dir2, + each_ref_entry_fn fn, void *cb_data) { int retval; int i1 = 0, i2 = 0; @@ -627,12 +638,10 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1, struct ref_entry *e1, *e2; int cmp; if (i1 == dir1->nr) { - return do_for_each_ref_in_dir(dir2, i2, - base, fn, trim, flags, cb_data); + return do_for_each_entry_in_dir(dir2, i2, fn, cb_data); } if (i2 == dir2->nr) { - return do_for_each_ref_in_dir(dir1, i1, - base, fn, trim, flags, cb_data); + return do_for_each_entry_in_dir(dir1, i1, fn, cb_data); } e1 = dir1->entries[i1]; e2 = dir2->entries[i2]; @@ -644,14 +653,13 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1, struct ref_dir *subdir2 = get_ref_dir(e2); sort_ref_dir(subdir1); sort_ref_dir(subdir2); - retval = do_for_each_ref_in_dirs( - subdir1, subdir2, - base, fn, trim, flags, cb_data); + retval = do_for_each_entry_in_dirs( + subdir1, subdir2, fn, cb_data); i1++; i2++; } else if (!(e1->flag & REF_DIR) && !(e2->flag & REF_DIR)) { /* Both are references; ignore the one from dir1. */ - retval = do_one_ref(base, fn, trim, flags, cb_data, e2); + retval = fn(e2, cb_data); i1++; i2++; } else { @@ -670,11 +678,10 @@ static int do_for_each_ref_in_dirs(struct ref_dir *dir1, if (e->flag & REF_DIR) { struct ref_dir *subdir = get_ref_dir(e); sort_ref_dir(subdir); - retval = do_for_each_ref_in_dir( - subdir, 0, - base, fn, trim, flags, cb_data); + retval = do_for_each_entry_in_dir( + subdir, 0, fn, cb_data); } else { - retval = do_one_ref(base, fn, trim, flags, cb_data, e); + retval = fn(e, cb_data); } } if (retval) @@ -703,14 +710,13 @@ struct name_conflict_cb { const char *conflicting_refname; }; -static int name_conflict_fn(const char *existingrefname, const unsigned char *sha1, - int flags, void *cb_data) +static int name_conflict_fn(struct ref_entry *entry, void *cb_data) { struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data; - if (data->oldrefname && !strcmp(data->oldrefname, existingrefname)) + if (data->oldrefname && !strcmp(data->oldrefname, entry->name)) return 0; - if (names_conflict(data->refname, existingrefname)) { - data->conflicting_refname = existingrefname; + if (names_conflict(data->refname, entry->name)) { + data->conflicting_refname = entry->name; return 1; } return 0; @@ -718,7 +724,7 @@ static int name_conflict_fn(const char *existingrefname, const unsigned char *sh /* * Return true iff a reference named refname could be created without - * conflicting with the name of an existing reference in array. If + * conflicting with the name of an existing reference in dir. If * oldrefname is non-NULL, ignore potential conflicts with oldrefname * (e.g., because oldrefname is scheduled for deletion in the same * operation). @@ -732,9 +738,7 @@ static int is_refname_available(const char *refname, const char *oldrefname, data.conflicting_refname = NULL; sort_ref_dir(dir); - if (do_for_each_ref_in_dir(dir, 0, "", name_conflict_fn, - 0, DO_FOR_EACH_INCLUDE_BROKEN, - &data)) { + if (do_for_each_entry_in_dir(dir, 0, name_conflict_fn, &data)) { error("'%s' exists; cannot create '%s'", data.conflicting_refname, refname); return 0; @@ -1422,16 +1426,14 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname) } /* - * Call fn for each reference in the specified submodule for which the - * refname begins with base. If trim is non-zero, then trim that many - * characters off the beginning of each refname before passing the - * refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to include - * broken references in the iteration. If fn ever returns a non-zero + * Call fn for each reference in the specified submodule, omitting + * references not in the containing_dir of base. fn is called for all + * references, including broken ones. If fn ever returns a non-zero * value, stop the iteration and return that value; otherwise, return * 0. */ -static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn, - int trim, int flags, void *cb_data) +static int do_for_each_entry(const char *submodule, const char *base, + each_ref_entry_fn fn, void *cb_data) { struct ref_cache *refs = get_ref_cache(submodule); struct ref_dir *packed_dir = get_packed_refs(refs); @@ -1446,24 +1448,43 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn if (packed_dir && loose_dir) { sort_ref_dir(packed_dir); sort_ref_dir(loose_dir); - retval = do_for_each_ref_in_dirs( - packed_dir, loose_dir, - base, fn, trim, flags, cb_data); + retval = do_for_each_entry_in_dirs( + packed_dir, loose_dir, fn, cb_data); } else if (packed_dir) { sort_ref_dir(packed_dir); - retval = do_for_each_ref_in_dir( - packed_dir, 0, - base, fn, trim, flags, cb_data); + retval = do_for_each_entry_in_dir( + packed_dir, 0, fn, cb_data); } else if (loose_dir) { sort_ref_dir(loose_dir); - retval = do_for_each_ref_in_dir( - loose_dir, 0, - base, fn, trim, flags, cb_data); + retval = do_for_each_entry_in_dir( + loose_dir, 0, fn, cb_data); } return retval; } +/* + * Call fn for each reference in the specified submodule for which the + * refname begins with base. If trim is non-zero, then trim that many + * characters off the beginning of each refname before passing the + * refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to include + * broken references in the iteration. If fn ever returns a non-zero + * value, stop the iteration and return that value; otherwise, return + * 0. + */ +static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn, + int trim, int flags, void *cb_data) +{ + struct ref_entry_cb data; + data.base = base; + data.trim = trim; + data.flags = flags; + data.fn = fn; + data.cb_data = cb_data; + + return do_for_each_entry(submodule, base, do_one_ref, &data); +} + static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data) { unsigned char sha1[20]; @@ -1873,20 +1894,21 @@ struct repack_without_ref_sb { int fd; }; -static int repack_without_ref_fn(const char *refname, const unsigned char *sha1, - int flags, void *cb_data) +static int repack_without_ref_fn(struct ref_entry *entry, void *cb_data) { struct repack_without_ref_sb *data = cb_data; char line[PATH_MAX + 100]; int len; - if (!strcmp(data->refname, refname)) + if (!strcmp(data->refname, entry->name)) return 0; + if (!ref_resolves_to_object(entry)) + return 0; /* Skip broken refs */ len = snprintf(line, sizeof(line), "%s %s\n", - sha1_to_hex(sha1), refname); + sha1_to_hex(entry->u.value.sha1), entry->name); /* this should not happen but just being defensive */ if (len > sizeof(line)) - die("too long a refname '%s'", refname); + die("too long a refname '%s'", entry->name); write_or_die(data->fd, line, len); return 0; } @@ -1910,7 +1932,7 @@ static int repack_without_ref(const char *refname) } clear_packed_ref_cache(refs); packed = get_packed_refs(refs); - do_for_each_ref_in_dir(packed, 0, "", repack_without_ref_fn, 0, 0, &data); + do_for_each_entry_in_dir(packed, 0, repack_without_ref_fn, &data); return commit_lock_file(&packlock); } -- cgit v1.2.3 From ab292bc4f30dd29d3d111b040b9e982f20b9ceb7 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:25 +0200 Subject: repack_without_ref(): silence errors for dangling packed refs Stop emitting an error message when deleting a packed reference if we find another dangling packed reference that is overridden by a loose reference. See the previous commit for a longer explanation of the issue. We have to be careful to make sure that the invalid packed reference really *is* overridden by a loose reference; otherwise what we have found is repository corruption, which we *should* report. Please note that this approach is vulnerable to a race condition similar to the race conditions already known to affect packed references [1]: * Process 1 tries to peel packed reference X as part of deleting another packed reference. It discovers that X does not refer to a valid object (because the object that it referred to has been garbage collected). * Process 2 tries to delete reference X. It starts by deleting the loose reference X. * Process 1 checks whether there is a loose reference X. There is not (it has just been deleted by process 2), so process 1 reports a spurious error "X does not point to a valid object!" The worst case seems relatively harmless, and the fix is identical to the fix that will be needed for the other race conditions (namely holding a lock on the packed-refs file during *all* reference deletions), so we leave the cleaning up of all of them as a future project. [1] http://thread.gmane.org/gmane.comp.version-control.git/211956 Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index d884f7d78f..002daebf6c 100644 --- a/refs.c +++ b/refs.c @@ -1902,8 +1902,41 @@ static int repack_without_ref_fn(struct ref_entry *entry, void *cb_data) if (!strcmp(data->refname, entry->name)) return 0; - if (!ref_resolves_to_object(entry)) - return 0; /* Skip broken refs */ + if (entry->flag & REF_ISBROKEN) { + /* This shouldn't happen to packed refs. */ + error("%s is broken!", entry->name); + return 0; + } + if (!has_sha1_file(entry->u.value.sha1)) { + unsigned char sha1[20]; + int flags; + + if (read_ref_full(entry->name, sha1, 0, &flags)) + /* We should at least have found the packed ref. */ + die("Internal error"); + if ((flags & REF_ISSYMREF) || !(flags & REF_ISPACKED)) + /* + * This packed reference is overridden by a + * loose reference, so it is OK that its value + * is no longer valid; for example, it might + * refer to an object that has been garbage + * collected. For this purpose we don't even + * care whether the loose reference itself is + * invalid, broken, symbolic, etc. Silently + * omit the packed reference from the output. + */ + return 0; + /* + * There is no overriding loose reference, so the fact + * that this reference doesn't refer to a valid object + * indicates some kind of repository corruption. + * Report the problem, then omit the reference from + * the output. + */ + error("%s does not point to a valid object!", entry->name); + return 0; + } + len = snprintf(line, sizeof(line), "%s %s\n", sha1_to_hex(entry->u.value.sha1), entry->name); /* this should not happen but just being defensive */ -- cgit v1.2.3 From 9fc0a64806aa72ca7ef1c20468ffcef5c86978b8 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:26 +0200 Subject: search_ref_dir(): return an index rather than a pointer Change search_ref_dir() to return the index of the sought entry (or -1 on error) rather than a pointer to the entry. This will make it more natural to use the function for removing an entry from the list. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 002daebf6c..3479713e81 100644 --- a/refs.c +++ b/refs.c @@ -367,18 +367,17 @@ static int ref_entry_cmp_sslice(const void *key_, const void *ent_) } /* - * Return the entry with the given refname from the ref_dir - * (non-recursively), sorting dir if necessary. Return NULL if no - * such entry is found. dir must already be complete. + * Return the index of the entry with the given refname from the + * ref_dir (non-recursively), sorting dir if necessary. Return -1 if + * no such entry is found. dir must already be complete. */ -static struct ref_entry *search_ref_dir(struct ref_dir *dir, - const char *refname, size_t len) +static int search_ref_dir(struct ref_dir *dir, const char *refname, size_t len) { struct ref_entry **r; struct string_slice key; if (refname == NULL || !dir->nr) - return NULL; + return -1; sort_ref_dir(dir); key.len = len; @@ -387,9 +386,9 @@ static struct ref_entry *search_ref_dir(struct ref_dir *dir, ref_entry_cmp_sslice); if (r == NULL) - return NULL; + return -1; - return *r; + return r - dir->entries; } /* @@ -403,8 +402,9 @@ static struct ref_dir *search_for_subdir(struct ref_dir *dir, const char *subdirname, size_t len, int mkdir) { - struct ref_entry *entry = search_ref_dir(dir, subdirname, len); - if (!entry) { + int entry_index = search_ref_dir(dir, subdirname, len); + struct ref_entry *entry; + if (entry_index == -1) { if (!mkdir) return NULL; /* @@ -415,6 +415,8 @@ static struct ref_dir *search_for_subdir(struct ref_dir *dir, */ entry = create_dir_entry(dir->ref_cache, subdirname, len, 0); add_entry_to_dir(dir, entry); + } else { + entry = dir->entries[entry_index]; } return get_ref_dir(entry); } @@ -453,12 +455,16 @@ static struct ref_dir *find_containing_dir(struct ref_dir *dir, */ static struct ref_entry *find_ref(struct ref_dir *dir, const char *refname) { + int entry_index; struct ref_entry *entry; dir = find_containing_dir(dir, refname, 0); if (!dir) return NULL; - entry = search_ref_dir(dir, refname, strlen(refname)); - return (entry && !(entry->flag & REF_DIR)) ? entry : NULL; + entry_index = search_ref_dir(dir, refname, strlen(refname)); + if (entry_index == -1) + return NULL; + entry = dir->entries[entry_index]; + return (entry->flag & REF_DIR) ? NULL : entry; } /* -- cgit v1.2.3 From 506a760db8c3fd510d7ebe70c189672f185ee108 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:27 +0200 Subject: refs: change how packed refs are deleted Add a function remove_ref(), which removes a single entry from a reference cache. Use this function to reimplement repack_without_ref(). The old version iterated over all refs, packing all of them except for the one to be deleted, then discarded the entire packed reference cache. The new version deletes the doomed reference from the cache *before* iterating. This has two advantages: * the code for writing packed-refs becomes simpler, because it doesn't have to exclude one of the references. * it is no longer necessary to discard the packed refs cache after deleting a reference: symbolic refs cannot be packed, so packed references cannot depend on each other, so the rest of the packed refs cache remains valid after a reference is deleted. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 68 insertions(+), 16 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 3479713e81..f8f295ea69 100644 --- a/refs.c +++ b/refs.c @@ -467,6 +467,57 @@ static struct ref_entry *find_ref(struct ref_dir *dir, const char *refname) return (entry->flag & REF_DIR) ? NULL : entry; } +/* + * Remove the entry with the given name from dir, recursing into + * subdirectories as necessary. If refname is the name of a directory + * (i.e., ends with '/'), then remove the directory and its contents. + * If the removal was successful, return the number of entries + * remaining in the directory entry that contained the deleted entry. + * If the name was not found, return -1. Please note that this + * function only deletes the entry from the cache; it does not delete + * it from the filesystem or ensure that other cache entries (which + * might be symbolic references to the removed entry) are updated. + * Nor does it remove any containing dir entries that might be made + * empty by the removal. dir must represent the top-level directory + * and must already be complete. + */ +static int remove_entry(struct ref_dir *dir, const char *refname) +{ + int refname_len = strlen(refname); + int entry_index; + struct ref_entry *entry; + int is_dir = refname[refname_len - 1] == '/'; + if (is_dir) { + /* + * refname represents a reference directory. Remove + * the trailing slash; otherwise we will get the + * directory *representing* refname rather than the + * one *containing* it. + */ + char *dirname = xmemdupz(refname, refname_len - 1); + dir = find_containing_dir(dir, dirname, 0); + free(dirname); + } else { + dir = find_containing_dir(dir, refname, 0); + } + if (!dir) + return -1; + entry_index = search_ref_dir(dir, refname, refname_len); + if (entry_index == -1) + return -1; + entry = dir->entries[entry_index]; + + memmove(&dir->entries[entry_index], + &dir->entries[entry_index + 1], + (dir->nr - entry_index - 1) * sizeof(*dir->entries) + ); + dir->nr--; + if (dir->sorted > entry_index) + dir->sorted--; + free_ref_entry(entry); + return dir->nr; +} + /* * Add a ref_entry to the ref_dir (unsorted), recursing into * subdirectories as necessary. dir must represent the top-level @@ -1895,19 +1946,12 @@ struct ref_lock *lock_any_ref_for_update(const char *refname, return lock_ref_sha1_basic(refname, old_sha1, flags, NULL); } -struct repack_without_ref_sb { - const char *refname; - int fd; -}; - -static int repack_without_ref_fn(struct ref_entry *entry, void *cb_data) +static int repack_ref_fn(struct ref_entry *entry, void *cb_data) { - struct repack_without_ref_sb *data = cb_data; + int *fd = cb_data; char line[PATH_MAX + 100]; int len; - if (!strcmp(data->refname, entry->name)) - return 0; if (entry->flag & REF_ISBROKEN) { /* This shouldn't happen to packed refs. */ error("%s is broken!", entry->name); @@ -1948,7 +1992,7 @@ static int repack_without_ref_fn(struct ref_entry *entry, void *cb_data) /* this should not happen but just being defensive */ if (len > sizeof(line)) die("too long a refname '%s'", entry->name); - write_or_die(data->fd, line, len); + write_or_die(*fd, line, len); return 0; } @@ -1956,22 +2000,30 @@ static struct lock_file packlock; static int repack_without_ref(const char *refname) { - struct repack_without_ref_sb data; + int fd; struct ref_cache *refs = get_ref_cache(NULL); struct ref_dir *packed; if (!get_packed_ref(refname)) return 0; /* refname does not exist in packed refs */ - data.refname = refname; - data.fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0); - if (data.fd < 0) { + fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0); + if (fd < 0) { unable_to_lock_error(git_path("packed-refs"), errno); return error("cannot delete '%s' from packed refs", refname); } clear_packed_ref_cache(refs); packed = get_packed_refs(refs); - do_for_each_entry_in_dir(packed, 0, repack_without_ref_fn, &data); + /* Remove refname from the cache. */ + if (remove_entry(packed, refname) == -1) { + /* + * The packed entry disappeared while we were + * acquiring the lock. + */ + rollback_lock_file(&packlock); + return 0; + } + do_for_each_entry_in_dir(packed, 0, repack_ref_fn, &fd); return commit_lock_file(&packlock); } @@ -2000,7 +2052,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) ret |= repack_without_ref(lock->ref_name); unlink_or_warn(git_path("logs/%s", lock->ref_name)); - invalidate_ref_cache(NULL); + clear_loose_ref_cache(get_ref_cache(NULL)); unlock_ref(lock); return ret; } -- cgit v1.2.3 From 694b7a1999fa331e00ed9297f3b6b01f7da7a104 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:29 +0200 Subject: repack_without_ref(): write peeled refs in the rewritten file When a reference that existed in the packed-refs file is deleted, the packed-refs file must be rewritten. Previously, the file was rewritten without any peeled refs, even if the file contained peeled refs when it was read. This was not a bug, because the packed-refs file header didn't claim that the file contained peeled values. But it had a performance cost, because the repository would lose the benefit of having precomputed peeled references until pack-refs was run again. Teach repack_without_ref() to write peeled refs to the packed-refs file (regardless of whether they were present in the old version of the file). This means that if the old version of the packed-refs file was not fully peeled, then repack_without_ref() will have to peel references. To avoid the expense of reading lots of loose references, we take two shortcuts relative to pack-refs: * If the peeled value of a reference is already known (i.e., because it was read from the old version of the packed-refs file), then output that peeled value again without any checks. This is the usual code path and should avoid any noticeable overhead. (This is different than pack-refs, which always re-peels references.) * We don't verify that the packed ref is still current. It could be that a packed references is overridden by a loose reference, in which case the packed ref is no longer needed and might even refer to an object that has been garbage collected. But we don't check; instead, we just try to peel all references. If peeling is successful, the peeled value is written out (even though it might not be needed any more); if not, then the reference is silently omitted from the output. The extra overhead of peeling references in repack_without_ref() should only be incurred the first time the packed-refs file is written by a version of Git that knows about the "fully-peeled" attribute. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'refs.c') diff --git a/refs.c b/refs.c index f8f295ea69..6032ba62c7 100644 --- a/refs.c +++ b/refs.c @@ -876,6 +876,13 @@ void invalidate_ref_cache(const char *submodule) /* The length of a peeled reference line in packed-refs, including EOL: */ #define PEELED_LINE_LENGTH 42 +/* + * The packed-refs header line that we write out. Perhaps other + * traits will be added later. The trailing space is required. + */ +static const char PACKED_REFS_HEADER[] = + "# pack-refs with: peeled fully-peeled \n"; + /* * Parse one line from a packed-refs file. Write the SHA1 to sha1. * Return a pointer to the refname within the line (null-terminated), @@ -1391,6 +1398,12 @@ static enum peel_status peel_object(const unsigned char *name, unsigned char *sh /* * Peel the entry (if possible) and return its new peel_status. + * + * It is OK to call this function with a packed reference entry that + * might be stale and might even refer to an object that has since + * been garbage-collected. In such a case, if the entry has + * REF_KNOWS_PEELED then leave the status unchanged and return + * PEEL_PEELED or PEEL_NON_TAG; otherwise, return PEEL_INVALID. */ static enum peel_status peel_entry(struct ref_entry *entry) { @@ -1993,6 +2006,15 @@ static int repack_ref_fn(struct ref_entry *entry, void *cb_data) if (len > sizeof(line)) die("too long a refname '%s'", entry->name); write_or_die(*fd, line, len); + if (!peel_entry(entry)) { + /* This reference could be peeled; write the peeled value: */ + if (snprintf(line, sizeof(line), "^%s\n", + sha1_to_hex(entry->u.value.peeled)) != + PEELED_LINE_LENGTH) + die("internal error"); + write_or_die(*fd, line, PEELED_LINE_LENGTH); + } + return 0; } @@ -2023,6 +2045,7 @@ static int repack_without_ref(const char *refname) rollback_lock_file(&packlock); return 0; } + write_or_die(fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER)); do_for_each_entry_in_dir(packed, 0, repack_ref_fn, &fd); return commit_lock_file(&packlock); } -- cgit v1.2.3 From fec3137ffcc49d58e6388b1c61da902b031e01ca Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:30 +0200 Subject: refs: extract a function write_packed_entry() Extract the I/O code from the "business logic" in repack_ref_fn(). Later there will be another caller for this function. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 6032ba62c7..e09c0457af 100644 --- a/refs.c +++ b/refs.c @@ -1959,12 +1959,36 @@ struct ref_lock *lock_any_ref_for_update(const char *refname, return lock_ref_sha1_basic(refname, old_sha1, flags, NULL); } -static int repack_ref_fn(struct ref_entry *entry, void *cb_data) +/* + * Write an entry to the packed-refs file for the specified refname. + * If peeled is non-NULL, write it as the entry's peeled value. + */ +static void write_packed_entry(int fd, char *refname, unsigned char *sha1, + unsigned char *peeled) { - int *fd = cb_data; char line[PATH_MAX + 100]; int len; + len = snprintf(line, sizeof(line), "%s %s\n", + sha1_to_hex(sha1), refname); + /* this should not happen but just being defensive */ + if (len > sizeof(line)) + die("too long a refname '%s'", refname); + write_or_die(fd, line, len); + + if (peeled) { + if (snprintf(line, sizeof(line), "^%s\n", + sha1_to_hex(peeled)) != PEELED_LINE_LENGTH) + die("internal error"); + write_or_die(fd, line, PEELED_LINE_LENGTH); + } +} + +static int repack_ref_fn(struct ref_entry *entry, void *cb_data) +{ + int *fd = cb_data; + enum peel_status peel_status; + if (entry->flag & REF_ISBROKEN) { /* This shouldn't happen to packed refs. */ error("%s is broken!", entry->name); @@ -2000,20 +2024,10 @@ static int repack_ref_fn(struct ref_entry *entry, void *cb_data) return 0; } - len = snprintf(line, sizeof(line), "%s %s\n", - sha1_to_hex(entry->u.value.sha1), entry->name); - /* this should not happen but just being defensive */ - if (len > sizeof(line)) - die("too long a refname '%s'", entry->name); - write_or_die(*fd, line, len); - if (!peel_entry(entry)) { - /* This reference could be peeled; write the peeled value: */ - if (snprintf(line, sizeof(line), "^%s\n", - sha1_to_hex(entry->u.value.peeled)) != - PEELED_LINE_LENGTH) - die("internal error"); - write_or_die(*fd, line, PEELED_LINE_LENGTH); - } + peel_status = peel_entry(entry); + write_packed_entry(*fd, entry->name, entry->u.value.sha1, + peel_status == PEEL_PEELED ? + entry->u.value.peeled : NULL); return 0; } -- cgit v1.2.3 From 32d462cea80cd52b2c3fa0d538aba7fcf079ba77 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:32 +0200 Subject: pack-refs: merge code from pack-refs.{c,h} into refs.{c,h} pack-refs.c doesn't contain much code, and the code it does contain is closely related to reference handling. Moreover, there is some duplication between pack_refs() and repack_without_ref(). Therefore, merge pack-refs.c into refs.c and pack-refs.h into refs.h. The code duplication will be addressed in future commits. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 144 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) (limited to 'refs.c') diff --git a/refs.c b/refs.c index e09c0457af..fcd7163581 100644 --- a/refs.c +++ b/refs.c @@ -1984,6 +1984,150 @@ static void write_packed_entry(int fd, char *refname, unsigned char *sha1, } } +struct ref_to_prune { + struct ref_to_prune *next; + unsigned char sha1[20]; + char name[FLEX_ARRAY]; +}; + +struct pack_refs_cb_data { + unsigned int flags; + struct ref_to_prune *ref_to_prune; + FILE *refs_file; +}; + +static int do_not_prune(int flags) +{ + /* If it is already packed or if it is a symref, + * do not prune it. + */ + return (flags & (REF_ISSYMREF|REF_ISPACKED)); +} + +static int pack_one_ref(const char *path, const unsigned char *sha1, + int flags, void *cb_data) +{ + struct pack_refs_cb_data *cb = cb_data; + struct object *o; + int is_tag_ref; + + /* Do not pack the symbolic refs */ + if ((flags & REF_ISSYMREF)) + return 0; + is_tag_ref = !prefixcmp(path, "refs/tags/"); + + /* ALWAYS pack refs that were already packed or are tags */ + if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED)) + return 0; + + fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path); + + o = parse_object_or_die(sha1, path); + if (o->type == OBJ_TAG) { + o = deref_tag(o, path, 0); + if (o) + fprintf(cb->refs_file, "^%s\n", + sha1_to_hex(o->sha1)); + } + + if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) { + int namelen = strlen(path) + 1; + struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen); + hashcpy(n->sha1, sha1); + strcpy(n->name, path); + n->next = cb->ref_to_prune; + cb->ref_to_prune = n; + } + return 0; +} + +/* + * Remove empty parents, but spare refs/ and immediate subdirs. + * Note: munges *name. + */ +static void try_remove_empty_parents(char *name) +{ + char *p, *q; + int i; + p = name; + for (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */ + while (*p && *p != '/') + p++; + /* tolerate duplicate slashes; see check_refname_format() */ + while (*p == '/') + p++; + } + for (q = p; *q; q++) + ; + while (1) { + while (q > p && *q != '/') + q--; + while (q > p && *(q-1) == '/') + q--; + if (q == p) + break; + *q = '\0'; + if (rmdir(git_path("%s", name))) + break; + } +} + +/* make sure nobody touched the ref, and unlink */ +static void prune_ref(struct ref_to_prune *r) +{ + struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1); + + if (lock) { + unlink_or_warn(git_path("%s", r->name)); + unlock_ref(lock); + try_remove_empty_parents(r->name); + } +} + +static void prune_refs(struct ref_to_prune *r) +{ + while (r) { + prune_ref(r); + r = r->next; + } +} + +static struct lock_file packed; + +int pack_refs(unsigned int flags) +{ + int fd; + struct pack_refs_cb_data cbdata; + + memset(&cbdata, 0, sizeof(cbdata)); + cbdata.flags = flags; + + fd = hold_lock_file_for_update(&packed, git_path("packed-refs"), + LOCK_DIE_ON_ERROR); + cbdata.refs_file = fdopen(fd, "w"); + if (!cbdata.refs_file) + die_errno("unable to create ref-pack file structure"); + + /* perhaps other traits later as well */ + fprintf(cbdata.refs_file, "# pack-refs with: peeled fully-peeled \n"); + + for_each_ref(pack_one_ref, &cbdata); + if (ferror(cbdata.refs_file)) + die("failed to write ref-pack file"); + if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file)) + die_errno("failed to write ref-pack file"); + /* + * Since the lock file was fdopen()'ed and then fclose()'ed above, + * assign -1 to the lock file descriptor so that commit_lock_file() + * won't try to close() it. + */ + packed.fd = -1; + if (commit_lock_file(&packed) < 0) + die_errno("unable to overwrite old ref-pack file"); + prune_refs(cbdata.ref_to_prune); + return 0; +} + static int repack_ref_fn(struct ref_entry *entry, void *cb_data) { int *fd = cb_data; -- cgit v1.2.3 From 3b4ae6d5023095114692d1e2e1a213611fec9314 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:33 +0200 Subject: pack_one_ref(): rename "path" parameter to "refname" Make this function conform to the naming convention established in 65385ef7d4 for the rest of the refs.c file. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index fcd7163581..b536af4cb0 100644 --- a/refs.c +++ b/refs.c @@ -2004,7 +2004,7 @@ static int do_not_prune(int flags) return (flags & (REF_ISSYMREF|REF_ISPACKED)); } -static int pack_one_ref(const char *path, const unsigned char *sha1, +static int pack_one_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data) { struct pack_refs_cb_data *cb = cb_data; @@ -2014,27 +2014,27 @@ static int pack_one_ref(const char *path, const unsigned char *sha1, /* Do not pack the symbolic refs */ if ((flags & REF_ISSYMREF)) return 0; - is_tag_ref = !prefixcmp(path, "refs/tags/"); + is_tag_ref = !prefixcmp(refname, "refs/tags/"); /* ALWAYS pack refs that were already packed or are tags */ if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED)) return 0; - fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path); + fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), refname); - o = parse_object_or_die(sha1, path); + o = parse_object_or_die(sha1, refname); if (o->type == OBJ_TAG) { - o = deref_tag(o, path, 0); + o = deref_tag(o, refname, 0); if (o) fprintf(cb->refs_file, "^%s\n", sha1_to_hex(o->sha1)); } if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) { - int namelen = strlen(path) + 1; + int namelen = strlen(refname) + 1; struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen); hashcpy(n->sha1, sha1); - strcpy(n->name, path); + strcpy(n->name, refname); n->next = cb->ref_to_prune; cb->ref_to_prune = n; } -- cgit v1.2.3 From d9470330379f5899dd117b7c720e67d278fed9e2 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:34 +0200 Subject: refs: use same lock_file object for both ref-packing functions Use a single struct lock_file for both pack_refs() and repack_without_ref(). Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index b536af4cb0..9b392bd96d 100644 --- a/refs.c +++ b/refs.c @@ -2092,7 +2092,7 @@ static void prune_refs(struct ref_to_prune *r) } } -static struct lock_file packed; +static struct lock_file packlock; int pack_refs(unsigned int flags) { @@ -2102,7 +2102,7 @@ int pack_refs(unsigned int flags) memset(&cbdata, 0, sizeof(cbdata)); cbdata.flags = flags; - fd = hold_lock_file_for_update(&packed, git_path("packed-refs"), + fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), LOCK_DIE_ON_ERROR); cbdata.refs_file = fdopen(fd, "w"); if (!cbdata.refs_file) @@ -2121,8 +2121,8 @@ int pack_refs(unsigned int flags) * assign -1 to the lock file descriptor so that commit_lock_file() * won't try to close() it. */ - packed.fd = -1; - if (commit_lock_file(&packed) < 0) + packlock.fd = -1; + if (commit_lock_file(&packlock) < 0) die_errno("unable to overwrite old ref-pack file"); prune_refs(cbdata.ref_to_prune); return 0; @@ -2176,8 +2176,6 @@ static int repack_ref_fn(struct ref_entry *entry, void *cb_data) return 0; } -static struct lock_file packlock; - static int repack_without_ref(const char *refname) { int fd; -- cgit v1.2.3 From 12e77559ec4bf863d3703a25ab79298d5ff89b3b Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:35 +0200 Subject: pack_refs(): change to use do_for_each_entry() pack_refs() was not using any of the extra features of for_each_ref(), so change it to use do_for_each_entry(). This also gives it access to the ref_entry and in particular its peeled field, which will be taken advantage of in the next commit. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 9b392bd96d..3465db64f9 100644 --- a/refs.c +++ b/refs.c @@ -2004,37 +2004,38 @@ static int do_not_prune(int flags) return (flags & (REF_ISSYMREF|REF_ISPACKED)); } -static int pack_one_ref(const char *refname, const unsigned char *sha1, - int flags, void *cb_data) +static int pack_one_ref(struct ref_entry *entry, void *cb_data) { struct pack_refs_cb_data *cb = cb_data; struct object *o; int is_tag_ref; - /* Do not pack the symbolic refs */ - if ((flags & REF_ISSYMREF)) + /* Do not pack symbolic or broken refs: */ + if ((entry->flag & REF_ISSYMREF) || !ref_resolves_to_object(entry)) return 0; - is_tag_ref = !prefixcmp(refname, "refs/tags/"); + is_tag_ref = !prefixcmp(entry->name, "refs/tags/"); /* ALWAYS pack refs that were already packed or are tags */ - if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED)) + if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && + !(entry->flag & REF_ISPACKED)) return 0; - fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), refname); + fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(entry->u.value.sha1), + entry->name); - o = parse_object_or_die(sha1, refname); + o = parse_object_or_die(entry->u.value.sha1, entry->name); if (o->type == OBJ_TAG) { - o = deref_tag(o, refname, 0); + o = deref_tag(o, entry->name, 0); if (o) fprintf(cb->refs_file, "^%s\n", sha1_to_hex(o->sha1)); } - if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(flags)) { - int namelen = strlen(refname) + 1; + if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(entry->flag)) { + int namelen = strlen(entry->name) + 1; struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen); - hashcpy(n->sha1, sha1); - strcpy(n->name, refname); + hashcpy(n->sha1, entry->u.value.sha1); + strcpy(n->name, entry->name); n->next = cb->ref_to_prune; cb->ref_to_prune = n; } @@ -2111,7 +2112,7 @@ int pack_refs(unsigned int flags) /* perhaps other traits later as well */ fprintf(cbdata.refs_file, "# pack-refs with: peeled fully-peeled \n"); - for_each_ref(pack_one_ref, &cbdata); + do_for_each_entry(NULL, "", pack_one_ref, &cbdata); if (ferror(cbdata.refs_file)) die("failed to write ref-pack file"); if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file)) -- cgit v1.2.3 From 8d3725b96fa6878fabe5ec715ff7b9f263481ea1 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:36 +0200 Subject: refs: inline function do_not_prune() Function do_not_prune() was redundantly checking REF_ISSYMREF, which was already tested at the top of pack_one_ref(), so remove that check. And the rest was trivial, so inline the function. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 3465db64f9..b1c15fda80 100644 --- a/refs.c +++ b/refs.c @@ -1996,14 +1996,6 @@ struct pack_refs_cb_data { FILE *refs_file; }; -static int do_not_prune(int flags) -{ - /* If it is already packed or if it is a symref, - * do not prune it. - */ - return (flags & (REF_ISSYMREF|REF_ISPACKED)); -} - static int pack_one_ref(struct ref_entry *entry, void *cb_data) { struct pack_refs_cb_data *cb = cb_data; @@ -2031,7 +2023,8 @@ static int pack_one_ref(struct ref_entry *entry, void *cb_data) sha1_to_hex(o->sha1)); } - if ((cb->flags & PACK_REFS_PRUNE) && !do_not_prune(entry->flag)) { + /* If the ref was already packed, there is no need to prune it. */ + if ((cb->flags & PACK_REFS_PRUNE) && !(entry->flag & REF_ISPACKED)) { int namelen = strlen(entry->name) + 1; struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen); hashcpy(n->sha1, entry->u.value.sha1); -- cgit v1.2.3 From f85354b5c7b800912743927f4abba022444163fd Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:37 +0200 Subject: pack_one_ref(): use function peel_entry() Change pack_one_ref() to call peel_entry() rather than using its own code for peeling references. Aside from sharing code, this lets it take advantage of the optimization introduced by 6c4a060d7d. Please note that we *could* use any peeled values that happen to already be stored in the ref_entries, which would avoid some object lookups for references that were already packed. But doing so would also propagate any peeling errors across runs of "git pack-refs" and give no way to recover from such errors. And "git pack-refs" isn't run often enough that the performance cost is a problem. So instead, add a new option to peel_entry() to force the entry to be re-peeled, and call it with that option set. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index b1c15fda80..7e4aca95bb 100644 --- a/refs.c +++ b/refs.c @@ -1397,7 +1397,9 @@ static enum peel_status peel_object(const unsigned char *name, unsigned char *sh } /* - * Peel the entry (if possible) and return its new peel_status. + * Peel the entry (if possible) and return its new peel_status. If + * repeel is true, re-peel the entry even if there is an old peeled + * value that is already stored in it. * * It is OK to call this function with a packed reference entry that * might be stale and might even refer to an object that has since @@ -1405,13 +1407,19 @@ static enum peel_status peel_object(const unsigned char *name, unsigned char *sh * REF_KNOWS_PEELED then leave the status unchanged and return * PEEL_PEELED or PEEL_NON_TAG; otherwise, return PEEL_INVALID. */ -static enum peel_status peel_entry(struct ref_entry *entry) +static enum peel_status peel_entry(struct ref_entry *entry, int repeel) { enum peel_status status; - if (entry->flag & REF_KNOWS_PEELED) - return is_null_sha1(entry->u.value.peeled) ? - PEEL_NON_TAG : PEEL_PEELED; + if (entry->flag & REF_KNOWS_PEELED) { + if (repeel) { + entry->flag &= ~REF_KNOWS_PEELED; + hashclr(entry->u.value.peeled); + } else { + return is_null_sha1(entry->u.value.peeled) ? + PEEL_NON_TAG : PEEL_PEELED; + } + } if (entry->flag & REF_ISBROKEN) return PEEL_BROKEN; if (entry->flag & REF_ISSYMREF) @@ -1430,7 +1438,7 @@ int peel_ref(const char *refname, unsigned char *sha1) if (current_ref && (current_ref->name == refname || !strcmp(current_ref->name, refname))) { - if (peel_entry(current_ref)) + if (peel_entry(current_ref, 0)) return -1; hashcpy(sha1, current_ref->u.value.peeled); return 0; @@ -1450,7 +1458,7 @@ int peel_ref(const char *refname, unsigned char *sha1) if (flag & REF_ISPACKED) { struct ref_entry *r = get_packed_ref(refname); if (r) { - if (peel_entry(r)) + if (peel_entry(r, 0)) return -1; hashcpy(sha1, r->u.value.peeled); return 0; @@ -1999,7 +2007,7 @@ struct pack_refs_cb_data { static int pack_one_ref(struct ref_entry *entry, void *cb_data) { struct pack_refs_cb_data *cb = cb_data; - struct object *o; + enum peel_status peel_status; int is_tag_ref; /* Do not pack symbolic or broken refs: */ @@ -2015,13 +2023,12 @@ static int pack_one_ref(struct ref_entry *entry, void *cb_data) fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(entry->u.value.sha1), entry->name); - o = parse_object_or_die(entry->u.value.sha1, entry->name); - if (o->type == OBJ_TAG) { - o = deref_tag(o, entry->name, 0); - if (o) - fprintf(cb->refs_file, "^%s\n", - sha1_to_hex(o->sha1)); - } + peel_status = peel_entry(entry, 1); + if (peel_status == PEEL_PEELED) + fprintf(cb->refs_file, "^%s\n", sha1_to_hex(entry->u.value.peeled)); + else if (peel_status != PEEL_NON_TAG) + die("internal error peeling reference %s (%s)", + entry->name, sha1_to_hex(entry->u.value.sha1)); /* If the ref was already packed, there is no need to prune it. */ if ((cb->flags & PACK_REFS_PRUNE) && !(entry->flag & REF_ISPACKED)) { @@ -2162,7 +2169,7 @@ static int repack_ref_fn(struct ref_entry *entry, void *cb_data) return 0; } - peel_status = peel_entry(entry); + peel_status = peel_entry(entry, 0); write_packed_entry(*fd, entry->name, entry->u.value.sha1, peel_status == PEEL_PEELED ? entry->u.value.peeled : NULL); -- cgit v1.2.3 From 0f29920f1e0ce00aaf867fdd9ad2174011179f47 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:38 +0200 Subject: pack_one_ref(): use write_packed_entry() to do the writing Change pack_refs() to work with a file descriptor instead of a FILE* (making the file-locking code less awkward) and use write_packed_entry() to do the writing. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 7e4aca95bb..00174e9498 100644 --- a/refs.c +++ b/refs.c @@ -2001,7 +2001,7 @@ struct ref_to_prune { struct pack_refs_cb_data { unsigned int flags; struct ref_to_prune *ref_to_prune; - FILE *refs_file; + int fd; }; static int pack_one_ref(struct ref_entry *entry, void *cb_data) @@ -2020,15 +2020,13 @@ static int pack_one_ref(struct ref_entry *entry, void *cb_data) !(entry->flag & REF_ISPACKED)) return 0; - fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(entry->u.value.sha1), - entry->name); - peel_status = peel_entry(entry, 1); - if (peel_status == PEEL_PEELED) - fprintf(cb->refs_file, "^%s\n", sha1_to_hex(entry->u.value.peeled)); - else if (peel_status != PEEL_NON_TAG) + if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG) die("internal error peeling reference %s (%s)", entry->name, sha1_to_hex(entry->u.value.sha1)); + write_packed_entry(cb->fd, entry->name, entry->u.value.sha1, + peel_status == PEEL_PEELED ? + entry->u.value.peeled : NULL); /* If the ref was already packed, there is no need to prune it. */ if ((cb->flags & PACK_REFS_PRUNE) && !(entry->flag & REF_ISPACKED)) { @@ -2097,32 +2095,17 @@ static struct lock_file packlock; int pack_refs(unsigned int flags) { - int fd; struct pack_refs_cb_data cbdata; memset(&cbdata, 0, sizeof(cbdata)); cbdata.flags = flags; - fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), - LOCK_DIE_ON_ERROR); - cbdata.refs_file = fdopen(fd, "w"); - if (!cbdata.refs_file) - die_errno("unable to create ref-pack file structure"); + cbdata.fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), + LOCK_DIE_ON_ERROR); - /* perhaps other traits later as well */ - fprintf(cbdata.refs_file, "# pack-refs with: peeled fully-peeled \n"); + write_or_die(cbdata.fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER)); do_for_each_entry(NULL, "", pack_one_ref, &cbdata); - if (ferror(cbdata.refs_file)) - die("failed to write ref-pack file"); - if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file)) - die_errno("failed to write ref-pack file"); - /* - * Since the lock file was fdopen()'ed and then fclose()'ed above, - * assign -1 to the lock file descriptor so that commit_lock_file() - * won't try to close() it. - */ - packlock.fd = -1; if (commit_lock_file(&packlock) < 0) die_errno("unable to overwrite old ref-pack file"); prune_refs(cbdata.ref_to_prune); -- cgit v1.2.3 From b2a8226d6360fc6e19af134236b3dd11f9d2040e Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:39 +0200 Subject: pack_one_ref(): do some cheap tests before a more expensive one Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 00174e9498..717aa54c03 100644 --- a/refs.c +++ b/refs.c @@ -2008,18 +2008,17 @@ static int pack_one_ref(struct ref_entry *entry, void *cb_data) { struct pack_refs_cb_data *cb = cb_data; enum peel_status peel_status; - int is_tag_ref; - - /* Do not pack symbolic or broken refs: */ - if ((entry->flag & REF_ISSYMREF) || !ref_resolves_to_object(entry)) - return 0; - is_tag_ref = !prefixcmp(entry->name, "refs/tags/"); + int is_tag_ref = !prefixcmp(entry->name, "refs/tags/"); /* ALWAYS pack refs that were already packed or are tags */ if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(entry->flag & REF_ISPACKED)) return 0; + /* Do not pack symbolic or broken refs: */ + if ((entry->flag & REF_ISSYMREF) || !ref_resolves_to_object(entry)) + return 0; + peel_status = peel_entry(entry, 1); if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG) die("internal error peeling reference %s (%s)", -- cgit v1.2.3 From 65cf102bb09804929261e1f6cbd2ff5e0af9f301 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:40 +0200 Subject: refs: change do_for_each_*() functions to take ref_cache arguments Change the callers convert submodule names into ref_cache pointers. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 717aa54c03..238670d8fc 100644 --- a/refs.c +++ b/refs.c @@ -1504,16 +1504,15 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname) } /* - * Call fn for each reference in the specified submodule, omitting + * Call fn for each reference in the specified ref_cache, omitting * references not in the containing_dir of base. fn is called for all * references, including broken ones. If fn ever returns a non-zero * value, stop the iteration and return that value; otherwise, return * 0. */ -static int do_for_each_entry(const char *submodule, const char *base, +static int do_for_each_entry(struct ref_cache *refs, const char *base, each_ref_entry_fn fn, void *cb_data) { - struct ref_cache *refs = get_ref_cache(submodule); struct ref_dir *packed_dir = get_packed_refs(refs); struct ref_dir *loose_dir = get_loose_refs(refs); int retval = 0; @@ -1542,7 +1541,7 @@ static int do_for_each_entry(const char *submodule, const char *base, } /* - * Call fn for each reference in the specified submodule for which the + * Call fn for each reference in the specified ref_cache for which the * refname begins with base. If trim is non-zero, then trim that many * characters off the beginning of each refname before passing the * refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to include @@ -1550,8 +1549,8 @@ static int do_for_each_entry(const char *submodule, const char *base, * value, stop the iteration and return that value; otherwise, return * 0. */ -static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn, - int trim, int flags, void *cb_data) +static int do_for_each_ref(struct ref_cache *refs, const char *base, + each_ref_fn fn, int trim, int flags, void *cb_data) { struct ref_entry_cb data; data.base = base; @@ -1560,7 +1559,7 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn data.fn = fn; data.cb_data = cb_data; - return do_for_each_entry(submodule, base, do_one_ref, &data); + return do_for_each_entry(refs, base, do_one_ref, &data); } static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data) @@ -1593,23 +1592,23 @@ int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data) int for_each_ref(each_ref_fn fn, void *cb_data) { - return do_for_each_ref(NULL, "", fn, 0, 0, cb_data); + return do_for_each_ref(get_ref_cache(NULL), "", fn, 0, 0, cb_data); } int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data) { - return do_for_each_ref(submodule, "", fn, 0, 0, cb_data); + return do_for_each_ref(get_ref_cache(submodule), "", fn, 0, 0, cb_data); } int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data) { - return do_for_each_ref(NULL, prefix, fn, strlen(prefix), 0, cb_data); + return do_for_each_ref(get_ref_cache(NULL), prefix, fn, strlen(prefix), 0, cb_data); } int for_each_ref_in_submodule(const char *submodule, const char *prefix, each_ref_fn fn, void *cb_data) { - return do_for_each_ref(submodule, prefix, fn, strlen(prefix), 0, cb_data); + return do_for_each_ref(get_ref_cache(submodule), prefix, fn, strlen(prefix), 0, cb_data); } int for_each_tag_ref(each_ref_fn fn, void *cb_data) @@ -1644,7 +1643,7 @@ int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void *c int for_each_replace_ref(each_ref_fn fn, void *cb_data) { - return do_for_each_ref(NULL, "refs/replace/", fn, 13, 0, cb_data); + return do_for_each_ref(get_ref_cache(NULL), "refs/replace/", fn, 13, 0, cb_data); } int head_ref_namespaced(each_ref_fn fn, void *cb_data) @@ -1667,7 +1666,7 @@ int for_each_namespaced_ref(each_ref_fn fn, void *cb_data) struct strbuf buf = STRBUF_INIT; int ret; strbuf_addf(&buf, "%srefs/", get_git_namespace()); - ret = do_for_each_ref(NULL, buf.buf, fn, 0, 0, cb_data); + ret = do_for_each_ref(get_ref_cache(NULL), buf.buf, fn, 0, 0, cb_data); strbuf_release(&buf); return ret; } @@ -1709,7 +1708,7 @@ int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data) int for_each_rawref(each_ref_fn fn, void *cb_data) { - return do_for_each_ref(NULL, "", fn, 0, + return do_for_each_ref(get_ref_cache(NULL), "", fn, 0, DO_FOR_EACH_INCLUDE_BROKEN, cb_data); } @@ -2104,7 +2103,7 @@ int pack_refs(unsigned int flags) write_or_die(cbdata.fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER)); - do_for_each_entry(NULL, "", pack_one_ref, &cbdata); + do_for_each_entry(get_ref_cache(NULL), "", pack_one_ref, &cbdata); if (commit_lock_file(&packlock) < 0) die_errno("unable to overwrite old ref-pack file"); prune_refs(cbdata.ref_to_prune); -- cgit v1.2.3 From 9da31cb027aa5dac3d4914a88faa8830f0578c88 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 22 Apr 2013 21:52:41 +0200 Subject: refs: handle the main ref_cache specially Hold the ref_cache instance for the main repository in a dedicated, statically-allocated instance to avoid the need for a function call and a linked-list traversal when it is needed. Suggested by: Heiko Voigt Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs.c | 60 +++++++++++++++++++++++++++++++----------------------------- 1 file changed, 31 insertions(+), 29 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 238670d8fc..fa53b39c28 100644 --- a/refs.c +++ b/refs.c @@ -811,9 +811,13 @@ static struct ref_cache { struct ref_cache *next; struct ref_entry *loose; struct ref_entry *packed; - /* The submodule name, or "" for the main repo. */ - char name[FLEX_ARRAY]; -} *ref_cache; + /* + * The submodule name, or "" for the main repo. We allocate + * length 1 rather than FLEX_ARRAY so that the main ref_cache + * is initialized correctly. + */ + char name[1]; +} ref_cache, *submodule_ref_caches; static void clear_packed_ref_cache(struct ref_cache *refs) { @@ -851,18 +855,18 @@ static struct ref_cache *create_ref_cache(const char *submodule) */ static struct ref_cache *get_ref_cache(const char *submodule) { - struct ref_cache *refs = ref_cache; - if (!submodule) - submodule = ""; - while (refs) { + struct ref_cache *refs; + + if (!submodule || !*submodule) + return &ref_cache; + + for (refs = submodule_ref_caches; refs; refs = refs->next) if (!strcmp(submodule, refs->name)) return refs; - refs = refs->next; - } refs = create_ref_cache(submodule); - refs->next = ref_cache; - ref_cache = refs; + refs->next = submodule_ref_caches; + submodule_ref_caches = refs; return refs; } @@ -1011,8 +1015,8 @@ static struct ref_dir *get_packed_refs(struct ref_cache *refs) void add_packed_ref(const char *refname, const unsigned char *sha1) { - add_ref(get_packed_refs(get_ref_cache(NULL)), - create_ref_entry(refname, sha1, REF_ISPACKED, 1)); + add_ref(get_packed_refs(&ref_cache), + create_ref_entry(refname, sha1, REF_ISPACKED, 1)); } /* @@ -1187,7 +1191,7 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sh */ static struct ref_entry *get_packed_ref(const char *refname) { - return find_ref(get_packed_refs(get_ref_cache(NULL)), refname); + return find_ref(get_packed_refs(&ref_cache), refname); } const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int reading, int *flag) @@ -1592,7 +1596,7 @@ int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data) int for_each_ref(each_ref_fn fn, void *cb_data) { - return do_for_each_ref(get_ref_cache(NULL), "", fn, 0, 0, cb_data); + return do_for_each_ref(&ref_cache, "", fn, 0, 0, cb_data); } int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data) @@ -1602,7 +1606,7 @@ int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data) int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data) { - return do_for_each_ref(get_ref_cache(NULL), prefix, fn, strlen(prefix), 0, cb_data); + return do_for_each_ref(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data); } int for_each_ref_in_submodule(const char *submodule, const char *prefix, @@ -1643,7 +1647,7 @@ int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void *c int for_each_replace_ref(each_ref_fn fn, void *cb_data) { - return do_for_each_ref(get_ref_cache(NULL), "refs/replace/", fn, 13, 0, cb_data); + return do_for_each_ref(&ref_cache, "refs/replace/", fn, 13, 0, cb_data); } int head_ref_namespaced(each_ref_fn fn, void *cb_data) @@ -1666,7 +1670,7 @@ int for_each_namespaced_ref(each_ref_fn fn, void *cb_data) struct strbuf buf = STRBUF_INIT; int ret; strbuf_addf(&buf, "%srefs/", get_git_namespace()); - ret = do_for_each_ref(get_ref_cache(NULL), buf.buf, fn, 0, 0, cb_data); + ret = do_for_each_ref(&ref_cache, buf.buf, fn, 0, 0, cb_data); strbuf_release(&buf); return ret; } @@ -1708,7 +1712,7 @@ int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data) int for_each_rawref(each_ref_fn fn, void *cb_data) { - return do_for_each_ref(get_ref_cache(NULL), "", fn, 0, + return do_for_each_ref(&ref_cache, "", fn, 0, DO_FOR_EACH_INCLUDE_BROKEN, cb_data); } @@ -1914,7 +1918,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, * name is a proper prefix of our refname. */ if (missing && - !is_refname_available(refname, NULL, get_packed_refs(get_ref_cache(NULL)))) { + !is_refname_available(refname, NULL, get_packed_refs(&ref_cache))) { last_errno = ENOTDIR; goto error_return; } @@ -2103,7 +2107,7 @@ int pack_refs(unsigned int flags) write_or_die(cbdata.fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER)); - do_for_each_entry(get_ref_cache(NULL), "", pack_one_ref, &cbdata); + do_for_each_entry(&ref_cache, "", pack_one_ref, &cbdata); if (commit_lock_file(&packlock) < 0) die_errno("unable to overwrite old ref-pack file"); prune_refs(cbdata.ref_to_prune); @@ -2161,7 +2165,6 @@ static int repack_ref_fn(struct ref_entry *entry, void *cb_data) static int repack_without_ref(const char *refname) { int fd; - struct ref_cache *refs = get_ref_cache(NULL); struct ref_dir *packed; if (!get_packed_ref(refname)) @@ -2172,8 +2175,8 @@ static int repack_without_ref(const char *refname) unable_to_lock_error(git_path("packed-refs"), errno); return error("cannot delete '%s' from packed refs", refname); } - clear_packed_ref_cache(refs); - packed = get_packed_refs(refs); + clear_packed_ref_cache(&ref_cache); + packed = get_packed_refs(&ref_cache); /* Remove refname from the cache. */ if (remove_entry(packed, refname) == -1) { /* @@ -2213,7 +2216,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) ret |= repack_without_ref(lock->ref_name); unlink_or_warn(git_path("logs/%s", lock->ref_name)); - clear_loose_ref_cache(get_ref_cache(NULL)); + clear_loose_ref_cache(&ref_cache); unlock_ref(lock); return ret; } @@ -2235,7 +2238,6 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms struct stat loginfo; int log = !lstat(git_path("logs/%s", oldrefname), &loginfo); const char *symref = NULL; - struct ref_cache *refs = get_ref_cache(NULL); if (log && S_ISLNK(loginfo.st_mode)) return error("reflog for %s is a symlink", oldrefname); @@ -2247,10 +2249,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms if (!symref) return error("refname %s not found", oldrefname); - if (!is_refname_available(newrefname, oldrefname, get_packed_refs(refs))) + if (!is_refname_available(newrefname, oldrefname, get_packed_refs(&ref_cache))) return 1; - if (!is_refname_available(newrefname, oldrefname, get_loose_refs(refs))) + if (!is_refname_available(newrefname, oldrefname, get_loose_refs(&ref_cache))) return 1; if (log && rename(git_path("logs/%s", oldrefname), git_path(TMP_RENAMED_LOG))) @@ -2506,7 +2508,7 @@ int write_ref_sha1(struct ref_lock *lock, unlock_ref(lock); return -1; } - clear_loose_ref_cache(get_ref_cache(NULL)); + clear_loose_ref_cache(&ref_cache); if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 || (strcmp(lock->ref_name, lock->orig_ref_name) && log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) { -- cgit v1.2.3