[PATCH 0/5] refs: remove long-dead code

[PATCH 0/5] refs: remove long-dead code

[Ævar Arnfjörð Bjarmason]

This removes dead code in the refs backend, mainly in the
ref-cache. By line count this is mostly a straightforward follow-up to
9939b33d6a3 (packed-backend: rip out some now-unused code, 2017-09-08)
and 9dd389f3d8d (packed_ref_store: get rid of the `ref_cache`
entirely, 2017-09-25).

Ævar Arnfjörð Bjarmason (5):
  refs.[ch]: remove unused ref_storage_backend_exists()
  refs/ref-cache.[ch]: remove unused remove_entry_from_dir()
  refs/ref-cache.[ch]: remove unused add_ref_entry()
  refs/ref-cache.c: remove "mkdir" parameter from find_containing_dir()
  refs/ref-cache.[ch]: remove "incomplete" from create_dir_entry()

 refs.c               |  5 ---
 refs.h               |  2 -
 refs/files-backend.c |  6 +--
 refs/ref-cache.c     | 87 ++++++++------------------------------------
 refs/ref-cache.h     | 26 +------------
 5 files changed, 19 insertions(+), 107 deletions(-)

-- 
2.33.0.1340.ge9f77250f2b

[PATCH 1/5] refs.[ch]: remove unused ref_storage_backend_exists()

[Ævar Arnfjörð Bjarmason]

This function was added in 3dce444f178 (refs: add a backend method
structure, 2016-09-04), but has never been used by anything. The only
caller that might care uses find_ref_storage_backend() directly.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 refs.c | 5 -----
 refs.h | 2 --
 2 files changed, 7 deletions(-)

diff --git a/refs.c b/refs.c
index 8b9f7c3a80a..32349a5585f 100644
--- a/refs.c
+++ b/refs.c
@@ -33,11 +33,6 @@ static struct ref_storage_be *find_ref_storage_backend(const char *name)
 	return NULL;
 }
 
-int ref_storage_backend_exists(const char *name)
-{
-	return find_ref_storage_backend(name) != NULL;
-}
-
 /*
  * How to handle various characters in refnames:
  * 0: An acceptable character for refs
diff --git a/refs.h b/refs.h
index 48970dfc7e0..573b69863d3 100644
--- a/refs.h
+++ b/refs.h
@@ -836,8 +836,6 @@ int reflog_expire(const char *refname, const struct object_id *oid,
 		  reflog_expiry_cleanup_fn cleanup_fn,
 		  void *policy_cb_data);
 
-int ref_storage_backend_exists(const char *name);
-
 struct ref_store *get_main_ref_store(struct repository *r);
 
 /**
-- 
2.33.0.1340.ge9f77250f2b

[PATCH 2/5] refs/ref-cache.[ch]: remove unused remove_entry_from_dir()

[Ævar Arnfjörð Bjarmason]

This function was missed in 9939b33d6a3 (packed-backend: rip out some
now-unused code, 2017-09-08), and has been orphaned since then. Let's
delete it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 refs/ref-cache.c | 35 -----------------------------------
 refs/ref-cache.h | 16 ----------------
 2 files changed, 51 deletions(-)

diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 49d732f6db9..e0feebf628f 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -212,41 +212,6 @@ struct ref_entry *find_ref_entry(struct ref_dir *dir, const char *refname)
 	return (entry->flag & REF_DIR) ? NULL : entry;
 }
 
-int remove_entry_from_dir(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];
-
-	MOVE_ARRAY(&dir->entries[entry_index],
-		   &dir->entries[entry_index + 1], dir->nr - entry_index - 1);
-	dir->nr--;
-	if (dir->sorted > entry_index)
-		dir->sorted--;
-	free_ref_entry(entry);
-	return dir->nr;
-}
-
 int add_ref_entry(struct ref_dir *dir, struct ref_entry *ref)
 {
 	dir = find_containing_dir(dir, ref->name, 1);
diff --git a/refs/ref-cache.h b/refs/ref-cache.h
index 3bfb89d2b34..bd1ff578ea2 100644
--- a/refs/ref-cache.h
+++ b/refs/ref-cache.h
@@ -199,22 +199,6 @@ void free_ref_cache(struct ref_cache *cache);
  */
 void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *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.
