[PATCH v4 0/7] Provide API to invalidate refs cache

[PATCH v4 0/7] Provide API to invalidate refs cache

[mhagger]

From: Michael Haggerty <mhagger@alum.mit.edu>

These patches are re-rolled onto the current master and incorporate
Junio's suggestion to rename "struct cached_refs" to "ref_cache".

This patch series provides an API for external code to invalidate the
ref cache that is used internally to refs.c.  It also allows code
*within* refs.c to invalidate only the packed or only the loose refs
for a module/submodule.

IMPORTANT:

I won't myself have time to figure out who, outside of refs.c, has to
*call* invalidate_ref_cache().  The candidates that I know off the top
of my head are git-clone, git-submodule [1], and git-pack-refs.  It
would be great if experts in those areas would insert calls to
invalidate_ref_cache() where needed.

Even better would be if the meddlesome code were changed to use the
refs API.  I'd be happy to help expanding the refs API if needed to
accommodate your needs.

This is why the API for invalidating only packed or loose refs is
private.  After code outside refs.c is changed to use the refs API, it
will get the optimal behavior for free (and at that time
invalidate_ref_cache() can be removed again).

[1] http://marc.info/?l=git&m=131827641227965&w=2
    In this mailing list thread, Heiko Voigt stated that git-submodule
    does not modify any references, so it should not have to use the
    API.

Michael Haggerty (7):
  invalidate_ref_cache(): rename function from invalidate_cached_refs()
  invalidate_ref_cache(): take the submodule as parameter
  invalidate_ref_cache(): expose this function in the refs API
  clear_ref_cache(): rename parameter
  clear_ref_cache(): extract two new functions
  write_ref_sha1(): only invalidate the loose ref cache
  clear_ref_cache(): inline function

 refs.c |   59 +++++++++++++++++++++++++++++++----------------------------
 refs.h |    8 ++++++++
 2 files changed, 39 insertions(+), 28 deletions(-)

-- 
1.7.7.rc2

[PATCH v4 1/7] invalidate_ref_cache(): rename function from invalidate_cached_refs()

[mhagger]

From: Michael Haggerty <mhagger@alum.mit.edu>

It is the cache that is being invalidated, not the references, and the
new name makes this unambiguous.  Rename other items analogously:

* struct cached_refs -> struct ref_cache
* cached_refs (the variable) -> ref_cache
* clear_cached_refs() -> clear_ref_cache()
* create_cached_refs() -> create_ref_cache()
* get_cached_refs() -> get_ref_cache()

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   40 ++++++++++++++++++++--------------------
 1 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/refs.c b/refs.c
index 9911c97..02b1ef0 100644
--- a/refs.c
+++ b/refs.c
@@ -134,15 +134,15 @@ static struct ref_entry *search_ref_array(struct ref_array *array, const char *n
  * Future: need to be in "struct repository"
  * when doing a full libification.
  */
-static struct cached_refs {
-	struct cached_refs *next;
+static struct ref_cache {
+	struct ref_cache *next;
 	char did_loose;
 	char did_packed;
 	struct ref_array loose;
 	struct ref_array packed;
 	/* The submodule name, or "" for the main repo. */
 	char name[FLEX_ARRAY];
-} *cached_refs;
+} *ref_cache;
 
 static struct ref_entry *current_ref;
 
@@ -158,7 +158,7 @@ static void free_ref_array(struct ref_array *array)
 	array->refs = NULL;
 }
 
-static void clear_cached_refs(struct cached_refs *ca)
+static void clear_ref_cache(struct ref_cache *ca)
 {
 	if (ca->did_loose)
 		free_ref_array(&ca->loose);
@@ -167,27 +167,27 @@ static void clear_cached_refs(struct cached_refs *ca)
 	ca->did_loose = ca->did_packed = 0;
 }
 
