threads / patch / 28699

v4, 7 partsinvalidate_ref_cache(): rename function from invalidate_cached_refs()

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

## tl;dr

11 messages between Oct 17, 2011 and Oct 17, 2011. Diffs are folded; open one to read it.

replies: 10people: 3as markdown or json

mhagger@alum.mit.edu· Oct 17, 2011, 02:38 UTC · lore

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

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

mhagger@alum.mit.edu· Oct 17, 2011, 02:38 UTC · re: mhagger@alum.mit.edu · lore

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(-)

Show changes to refs.c +20 −20

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

mhagger@alum.mit.edu· Oct 17, 2011, 02:38 UTC · re: mhagger@alum.mit.edu · lore

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

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(-)

Show changes to refs.c +4 −8

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

mhagger@alum.mit.edu· Oct 17, 2011, 02:38 UTC · re: mhagger@alum.mit.edu · lore

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

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(-)

Show changes to 2 files +9 −1

refs.c, refs.h

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

mhagger@alum.mit.edu· Oct 17, 2011, 02:38 UTC · re: mhagger@alum.mit.edu · lore

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

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(-)

Show changes to refs.c +6 −6

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

mhagger@alum.mit.edu· Oct 17, 2011, 02:38 UTC · re: mhagger@alum.mit.edu · lore

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

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(-)

Show changes to refs.c +15 −4

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

mhagger@alum.mit.edu· Oct 17, 2011, 02:38 UTC · re: mhagger@alum.mit.edu · lore

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

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(-)

Show changes to refs.c +1 −1

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

mhagger@alum.mit.edu· Oct 17, 2011, 02:38 UTC · re: mhagger@alum.mit.edu · lore

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

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(-)

Show changes to refs.c +3 −7

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

Junio C Hamano· Oct 17, 2011, 05:40 UTC · re: mhagger@alum.mit.edu · lore

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

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

Heiko Voigt· Oct 17, 2011, 18:47 UTC · re: mhagger@alum.mit.edu · lore

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

Hi,

On Mon, Oct 17, 2011 at 04:38:04AM +0200, mhagger@alum.mit.edu wrote:

Show 5 quoted lines

> 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

Michael Haggerty· Oct 17, 2011, 19:43 UTC · re: Heiko Voigt · lore

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

On 10/17/2011 08:47 PM, Heiko Voigt wrote:

Show 10 quoted lines

> 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/