- */
-int remove_entry_from_dir(struct ref_dir *dir, const char *refname);
-
 /*
  * Add a ref_entry to the ref_dir (unsorted), recursing into
  * subdirectories as necessary.  dir must represent the top-level
-- 
2.33.0.1340.ge9f77250f2b

[PATCH 3/5] refs/ref-cache.[ch]: remove unused add_ref_entry()

[Ævar Arnfjörð Bjarmason]

This function has not been used since 9dd389f3d8d (packed_ref_store:
get rid of the `ref_cache` entirely, 2017-09-25).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 refs/ref-cache.c | 9 ---------
 refs/ref-cache.h | 7 -------
 2 files changed, 16 deletions(-)

diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index e0feebf628f..a28883768c9 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -212,15 +212,6 @@ struct ref_entry *find_ref_entry(struct ref_dir *dir, const char *refname)
 	return (entry->flag & REF_DIR) ? NULL : entry;
 }
 
-int add_ref_entry(struct ref_dir *dir, struct ref_entry *ref)
-{
-	dir = find_containing_dir(dir, ref->name, 1);
-	if (!dir)
-		return -1;
-	add_entry_to_dir(dir, ref);
-	return 0;
-}
-
 /*
  * Emit a warning and return true iff ref1 and ref2 have the same name
  * and the same oid. Die if they have the same name but different
diff --git a/refs/ref-cache.h b/refs/ref-cache.h
index bd1ff578ea2..580d4038f62 100644
--- a/refs/ref-cache.h
+++ b/refs/ref-cache.h
@@ -199,13 +199,6 @@ void free_ref_cache(struct ref_cache *cache);
  */
 void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);
 
-/*
- * Add a ref_entry to the ref_dir (unsorted), recursing into
- * subdirectories as necessary.  dir must represent the top-level
- * directory.  Return 0 on success.
- */
-int add_ref_entry(struct ref_dir *dir, struct ref_entry *ref);
-
 /*
  * Find the value entry with the given name in dir, sorting ref_dirs
  * and recursing into subdirectories as necessary.  If the name is not
-- 
2.33.0.1340.ge9f77250f2b

[PATCH 4/5] refs/ref-cache.c: remove "mkdir" parameter from find_containing_dir()

[Ævar Arnfjörð Bjarmason]

Remove the "mkdir" parameter from the find_containing_dir() function,
the add_ref_entry() function removed in the preceding commit was its
last user.

Since "mkdir" is always "0" we can also remove the parameter from
search_for_subdir(), which in turn means that we can delete most of
that function.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 refs/ref-cache.c | 36 ++++++++++++------------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index a28883768c9..73b338f5ff2 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -144,30 +144,19 @@ int search_ref_dir(struct ref_dir *dir, const char *refname, size_t len)
 /*
  * Search for a directory entry directly within dir (without
  * recursing).  Sort dir if necessary.  subdirname must be a directory
- * name (i.e., end in '/').  If mkdir is set, then create the
- * directory if it is missing; otherwise, return NULL if the desired
+ * name (i.e., end in '/'). Returns NULL if the desired
  * directory cannot be found.  dir must already be complete.
  */
 static struct ref_dir *search_for_subdir(struct ref_dir *dir,
-					 const char *subdirname, size_t len,
-					 int mkdir)
+					 const char *subdirname, size_t len)
 {
 	int entry_index = search_ref_dir(dir, subdirname, len);
 	struct ref_entry *entry;
-	if (entry_index == -1) {
-		if (!mkdir)
-			return NULL;
-		/*
-		 * Since dir is complete, the absence of a subdir
-		 * means that the subdir really doesn't exist;
-		 * therefore, create an empty record for it but mark
-		 * the record complete.
-		 */
-		entry = create_dir_entry(dir->cache, subdirname, len, 0);
-		add_entry_to_dir(dir, entry);
-	} else {
-		entry = dir->entries[entry_index];
-	}
+
+	if (entry_index == -1)
+		return NULL;
+
+	entry = dir->entries[entry_index];
 	return get_ref_dir(entry);
 }
 