-static struct cached_refs *create_cached_refs(const char *submodule)
+static struct ref_cache *create_ref_cache(const char *submodule)
 {
 	int len;
-	struct cached_refs *refs;
+	struct ref_cache *refs;
 	if (!submodule)
 		submodule = "";
 	len = strlen(submodule) + 1;
-	refs = xcalloc(1, sizeof(struct cached_refs) + len);
+	refs = xcalloc(1, sizeof(struct ref_cache) + len);
 	memcpy(refs->name, submodule, len);
 	return refs;
 }
 
 /*
- * Return a pointer to a cached_refs for the specified submodule. For
+ * Return a pointer to a ref_cache for the specified submodule. For
  * the main repository, use submodule==NULL. The returned structure
  * will be allocated and initialized but not necessarily populated; it
  * should not be freed.
  */
-static struct cached_refs *get_cached_refs(const char *submodule)
+static struct ref_cache *get_ref_cache(const char *submodule)
 {
-	struct cached_refs *refs = cached_refs;
+	struct ref_cache *refs = ref_cache;
 	if (!submodule)
 		submodule = "";
 	while (refs) {
@@ -196,17 +196,17 @@ static struct cached_refs *get_cached_refs(const char *submodule)
 		refs = refs->next;
 	}
 
-	refs = create_cached_refs(submodule);
-	refs->next = cached_refs;
-	cached_refs = refs;
+	refs = create_ref_cache(submodule);
+	refs->next = ref_cache;
+	ref_cache = refs;
 	return refs;
 }
 
-static void invalidate_cached_refs(void)
+static void invalidate_ref_cache(void)
 {
-	struct cached_refs *refs = cached_refs;
+	struct ref_cache *refs = ref_cache;
 	while (refs) {
-		clear_cached_refs(refs);
+		clear_ref_cache(refs);
 		refs = refs->next;
 	}
 }
@@ -257,7 +257,7 @@ void clear_extra_refs(void)
 
 static struct ref_array *get_packed_refs(const char *submodule)
 {
-	struct cached_refs *refs = get_cached_refs(submodule);
+	struct ref_cache *refs = get_ref_cache(submodule);
 
 	if (!refs->did_packed) {
 		const char *packed_refs_file;
@@ -379,7 +379,7 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
 
 static struct ref_array *get_loose_refs(const char *submodule)
 {
-	struct cached_refs *refs = get_cached_refs(submodule);
+	struct ref_cache *refs = get_ref_cache(submodule);
 
 	if (!refs->did_loose) {
 		get_ref_dir(submodule, "refs", &refs->loose);
@@ -1228,7 +1228,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 	ret |= repack_without_ref(refname);
 
 	unlink_or_warn(git_path("logs/%s", lock->ref_name));
-	invalidate_cached_refs();
+	invalidate_ref_cache();
 	unlock_ref(lock);
 	return ret;
 }
@@ -1527,7 +1527,7 @@ int write_ref_sha1(struct ref_lock *lock,
 		unlock_ref(lock);
 		return -1;
 	}
-	invalidate_cached_refs();
+	invalidate_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)) {
-- 
1.7.7.rc2

[PATCH v4 2/7] invalidate_ref_cache(): take the submodule as parameter

[mhagger]

From: Michael Haggerty <mhagger@alum.mit.edu>

Instead of invalidating the ref cache on an all-or-nothing basis,
invalidate the cache for a specific submodule.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/refs.c b/refs.c
index 02b1ef0..68b73aa 100644
--- a/refs.c
+++ b/refs.c
@@ -202,13 +202,9 @@ static struct ref_cache *get_ref_cache(const char *submodule)
 	return refs;
 }
 
-static void invalidate_ref_cache(void)
+static void invalidate_ref_cache(const char *submodule)
 {
-	struct ref_cache *refs = ref_cache;
-	while (refs) {
-		clear_ref_cache(refs);
-		refs = refs->next;
-	}
+	clear_ref_cache(get_ref_cache(submodule));
 }
 
 static void read_packed_refs(FILE *f, struct ref_array *array)
@@ -1228,7 +1224,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 	ret |= repack_without_ref(refname);
 
 	unlink_or_warn(git_path("logs/%s", lock->ref_name));
-	invalidate_ref_cache();
+	invalidate_ref_cache(NULL);
 	unlock_ref(lock);
 	return ret;
 }