@@ -176,18 +165,17 @@ static struct ref_dir *search_for_subdir(struct ref_dir *dir,
  * tree that should hold refname. If refname is a directory name
  * (i.e., it ends in '/'), then return that ref_dir itself. dir must
  * represent the top-level directory and must already be complete.
- * Sort ref_dirs and recurse into subdirectories as necessary. If
- * mkdir is set, then create any missing directories; otherwise,
+ * Sort ref_dirs and recurse into subdirectories as necessary. Will
  * return NULL if the desired directory cannot be found.
  */
 static struct ref_dir *find_containing_dir(struct ref_dir *dir,
-					   const char *refname, int mkdir)
+					   const char *refname)
 {
 	const char *slash;
 	for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {
 		size_t dirnamelen = slash - refname + 1;
 		struct ref_dir *subdir;
-		subdir = search_for_subdir(dir, refname, dirnamelen, mkdir);
+		subdir = search_for_subdir(dir, refname, dirnamelen);
 		if (!subdir) {
 			dir = NULL;
 			break;
@@ -202,7 +190,7 @@ struct ref_entry *find_ref_entry(struct ref_dir *dir, const char *refname)
 {
 	int entry_index;
 	struct ref_entry *entry;
-	dir = find_containing_dir(dir, refname, 0);
+	dir = find_containing_dir(dir, refname);
 	if (!dir)
 		return NULL;
 	entry_index = search_ref_dir(dir, refname, strlen(refname));
@@ -478,7 +466,7 @@ struct ref_iterator *cache_ref_iterator_begin(struct ref_cache *cache,
 
 	dir = get_ref_dir(cache->root);
 	if (prefix && *prefix)
-		dir = find_containing_dir(dir, prefix, 0);
+		dir = find_containing_dir(dir, prefix);
 	if (!dir)
 		/* There's nothing to iterate over. */
 		return empty_ref_iterator_begin();
-- 
2.33.0.1340.ge9f77250f2b

[PATCH 5/5] refs/ref-cache.[ch]: remove "incomplete" from create_dir_entry()

[Ævar Arnfjörð Bjarmason]

Remove the now-unused "incomplete" parameter from create_dir_entry(),
all its callers specify it as "1", so let's drop the "incomplete=0"
case. The last caller to use it was search_for_subdir(), but that code
was removed in the preceding commit.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 refs/files-backend.c | 6 +++---
 refs/ref-cache.c     | 7 +++----
 refs/ref-cache.h     | 3 +--
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 74c03858736..8fa328108ac 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -227,7 +227,7 @@ static void add_per_worktree_entries_to_dir(struct ref_dir *dir, const char *dir
 		pos = search_ref_dir(dir, prefix, prefix_len);
 		if (pos >= 0)
 			continue;
-		child_entry = create_dir_entry(dir->cache, prefix, prefix_len, 1);
+		child_entry = create_dir_entry(dir->cache, prefix, prefix_len);
 		add_entry_to_dir(dir, child_entry);
 	}
 }
@@ -278,7 +278,7 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
 			strbuf_addch(&refname, '/');
 			add_entry_to_dir(dir,
 					 create_dir_entry(dir->cache, refname.buf,
-							  refname.len, 1));
+							  refname.len));
 		} else {
 			if (!refs_resolve_ref_unsafe(&refs->base,
 						     refname.buf,
@@ -336,7 +336,7 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)
 		 * lazily):
 		 */
 		add_entry_to_dir(get_ref_dir(refs->loose->root),
-				 create_dir_entry(refs->loose, "refs/", 5, 1));
+				 create_dir_entry(refs->loose, "refs/", 5));
 	}
 	return refs->loose;
 }
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 73b338f5ff2..a5ad8a39fb4 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -49,7 +49,7 @@ struct ref_cache *create_ref_cache(struct ref_store *refs,
 
 	ret->ref_store = refs;
 	ret->fill_ref_dir = fill_ref_dir;
-	ret->root = create_dir_entry(ret, "", 0, 1);
+	ret->root = create_dir_entry(ret, "", 0);
 	return ret;
 }
 