@@ -1527,7 +1523,7 @@ int write_ref_sha1(struct ref_lock *lock,
 		unlock_ref(lock);
 		return -1;
 	}
-	invalidate_ref_cache();
+	invalidate_ref_cache(NULL);
 	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)) {
-- 
1.7.7.rc2

[PATCH v4 3/7] invalidate_ref_cache(): expose this function in the refs API

[mhagger]

From: Michael Haggerty <mhagger@alum.mit.edu>

Make invalidate_ref_cache() an official part of the refs API.  It is
currently a fact of life that code outside of refs.c mucks about with
references.  This change gives such code a way of informing the refs
module that it should no longer trust its cache.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |    2 +-
 refs.h |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index 68b73aa..ddd799e 100644
--- a/refs.c
+++ b/refs.c
@@ -202,7 +202,7 @@ static struct ref_cache *get_ref_cache(const char *submodule)
 	return refs;
 }
 
-static void invalidate_ref_cache(const char *submodule)
+void invalidate_ref_cache(const char *submodule)
 {
 	clear_ref_cache(get_ref_cache(submodule));
 }
diff --git a/refs.h b/refs.h
index 0229c57..f439c54 100644
--- a/refs.h
+++ b/refs.h
@@ -80,6 +80,14 @@ extern void unlock_ref(struct ref_lock *lock);
 /** Writes sha1 into the ref specified by the lock. **/
 extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);
 
+/*
+ * Invalidate the reference cache for the specified submodule.  Use
+ * submodule=NULL to invalidate the cache for the main module.  This
+ * function must be called if references are changed via a mechanism
+ * other than the refs API.
+ */
+extern void invalidate_ref_cache(const char *submodule);
+
 /** Setup reflog before using. **/
 int log_ref_setup(const char *ref_name, char *logfile, int bufsize);
 
-- 
1.7.7.rc2

[PATCH v4 4/7] clear_ref_cache(): rename parameter

[mhagger]

From: Michael Haggerty <mhagger@alum.mit.edu>

...for consistency with the rest of this module.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index ddd799e..623d673 100644
--- a/refs.c
+++ b/refs.c
@@ -158,13 +158,13 @@ static void free_ref_array(struct ref_array *array)
 	array->refs = NULL;
 }
 
-static void clear_ref_cache(struct ref_cache *ca)
+static void clear_ref_cache(struct ref_cache *refs)
 {
-	if (ca->did_loose)
-		free_ref_array(&ca->loose);
-	if (ca->did_packed)
-		free_ref_array(&ca->packed);
-	ca->did_loose = ca->did_packed = 0;
+	if (refs->did_loose)
+		free_ref_array(&refs->loose);
+	if (refs->did_packed)
+		free_ref_array(&refs->packed);
+	refs->did_loose = refs->did_packed = 0;
 }
 
 static struct ref_cache *create_ref_cache(const char *submodule)
-- 
1.7.7.rc2

[PATCH v4 5/7] clear_ref_cache(): extract two new functions

[mhagger]

From: Michael Haggerty <mhagger@alum.mit.edu>

Extract two new functions from clear_cached_refs():
clear_loose_ref_cache() and clear_packed_ref_cache().

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index 623d673..2a21dc3 100644
--- a/refs.c
+++ b/refs.c
@@ -158,13 +158,24 @@ static void free_ref_array(struct ref_array *array)
 	array->refs = NULL;
 }
 
-static void clear_ref_cache(struct ref_cache *refs)
+static void clear_packed_ref_cache(struct ref_cache *refs)
 {
-	if (refs->did_loose)
-		free_ref_array(&refs->loose);
 	if (refs->did_packed)
 		free_ref_array(&refs->packed);
-	refs->did_loose = refs->did_packed = 0;
+	refs->did_packed = 0;
+}
+
+static void clear_loose_ref_cache(struct ref_cache *refs)
+{
+	if (refs->did_loose)
+		free_ref_array(&refs->loose);
+	refs->did_loose = 0;
+}
+
+static void clear_ref_cache(struct ref_cache *refs)
+{
+	clear_packed_ref_cache(refs);
+	clear_loose_ref_cache(refs);
 }
 
 static struct ref_cache *create_ref_cache(const char *submodule)