@@ -86,14 +86,13 @@ static void clear_ref_dir(struct ref_dir *dir)
 }
 
 struct ref_entry *create_dir_entry(struct ref_cache *cache,
-				   const char *dirname, size_t len,
-				   int incomplete)
+				   const char *dirname, size_t len)
 {
 	struct ref_entry *direntry;
 
 	FLEX_ALLOC_MEM(direntry, name, dirname, len);
 	direntry->u.subdir.cache = cache;
-	direntry->flag = REF_DIR | (incomplete ? REF_INCOMPLETE : 0);
+	direntry->flag = REF_DIR | REF_INCOMPLETE;
 	return direntry;
 }
 
diff --git a/refs/ref-cache.h b/refs/ref-cache.h
index 580d4038f62..5c042ae718c 100644
--- a/refs/ref-cache.h
+++ b/refs/ref-cache.h
@@ -169,8 +169,7 @@ struct ref_dir *get_ref_dir(struct ref_entry *entry);
  * "refs/heads/") or "" for the top-level directory.
  */
 struct ref_entry *create_dir_entry(struct ref_cache *cache,
-				   const char *dirname, size_t len,
-				   int incomplete);
+				   const char *dirname, size_t len);
 
 struct ref_entry *create_ref_entry(const char *refname,
 				   const struct object_id *oid, int flag);
-- 
2.33.0.1340.ge9f77250f2b

Re: [PATCH 0/5] refs: remove long-dead code

[Jeff King]

On Tue, Sep 28, 2021 at 03:02:19PM +0200, Ævar Arnfjörð Bjarmason wrote:

> This removes dead code in the refs backend, mainly in the
> ref-cache. By line count this is mostly a straightforward follow-up to
> 9939b33d6a3 (packed-backend: rip out some now-unused code, 2017-09-08)
> and 9dd389f3d8d (packed_ref_store: get rid of the `ref_cache`
> entirely, 2017-09-25).

Thanks, all five look good to me.

Reading through I was surprised the loose backend didn't use
add_ref_entry(), but that's because it uses add_entry_to_dir() more
directly. And that also explains why the "incomplete" parameter in the
final patch was always 1: in the loose world, we are always reading as
incrementally as possible.

So patches 2-4 are really just fallout from switching the packed-refs
reading away from ref-cache. Makes sense.

-Peff

Re: [PATCH 0/5] refs: remove long-dead code

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> This removes dead code in the refs backend, mainly in the
> ref-cache. By line count this is mostly a straightforward follow-up to
> 9939b33d6a3 (packed-backend: rip out some now-unused code, 2017-09-08)
> and 9dd389f3d8d (packed_ref_store: get rid of the `ref_cache`
> entirely, 2017-09-25).
>
> Ævar Arnfjörð Bjarmason (5):
>   refs.[ch]: remove unused ref_storage_backend_exists()
>   refs/ref-cache.[ch]: remove unused remove_entry_from_dir()
>   refs/ref-cache.[ch]: remove unused add_ref_entry()
>   refs/ref-cache.c: remove "mkdir" parameter from find_containing_dir()
>   refs/ref-cache.[ch]: remove "incomplete" from create_dir_entry()

Looking good.  Thanks.