-- 
1.7.7.rc2

[PATCH v4 6/7] write_ref_sha1(): only invalidate the loose ref cache

[mhagger]

From: Michael Haggerty <mhagger@alum.mit.edu>

Since write_ref_sha1() can only write loose refs and cannot write
symbolic refs, there is no need for it to invalidate the packed ref
cache.

Suggested by: Martin Fick <mfick@codeaurora.org>

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index 2a21dc3..a888cea 100644
--- a/refs.c
+++ b/refs.c
@@ -1534,7 +1534,7 @@ int write_ref_sha1(struct ref_lock *lock,
 		unlock_ref(lock);
 		return -1;
 	}
-	invalidate_ref_cache(NULL);
+	clear_loose_ref_cache(get_ref_cache(NULL));
 	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)) {
-- 
1.7.7.rc2

[PATCH v4 7/7] clear_ref_cache(): inline function

[mhagger]

From: Michael Haggerty <mhagger@alum.mit.edu>

clear_ref_cache() was only called from one place, so inline it
there.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index a888cea..d8a4fa3 100644
--- a/refs.c
+++ b/refs.c
@@ -172,12 +172,6 @@ static void clear_loose_ref_cache(struct ref_cache *refs)
 	refs->did_loose = 0;
 }
 
-static void clear_ref_cache(struct ref_cache *refs)
-{
-	clear_packed_ref_cache(refs);
-	clear_loose_ref_cache(refs);
-}
-
 static struct ref_cache *create_ref_cache(const char *submodule)
 {
 	int len;
@@ -215,7 +209,9 @@ static struct ref_cache *get_ref_cache(const char *submodule)
 
 void invalidate_ref_cache(const char *submodule)
 {
-	clear_ref_cache(get_ref_cache(submodule));
+	struct ref_cache *refs = get_ref_cache(submodule);
+	clear_packed_ref_cache(refs);
+	clear_loose_ref_cache(refs);
 }
 
 static void read_packed_refs(FILE *f, struct ref_array *array)
-- 
1.7.7.rc2

Re: [PATCH v4 0/7] Provide API to invalidate refs cache

[Junio C Hamano]

Thanks; will replace the topic with these and re-queue.

Re: [PATCH v4 0/7] Provide API to invalidate refs cache

[Heiko Voigt]

Hi,

On Mon, Oct 17, 2011 at 04:38:04AM +0200, mhagger@alum.mit.edu wrote:
> I won't myself have time to figure out who, outside of refs.c, has to
> *call* invalidate_ref_cache().  The candidates that I know off the top
> of my head are git-clone, git-submodule [1], and git-pack-refs.  It
> would be great if experts in those areas would insert calls to
> invalidate_ref_cache() where needed.

[...]

> [1] http://marc.info/?l=git&m=131827641227965&w=2
>     In this mailing list thread, Heiko Voigt stated that git-submodule
>     does not modify any references, so it should not have to use the
>     API.

This is not entirely true. I was saying that my submodule-merge code is
currently the only one using the refs api for submodules and that does
not need to modify submodule refs. I imagine that there will be some
users when submodule support matures (e.g. recursive push).

Cheers Heiko

Re: [PATCH v4 0/7] Provide API to invalidate refs cache

[Michael Haggerty]

On 10/17/2011 08:47 PM, Heiko Voigt wrote:
> On Mon, Oct 17, 2011 at 04:38:04AM +0200, mhagger@alum.mit.edu wrote:
>> [1] http://marc.info/?l=git&m=131827641227965&w=2
>>     In this mailing list thread, Heiko Voigt stated that git-submodule
>>     does not modify any references, so it should not have to use the
>>     API.
> 
> This is not entirely true. I was saying that my submodule-merge code is
> currently the only one using the refs api for submodules and that does
> not need to modify submodule refs. I imagine that there will be some
> users when submodule support matures (e.g. recursive push).

Sorry for misunderstanding/misrepresenting you; thanks for the
clarification.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/