[RFC PATCH 00/19] object-store refactoring 3 (replace objects, main ref store)

[RFC PATCH 00/19] object-store refactoring 3 (replace objects, main ref store)

[Stefan Beller]

This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c
(sb/packfiles-in-repository).
It is also available at https://github.com/stefanbeller/git/tree/object-store-3

This series will bring the replacement mechanism (git replace)
into the object store.

The first patches are cleaning up a bit, and patches 6-19 are converting
one function at a time using the tick-tock pattern with the #define trick.
See cfc62fc98c (sha1_file: add repository argument to link_alt_odb_entry,
2018-03-23) for explanation.

Thanks,
Stefan

Stefan Beller (19):
  replace_object.c: rename to use dash in file name
  replace-object: move replace_object to object store
  object-store: move lookup_replace_object to replace-object.h
  replace-object: move replace objects prepared flag to object store
  replace-object: check_replace_refs is safe in multi repo environment
  refs: add repository argument to get_main_ref_store
  refs: add repository argument to for_each_replace_ref
  replace-object: add repository argument to replace_object_pos
  replace-object: add repository argument to register_replace_object
  replace-object: add repository argument to prepare_replace_object
  replace-object: add repository argument to do_lookup_replace_object
  replace-object: add repository argument to lookup_replace_object
  refs: store the main ref store inside the repository struct
  refs: allow for_each_replace_ref to handle arbitrary repositories
  replace-object: allow replace_object_pos to handle arbitrary
    repositories
  replace-object: allow register_replace_object to handle arbitrary
    repositories
  replace-object: allow prepare_replace_object to handle arbitrary
    repositories
  replace-object: allow do_lookup_replace_object to handle arbitrary
    repositories
  replace-object: allow lookup_replace_object to handle arbitrary
    repositories

 Makefile                             |  2 +-
 builtin/mktag.c                      |  3 +-
 builtin/pack-refs.c                  |  3 +-
 builtin/replace.c                    |  4 +-
 cache.h                              | 19 -------
 environment.c                        |  2 +-
 object-store.h                       | 16 ++++++
 object.c                             |  3 +-
 refs.c                               | 80 ++++++++++++++--------------
 refs.h                               |  4 +-
 replace_object.c => replace-object.c | 66 +++++++++++------------
 replace-object.h                     | 35 ++++++++++++
 repository.h                         |  5 ++
 revision.c                           |  5 +-
 sha1_file.c                          |  7 +--
 streaming.c                          |  3 +-
 t/helper/test-ref-store.c            |  3 +-
 17 files changed, 149 insertions(+), 111 deletions(-)
 rename replace_object.c => replace-object.c (56%)
 create mode 100644 replace-object.h
 
 
 $ git diff 464416a2eaadf84d2bfdf795007863d03b222b7c..HEAD -- object-store.h repository.h
diff --git a/object-store.h b/object-store.h
index fef33f345f..be90c02db6 100644
--- a/object-store.h
+++ b/object-store.h
@@ -93,6 +93,22 @@ struct raw_object_store {
        struct alternate_object_database *alt_odb_list;
        struct alternate_object_database **alt_odb_tail;
 
+       /*
+        * Objects that should be substituted by other objects
+        * (see git-replace(1)).
+        */
+       struct replace_objects {
+               /*
+                * An array of replacements.  The array is kept sorted by the original
+                * sha1.
+                */
+               struct replace_object **items;
+
+               int alloc, nr;
+
+               unsigned prepared : 1;
+       } replacements;
+
        /*
         * private data
         *
diff --git a/repository.h b/repository.h
index 09df94a472..2922d3a28b 100644
--- a/repository.h
+++ b/repository.h
@@ -26,6 +26,11 @@ struct repository {
         */
        struct raw_object_store *objects;
 
+       /*
+        * The store in which the refs are hold.
+        */
+       struct ref_store *main_ref_store;
+
        /*
         * Path to the repository's graft file.
         * Cannot be NULL after initialization.

-- 
2.17.0.484.g0c8726318c-goog

[PATCH 01/19] replace_object.c: rename to use dash in file name

[Stefan Beller]

This is more consistent with the project style. The majority of
Git's source files use dashes in preference to underscores in their file
names.

Noticed while adding a header corresponding to this file.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Makefile                             | 2 +-
 replace_object.c => replace-object.c | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename replace_object.c => replace-object.c (100%)

diff --git a/Makefile b/Makefile
index 5bcd83ddf3..e345e9e75d 100644
--- a/Makefile
+++ b/Makefile
@@ -871,7 +871,7 @@ LIB_OBJS += refs/packed-backend.o
 LIB_OBJS += refs/ref-cache.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
-LIB_OBJS += replace_object.o
+LIB_OBJS += replace-object.o
 LIB_OBJS += repository.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
diff --git a/replace_object.c b/replace-object.c
similarity index 100%
rename from replace_object.c
rename to replace-object.c
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 02/19] replace-object: move replace_object to object store

[Stefan Beller]

Refs belong to particular repositories, so the replacements defined by
them should belong to a particular repository as well.

Move the definition of a single object replacement to a new header
"replace-object.h". While at it replace the hardcoded 20 by GIT_MAX_RAWSZ.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 object-store.h   | 14 ++++++++++++++
 replace-object.c | 40 ++++++++++++++++++----------------------
 replace-object.h |  9 +++++++++
 3 files changed, 41 insertions(+), 22 deletions(-)
 create mode 100644 replace-object.h

diff --git a/object-store.h b/object-store.h
index fef33f345f..da639b3184 100644
--- a/object-store.h
+++ b/object-store.h
@@ -93,6 +93,20 @@ struct raw_object_store {
 	struct alternate_object_database *alt_odb_list;
 	struct alternate_object_database **alt_odb_tail;
 
+	/*
+	 * Objects that should be substituted by other objects
+	 * (see git-replace(1)).
+	 */
+	struct replace_objects {
+		/*
+		 * An array of replacements.  The array is kept sorted by the original
+		 * sha1.
+		 */
+		struct replace_object **items;
+
+		int alloc, nr;
+	} replacements;
+
 	/*
 	 * private data
 	 *
diff --git a/replace-object.c b/replace-object.c
index 3e49965d05..a7eb31026e 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -1,19 +1,11 @@
 #include "cache.h"
+#include "replace-object.h"
+#include "object-store.h"
 #include "sha1-lookup.h"
 #include "refs.h"
+#include "repository.h"
 #include "commit.h"
 
-/*
- * An array of replacements.  The array is kept sorted by the original
- * sha1.
- */
-static struct replace_object {
-	unsigned char original[20];
-	unsigned char replacement[20];
-} **replace_object;
-
-static int replace_object_alloc, replace_object_nr;
-
 static const unsigned char *replace_sha1_access(size_t index, void *table)
 {
 	struct replace_object **replace = table;
@@ -22,7 +14,8 @@ static const unsigned char *replace_sha1_access(size_t index, void *table)
 
 static int replace_object_pos(const unsigned char *sha1)
 {
-	return sha1_pos(sha1, replace_object, replace_object_nr,
+	return sha1_pos(sha1,  the_repository->objects->replacements.items,
+			 the_repository->objects->replacements.nr,
 			replace_sha1_access);
 }
 
@@ -35,18 +28,21 @@ static int register_replace_object(struct replace_object *replace,
 		if (ignore_dups)
 			free(replace);
 		else {
-			free(replace_object[pos]);
-			replace_object[pos] = replace;
+			free( the_repository->objects->replacements.items[pos]);
+			 the_repository->objects->replacements.items[pos] = replace;
 		}
 		return 1;
 	}
 	pos = -pos - 1;
-	ALLOC_GROW(replace_object, replace_object_nr + 1, replace_object_alloc);
-	replace_object_nr++;
-	if (pos < replace_object_nr)
-		MOVE_ARRAY(replace_object + pos + 1, replace_object + pos,
-			   replace_object_nr - pos - 1);
-	replace_object[pos] = replace;
+	ALLOC_GROW( the_repository->objects->replacements.items,
+		    the_repository->objects->replacements.nr + 1,
+		    the_repository->objects->replacements.alloc);
+	 the_repository->objects->replacements.nr++;
+	if (pos <  the_repository->objects->replacements.nr)
+		MOVE_ARRAY( the_repository->objects->replacements.items + pos + 1,
+			    the_repository->objects->replacements.items + pos,
+			    the_repository->objects->replacements.nr - pos - 1);
+	 the_repository->objects->replacements.items[pos] = replace;
 	return 0;
 }
 
@@ -84,7 +80,7 @@ static void prepare_replace_object(void)
 
 	for_each_replace_ref(register_replace_ref, NULL);
 	replace_object_prepared = 1;
-	if (!replace_object_nr)
+	if (!the_repository->objects->replacements.nr)
 		check_replace_refs = 0;
 }
 
@@ -113,7 +109,7 @@ const unsigned char *do_lookup_replace_object(const unsigned char *sha1)
 
 		pos = replace_object_pos(cur);
 		if (0 <= pos)
-			cur = replace_object[pos]->replacement;
+			cur = the_repository->objects->replacements.items[pos]->replacement;
 	} while (0 <= pos);
 
 	return cur;
diff --git a/replace-object.h b/replace-object.h
new file mode 100644
index 0000000000..50731ec9c2
--- /dev/null
+++ b/replace-object.h
@@ -0,0 +1,9 @@
+#ifndef REPLACE_OBJECT_H
+#define REPLACE_OBJECT_H
+
+struct replace_object {
+	unsigned char original[GIT_MAX_RAWSZ];
+	unsigned char replacement[GIT_MAX_RAWSZ];
+};
+
+#endif /* REPLACE_OBJECT_H */
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 02/19] replace-object: move replace_object to object store

[Derrick Stolee]

On 4/6/2018 7:21 PM, Stefan Beller wrote:
> Refs belong to particular repositories, so the replacements defined by
> them should belong to a particular repository as well.
>
> Move the definition of a single object replacement to a new header
> "replace-object.h". While at it replace the hardcoded 20 by GIT_MAX_RAWSZ.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>   object-store.h   | 14 ++++++++++++++
>   replace-object.c | 40 ++++++++++++++++++----------------------
>   replace-object.h |  9 +++++++++
>   3 files changed, 41 insertions(+), 22 deletions(-)
>   create mode 100644 replace-object.h

Throughout this commit, there appears to be an extra space inserted 
before 'the_repository'. Some are more obvious than others (such as a 
'free( the_repository->...)' but others are after the indentation.

> diff --git a/object-store.h b/object-store.h
> index fef33f345f..da639b3184 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -93,6 +93,20 @@ struct raw_object_store {
>   	struct alternate_object_database *alt_odb_list;
>   	struct alternate_object_database **alt_odb_tail;
>   
> +	/*
> +	 * Objects that should be substituted by other objects
> +	 * (see git-replace(1)).
> +	 */
> +	struct replace_objects {
> +		/*
> +		 * An array of replacements.  The array is kept sorted by the original
> +		 * sha1.
> +		 */
> +		struct replace_object **items;
> +
> +		int alloc, nr;
> +	} replacements;
> +
>   	/*
>   	 * private data
>   	 *
> diff --git a/replace-object.c b/replace-object.c
> index 3e49965d05..a7eb31026e 100644
> --- a/replace-object.c
> +++ b/replace-object.c
> @@ -1,19 +1,11 @@
>   #include "cache.h"
> +#include "replace-object.h"
> +#include "object-store.h"
>   #include "sha1-lookup.h"
>   #include "refs.h"
> +#include "repository.h"
>   #include "commit.h"
>   
> -/*
> - * An array of replacements.  The array is kept sorted by the original
> - * sha1.
> - */
> -static struct replace_object {
> -	unsigned char original[20];
> -	unsigned char replacement[20];
> -} **replace_object;
> -
> -static int replace_object_alloc, replace_object_nr;
> -
>   static const unsigned char *replace_sha1_access(size_t index, void *table)
>   {
>   	struct replace_object **replace = table;
> @@ -22,7 +14,8 @@ static const unsigned char *replace_sha1_access(size_t index, void *table)
>   
>   static int replace_object_pos(const unsigned char *sha1)
>   {
> -	return sha1_pos(sha1, replace_object, replace_object_nr,
> +	return sha1_pos(sha1,  the_repository->objects->replacements.items,
> +			 the_repository->objects->replacements.nr,
>   			replace_sha1_access);
>   }
>   
> @@ -35,18 +28,21 @@ static int register_replace_object(struct replace_object *replace,
>   		if (ignore_dups)
>   			free(replace);
>   		else {
> -			free(replace_object[pos]);
> -			replace_object[pos] = replace;
> +			free( the_repository->objects->replacements.items[pos]);
> +			 the_repository->objects->replacements.items[pos] = replace;
>   		}
>   		return 1;
>   	}
>   	pos = -pos - 1;
> -	ALLOC_GROW(replace_object, replace_object_nr + 1, replace_object_alloc);
> -	replace_object_nr++;
> -	if (pos < replace_object_nr)
> -		MOVE_ARRAY(replace_object + pos + 1, replace_object + pos,
> -			   replace_object_nr - pos - 1);
> -	replace_object[pos] = replace;
> +	ALLOC_GROW( the_repository->objects->replacements.items,
> +		    the_repository->objects->replacements.nr + 1,
> +		    the_repository->objects->replacements.alloc);
> +	 the_repository->objects->replacements.nr++;
> +	if (pos <  the_repository->objects->replacements.nr)
> +		MOVE_ARRAY( the_repository->objects->replacements.items + pos + 1,
> +			    the_repository->objects->replacements.items + pos,
> +			    the_repository->objects->replacements.nr - pos - 1);
> +	 the_repository->objects->replacements.items[pos] = replace;
>   	return 0;
>   }
>   
> @@ -84,7 +80,7 @@ static void prepare_replace_object(void)
>   
>   	for_each_replace_ref(register_replace_ref, NULL);
>   	replace_object_prepared = 1;
> -	if (!replace_object_nr)
> +	if (!the_repository->objects->replacements.nr)
>   		check_replace_refs = 0;
>   }
>   
> @@ -113,7 +109,7 @@ const unsigned char *do_lookup_replace_object(const unsigned char *sha1)
>   
>   		pos = replace_object_pos(cur);
>   		if (0 <= pos)
> -			cur = replace_object[pos]->replacement;
> +			cur = the_repository->objects->replacements.items[pos]->replacement;
>   	} while (0 <= pos);
>   
>   	return cur;
> diff --git a/replace-object.h b/replace-object.h
> new file mode 100644
> index 0000000000..50731ec9c2
> --- /dev/null
> +++ b/replace-object.h
> @@ -0,0 +1,9 @@
> +#ifndef REPLACE_OBJECT_H
> +#define REPLACE_OBJECT_H
> +
> +struct replace_object {
> +	unsigned char original[GIT_MAX_RAWSZ];
> +	unsigned char replacement[GIT_MAX_RAWSZ];
> +};
> +
> +#endif /* REPLACE_OBJECT_H */

[PATCH 03/19] object-store: move lookup_replace_object to replace-object.h

[Stefan Beller]

lookup_replace_object is a low-level function that most users of the
object store do not need to use directly.

Move it to replace-object.h to avoid a dependency loop in an upcoming
change to its inline definition that will make use of repository.h.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/mktag.c  |  1 +
 cache.h          | 19 -------------------
 object.c         |  1 +
 replace-object.h | 22 ++++++++++++++++++++++
 sha1_file.c      |  1 +
 streaming.c      |  1 +
 6 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/builtin/mktag.c b/builtin/mktag.c
index 031b750f06..5e40e2152f 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "tag.h"
+#include "replace-object.h"
 
 /*
  * A signature file has a very simple fixed format: four lines
diff --git a/cache.h b/cache.h
index 720664e394..2a238d6398 100644
--- a/cache.h
+++ b/cache.h
@@ -1214,25 +1214,6 @@ static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *
 	return read_sha1_file_extended(sha1, type, size, 1);
 }
 
-/*
- * This internal function is only declared here for the benefit of
- * lookup_replace_object().  Please do not call it directly.
- */
-extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1);
-
-/*
- * If object sha1 should be replaced, return the replacement object's
- * name (replaced recursively, if necessary).  The return value is
- * either sha1 or a pointer to a permanently-allocated value.  When
- * object replacement is suppressed, always return sha1.
- */
-static inline const unsigned char *lookup_replace_object(const unsigned char *sha1)
-{
-	if (!check_replace_refs)
-		return sha1;
-	return do_lookup_replace_object(sha1);
-}
-
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
 extern int sha1_object_info(const unsigned char *, unsigned long *);
 extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1);
diff --git a/object.c b/object.c
index 4c2cf7ff5d..9f381c44eb 100644
--- a/object.c
+++ b/object.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "object.h"
+#include "replace-object.h"
 #include "blob.h"
 #include "tree.h"
 #include "commit.h"
diff --git a/replace-object.h b/replace-object.h
index 50731ec9c2..1e3e8805b9 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -1,9 +1,31 @@
 #ifndef REPLACE_OBJECT_H
 #define REPLACE_OBJECT_H
 
+#include "cache.h"
+#include "repository.h"
+
 struct replace_object {
 	unsigned char original[GIT_MAX_RAWSZ];
 	unsigned char replacement[GIT_MAX_RAWSZ];
 };
 
+/*
+ * This internal function is only declared here for the benefit of
+ * lookup_replace_object().  Please do not call it directly.
+ */
+extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1);
+
+/*
+ * If object sha1 should be replaced, return the replacement object's
+ * name (replaced recursively, if necessary).  The return value is
+ * either sha1 or a pointer to a permanently-allocated value.  When
+ * object replacement is suppressed, always return sha1.
+ */
+static inline const unsigned char *lookup_replace_object(const unsigned char *sha1)
+{
+	if (!check_replace_refs)
+		return sha1;
+	return do_lookup_replace_object(sha1);
+}
+
 #endif /* REPLACE_OBJECT_H */
diff --git a/sha1_file.c b/sha1_file.c
index 314ff55b47..68f1735476 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -23,6 +23,7 @@
 #include "sha1-lookup.h"
 #include "bulk-checkin.h"
 #include "repository.h"
+#include "replace-object.h"
 #include "streaming.h"
 #include "dir.h"
 #include "list.h"
diff --git a/streaming.c b/streaming.c
index 22d27df55e..29632065d0 100644
--- a/streaming.c
+++ b/streaming.c
@@ -5,6 +5,7 @@
 #include "streaming.h"
 #include "repository.h"
 #include "object-store.h"
+#include "replace-object.h"
 #include "packfile.h"
 
 enum input_source {
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 04/19] replace-object: move replace objects prepared flag to object store

[Stefan Beller]

Remove another global variable on the way.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 object-store.h   | 2 ++
 replace-object.c | 6 ++----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/object-store.h b/object-store.h
index da639b3184..be90c02db6 100644
--- a/object-store.h
+++ b/object-store.h
@@ -105,6 +105,8 @@ struct raw_object_store {
 		struct replace_object **items;
 
 		int alloc, nr;
+
+		unsigned prepared : 1;
 	} replacements;
 
 	/*
diff --git a/replace-object.c b/replace-object.c
index a7eb31026e..603d11e931 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -73,13 +73,11 @@ static int register_replace_ref(const char *refname,
 
 static void prepare_replace_object(void)
 {
-	static int replace_object_prepared;
-
-	if (replace_object_prepared)
+	if (the_repository->objects->replacements.prepared)
 		return;
 
 	for_each_replace_ref(register_replace_ref, NULL);
-	replace_object_prepared = 1;
+	the_repository->objects->replacements.prepared = 1;
 	if (!the_repository->objects->replacements.nr)
 		check_replace_refs = 0;
 }
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 05/19] replace-object: check_replace_refs is safe in multi repo environment

[Stefan Beller]

In e1111cef23 (inline lookup_replace_object() calls, 2011-05-15) a shortcut
for checking the object replacement was added by setting check_replace_refs
to 0 once the replacements were evaluated to not exist. This works fine in
with the assumption of only one repository in existence.

The assumption won't hold true any more when we work on multiple instances
of a repository structs (e.g. one struct per submodule), as the first
repository to be inspected may have no replacements and would set the
global variable. Other repositories would then completely omit their
evaluation of replacements.

This reverts back the meaning of the flag `check_replace_refs` of
"Do we need to check with the lookup table?" to "Do we need to read
the replacement definition?", adding the bypassing logic to
lookup_replace_object after the replacement definition was read.
As with the original patch, delay the renaming of the global variable

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 environment.c    | 2 +-
 replace-object.c | 2 --
 replace-object.h | 5 ++++-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/environment.c b/environment.c
index 93c9fbb0ba..29f677c650 100644
--- a/environment.c
+++ b/environment.c
@@ -50,7 +50,7 @@ const char *editor_program;
 const char *askpass_program;
 const char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
-int check_replace_refs = 1;
+int check_replace_refs = 1; /* NEEDSWORK: rename to read_replace_refs */
 char *git_replace_ref_base;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
diff --git a/replace-object.c b/replace-object.c
index 603d11e931..c6d08c2e17 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -78,8 +78,6 @@ static void prepare_replace_object(void)
 
 	for_each_replace_ref(register_replace_ref, NULL);
 	the_repository->objects->replacements.prepared = 1;
-	if (!the_repository->objects->replacements.nr)
-		check_replace_refs = 0;
 }
 
 /* We allow "recursive" replacement. Only within reason, though */
diff --git a/replace-object.h b/replace-object.h
index 1e3e8805b9..f2555cddb9 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -3,6 +3,7 @@
 
 #include "cache.h"
 #include "repository.h"
+#include "object-store.h"
 
 struct replace_object {
 	unsigned char original[GIT_MAX_RAWSZ];
@@ -23,7 +24,9 @@ extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1);
  */
 static inline const unsigned char *lookup_replace_object(const unsigned char *sha1)
 {
-	if (!check_replace_refs)
+	if (!check_replace_refs ||
+	    (the_repository->objects->replacements.prepared &&
+	     the_repository->objects->replacements.nr == 0))
 		return sha1;
 	return do_lookup_replace_object(sha1);
 }
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 06/19] refs: add repository argument to get_main_ref_store

[Stefan Beller]

Add a repository argument to allow the get_main_ref_store caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>

# Conflicts:
#	t/helper/test-ref-store.c
---
 builtin/pack-refs.c       |  3 +-
 refs.c                    | 67 ++++++++++++++++++++-------------------
 refs.h                    |  4 ++-
 revision.c                |  5 +--
 t/helper/test-ref-store.c |  3 +-
 5 files changed, 44 insertions(+), 38 deletions(-)

diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index b106a392a4..f3353564f9 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "parse-options.h"
 #include "refs.h"
+#include "repository.h"
 
 static char const * const pack_refs_usage[] = {
 	N_("git pack-refs [<options>]"),
@@ -17,5 +18,5 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
 	};
 	if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
 		usage_with_options(pack_refs_usage, opts);
-	return refs_pack_refs(get_main_ref_store(), flags);
+	return refs_pack_refs(get_main_ref_store(the_repository), flags);
 }
diff --git a/refs.c b/refs.c
index 20ba82b434..addc9d4934 100644
--- a/refs.c
+++ b/refs.c
@@ -13,6 +13,7 @@
 #include "tag.h"
 #include "submodule.h"
 #include "worktree.h"
+#include "repository.h"
 
 /*
  * List of all available backends
@@ -206,7 +207,7 @@ char *refs_resolve_refdup(struct ref_store *refs,
 char *resolve_refdup(const char *refname, int resolve_flags,
 		     struct object_id *oid, int *flags)
 {
-	return refs_resolve_refdup(get_main_ref_store(),
+	return refs_resolve_refdup(get_main_ref_store(the_repository),
 				   refname, resolve_flags,
 				   oid, flags);
 }
@@ -228,7 +229,7 @@ int refs_read_ref_full(struct ref_store *refs, const char *refname,
 
 int read_ref_full(const char *refname, int resolve_flags, struct object_id *oid, int *flags)
 {
-	return refs_read_ref_full(get_main_ref_store(), refname,
+	return refs_read_ref_full(get_main_ref_store(the_repository), refname,
 				  resolve_flags, oid, flags);
 }
 
@@ -375,7 +376,7 @@ int refs_for_each_tag_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_tag_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_tag_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_tag_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
@@ -385,7 +386,7 @@ int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_da
 
 int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_branch_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_branch_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
@@ -395,7 +396,7 @@ int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_da
 
 int for_each_remote_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_remote_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_remote_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int head_ref_namespaced(each_ref_fn fn, void *cb_data)
@@ -730,7 +731,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
 	struct strbuf err = STRBUF_INIT;
 
 	if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
-		assert(refs == get_main_ref_store());
+		assert(refs == get_main_ref_store(the_repository));
 		return delete_pseudoref(refname, old_oid);
 	}
 
@@ -752,7 +753,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
 int delete_ref(const char *msg, const char *refname,
 	       const struct object_id *old_oid, unsigned int flags)
 {
-	return refs_delete_ref(get_main_ref_store(), msg, refname,
+	return refs_delete_ref(get_main_ref_store(the_repository), msg, refname,
 			       old_oid, flags);
 }
 
@@ -928,7 +929,7 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
 
 struct ref_transaction *ref_transaction_begin(struct strbuf *err)
 {
-	return ref_store_transaction_begin(get_main_ref_store(), err);
+	return ref_store_transaction_begin(get_main_ref_store(the_repository), err);
 }
 
 void ref_transaction_free(struct ref_transaction *transaction)
@@ -1060,7 +1061,7 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
 	int ret = 0;
 
 	if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
-		assert(refs == get_main_ref_store());
+		assert(refs == get_main_ref_store(the_repository));
 		ret = write_pseudoref(refname, new_oid, old_oid, &err);
 	} else {
 		t = ref_store_transaction_begin(refs, &err);
@@ -1099,7 +1100,7 @@ int update_ref(const char *msg, const char *refname,
 	       const struct object_id *old_oid,
 	       unsigned int flags, enum action_on_err onerr)
 {
-	return refs_update_ref(get_main_ref_store(), msg, refname, new_oid,
+	return refs_update_ref(get_main_ref_store(the_repository), msg, refname, new_oid,
 			       old_oid, flags, onerr);
 }
 
@@ -1320,7 +1321,7 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int head_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_head_ref(get_main_ref_store(), fn, cb_data);
+	return refs_head_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 struct ref_iterator *refs_ref_iterator_begin(
@@ -1379,7 +1380,7 @@ int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
@@ -1390,7 +1391,7 @@ int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
 
 int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_ref_in(get_main_ref_store(), prefix, fn, cb_data);
+	return refs_for_each_ref_in(get_main_ref_store(the_repository), prefix, fn, cb_data);
 }
 
 int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)
@@ -1399,7 +1400,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsig
 
 	if (broken)
 		flag = DO_FOR_EACH_INCLUDE_BROKEN;
-	return do_for_each_ref(get_main_ref_store(),
+	return do_for_each_ref(get_main_ref_store(the_repository),
 			       prefix, fn, 0, flag, cb_data);
 }
 
@@ -1416,7 +1417,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 
 int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(get_main_ref_store(),
+	return do_for_each_ref(get_main_ref_store(the_repository),
 			       git_replace_ref_base, fn,
 			       strlen(git_replace_ref_base),
 			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
@@ -1427,7 +1428,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_main_ref_store(),
+	ret = do_for_each_ref(get_main_ref_store(the_repository),
 			      buf.buf, fn, 0, 0, cb_data);
 	strbuf_release(&buf);
 	return ret;
@@ -1441,7 +1442,7 @@ int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_rawref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_rawref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_rawref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_read_raw_ref(struct ref_store *ref_store,
@@ -1547,7 +1548,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 /* backend functions */
 int refs_init_db(struct strbuf *err)
 {
-	struct ref_store *refs = get_main_ref_store();
+	struct ref_store *refs = get_main_ref_store(the_repository);
 
 	return refs->be->init_db(refs, err);
 }
@@ -1555,7 +1556,7 @@ int refs_init_db(struct strbuf *err)
 const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
 			       struct object_id *oid, int *flags)
 {
-	return refs_resolve_ref_unsafe(get_main_ref_store(), refname,
+	return refs_resolve_ref_unsafe(get_main_ref_store(the_repository), refname,
 				       resolve_flags, oid, flags);
 }
 
@@ -1651,7 +1652,7 @@ static struct ref_store *ref_store_init(const char *gitdir,
 	return refs;
 }
 
-struct ref_store *get_main_ref_store(void)
+struct ref_store *get_main_ref_store_the_repository(void)
 {
 	if (main_ref_store)
 		return main_ref_store;
@@ -1726,7 +1727,7 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)
 	const char *id;
 
 	if (wt->is_current)
-		return get_main_ref_store();
+		return get_main_ref_store(the_repository);
 
 	id = wt->id ? wt->id : "/";
 	refs = lookup_ref_store_map(&worktree_ref_stores, id);
@@ -1782,7 +1783,7 @@ int refs_peel_ref(struct ref_store *refs, const char *refname,
 
 int peel_ref(const char *refname, struct object_id *oid)
 {
-	return refs_peel_ref(get_main_ref_store(), refname, oid);
+	return refs_peel_ref(get_main_ref_store(the_repository), refname, oid);
 }
 
 int refs_create_symref(struct ref_store *refs,
@@ -1798,7 +1799,7 @@ int refs_create_symref(struct ref_store *refs,
 int create_symref(const char *ref_target, const char *refs_heads_master,
 		  const char *logmsg)
 {
-	return refs_create_symref(get_main_ref_store(), ref_target,
+	return refs_create_symref(get_main_ref_store(the_repository), ref_target,
 				  refs_heads_master, logmsg);
 }
 
@@ -2006,7 +2007,7 @@ int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_reflog(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_reflog(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_reflog(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_reflog_ent_reverse(struct ref_store *refs,
@@ -2021,7 +2022,7 @@ int refs_for_each_reflog_ent_reverse(struct ref_store *refs,
 int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
 				void *cb_data)
 {
-	return refs_for_each_reflog_ent_reverse(get_main_ref_store(),
+	return refs_for_each_reflog_ent_reverse(get_main_ref_store(the_repository),
 						refname, fn, cb_data);
 }
 
@@ -2034,7 +2035,7 @@ int refs_for_each_reflog_ent(struct ref_store *refs, const char *refname,
 int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn,
 			void *cb_data)
 {
-	return refs_for_each_reflog_ent(get_main_ref_store(), refname,
+	return refs_for_each_reflog_ent(get_main_ref_store(the_repository), refname,
 					fn, cb_data);
 }
 
@@ -2045,7 +2046,7 @@ int refs_reflog_exists(struct ref_store *refs, const char *refname)
 
 int reflog_exists(const char *refname)
 {
-	return refs_reflog_exists(get_main_ref_store(), refname);
+	return refs_reflog_exists(get_main_ref_store(the_repository), refname);
 }
 
 int refs_create_reflog(struct ref_store *refs, const char *refname,
@@ -2057,7 +2058,7 @@ int refs_create_reflog(struct ref_store *refs, const char *refname,
 int safe_create_reflog(const char *refname, int force_create,
 		       struct strbuf *err)
 {
-	return refs_create_reflog(get_main_ref_store(), refname,
+	return refs_create_reflog(get_main_ref_store(the_repository), refname,
 				  force_create, err);
 }
 
@@ -2068,7 +2069,7 @@ int refs_delete_reflog(struct ref_store *refs, const char *refname)
 
 int delete_reflog(const char *refname)
 {
-	return refs_delete_reflog(get_main_ref_store(), refname);
+	return refs_delete_reflog(get_main_ref_store(the_repository), refname);
 }
 
 int refs_reflog_expire(struct ref_store *refs,
@@ -2091,7 +2092,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,
 		  reflog_expiry_cleanup_fn cleanup_fn,
 		  void *policy_cb_data)
 {
-	return refs_reflog_expire(get_main_ref_store(),
+	return refs_reflog_expire(get_main_ref_store(the_repository),
 				  refname, oid, flags,
 				  prepare_fn, should_prune_fn,
 				  cleanup_fn, policy_cb_data);
@@ -2114,7 +2115,7 @@ int refs_delete_refs(struct ref_store *refs, const char *msg,
 int delete_refs(const char *msg, struct string_list *refnames,
 		unsigned int flags)
 {
-	return refs_delete_refs(get_main_ref_store(), msg, refnames, flags);
+	return refs_delete_refs(get_main_ref_store(the_repository), msg, refnames, flags);
 }
 
 int refs_rename_ref(struct ref_store *refs, const char *oldref,
@@ -2125,7 +2126,7 @@ int refs_rename_ref(struct ref_store *refs, const char *oldref,
 
 int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 {
-	return refs_rename_ref(get_main_ref_store(), oldref, newref, logmsg);
+	return refs_rename_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
 }
 
 int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
@@ -2136,5 +2137,5 @@ int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
 
 int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)
 {
-	return refs_copy_existing_ref(get_main_ref_store(), oldref, newref, logmsg);
+	return refs_copy_existing_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
 }
diff --git a/refs.h b/refs.h
index 01be5ae32f..0d013377ce 100644
--- a/refs.h
+++ b/refs.h
@@ -758,7 +758,9 @@ int reflog_expire(const char *refname, const struct object_id *oid,
 
 int ref_storage_backend_exists(const char *name);
 
-struct ref_store *get_main_ref_store(void);
+#define get_main_ref_store(r) \
+	get_main_ref_store_##r()
+struct ref_store *get_main_ref_store_the_repository(void);
 /*
  * Return the ref_store instance for the specified submodule. For the
  * main repository, use submodule==NULL; such a call cannot fail. For
diff --git a/revision.c b/revision.c
index 5ce9b93baa..8fa5a3c7aa 100644
--- a/revision.c
+++ b/revision.c
@@ -6,6 +6,7 @@
 #include "diff.h"
 #include "refs.h"
 #include "revision.h"
+#include "repository.h"
 #include "graph.h"
 #include "grep.h"
 #include "reflog-walk.h"
@@ -1289,7 +1290,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)
 
 	cb.all_revs = revs;
 	cb.all_flags = flags;
-	cb.refs = get_main_ref_store();
+	cb.refs = get_main_ref_store(the_repository);
 	for_each_reflog(handle_one_reflog, &cb);
 
 	if (!revs->single_worktree)
@@ -2182,7 +2183,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 			die("BUG: --single-worktree cannot be used together with submodule");
 		refs = get_submodule_ref_store(submodule);
 	} else
-		refs = get_main_ref_store();
+		refs = get_main_ref_store(the_repository);
 
 	/*
 	 * NOTE!
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 7314b5943e..e8c328ec0e 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -2,6 +2,7 @@
 #include "refs.h"
 #include "worktree.h"
 #include "object-store.h"
+#include "repository.h"
 
 static const char *notnull(const char *arg, const char *name)
 {
@@ -22,7 +23,7 @@ static const char **get_store(const char **argv, struct ref_store **refs)
 	if (!argv[0]) {
 		die("ref store required");
 	} else if (!strcmp(argv[0], "main")) {
-		*refs = get_main_ref_store();
+		*refs = get_main_ref_store(the_repository);
 	} else if (skip_prefix(argv[0], "submodule:", &gitdir)) {
 		struct strbuf sb = STRBUF_INIT;
 		int ret;
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 06/19] refs: add repository argument to get_main_ref_store

[Eric Sunshine]

On Fri, Apr 6, 2018 at 7:21 PM, Stefan Beller <sbeller@google.com> wrote:
> Add a repository argument to allow the get_main_ref_store caller
> to be more specific about which repository to handle. This is a small
> mechanical change; it doesn't change the implementation to handle
> repositories other than the_repository yet.
>
> As with the previous commits, use a macro to catch callers passing a
> repository other than the_repository at compile time.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
>
> # Conflicts:
> #       t/helper/test-ref-store.c

Meh.

Re: [PATCH 06/19] refs: add repository argument to get_main_ref_store

[Stefan Beller]

Hi Eric,

On Fri, Apr 6, 2018 at 11:53 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>
>> # Conflicts:
>> #       t/helper/test-ref-store.c
>
> Meh.

Fixed in a reroll.

[PATCH 07/19] refs: add repository argument to for_each_replace_ref

[Stefan Beller]

Add a repository argument to allow for_each_replace_ref callers to be
more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/replace.c | 4 +++-
 refs.c            | 2 +-
 refs.h            | 4 +++-
 replace-object.c  | 2 +-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 10078ae371..6d27dd73f0 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -14,6 +14,8 @@
 #include "refs.h"
 #include "parse-options.h"
 #include "run-command.h"
+#include "object-store.h"
+#include "repository.h"
 #include "tag.h"
 
 static const char * const git_replace_usage[] = {
@@ -83,7 +85,7 @@ static int list_replace_refs(const char *pattern, const char *format)
 		    "valid formats are 'short', 'medium' and 'long'\n",
 		    format);
 
-	for_each_replace_ref(show_reference, (void *)&data);
+	for_each_replace_ref(the_repository, show_reference, (void *)&data);
 
 	return 0;
 }
diff --git a/refs.c b/refs.c
index addc9d4934..0352cbdcc1 100644
--- a/refs.c
+++ b/refs.c
@@ -1415,7 +1415,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 	return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
 }
 
-int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(get_main_ref_store(the_repository),
 			       git_replace_ref_base, fn,
diff --git a/refs.h b/refs.h
index 0d013377ce..ab3d2bec2f 100644
--- a/refs.h
+++ b/refs.h
@@ -300,7 +300,9 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,
 int for_each_tag_ref(each_ref_fn fn, void *cb_data);
 int for_each_branch_ref(each_ref_fn fn, void *cb_data);
 int for_each_remote_ref(each_ref_fn fn, void *cb_data);
-int for_each_replace_ref(each_ref_fn fn, void *cb_data);
+#define for_each_replace_ref(r, fn, cb) \
+	for_each_replace_ref_##r(fn, cb)
+int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data);
 int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
 int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
 			 const char *prefix, void *cb_data);
diff --git a/replace-object.c b/replace-object.c
index c6d08c2e17..a31e930cfe 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -76,7 +76,7 @@ static void prepare_replace_object(void)
 	if (the_repository->objects->replacements.prepared)
 		return;
 
-	for_each_replace_ref(register_replace_ref, NULL);
+	for_each_replace_ref(the_repository, register_replace_ref, NULL);
 	the_repository->objects->replacements.prepared = 1;
 }
 
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 08/19] replace-object: add repository argument to replace_object_pos

[Stefan Beller]

Add a repository argument to allow the replace_object_pos caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index a31e930cfe..52fc59c4f7 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -12,7 +12,9 @@ static const unsigned char *replace_sha1_access(size_t index, void *table)
 	return replace[index]->original;
 }
 
-static int replace_object_pos(const unsigned char *sha1)
+#define replace_object_pos(r, s) \
+	replace_object_pos_##r(s)
+static int replace_object_pos_the_repository(const unsigned char *sha1)
 {
 	return sha1_pos(sha1,  the_repository->objects->replacements.items,
 			 the_repository->objects->replacements.nr,
@@ -22,7 +24,7 @@ static int replace_object_pos(const unsigned char *sha1)
 static int register_replace_object(struct replace_object *replace,
 				   int ignore_dups)
 {
-	int pos = replace_object_pos(replace->original);
+	int pos = replace_object_pos(the_repository, replace->original);
 
 	if (0 <= pos) {
 		if (ignore_dups)
@@ -103,7 +105,7 @@ const unsigned char *do_lookup_replace_object(const unsigned char *sha1)
 			die("replace depth too high for object %s",
 			    sha1_to_hex(sha1));
 
-		pos = replace_object_pos(cur);
+		pos = replace_object_pos(the_repository, cur);
 		if (0 <= pos)
 			cur = the_repository->objects->replacements.items[pos]->replacement;
 	} while (0 <= pos);
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 09/19] replace-object: add repository argument to register_replace_object

[Stefan Beller]

Add a repository argument to allow the register_replace_object caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index 52fc59c4f7..e9aa964978 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -21,8 +21,10 @@ static int replace_object_pos_the_repository(const unsigned char *sha1)
 			replace_sha1_access);
 }
 
-static int register_replace_object(struct replace_object *replace,
-				   int ignore_dups)
+#define register_replace_object(r, rp, i) \
+	register_replace_object_##r(rp, i)
+static int register_replace_object_the_repository(struct replace_object *replace,
+						  int ignore_dups)
 {
 	int pos = replace_object_pos(the_repository, replace->original);
 
@@ -67,7 +69,7 @@ static int register_replace_ref(const char *refname,
 	hashcpy(repl_obj->replacement, oid->hash);
 
 	/* Register new object */
-	if (register_replace_object(repl_obj, 1))
+	if (register_replace_object(the_repository, repl_obj, 1))
 		die("duplicate replace ref: %s", refname);
 
 	return 0;
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 10/19] replace-object: add repository argument to prepare_replace_object

[Stefan Beller]

Add a repository argument to allow the prepare_replace_object caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index e9aa964978..3a3d971732 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -75,7 +75,9 @@ static int register_replace_ref(const char *refname,
 	return 0;
 }
 
-static void prepare_replace_object(void)
+#define prepare_replace_object(r) \
+	prepare_replace_object_##r()
+static void prepare_replace_object_the_repository(void)
 {
 	if (the_repository->objects->replacements.prepared)
 		return;
@@ -99,7 +101,7 @@ const unsigned char *do_lookup_replace_object(const unsigned char *sha1)
 	int pos, depth = MAXREPLACEDEPTH;
 	const unsigned char *cur = sha1;
 
-	prepare_replace_object();
+	prepare_replace_object(the_repository);
 
 	/* Try to recursively replace the object */
 	do {
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 11/19] replace-object: add repository argument to do_lookup_replace_object

[Stefan Beller]

Add a repository argument to allow the do_lookup_replace_object caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 2 +-
 replace-object.h | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index 3a3d971732..944bcaf242 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -96,7 +96,7 @@ static void prepare_replace_object_the_repository(void)
  * permanently-allocated value.  This function always respects replace
  * references, regardless of the value of check_replace_refs.
  */
-const unsigned char *do_lookup_replace_object(const unsigned char *sha1)
+const unsigned char *do_lookup_replace_object_the_repository(const unsigned char *sha1)
 {
 	int pos, depth = MAXREPLACEDEPTH;
 	const unsigned char *cur = sha1;
diff --git a/replace-object.h b/replace-object.h
index f2555cddb9..dbb128f4c3 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -14,7 +14,8 @@ struct replace_object {
  * This internal function is only declared here for the benefit of
  * lookup_replace_object().  Please do not call it directly.
  */
-extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1);
+#define do_lookup_replace_object(r, s) do_lookup_replace_object_##r(s)
+extern const unsigned char *do_lookup_replace_object_the_repository(const unsigned char *sha1);
 
 /*
  * If object sha1 should be replaced, return the replacement object's
@@ -28,7 +29,7 @@ static inline const unsigned char *lookup_replace_object(const unsigned char *sh
 	    (the_repository->objects->replacements.prepared &&
 	     the_repository->objects->replacements.nr == 0))
 		return sha1;
-	return do_lookup_replace_object(sha1);
+	return do_lookup_replace_object(the_repository, sha1);
 }
 
 #endif /* REPLACE_OBJECT_H */
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 12/19] replace-object: add repository argument to lookup_replace_object

[Stefan Beller]

Add a repository argument to allow callers of lookup_replace_object
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/mktag.c  | 2 +-
 object.c         | 2 +-
 replace-object.h | 3 ++-
 sha1_file.c      | 6 +++---
 streaming.c      | 2 +-
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/mktag.c b/builtin/mktag.c
index 5e40e2152f..3dd937d4ef 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -25,7 +25,7 @@ static int verify_object(const unsigned char *sha1, const char *expected_type)
 	enum object_type type;
 	unsigned long size;
 	void *buffer = read_sha1_file(sha1, &type, &size);
-	const unsigned char *repl = lookup_replace_object(sha1);
+	const unsigned char *repl = lookup_replace_object(the_repository, sha1);
 
 	if (buffer) {
 		if (type == type_from_string(expected_type))
diff --git a/object.c b/object.c
index 9f381c44eb..432a1b2b2a 100644
--- a/object.c
+++ b/object.c
@@ -247,7 +247,7 @@ struct object *parse_object(const struct object_id *oid)
 	unsigned long size;
 	enum object_type type;
 	int eaten;
-	const unsigned char *repl = lookup_replace_object(oid->hash);
+	const unsigned char *repl = lookup_replace_object(the_repository, oid->hash);
 	void *buffer;
 	struct object *obj;
 
diff --git a/replace-object.h b/replace-object.h
index dbb128f4c3..6a2d37d717 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -23,7 +23,8 @@ extern const unsigned char *do_lookup_replace_object_the_repository(const unsign
  * either sha1 or a pointer to a permanently-allocated value.  When
  * object replacement is suppressed, always return sha1.
  */
-static inline const unsigned char *lookup_replace_object(const unsigned char *sha1)
+#define lookup_replace_object(r, s) lookup_replace_object_##r(s)
+static inline const unsigned char *lookup_replace_object_the_repository(const unsigned char *sha1)
 {
 	if (!check_replace_refs ||
 	    (the_repository->objects->replacements.prepared &&
diff --git a/sha1_file.c b/sha1_file.c
index 68f1735476..0dad6c211e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1236,7 +1236,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,
 	struct pack_entry e;
 	int rtype;
 	const unsigned char *real = (flags & OBJECT_INFO_LOOKUP_REPLACE) ?
-				    lookup_replace_object(sha1) :
+				    lookup_replace_object(the_repository, sha1) :
 				    sha1;
 	int already_retried = 0;
 
@@ -1374,8 +1374,8 @@ void *read_sha1_file_extended(const unsigned char *sha1,
 	const struct packed_git *p;
 	const char *path;
 	struct stat st;
-	const unsigned char *repl = lookup_replace ? lookup_replace_object(sha1)
-						   : sha1;
+	const unsigned char *repl = lookup_replace ?
+		lookup_replace_object(the_repository, sha1) : sha1;
 
 	errno = 0;
 	data = read_object(repl, type, size);
diff --git a/streaming.c b/streaming.c
index 29632065d0..b36b3cc6b8 100644
--- a/streaming.c
+++ b/streaming.c
@@ -140,7 +140,7 @@ struct git_istream *open_istream(const unsigned char *sha1,
 {
 	struct git_istream *st;
 	struct object_info oi = OBJECT_INFO_INIT;
-	const unsigned char *real = lookup_replace_object(sha1);
+	const unsigned char *real = lookup_replace_object(the_repository, sha1);
 	enum input_source src = istream_source(real, type, &oi);
 
 	if (src < 0)
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 13/19] refs: store the main ref store inside the repository struct

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 refs.c       | 13 +++++--------
 refs.h       |  4 +---
 repository.h |  5 +++++
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/refs.c b/refs.c
index 0352cbdcc1..793b827e19 100644
--- a/refs.c
+++ b/refs.c
@@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry(
 	return entry;
 }
 
-/* A pointer to the ref_store for the main repository: */
-static struct ref_store *main_ref_store;
-
 /* A hashmap of ref_stores, stored by submodule name: */
 static struct hashmap submodule_ref_stores;
 
@@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char *gitdir,
 	return refs;
 }
 
-struct ref_store *get_main_ref_store_the_repository(void)
+struct ref_store *get_main_ref_store(struct repository *r)
 {
-	if (main_ref_store)
-		return main_ref_store;
+	if (r->main_ref_store)
+		return r->main_ref_store;
 
-	main_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);
-	return main_ref_store;
+	r->main_ref_store = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
+	return r->main_ref_store;
 }
 
 /*
diff --git a/refs.h b/refs.h
index ab3d2bec2f..f5ab68c0ed 100644
--- a/refs.h
+++ b/refs.h
@@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,
 
 int ref_storage_backend_exists(const char *name);
 
-#define get_main_ref_store(r) \
-	get_main_ref_store_##r()
-struct ref_store *get_main_ref_store_the_repository(void);
+struct ref_store *get_main_ref_store(struct repository *r);
 /*
  * Return the ref_store instance for the specified submodule. For the
  * main repository, use submodule==NULL; such a call cannot fail. For
diff --git a/repository.h b/repository.h
index 09df94a472..2922d3a28b 100644
--- a/repository.h
+++ b/repository.h
@@ -26,6 +26,11 @@ struct repository {
 	 */
 	struct raw_object_store *objects;
 
+	/*
+	 * The store in which the refs are hold.
+	 */
+	struct ref_store *main_ref_store;
+
 	/*
 	 * Path to the repository's graft file.
 	 * Cannot be NULL after initialization.
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 13/19] refs: store the main ref store inside the repository struct

[Eric Sunshine]

On Fri, Apr 6, 2018 at 7:21 PM, Stefan Beller <sbeller@google.com> wrote:
> diff --git a/repository.h b/repository.h
> @@ -26,6 +26,11 @@ struct repository {
> +       /*
> +        * The store in which the refs are hold.
> +        */

s/hold/held/

Also, this comment is short enough to fit on one line: /* ... */

> +       struct ref_store *main_ref_store;

[PATCH 14/19] refs: allow for_each_replace_ref to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 refs.c | 4 ++--
 refs.h | 4 +---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/refs.c b/refs.c
index 793b827e19..ee71733a74 100644
--- a/refs.c
+++ b/refs.c
@@ -1415,9 +1415,9 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 	return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
 }
 
-int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data)
+int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(get_main_ref_store(the_repository),
+	return do_for_each_ref(get_main_ref_store(r),
 			       git_replace_ref_base, fn,
 			       strlen(git_replace_ref_base),
 			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
diff --git a/refs.h b/refs.h
index f5ab68c0ed..15f3a91cc4 100644
--- a/refs.h
+++ b/refs.h
@@ -300,9 +300,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,
 int for_each_tag_ref(each_ref_fn fn, void *cb_data);
 int for_each_branch_ref(each_ref_fn fn, void *cb_data);
 int for_each_remote_ref(each_ref_fn fn, void *cb_data);
-#define for_each_replace_ref(r, fn, cb) \
-	for_each_replace_ref_##r(fn, cb)
-int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data);
+int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data);
 int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
 int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
 			 const char *prefix, void *cb_data);
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 15/19] replace-object: allow replace_object_pos to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index 944bcaf242..9272c7acbf 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -12,12 +12,10 @@ static const unsigned char *replace_sha1_access(size_t index, void *table)
 	return replace[index]->original;
 }
 
-#define replace_object_pos(r, s) \
-	replace_object_pos_##r(s)
-static int replace_object_pos_the_repository(const unsigned char *sha1)
+static int replace_object_pos(struct repository *r, const unsigned char *sha1)
 {
-	return sha1_pos(sha1,  the_repository->objects->replacements.items,
-			 the_repository->objects->replacements.nr,
+	return sha1_pos(sha1, r->objects->replacements.items,
+			r->objects->replacements.nr,
 			replace_sha1_access);
 }
 
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 16/19] replace-object: allow register_replace_object to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index 9272c7acbf..b3bfd21541 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -19,32 +19,31 @@ static int replace_object_pos(struct repository *r, const unsigned char *sha1)
 			replace_sha1_access);
 }
 
-#define register_replace_object(r, rp, i) \
-	register_replace_object_##r(rp, i)
-static int register_replace_object_the_repository(struct replace_object *replace,
-						  int ignore_dups)
+static int register_replace_object(struct repository *r,
+				   struct replace_object *replace,
+				   int ignore_dups)
 {
-	int pos = replace_object_pos(the_repository, replace->original);
+	int pos = replace_object_pos(r, replace->original);
 
 	if (0 <= pos) {
 		if (ignore_dups)
 			free(replace);
 		else {
-			free( the_repository->objects->replacements.items[pos]);
-			 the_repository->objects->replacements.items[pos] = replace;
+			free(r->objects->replacements.items[pos]);
+			r->objects->replacements.items[pos] = replace;
 		}
 		return 1;
 	}
 	pos = -pos - 1;
-	ALLOC_GROW( the_repository->objects->replacements.items,
-		    the_repository->objects->replacements.nr + 1,
-		    the_repository->objects->replacements.alloc);
-	 the_repository->objects->replacements.nr++;
-	if (pos <  the_repository->objects->replacements.nr)
-		MOVE_ARRAY( the_repository->objects->replacements.items + pos + 1,
-			    the_repository->objects->replacements.items + pos,
-			    the_repository->objects->replacements.nr - pos - 1);
-	 the_repository->objects->replacements.items[pos] = replace;
+	ALLOC_GROW(r->objects->replacements.items,
+		   r->objects->replacements.nr + 1,
+		   r->objects->replacements.alloc);
+	r->objects->replacements.nr++;
+	if (pos < r->objects->replacements.nr)
+		MOVE_ARRAY(r->objects->replacements.items + pos + 1,
+			   r->objects->replacements.items + pos,
+			   r->objects->replacements.nr - pos - 1);
+	r->objects->replacements.items[pos] = replace;
 	return 0;
 }
 
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 17/19] replace-object: allow prepare_replace_object to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index b3bfd21541..967d794687 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -72,15 +72,13 @@ static int register_replace_ref(const char *refname,
 	return 0;
 }
 
-#define prepare_replace_object(r) \
-	prepare_replace_object_##r()
-static void prepare_replace_object_the_repository(void)
+static void prepare_replace_object(struct repository *r)
 {
-	if (the_repository->objects->replacements.prepared)
+	if (r->objects->replacements.prepared)
 		return;
 
-	for_each_replace_ref(the_repository, register_replace_ref, NULL);
-	the_repository->objects->replacements.prepared = 1;
+	for_each_replace_ref(r, register_replace_ref, r);
+	r->objects->replacements.prepared = 1;
 }
 
 /* We allow "recursive" replacement. Only within reason, though */
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 18/19] replace-object: allow do_lookup_replace_object to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 9 +++++----
 replace-object.h | 3 +--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index 967d794687..075a48b661 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -91,12 +91,13 @@ static void prepare_replace_object(struct repository *r)
  * permanently-allocated value.  This function always respects replace
  * references, regardless of the value of check_replace_refs.
  */
-const unsigned char *do_lookup_replace_object_the_repository(const unsigned char *sha1)
+const unsigned char *do_lookup_replace_object(struct repository *r,
+					      const unsigned char *sha1)
 {
 	int pos, depth = MAXREPLACEDEPTH;
 	const unsigned char *cur = sha1;
 
-	prepare_replace_object(the_repository);
+	prepare_replace_object(r);
 
 	/* Try to recursively replace the object */
 	do {
@@ -104,9 +105,9 @@ const unsigned char *do_lookup_replace_object_the_repository(const unsigned char
 			die("replace depth too high for object %s",
 			    sha1_to_hex(sha1));
 
-		pos = replace_object_pos(the_repository, cur);
+		pos = replace_object_pos(r, cur);
 		if (0 <= pos)
-			cur = the_repository->objects->replacements.items[pos]->replacement;
+			cur = r->objects->replacements.items[pos]->replacement;
 	} while (0 <= pos);
 
 	return cur;
diff --git a/replace-object.h b/replace-object.h
index 6a2d37d717..e15f0725cb 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -14,8 +14,7 @@ struct replace_object {
  * This internal function is only declared here for the benefit of
  * lookup_replace_object().  Please do not call it directly.
  */
-#define do_lookup_replace_object(r, s) do_lookup_replace_object_##r(s)
-extern const unsigned char *do_lookup_replace_object_the_repository(const unsigned char *sha1);
+extern const unsigned char *do_lookup_replace_object(struct repository *r, const unsigned char *sha1);
 
 /*
  * If object sha1 should be replaced, return the replacement object's
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 19/19] replace-object: allow lookup_replace_object to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/replace-object.h b/replace-object.h
index e15f0725cb..e7511ea54c 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -22,14 +22,14 @@ extern const unsigned char *do_lookup_replace_object(struct repository *r, const
  * either sha1 or a pointer to a permanently-allocated value.  When
  * object replacement is suppressed, always return sha1.
  */
-#define lookup_replace_object(r, s) lookup_replace_object_##r(s)
-static inline const unsigned char *lookup_replace_object_the_repository(const unsigned char *sha1)
+static inline const unsigned char *lookup_replace_object(struct repository *r,
+							 const unsigned char *sha1)
 {
 	if (!check_replace_refs ||
-	    (the_repository->objects->replacements.prepared &&
-	     the_repository->objects->replacements.nr == 0))
+	    (r->objects->replacements.prepared &&
+	     r->objects->replacements.nr == 0))
 		return sha1;
-	return do_lookup_replace_object(the_repository, sha1);
+	return do_lookup_replace_object(r, sha1);
 }
 
 #endif /* REPLACE_OBJECT_H */
-- 
2.17.0.484.g0c8726318c-goog

Re: [RFC PATCH 00/19] object-store refactoring 3 (replace objects, main ref store)

[René Scharfe]

Am 07.04.2018 um 01:21 schrieb Stefan Beller:
> This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c
> (sb/packfiles-in-repository).
> It is also available at https://github.com/stefanbeller/git/tree/object-store-3

This series conflicts with 1731a1e239 (replace_object: convert struct
replace_object to object_id) and b383a13cc0 (Convert
lookup_replace_object to struct object_id), which are in next.

> This series will bring the replacement mechanism (git replace)
> into the object store.

Good idea.

>   $ git diff 464416a2eaadf84d2bfdf795007863d03b222b7c..HEAD -- object-store.h repository.h
> diff --git a/object-store.h b/object-store.h
> index fef33f345f..be90c02db6 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -93,6 +93,22 @@ struct raw_object_store {
>          struct alternate_object_database *alt_odb_list;
>          struct alternate_object_database **alt_odb_tail;
>   
> +       /*
> +        * Objects that should be substituted by other objects
> +        * (see git-replace(1)).
> +        */
> +       struct replace_objects {
> +               /*
> +                * An array of replacements.  The array is kept sorted by the original
> +                * sha1.
> +                */
> +               struct replace_object **items;
> +
> +               int alloc, nr;
> +
> +               unsigned prepared : 1;
> +       } replacements;

An oidmap would be a better fit -- lookups should be quicker and
memory consumption not much worse.  I meant to submit something like
this eventually after Brian's series lands:

-- >8 --
Subject: [PATCH] replace_object: use oidmap

Load the replace objects into an oidmap to allow for easy lookups in
constant time.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
This is on top of next.

 replace_object.c | 76 ++++++++++--------------------------------------
 1 file changed, 16 insertions(+), 60 deletions(-)

diff --git a/replace_object.c b/replace_object.c
index 336357394d..a757a5ebf2 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -1,54 +1,14 @@
 #include "cache.h"
-#include "sha1-lookup.h"
+#include "oidmap.h"
 #include "refs.h"
 #include "commit.h"
 
-/*
- * An array of replacements.  The array is kept sorted by the original
- * sha1.
- */
-static struct replace_object {
-	struct object_id original;
+struct replace_object {
+	struct oidmap_entry original;
 	struct object_id replacement;
-} **replace_object;
-
-static int replace_object_alloc, replace_object_nr;
+};
 
-static const unsigned char *replace_sha1_access(size_t index, void *table)
-{
-	struct replace_object **replace = table;
-	return replace[index]->original.hash;
-}
-
-static int replace_object_pos(const unsigned char *sha1)
-{
-	return sha1_pos(sha1, replace_object, replace_object_nr,
-			replace_sha1_access);
-}
-
-static int register_replace_object(struct replace_object *replace,
-				   int ignore_dups)
-{
-	int pos = replace_object_pos(replace->original.hash);
-
-	if (0 <= pos) {
-		if (ignore_dups)
-			free(replace);
-		else {
-			free(replace_object[pos]);
-			replace_object[pos] = replace;
-		}
-		return 1;
-	}
-	pos = -pos - 1;
-	ALLOC_GROW(replace_object, replace_object_nr + 1, replace_object_alloc);
-	replace_object_nr++;
-	if (pos < replace_object_nr)
-		MOVE_ARRAY(replace_object + pos + 1, replace_object + pos,
-			   replace_object_nr - pos - 1);
-	replace_object[pos] = replace;
-	return 0;
-}
+static struct oidmap replace_map = OIDMAP_INIT;
 
 static int register_replace_ref(const char *refname,
 				const struct object_id *oid,
@@ -59,7 +19,7 @@ static int register_replace_ref(const char *refname,
 	const char *hash = slash ? slash + 1 : refname;
 	struct replace_object *repl_obj = xmalloc(sizeof(*repl_obj));
 
-	if (get_oid_hex(hash, &repl_obj->original)) {
+	if (get_oid_hex(hash, &repl_obj->original.oid)) {
 		free(repl_obj);
 		warning("bad replace ref name: %s", refname);
 		return 0;
@@ -69,7 +29,7 @@ static int register_replace_ref(const char *refname,
 	oidcpy(&repl_obj->replacement, oid);
 
 	/* Register new object */
-	if (register_replace_object(repl_obj, 1))
+	if (oidmap_put(&replace_map, repl_obj))
 		die("duplicate replace ref: %s", refname);
 
 	return 0;
@@ -84,7 +44,7 @@ static void prepare_replace_object(void)
 
 	for_each_replace_ref(register_replace_ref, NULL);
 	replace_object_prepared = 1;
-	if (!replace_object_nr)
+	if (!replace_map.map.tablesize)
 		check_replace_refs = 0;
 }
 
@@ -100,21 +60,17 @@ static void prepare_replace_object(void)
  */
 const struct object_id *do_lookup_replace_object(const struct object_id *oid)
 {
-	int pos, depth = MAXREPLACEDEPTH;
+	int depth = MAXREPLACEDEPTH;
 	const struct object_id *cur = oid;
 
 	prepare_replace_object();
 
 	/* Try to recursively replace the object */
-	do {
-		if (--depth < 0)
-			die("replace depth too high for object %s",
-			    oid_to_hex(oid));
-
-		pos = replace_object_pos(cur->hash);
-		if (0 <= pos)
-			cur = &replace_object[pos]->replacement;
-	} while (0 <= pos);
-
-	return cur;
+	while (depth-- > 0) {
+		struct replace_object *repl_obj = oidmap_get(&replace_map, cur);
+		if (!repl_obj)
+			return cur;
+		cur = &repl_obj->replacement;
+	}
+	die("replace depth too high for object %s", oid_to_hex(oid));
 }
-- 
2.17.0

Re: [RFC PATCH 00/19] object-store refactoring 3 (replace objects, main ref store)

[Stefan Beller]

Hi René,

On Fri, Apr 6, 2018 at 9:58 PM, René Scharfe <l.s.r@web.de> wrote:
> Am 07.04.2018 um 01:21 schrieb Stefan Beller:
>> This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c
>> (sb/packfiles-in-repository).
>> It is also available at https://github.com/stefanbeller/git/tree/object-store-3
>
> This series conflicts with 1731a1e239 (replace_object: convert struct
> replace_object to object_id) and b383a13cc0 (Convert
> lookup_replace_object to struct object_id), which are in next.

ok, I'll investigate. Maybe the reroll will be on top of a merge
of brians series and sb/packfiles-in-repository, both of which go to master
quickly now that 2.17 is out?

>
>> This series will bring the replacement mechanism (git replace)
>> into the object store.
>
> Good idea.

heh, thanks. I think this is the smallest unit in which the community is
willing to digest it. (The largest coherent patch series was the demo of
"everything in struct repo"[1])

[1] https://public-inbox.org/git/20180205235508.216277-1-sbeller@google.com/

>
>>   $ git diff 464416a2eaadf84d2bfdf795007863d03b222b7c..HEAD -- object-store.h repository.h
>> diff --git a/object-store.h b/object-store.h
>> index fef33f345f..be90c02db6 100644
>> --- a/object-store.h
>> +++ b/object-store.h
>> @@ -93,6 +93,22 @@ struct raw_object_store {
>>          struct alternate_object_database *alt_odb_list;
>>          struct alternate_object_database **alt_odb_tail;
>>
>> +       /*
>> +        * Objects that should be substituted by other objects
>> +        * (see git-replace(1)).
>> +        */
>> +       struct replace_objects {
>> +               /*
>> +                * An array of replacements.  The array is kept sorted by the original
>> +                * sha1.
>> +                */
>> +               struct replace_object **items;
>> +
>> +               int alloc, nr;
>> +
>> +               unsigned prepared : 1;
>> +       } replacements;
>
> An oidmap would be a better fit -- lookups should be quicker and
> memory consumption not much worse.  I meant to submit something like
> this eventually after Brian's series lands:
>
> -- >8 --
> Subject: [PATCH] replace_object: use oidmap
>
> Load the replace objects into an oidmap to allow for easy lookups in
> constant time.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> This is on top of next.

So this is on top of brians series (modulo other next-ish stuff)
which this series ought to merge with?

The patch looks good -- just from the diff stat alone.

Thanks,
Stefan


>
>  replace_object.c | 76 ++++++++++--------------------------------------
>  1 file changed, 16 insertions(+), 60 deletions(-)
>
> diff --git a/replace_object.c b/replace_object.c
> index 336357394d..a757a5ebf2 100644
> --- a/replace_object.c
> +++ b/replace_object.c
> @@ -1,54 +1,14 @@
>  #include "cache.h"
> -#include "sha1-lookup.h"
> +#include "oidmap.h"
>  #include "refs.h"
>  #include "commit.h"
>
> -/*
> - * An array of replacements.  The array is kept sorted by the original
> - * sha1.
> - */
> -static struct replace_object {
> -       struct object_id original;
> +struct replace_object {
> +       struct oidmap_entry original;
>         struct object_id replacement;
> -} **replace_object;
> -
> -static int replace_object_alloc, replace_object_nr;
> +};
>
> -static const unsigned char *replace_sha1_access(size_t index, void *table)
> -{
> -       struct replace_object **replace = table;
> -       return replace[index]->original.hash;
> -}
> -
> -static int replace_object_pos(const unsigned char *sha1)
> -{
> -       return sha1_pos(sha1, replace_object, replace_object_nr,
> -                       replace_sha1_access);
> -}
> -
> -static int register_replace_object(struct replace_object *replace,
> -                                  int ignore_dups)
> -{
> -       int pos = replace_object_pos(replace->original.hash);
> -
> -       if (0 <= pos) {
> -               if (ignore_dups)
> -                       free(replace);
> -               else {
> -                       free(replace_object[pos]);
> -                       replace_object[pos] = replace;
> -               }
> -               return 1;
> -       }
> -       pos = -pos - 1;
> -       ALLOC_GROW(replace_object, replace_object_nr + 1, replace_object_alloc);
> -       replace_object_nr++;
> -       if (pos < replace_object_nr)
> -               MOVE_ARRAY(replace_object + pos + 1, replace_object + pos,
> -                          replace_object_nr - pos - 1);
> -       replace_object[pos] = replace;
> -       return 0;
> -}
> +static struct oidmap replace_map = OIDMAP_INIT;
>
>  static int register_replace_ref(const char *refname,
>                                 const struct object_id *oid,
> @@ -59,7 +19,7 @@ static int register_replace_ref(const char *refname,
>         const char *hash = slash ? slash + 1 : refname;
>         struct replace_object *repl_obj = xmalloc(sizeof(*repl_obj));
>
> -       if (get_oid_hex(hash, &repl_obj->original)) {
> +       if (get_oid_hex(hash, &repl_obj->original.oid)) {
>                 free(repl_obj);
>                 warning("bad replace ref name: %s", refname);
>                 return 0;
> @@ -69,7 +29,7 @@ static int register_replace_ref(const char *refname,
>         oidcpy(&repl_obj->replacement, oid);
>
>         /* Register new object */
> -       if (register_replace_object(repl_obj, 1))
> +       if (oidmap_put(&replace_map, repl_obj))
>                 die("duplicate replace ref: %s", refname);
>
>         return 0;
> @@ -84,7 +44,7 @@ static void prepare_replace_object(void)
>
>         for_each_replace_ref(register_replace_ref, NULL);
>         replace_object_prepared = 1;
> -       if (!replace_object_nr)
> +       if (!replace_map.map.tablesize)
>                 check_replace_refs = 0;
>  }
>
> @@ -100,21 +60,17 @@ static void prepare_replace_object(void)
>   */
>  const struct object_id *do_lookup_replace_object(const struct object_id *oid)
>  {
> -       int pos, depth = MAXREPLACEDEPTH;
> +       int depth = MAXREPLACEDEPTH;
>         const struct object_id *cur = oid;
>
>         prepare_replace_object();
>
>         /* Try to recursively replace the object */
> -       do {
> -               if (--depth < 0)
> -                       die("replace depth too high for object %s",
> -                           oid_to_hex(oid));
> -
> -               pos = replace_object_pos(cur->hash);
> -               if (0 <= pos)
> -                       cur = &replace_object[pos]->replacement;
> -       } while (0 <= pos);
> -
> -       return cur;
> +       while (depth-- > 0) {
> +               struct replace_object *repl_obj = oidmap_get(&replace_map, cur);
> +               if (!repl_obj)
> +                       return cur;
> +               cur = &repl_obj->replacement;
> +       }
> +       die("replace depth too high for object %s", oid_to_hex(oid));
>  }
> --
> 2.17.0

Re: [RFC PATCH 00/19] object-store refactoring 3 (replace objects, main ref store)

[Duy Nguyen]

On Sat, Apr 7, 2018 at 1:21 AM, Stefan Beller <sbeller@google.com> wrote:     *
> diff --git a/repository.h b/repository.h
> index 09df94a472..2922d3a28b 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -26,6 +26,11 @@ struct repository {
>          */
>         struct raw_object_store *objects;
>
> +       /*
> +        * The store in which the refs are hold.
> +        */
> +       struct ref_store *main_ref_store;
> +

We probably should drop the main_ prefix here because this could also
be a submodule ref store (when the repository is about a submodule).
worktree ref store is a different story and I don't know how to best
present it here yet (I'm still thinking a separate struct repository).
But we can worry about that when struct repository supports multiple
worktree.
-- 
Duy

Re: [RFC PATCH 00/19] object-store refactoring 3 (replace objects, main ref store)

[Stefan Beller]

On Sat, Apr 7, 2018 at 2:50 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sat, Apr 7, 2018 at 1:21 AM, Stefan Beller <sbeller@google.com> wrote:     *
>> diff --git a/repository.h b/repository.h
>> index 09df94a472..2922d3a28b 100644
>> --- a/repository.h
>> +++ b/repository.h
>> @@ -26,6 +26,11 @@ struct repository {
>>          */
>>         struct raw_object_store *objects;
>>
>> +       /*
>> +        * The store in which the refs are hold.
>> +        */
>> +       struct ref_store *main_ref_store;
>> +
>
> We probably should drop the main_ prefix here because this could also
> be a submodule ref store (when the repository is about a submodule).

it's still the main ref store of the submodule, then. :)
But yes, I agree we should rename it. Now or eventually later?

> worktree ref store is a different story and I don't know how to best
> present it here yet (I'm still thinking a separate struct repository).

I imagine that we'd rather want to have arrays of config/worktree path/refs
when needed and the worktree is just an index into all of these things?

> But we can worry about that when struct repository supports multiple
> worktree.

ok. Thanks for bringing this to my attention.

Thanks,
Stefan

Re: [RFC PATCH 00/19] object-store refactoring 3 (replace objects, main ref store)

[Derrick Stolee]

On 4/6/2018 7:21 PM, Stefan Beller wrote:
> This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c
> (sb/packfiles-in-repository).
> It is also available at https://github.com/stefanbeller/git/tree/object-store-3
>
> This series will bring the replacement mechanism (git replace)
> into the object store.
>
> The first patches are cleaning up a bit, and patches 6-19 are converting
> one function at a time using the tick-tock pattern with the #define trick.
> See cfc62fc98c (sha1_file: add repository argument to link_alt_odb_entry,
> 2018-03-23) for explanation.
>
> Thanks,
> Stefan

I looked through these patches and only found one set of whitespace 
errors. Compiles and tests fine on my machine.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>

Re: [RFC PATCH 00/19] object-store refactoring 3 (replace objects, main ref store)

[Stefan Beller]

+cc list

On Mon, Apr 9, 2018 at 1:39 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Stefan,
>
> On Fri, 6 Apr 2018, Stefan Beller wrote:
>
>> See cfc62fc98c (sha1_file: add repository argument to link_alt_odb_entry,
>> 2018-03-23) for explanation.
>
> "See ... for explanation." ... are you going full Russian on us? ;-)

That commit is supposed to merge to master now, so we don't lose it.
I just dislike to repeat myself, and there we explained that trick already.
As commit messages don't link well together in email clients, I will
repeat the important part.

Thanks,
Stefan

>
> Ciao,
> Dscho

[PATCHv2 00/16] object-store refactoring 3 (replace objects, main ref store)

[Stefan Beller]

v2:
This applies on top of a merge of
origin/bc/object-id and origin/sb/packfiles-in-repository,
both of which are pending merge to master. It is also available at
https://github.com/stefanbeller/git/tree/object-store-3

* removed whitespaces as noted by Stolee
* incorporated Renes patch as the first patch of this series
  (It may go independently if this series takes too long)
* Adressed Erics concern regarding sloppy commit messages
  (removed #Conflict markers), typo in comment
* I did not drop the main_ from the ref store, yet, as asked by Duy.

Thanks,
Stefan

v1:
This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c
(sb/packfiles-in-repository).
It is also available at https://github.com/stefanbeller/git/tree/object-store-3

This series will bring the replacement mechanism (git replace)
into the object store.

The first patches are cleaning up a bit, and patches 6-19 are converting
one function at a time using the tick-tock pattern with the #define trick.
See cfc62fc98c (sha1_file: add repository argument to link_alt_odb_entry,
2018-03-23) for explanation.

Thanks,
Stefan

René Scharfe (1):
  replace_object: use oidmap

Stefan Beller (15):
  replace_object.c: rename to use dash in file name
  replace-object: move replace_map to object store
  object-store: move lookup_replace_object to replace-object.h
  replace-object: eliminate replace objects prepared flag
  replace-object: check_replace_refs is safe in multi repo environment
  refs: add repository argument to get_main_ref_store
  refs: add repository argument to for_each_replace_ref
  replace-object: add repository argument to prepare_replace_object
  replace-object: add repository argument to do_lookup_replace_object
  replace-object: add repository argument to lookup_replace_object
  refs: store the main ref store inside the repository struct
  refs: allow for_each_replace_ref to handle arbitrary repositories
  replace-object: allow prepare_replace_object to handle arbitrary
    repositories
  replace-object: allow do_lookup_replace_object to handle arbitrary
    repositories
  replace-object: allow lookup_replace_object to handle arbitrary
    repositories

 Makefile                  |   2 +-
 builtin/mktag.c           |   3 +-
 builtin/pack-refs.c       |   3 +-
 builtin/replace.c         |   4 +-
 cache.h                   |  19 ------
 environment.c             |   2 +-
 object-store.h            |   8 +++
 object.c                  |   3 +-
 refs.c                    |  80 +++++++++++++------------
 refs.h                    |   4 +-
 replace-object.c          |  73 +++++++++++++++++++++++
 replace-object.h          |  36 ++++++++++++
 replace_object.c          | 120 --------------------------------------
 repository.h              |   3 +
 revision.c                |   5 +-
 sha1_file.c               |   7 ++-
 streaming.c               |   3 +-
 t/helper/test-ref-store.c |   3 +-
 18 files changed, 183 insertions(+), 195 deletions(-)
 create mode 100644 replace-object.c
 create mode 100644 replace-object.h
 delete mode 100644 replace_object.c

-- 
2.17.0.484.g0c8726318c-goog

[PATCH 01/16] replace_object: use oidmap

[Stefan Beller]

From: René Scharfe <l.s.r@web.de>

Load the replace objects into an oidmap to allow for easy lookups in
constant time.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace_object.c | 76 ++++++++++--------------------------------------
 1 file changed, 16 insertions(+), 60 deletions(-)

diff --git a/replace_object.c b/replace_object.c
index 336357394d..a757a5ebf2 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -1,54 +1,14 @@
 #include "cache.h"
-#include "sha1-lookup.h"
+#include "oidmap.h"
 #include "refs.h"
 #include "commit.h"
 
-/*
- * An array of replacements.  The array is kept sorted by the original
- * sha1.
- */
-static struct replace_object {
-	struct object_id original;
+struct replace_object {
+	struct oidmap_entry original;
 	struct object_id replacement;
-} **replace_object;
-
-static int replace_object_alloc, replace_object_nr;
+};
 
-static const unsigned char *replace_sha1_access(size_t index, void *table)
-{
-	struct replace_object **replace = table;
-	return replace[index]->original.hash;
-}
-
-static int replace_object_pos(const unsigned char *sha1)
-{
-	return sha1_pos(sha1, replace_object, replace_object_nr,
-			replace_sha1_access);
-}
-
-static int register_replace_object(struct replace_object *replace,
-				   int ignore_dups)
-{
-	int pos = replace_object_pos(replace->original.hash);
-
-	if (0 <= pos) {
-		if (ignore_dups)
-			free(replace);
-		else {
-			free(replace_object[pos]);
-			replace_object[pos] = replace;
-		}
-		return 1;
-	}
-	pos = -pos - 1;
-	ALLOC_GROW(replace_object, replace_object_nr + 1, replace_object_alloc);
-	replace_object_nr++;
-	if (pos < replace_object_nr)
-		MOVE_ARRAY(replace_object + pos + 1, replace_object + pos,
-			   replace_object_nr - pos - 1);
-	replace_object[pos] = replace;
-	return 0;
-}
+static struct oidmap replace_map = OIDMAP_INIT;
 
 static int register_replace_ref(const char *refname,
 				const struct object_id *oid,
@@ -59,7 +19,7 @@ static int register_replace_ref(const char *refname,
 	const char *hash = slash ? slash + 1 : refname;
 	struct replace_object *repl_obj = xmalloc(sizeof(*repl_obj));
 
-	if (get_oid_hex(hash, &repl_obj->original)) {
+	if (get_oid_hex(hash, &repl_obj->original.oid)) {
 		free(repl_obj);
 		warning("bad replace ref name: %s", refname);
 		return 0;
@@ -69,7 +29,7 @@ static int register_replace_ref(const char *refname,
 	oidcpy(&repl_obj->replacement, oid);
 
 	/* Register new object */
-	if (register_replace_object(repl_obj, 1))
+	if (oidmap_put(&replace_map, repl_obj))
 		die("duplicate replace ref: %s", refname);
 
 	return 0;
@@ -84,7 +44,7 @@ static void prepare_replace_object(void)
 
 	for_each_replace_ref(register_replace_ref, NULL);
 	replace_object_prepared = 1;
-	if (!replace_object_nr)
+	if (!replace_map.map.tablesize)
 		check_replace_refs = 0;
 }
 
@@ -100,21 +60,17 @@ static void prepare_replace_object(void)
  */
 const struct object_id *do_lookup_replace_object(const struct object_id *oid)
 {
-	int pos, depth = MAXREPLACEDEPTH;
+	int depth = MAXREPLACEDEPTH;
 	const struct object_id *cur = oid;
 
 	prepare_replace_object();
 
 	/* Try to recursively replace the object */
-	do {
-		if (--depth < 0)
-			die("replace depth too high for object %s",
-			    oid_to_hex(oid));
-
-		pos = replace_object_pos(cur->hash);
-		if (0 <= pos)
-			cur = &replace_object[pos]->replacement;
-	} while (0 <= pos);
-
-	return cur;
+	while (depth-- > 0) {
+		struct replace_object *repl_obj = oidmap_get(&replace_map, cur);
+		if (!repl_obj)
+			return cur;
+		cur = &repl_obj->replacement;
+	}
+	die("replace depth too high for object %s", oid_to_hex(oid));
 }
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 01/16] replace_object: use oidmap

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> -static struct replace_object {
> -	struct object_id original;
> +struct replace_object {
> +	struct oidmap_entry original;
>  	struct object_id replacement;
> -} **replace_object;
> -
> -static int replace_object_alloc, replace_object_nr;
> +};

The oidmap key can also serve as the original side, which is good
;-)

> @@ -84,7 +44,7 @@ static void prepare_replace_object(void)
>  
>  	for_each_replace_ref(register_replace_ref, NULL);
>  	replace_object_prepared = 1;
> -	if (!replace_object_nr)
> +	if (!replace_map.map.tablesize)
>  		check_replace_refs = 0;

The original checks _nr (i.e. how many do we actually have) as
opposed to _alloc; on the other hand, .tablesize is not about the
number of contents in the table.

But before or after the patch, what the code really wants to check
is if the table is _initialized_, so from that point of view,
checking .tablesize is good enough.

So, I guess this step is quite right ;-)

Re: [PATCH 01/16] replace_object: use oidmap

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> -static struct replace_object {
> -	struct object_id original;
> +struct replace_object {
> +	struct oidmap_entry original;
>  	struct object_id replacement;
> -} **replace_object;
> -
> -static int replace_object_alloc, replace_object_nr;
> +};

The oidmap key can also serve as the original side, which is good
;-)

> @@ -84,7 +44,7 @@ static void prepare_replace_object(void)
>  
>  	for_each_replace_ref(register_replace_ref, NULL);
>  	replace_object_prepared = 1;
> -	if (!replace_object_nr)
> +	if (!replace_map.map.tablesize)
>  		check_replace_refs = 0;

The original checks _nr (i.e. how many do we actually have) as
opposed to _alloc; on the other hand, .tablesize is not about the
number of contents in the table.

But before or after the patch, what the code really wants to check
is if the table is _initialized_, so from that point of view,
checking .tablesize is good enough.

So, I guess this step is quite right ;-)

[PATCH 02/16] replace_object.c: rename to use dash in file name

[Stefan Beller]

This is more consistent with the project style. The majority of
Git's source files use dashes in preference to underscores in their file
names.

Noticed while adding a header corresponding to this file.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Makefile                             | 2 +-
 replace_object.c => replace-object.c | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename replace_object.c => replace-object.c (100%)

diff --git a/Makefile b/Makefile
index de4b8f0c02..94e0bf47b1 100644
--- a/Makefile
+++ b/Makefile
@@ -871,7 +871,7 @@ LIB_OBJS += refs/packed-backend.o
 LIB_OBJS += refs/ref-cache.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
-LIB_OBJS += replace_object.o
+LIB_OBJS += replace-object.o
 LIB_OBJS += repository.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
diff --git a/replace_object.c b/replace-object.c
similarity index 100%
rename from replace_object.c
rename to replace-object.c
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 02/16] replace_object.c: rename to use dash in file name

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> This is more consistent with the project style. The majority of
> Git's source files use dashes in preference to underscores in their file
> names.
>
> Noticed while adding a header corresponding to this file.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---

Hmph, is this authored by Jonathan?

There are sha1_{file,name}.c, exec_cmd.[ch], and unicode_width.h
remaining, though ;-)

>  Makefile                             | 2 +-
>  replace_object.c => replace-object.c | 0
>  2 files changed, 1 insertion(+), 1 deletion(-)
>  rename replace_object.c => replace-object.c (100%)
>
> diff --git a/Makefile b/Makefile
> index de4b8f0c02..94e0bf47b1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -871,7 +871,7 @@ LIB_OBJS += refs/packed-backend.o
>  LIB_OBJS += refs/ref-cache.o
>  LIB_OBJS += ref-filter.o
>  LIB_OBJS += remote.o
> -LIB_OBJS += replace_object.o
> +LIB_OBJS += replace-object.o
>  LIB_OBJS += repository.o
>  LIB_OBJS += rerere.o
>  LIB_OBJS += resolve-undo.o
> diff --git a/replace_object.c b/replace-object.c
> similarity index 100%
> rename from replace_object.c
> rename to replace-object.c

Re: [PATCH 02/16] replace_object.c: rename to use dash in file name

[Stefan Beller]

Hi Junio,

On Mon, Apr 9, 2018 at 8:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> This is more consistent with the project style. The majority of
>> Git's source files use dashes in preference to underscores in their file
>> names.
>>
>> Noticed while adding a header corresponding to this file.
>>
>> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>
> Hmph, is this authored by Jonathan?

This one (as well as other patches that are also signed off by Jonathan)
was cherry-picked from the long series[1], which was done partially
in a pair programming session or by passing patches back and forth.

Most of the mechanical changes were done by one author and we
just added the others sign off to have the whole series look like pair
programming.

This patch is [2], as-is, so I did not mess up the original authorship.

We decided to not use another trailer, such as co-authored-by or such
as we'd have to sign off anyway.

[1] https://public-inbox.org/git/20180205235508.216277-1-sbeller@google.com/
[2] https://public-inbox.org/git/20180205235735.216710-19-sbeller@google.com/

>
> There are sha1_{file,name}.c, exec_cmd.[ch], and unicode_width.h
> remaining, though ;-)

Noted.

Thanks,
Stefan

[PATCH 0/6] Rename files to use dashes instead of underscores

[Stefan Beller]

This is the followup for 
https://public-inbox.org/git/xmqqbmer4vfh.fsf@gitster-ct.c.googlers.com/

We have no files left with underscores in their names.

Thanks,
Stefan

Stefan Beller (6):
  write_or_die.c: rename to use dashes in file name
  unicode_width.h: rename to use dash in file name
  exec_cmd: rename to use dash in file name
  sha1_name.c: rename to use dash in file name
  sha1_file.c: rename to use dash in file name
  replace_object.c: rename to use dash in file name

 Documentation/technical/api-object-access.txt |  2 +-
 Makefile                                      | 14 +++++++-------
 attr.c                                        |  2 +-
 builtin/add.c                                 |  2 +-
 builtin/am.c                                  |  2 +-
 builtin/describe.c                            |  2 +-
 builtin/difftool.c                            |  2 +-
 builtin/hash-object.c                         |  2 +-
 builtin/help.c                                |  2 +-
 builtin/index-pack.c                          |  4 ++--
 builtin/init-db.c                             |  2 +-
 builtin/merge-tree.c                          |  2 +-
 builtin/notes.c                               |  2 +-
 builtin/pull.c                                |  2 +-
 builtin/receive-pack.c                        |  2 +-
 common-main.c                                 |  2 +-
 config.c                                      |  2 +-
 contrib/update-unicode/README                 |  6 +++---
 contrib/update-unicode/update_unicode.sh      |  2 +-
 exec_cmd.c => exec-cmd.c                      |  2 +-
 exec_cmd.h => exec-cmd.h                      |  0
 fetch-pack.c                                  |  2 +-
 git.c                                         |  2 +-
 help.c                                        |  2 +-
 http-backend.c                                |  2 +-
 http-fetch.c                                  |  2 +-
 http-push.c                                   |  2 +-
 imap-send.c                                   |  2 +-
 list-objects-filter.c                         |  2 +-
 object.h                                      |  2 +-
 remote-curl.c                                 |  2 +-
 remote-testsvn.c                              |  2 +-
 replace_object.c => replace-object.c          |  0
 run-command.c                                 |  2 +-
 sequencer.c                                   |  2 +-
 sha1_file.c => sha1-file.c                    |  0
 sha1_name.c => sha1-name.c                    |  0
 shell.c                                       |  2 +-
 unicode_width.h => unicode-width.h            |  0
 upload-pack.c                                 |  2 +-
 utf8.c                                        |  2 +-
 write_or_die.c => write-or-die.c              |  0
 42 files changed, 45 insertions(+), 45 deletions(-)
 rename exec_cmd.c => exec-cmd.c (99%)
 rename exec_cmd.h => exec-cmd.h (100%)
 rename replace_object.c => replace-object.c (100%)
 rename sha1_file.c => sha1-file.c (100%)
 rename sha1_name.c => sha1-name.c (100%)
 rename unicode_width.h => unicode-width.h (100%)
 rename write_or_die.c => write-or-die.c (100%)

-- 
2.17.0.484.g0c8726318c-goog

[PATCH 1/6] write_or_die.c: rename to use dashes in file name

[Stefan Beller]

This is more consistent with the project style. The majority of Git's
source files use dashes in preference to underscores in their file names.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Makefile                         | 2 +-
 write_or_die.c => write-or-die.c | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename write_or_die.c => write-or-die.c (100%)

diff --git a/Makefile b/Makefile
index 96f6138f63..be4ac5b2a6 100644
--- a/Makefile
+++ b/Makefile
@@ -933,7 +933,7 @@ LIB_OBJS += walker.o
 LIB_OBJS += wildmatch.o
 LIB_OBJS += worktree.o
 LIB_OBJS += wrapper.o
-LIB_OBJS += write_or_die.o
+LIB_OBJS += write-or-die.o
 LIB_OBJS += ws.o
 LIB_OBJS += wt-status.o
 LIB_OBJS += xdiff-interface.o
diff --git a/write_or_die.c b/write-or-die.c
similarity index 100%
rename from write_or_die.c
rename to write-or-die.c
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 2/6] unicode_width.h: rename to use dash in file name

[Stefan Beller]

This is more consistent with the project style. The majority of Git's
source files use dashes in preference to underscores in their file names.

Also adjust contrib/update-unicode as well.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 contrib/update-unicode/README            | 6 +++---
 contrib/update-unicode/update_unicode.sh | 2 +-
 unicode_width.h => unicode-width.h       | 0
 utf8.c                                   | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)
 rename unicode_width.h => unicode-width.h (100%)

diff --git a/contrib/update-unicode/README b/contrib/update-unicode/README
index b9e2fc8540..151a197041 100644
--- a/contrib/update-unicode/README
+++ b/contrib/update-unicode/README
@@ -1,10 +1,10 @@
 TL;DR: Run update_unicode.sh after the publication of a new Unicode
-standard and commit the resulting unicode_widths.h file.
+standard and commit the resulting unicode-widths.h file.
 
 The long version
 ================
 
-The Git source code ships the file unicode_widths.h which contains
+The Git source code ships the file unicode-widths.h which contains
 tables of zero and double width Unicode code points, respectively.
 These tables are generated using update_unicode.sh in this directory.
 update_unicode.sh itself uses a third-party tool, uniset, to query two
@@ -16,5 +16,5 @@ This requires a current-ish version of autoconf (2.69 works per December
 
 On each run, update_unicode.sh checks whether more recent Unicode data
 files are available from the Unicode consortium, and rebuilds the header
-unicode_widths.h with the new data. The new header can then be
+unicode-widths.h with the new data. The new header can then be
 committed.
diff --git a/contrib/update-unicode/update_unicode.sh b/contrib/update-unicode/update_unicode.sh
index e05db92d3f..aa90865bef 100755
--- a/contrib/update-unicode/update_unicode.sh
+++ b/contrib/update-unicode/update_unicode.sh
@@ -6,7 +6,7 @@
 #Cf Format          a format control character
 #
 cd "$(dirname "$0")"
-UNICODEWIDTH_H=$(git rev-parse --show-toplevel)/unicode_width.h
+UNICODEWIDTH_H=$(git rev-parse --show-toplevel)/unicode-width.h
 
 wget -N http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt \
 	http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt &&
diff --git a/unicode_width.h b/unicode-width.h
similarity index 100%
rename from unicode_width.h
rename to unicode-width.h
diff --git a/utf8.c b/utf8.c
index 2c27ce0137..4419055b48 100644
--- a/utf8.c
+++ b/utf8.c
@@ -81,7 +81,7 @@ static int git_wcwidth(ucs_char_t ch)
 	/*
 	 * Sorted list of non-overlapping intervals of non-spacing characters,
 	 */
-#include "unicode_width.h"
+#include "unicode-width.h"
 
 	/* test for 8-bit control characters */
 	if (ch == 0)
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 3/6] exec_cmd: rename to use dash in file name

[Stefan Beller]

This is more consistent with the project style. The majority of Git's
source files use dashes in preference to underscores in their file names.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Makefile                 | 6 +++---
 attr.c                   | 2 +-
 builtin/add.c            | 2 +-
 builtin/am.c             | 2 +-
 builtin/describe.c       | 2 +-
 builtin/difftool.c       | 2 +-
 builtin/hash-object.c    | 2 +-
 builtin/help.c           | 2 +-
 builtin/index-pack.c     | 2 +-
 builtin/init-db.c        | 2 +-
 builtin/merge-tree.c     | 2 +-
 builtin/notes.c          | 2 +-
 builtin/pull.c           | 2 +-
 builtin/receive-pack.c   | 2 +-
 common-main.c            | 2 +-
 config.c                 | 2 +-
 exec_cmd.c => exec-cmd.c | 2 +-
 exec_cmd.h => exec-cmd.h | 0
 fetch-pack.c             | 2 +-
 git.c                    | 2 +-
 help.c                   | 2 +-
 http-backend.c           | 2 +-
 http-fetch.c             | 2 +-
 http-push.c              | 2 +-
 imap-send.c              | 2 +-
 remote-curl.c            | 2 +-
 remote-testsvn.c         | 2 +-
 run-command.c            | 2 +-
 sequencer.c              | 2 +-
 shell.c                  | 2 +-
 upload-pack.c            | 2 +-
 31 files changed, 32 insertions(+), 32 deletions(-)
 rename exec_cmd.c => exec-cmd.c (99%)
 rename exec_cmd.h => exec-cmd.h (100%)

diff --git a/Makefile b/Makefile
index be4ac5b2a6..f608c592b7 100644
--- a/Makefile
+++ b/Makefile
@@ -815,7 +815,7 @@ LIB_OBJS += ewah/bitmap.o
 LIB_OBJS += ewah/ewah_bitmap.o
 LIB_OBJS += ewah/ewah_io.o
 LIB_OBJS += ewah/ewah_rlw.o
-LIB_OBJS += exec_cmd.o
+LIB_OBJS += exec-cmd.o
 LIB_OBJS += fetch-object.o
 LIB_OBJS += fetch-pack.o
 LIB_OBJS += fsck.o
@@ -2152,8 +2152,8 @@ else
 $(OBJECTS): $(LIB_H)
 endif
 
-exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX
-exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \
+exec-cmd.sp exec-cmd.s exec-cmd.o: GIT-PREFIX
+exec-cmd.sp exec-cmd.s exec-cmd.o: EXTRA_CPPFLAGS = \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
 	'-DBINDIR="$(bindir_relative_SQ)"' \
 	'-DPREFIX="$(prefix_SQ)"'
diff --git a/attr.c b/attr.c
index dfc3a558d8..03a678fa9b 100644
--- a/attr.c
+++ b/attr.c
@@ -10,7 +10,7 @@
 #define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "config.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "attr.h"
 #include "dir.h"
 #include "utf8.h"
diff --git a/builtin/add.c b/builtin/add.c
index 9ef7fb02d5..c9e2619a9a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -9,7 +9,7 @@
 #include "lockfile.h"
 #include "dir.h"
 #include "pathspec.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "cache-tree.h"
 #include "run-command.h"
 #include "parse-options.h"
diff --git a/builtin/am.c b/builtin/am.c
index 1bcc3606c5..269743ff76 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -6,7 +6,7 @@
 #include "cache.h"
 #include "config.h"
 #include "builtin.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "parse-options.h"
 #include "dir.h"
 #include "run-command.h"
diff --git a/builtin/describe.c b/builtin/describe.c
index de840f96a4..b5afc45846 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -6,7 +6,7 @@
 #include "blob.h"
 #include "refs.h"
 #include "builtin.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "parse-options.h"
 #include "revision.h"
 #include "diff.h"
diff --git a/builtin/difftool.c b/builtin/difftool.c
index ee8dce019e..aad0e073ee 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -15,7 +15,7 @@
 #include "config.h"
 #include "builtin.h"
 #include "run-command.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "parse-options.h"
 #include "argv-array.h"
 #include "strbuf.h"
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 526da5c185..a9a3a198c3 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -9,7 +9,7 @@
 #include "blob.h"
 #include "quote.h"
 #include "parse-options.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 
 /*
  * This is to create corrupt objects for debugging and as such it
diff --git a/builtin/help.c b/builtin/help.c
index 598867cfea..2d51071429 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -4,7 +4,7 @@
 #include "cache.h"
 #include "config.h"
 #include "builtin.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "parse-options.h"
 #include "run-command.h"
 #include "column.h"
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 657a5dda06..e89c039b56 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -9,7 +9,7 @@
 #include "tree.h"
 #include "progress.h"
 #include "fsck.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "streaming.h"
 #include "thread-utils.h"
 #include "packfile.h"
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 68ff4ad75a..2542c5244e 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -7,7 +7,7 @@
 #include "config.h"
 #include "refs.h"
 #include "builtin.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "parse-options.h"
 
 #ifndef DEFAULT_GIT_TEMPLATE_DIR
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 32736e0b10..bf01e05808 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -2,7 +2,7 @@
 #include "tree-walk.h"
 #include "xdiff-interface.h"
 #include "blob.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "merge-blobs.h"
 
 static const char merge_tree_usage[] = "git merge-tree <base-tree> <branch1> <branch2>";
diff --git a/builtin/notes.c b/builtin/notes.c
index 921e08d5bf..e5bf80eef1 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -14,7 +14,7 @@
 #include "blob.h"
 #include "pretty.h"
 #include "refs.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "run-command.h"
 #include "parse-options.h"
 #include "string-list.h"
diff --git a/builtin/pull.c b/builtin/pull.c
index e32d6cd5b4..71aac5005e 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -9,7 +9,7 @@
 #include "config.h"
 #include "builtin.h"
 #include "parse-options.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "run-command.h"
 #include "sha1-array.h"
 #include "remote.h"
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 2bf7f2d1a3..3bbb7097aa 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -6,7 +6,7 @@
 #include "pkt-line.h"
 #include "sideband.h"
 #include "run-command.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "commit.h"
 #include "object.h"
 #include "remote.h"
diff --git a/common-main.c b/common-main.c
index 7d716d5a54..b989e136b5 100644
--- a/common-main.c
+++ b/common-main.c
@@ -1,5 +1,5 @@
 #include "cache.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "attr.h"
 
 /*
diff --git a/config.c b/config.c
index c698988f5e..e2b87b4764 100644
--- a/config.c
+++ b/config.c
@@ -9,7 +9,7 @@
 #include "config.h"
 #include "repository.h"
 #include "lockfile.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "strbuf.h"
 #include "quote.h"
 #include "hashmap.h"
diff --git a/exec_cmd.c b/exec-cmd.c
similarity index 99%
rename from exec_cmd.c
rename to exec-cmd.c
index ce192a2d64..8a8261746a 100644
--- a/exec_cmd.c
+++ b/exec-cmd.c
@@ -1,5 +1,5 @@
 #include "cache.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "quote.h"
 #include "argv-array.h"
 #define MAX_ARGS	32
diff --git a/exec_cmd.h b/exec-cmd.h
similarity index 100%
rename from exec_cmd.h
rename to exec-cmd.h
diff --git a/fetch-pack.c b/fetch-pack.c
index 52932b37f8..1c8b60c434 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -5,7 +5,7 @@
 #include "pkt-line.h"
 #include "commit.h"
 #include "tag.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "pack.h"
 #include "sideband.h"
 #include "fetch-pack.h"
diff --git a/git.c b/git.c
index 3a89893712..f598fae7b7 100644
--- a/git.c
+++ b/git.c
@@ -1,6 +1,6 @@
 #include "builtin.h"
 #include "config.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "help.h"
 #include "run-command.h"
 
diff --git a/help.c b/help.c
index 60071a9bea..a4feef2ffe 100644
--- a/help.c
+++ b/help.c
@@ -1,7 +1,7 @@
 #include "cache.h"
 #include "config.h"
 #include "builtin.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "run-command.h"
 #include "levenshtein.h"
 #include "help.h"
diff --git a/http-backend.c b/http-backend.c
index f3dc218b2a..7f62efcd89 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -4,7 +4,7 @@
 #include "pkt-line.h"
 #include "object.h"
 #include "tag.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "run-command.h"
 #include "string-list.h"
 #include "url.h"
diff --git a/http-fetch.c b/http-fetch.c
index 8af380050c..885e471501 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -1,6 +1,6 @@
 #include "cache.h"
 #include "config.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "http.h"
 #include "walker.h"
 
diff --git a/http-push.c b/http-push.c
index ff82b63133..fcd3a6ce83 100644
--- a/http-push.c
+++ b/http-push.c
@@ -6,7 +6,7 @@
 #include "refs.h"
 #include "diff.h"
 #include "revision.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "remote.h"
 #include "list-objects.h"
 #include "sigchain.h"
diff --git a/imap-send.c b/imap-send.c
index ffb0a6eca8..3573cbfb0f 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -24,7 +24,7 @@
 #include "cache.h"
 #include "config.h"
 #include "credential.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "run-command.h"
 #include "parse-options.h"
 #ifdef NO_OPENSSL
diff --git a/remote-curl.c b/remote-curl.c
index a7c4c9b5ff..8d2ffaf8de 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -4,7 +4,7 @@
 #include "strbuf.h"
 #include "walker.h"
 #include "http.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "run-command.h"
 #include "pkt-line.h"
 #include "string-list.h"
diff --git a/remote-testsvn.c b/remote-testsvn.c
index c4bb9a8ba9..444d98059f 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -3,7 +3,7 @@
 #include "remote.h"
 #include "strbuf.h"
 #include "url.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "run-command.h"
 #include "vcs-svn/svndump.h"
 #include "notes.h"
diff --git a/run-command.c b/run-command.c
index 84899e423f..12c94c1dbe 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1,6 +1,6 @@
 #include "cache.h"
 #include "run-command.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "sigchain.h"
 #include "argv-array.h"
 #include "thread-utils.h"
diff --git a/sequencer.c b/sequencer.c
index 667f35ebdf..6d631d25c6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -7,7 +7,7 @@
 #include "sequencer.h"
 #include "tag.h"
 #include "run-command.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "utf8.h"
 #include "cache-tree.h"
 #include "diff.h"
diff --git a/shell.c b/shell.c
index 234b2d4f16..0200d10796 100644
--- a/shell.c
+++ b/shell.c
@@ -1,6 +1,6 @@
 #include "cache.h"
 #include "quote.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "strbuf.h"
 #include "run-command.h"
 
diff --git a/upload-pack.c b/upload-pack.c
index 4a82602be5..6261d4fab3 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -6,7 +6,7 @@
 #include "tag.h"
 #include "object.h"
 #include "commit.h"
-#include "exec_cmd.h"
+#include "exec-cmd.h"
 #include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 4/6] sha1_name.c: rename to use dash in file name

[Stefan Beller]

This is more consistent with the project style. The majority of Git's
source files use dashes in preference to underscores in their file names.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Makefile                   | 2 +-
 list-objects-filter.c      | 2 +-
 object.h                   | 2 +-
 sha1_name.c => sha1-name.c | 0
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename sha1_name.c => sha1-name.c (100%)

diff --git a/Makefile b/Makefile
index f608c592b7..a54eef2f23 100644
--- a/Makefile
+++ b/Makefile
@@ -898,7 +898,7 @@ LIB_OBJS += setup.o
 LIB_OBJS += sha1-array.o
 LIB_OBJS += sha1-lookup.o
 LIB_OBJS += sha1_file.o
-LIB_OBJS += sha1_name.o
+LIB_OBJS += sha1-name.o
 LIB_OBJS += shallow.o
 LIB_OBJS += sideband.o
 LIB_OBJS += sigchain.o
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 0ec83aaf18..247717561f 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -19,7 +19,7 @@
  * in the traversal (until we mark it SEEN).  This is a way to
  * let us silently de-dup calls to show() in the caller.  This
  * is subtly different from the "revision.h:SHOWN" and the
- * "sha1_name.c:ONELINE_SEEN" bits.  And also different from
+ * "sha1-name.c:ONELINE_SEEN" bits.  And also different from
  * the non-de-dup usage in pack-bitmap.c
  */
 #define FILTER_SHOWN_BUT_REVISIT (1<<21)
diff --git a/object.h b/object.h
index f13f85b2a9..b8e70e5519 100644
--- a/object.h
+++ b/object.h
@@ -37,7 +37,7 @@ struct object_array {
  * bundle.c:                                        16
  * http-push.c:                                     16-----19
  * commit.c:                                        16-----19
- * sha1_name.c:                                              20
+ * sha1-name.c:                                              20
  * list-objects-filter.c:                                      21
  * builtin/fsck.c:           0--3
  * builtin/index-pack.c:                                     2021
diff --git a/sha1_name.c b/sha1-name.c
similarity index 100%
rename from sha1_name.c
rename to sha1-name.c
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 5/6] sha1_file.c: rename to use dash in file name

[Stefan Beller]

This is more consistent with the project style. The majority of Git's
source files use dashes in preference to underscores in their file names.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/technical/api-object-access.txt | 2 +-
 Makefile                                      | 2 +-
 builtin/index-pack.c                          | 2 +-
 sha1_file.c => sha1-file.c                    | 0
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename sha1_file.c => sha1-file.c (100%)

diff --git a/Documentation/technical/api-object-access.txt b/Documentation/technical/api-object-access.txt
index a1162e5bcd..5b29622d00 100644
--- a/Documentation/technical/api-object-access.txt
+++ b/Documentation/technical/api-object-access.txt
@@ -1,7 +1,7 @@
 object access API
 =================
 
-Talk about <sha1_file.c> and <object.h> family, things like
+Talk about <sha1-file.c> and <object.h> family, things like
 
 * read_sha1_file()
 * read_object_with_reference()
diff --git a/Makefile b/Makefile
index a54eef2f23..d24695f292 100644
--- a/Makefile
+++ b/Makefile
@@ -897,7 +897,7 @@ LIB_OBJS += server-info.o
 LIB_OBJS += setup.o
 LIB_OBJS += sha1-array.o
 LIB_OBJS += sha1-lookup.o
-LIB_OBJS += sha1_file.o
+LIB_OBJS += sha1-file.o
 LIB_OBJS += sha1-name.o
 LIB_OBJS += shallow.o
 LIB_OBJS += sideband.o
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index e89c039b56..a9b0fefc94 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1592,7 +1592,7 @@ static void read_idx_option(struct pack_idx_option *opts, const char *pack_name)
 	/*
 	 * Get rid of the idx file as we do not need it anymore.
 	 * NEEDSWORK: extract this bit from free_pack_by_name() in
-	 * sha1_file.c, perhaps?  It shouldn't matter very much as we
+	 * sha1-file.c, perhaps?  It shouldn't matter very much as we
 	 * know we haven't installed this pack (hence we never have
 	 * read anything from it).
 	 */
diff --git a/sha1_file.c b/sha1-file.c
similarity index 100%
rename from sha1_file.c
rename to sha1-file.c
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 6/6] replace_object.c: rename to use dash in file name

[Stefan Beller]

This is more consistent with the project style. The majority of
Git's source files use dashes in preference to underscores in their file
names.

Noticed while adding a header corresponding to this file.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Makefile                             | 2 +-
 replace_object.c => replace-object.c | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename replace_object.c => replace-object.c (100%)

diff --git a/Makefile b/Makefile
index d24695f292..6a8168b858 100644
--- a/Makefile
+++ b/Makefile
@@ -885,7 +885,7 @@ LIB_OBJS += refs/packed-backend.o
 LIB_OBJS += refs/ref-cache.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
-LIB_OBJS += replace_object.o
+LIB_OBJS += replace-object.o
 LIB_OBJS += repository.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
diff --git a/replace_object.c b/replace-object.c
similarity index 100%
rename from replace_object.c
rename to replace-object.c
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 0/6] Rename files to use dashes instead of underscores

[Stefan Beller]

On Tue, Apr 10, 2018 at 2:26 PM, Stefan Beller <sbeller@google.com> wrote:
> This is the followup for
> https://public-inbox.org/git/xmqqbmer4vfh.fsf@gitster-ct.c.googlers.com/
>
> We have no files left with underscores in their names.
>

It applies on master, and also contains that patch for replace_objects,
which I would intend to drop from that other series.

Re: [PATCH 0/6] Rename files to use dashes instead of underscores

[Johannes Schindelin]

Hi Stefan,

On Tue, 10 Apr 2018, Stefan Beller wrote:

> This is the followup for 
> https://public-inbox.org/git/xmqqbmer4vfh.fsf@gitster-ct.c.googlers.com/
> 
> We have no files left with underscores in their names.

Yaaay!

> Stefan Beller (6):
>   write_or_die.c: rename to use dashes in file name
>   unicode_width.h: rename to use dash in file name
>   exec_cmd: rename to use dash in file name
>   sha1_name.c: rename to use dash in file name
>   sha1_file.c: rename to use dash in file name
>   replace_object.c: rename to use dash in file name

These are all obviously correct (I did not apply the series and used `git
grep` to verify that nothing underscored is left there, but I trust you to
have done that already).

Ciao,
Dscho

Re: [PATCH 0/6] Rename files to use dashes instead of underscores

[Stefan Beller]

Hi Johannes,

On Tue, Apr 10, 2018 at 3:39 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Stefan,
>
> On Tue, 10 Apr 2018, Stefan Beller wrote:
>
>> This is the followup for
>> https://public-inbox.org/git/xmqqbmer4vfh.fsf@gitster-ct.c.googlers.com/
>>
>> We have no files left with underscores in their names.
>
> Yaaay!
>
>> Stefan Beller (6):
>>   write_or_die.c: rename to use dashes in file name
>>   unicode_width.h: rename to use dash in file name
>>   exec_cmd: rename to use dash in file name
>>   sha1_name.c: rename to use dash in file name
>>   sha1_file.c: rename to use dash in file name
>>   replace_object.c: rename to use dash in file name
>
> These are all obviously correct (I did not apply the series and used `git
> grep` to verify that nothing underscored is left there, but I trust you to
> have done that already).

Yes and I did not tell the full story.

There is still 'check_bindir' as compile output, such that I ignored it.

And there was sha1dc_git.{h, c} which I also ignored as it is unclear
to me how our relationship with the DC library is.
I think that could also be converted as it seems to be a shallow wrapper
around that lib.

Re: [PATCH 0/6] Rename files to use dashes instead of underscores

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 365 bytes --]

On Tue, Apr 10, 2018 at 02:26:15PM -0700, Stefan Beller wrote:
> This is the followup for 
> https://public-inbox.org/git/xmqqbmer4vfh.fsf@gitster-ct.c.googlers.com/
> 
> We have no files left with underscores in their names.

This series looked good to me.  It's a nice change.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

[PATCH 03/16] replace-object: move replace_map to object store

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 object-store.h   |  8 ++++++++
 replace-object.c | 17 +++++++----------
 replace-object.h |  9 +++++++++
 3 files changed, 24 insertions(+), 10 deletions(-)
 create mode 100644 replace-object.h

diff --git a/object-store.h b/object-store.h
index fef33f345f..c04b4c95eb 100644
--- a/object-store.h
+++ b/object-store.h
@@ -1,6 +1,8 @@
 #ifndef OBJECT_STORE_H
 #define OBJECT_STORE_H
 
+#include "oidmap.h"
+
 struct alternate_object_database {
 	struct alternate_object_database *next;
 
@@ -93,6 +95,12 @@ struct raw_object_store {
 	struct alternate_object_database *alt_odb_list;
 	struct alternate_object_database **alt_odb_tail;
 
+	/*
+	 * Objects that should be substituted by other objects
+	 * (see git-replace(1)).
+	 */
+	struct oidmap replace_map;
+
 	/*
 	 * private data
 	 *
diff --git a/replace-object.c b/replace-object.c
index a757a5ebf2..afbdf2df25 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -1,15 +1,11 @@
 #include "cache.h"
 #include "oidmap.h"
+#include "object-store.h"
+#include "replace-object.h"
 #include "refs.h"
+#include "repository.h"
 #include "commit.h"
 
-struct replace_object {
-	struct oidmap_entry original;
-	struct object_id replacement;
-};
-
-static struct oidmap replace_map = OIDMAP_INIT;
-
 static int register_replace_ref(const char *refname,
 				const struct object_id *oid,
 				int flag, void *cb_data)
@@ -29,7 +25,7 @@ static int register_replace_ref(const char *refname,
 	oidcpy(&repl_obj->replacement, oid);
 
 	/* Register new object */
-	if (oidmap_put(&replace_map, repl_obj))
+	if (oidmap_put(&the_repository->objects->replace_map, repl_obj))
 		die("duplicate replace ref: %s", refname);
 
 	return 0;
@@ -44,7 +40,7 @@ static void prepare_replace_object(void)
 
 	for_each_replace_ref(register_replace_ref, NULL);
 	replace_object_prepared = 1;
-	if (!replace_map.map.tablesize)
+	if (!the_repository->objects->replace_map.map.tablesize)
 		check_replace_refs = 0;
 }
 
@@ -67,7 +63,8 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)
 
 	/* Try to recursively replace the object */
 	while (depth-- > 0) {
-		struct replace_object *repl_obj = oidmap_get(&replace_map, cur);
+		struct replace_object *repl_obj =
+			oidmap_get(&the_repository->objects->replace_map, cur);
 		if (!repl_obj)
 			return cur;
 		cur = &repl_obj->replacement;
diff --git a/replace-object.h b/replace-object.h
new file mode 100644
index 0000000000..f9a2b70eb8
--- /dev/null
+++ b/replace-object.h
@@ -0,0 +1,9 @@
+#ifndef REPLACE_OBJECT_H
+#define REPLACE_OBJECT_H
+
+struct replace_object {
+	struct oidmap_entry original;
+	struct object_id replacement;
+};
+
+#endif /* REPLACE_OBJECT_H */
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 03/16] replace-object: move replace_map to object store

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  object-store.h   |  8 ++++++++
>  replace-object.c | 17 +++++++----------
>  replace-object.h |  9 +++++++++
>  3 files changed, 24 insertions(+), 10 deletions(-)
>  create mode 100644 replace-object.h

With this, the relationship between an object X and another object Y
that replaces the object X is defined only within the scope of a
single repository.

The exception in reachability rule around these replacement objects
is also local to a repository (i.e. if traversal from refs reaches
X, then both X and Y are reachable and need to be kept from gc), so
I think this is a reasonable way to arrange things.

[PATCH 04/16] object-store: move lookup_replace_object to replace-object.h

[Stefan Beller]

lookup_replace_object is a low-level function that most users of the
object store do not need to use directly.

Move it to replace-object.h to avoid a dependency loop in an upcoming
change to its inline definition that will make use of repository.h.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/mktag.c  |  1 +
 cache.h          | 19 -------------------
 object.c         |  1 +
 replace-object.h | 22 ++++++++++++++++++++++
 sha1_file.c      |  1 +
 streaming.c      |  1 +
 6 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/builtin/mktag.c b/builtin/mktag.c
index 9f5a50a8fd..e3d20a7722 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "tag.h"
+#include "replace-object.h"
 
 /*
  * A signature file has a very simple fixed format: four lines
diff --git a/cache.h b/cache.h
index a5c4fddf77..e3c6cba514 100644
--- a/cache.h
+++ b/cache.h
@@ -1187,25 +1187,6 @@ static inline void *read_object_file(const struct object_id *oid, enum object_ty
 	return read_object_file_extended(oid, type, size, 1);
 }
 
-/*
- * This internal function is only declared here for the benefit of
- * lookup_replace_object().  Please do not call it directly.
- */
-extern const struct object_id *do_lookup_replace_object(const struct object_id *oid);
-
-/*
- * If object sha1 should be replaced, return the replacement object's
- * name (replaced recursively, if necessary).  The return value is
- * either sha1 or a pointer to a permanently-allocated value.  When
- * object replacement is suppressed, always return sha1.
- */
-static inline const struct object_id *lookup_replace_object(const struct object_id *oid)
-{
-	if (!check_replace_refs)
-		return oid;
-	return do_lookup_replace_object(oid);
-}
-
 /* Read and unpack an object file into memory, write memory to an object file */
 extern int oid_object_info(const struct object_id *, unsigned long *);
 
diff --git a/object.c b/object.c
index a0a756f24f..998ec2a25f 100644
--- a/object.c
+++ b/object.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "object.h"
+#include "replace-object.h"
 #include "blob.h"
 #include "tree.h"
 #include "commit.h"
diff --git a/replace-object.h b/replace-object.h
index f9a2b70eb8..15315311fb 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -1,9 +1,31 @@
 #ifndef REPLACE_OBJECT_H
 #define REPLACE_OBJECT_H
 
+#include "oidmap.h"
+#include "repository.h"
+
 struct replace_object {
 	struct oidmap_entry original;
 	struct object_id replacement;
 };
 
+/*
+ * This internal function is only declared here for the benefit of
+ * lookup_replace_object().  Please do not call it directly.
+ */
+extern const struct object_id *do_lookup_replace_object(const struct object_id *oid);
+
+/*
+ * If object sha1 should be replaced, return the replacement object's
+ * name (replaced recursively, if necessary).  The return value is
+ * either sha1 or a pointer to a permanently-allocated value.  When
+ * object replacement is suppressed, always return sha1.
+ */
+static inline const struct object_id *lookup_replace_object(const struct object_id *oid)
+{
+	if (!check_replace_refs)
+		return oid;
+	return do_lookup_replace_object(oid);
+}
+
 #endif /* REPLACE_OBJECT_H */
diff --git a/sha1_file.c b/sha1_file.c
index 3e0af41892..c38e41e49e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -23,6 +23,7 @@
 #include "sha1-lookup.h"
 #include "bulk-checkin.h"
 #include "repository.h"
+#include "replace-object.h"
 #include "streaming.h"
 #include "dir.h"
 #include "list.h"
diff --git a/streaming.c b/streaming.c
index 7d55ba64c7..a6e1162946 100644
--- a/streaming.c
+++ b/streaming.c
@@ -5,6 +5,7 @@
 #include "streaming.h"
 #include "repository.h"
 #include "object-store.h"
+#include "replace-object.h"
 #include "packfile.h"
 
 enum input_source {
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 05/16] replace-object: eliminate replace objects prepared flag

[Stefan Beller]

By making the oidmap a pointer, we eliminate the need for
the global boolean variable 'replace_object_prepared'.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 object-store.h   |  2 +-
 replace-object.c | 16 +++++++++-------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/object-store.h b/object-store.h
index c04b4c95eb..1ff862c7f9 100644
--- a/object-store.h
+++ b/object-store.h
@@ -99,7 +99,7 @@ struct raw_object_store {
 	 * Objects that should be substituted by other objects
 	 * (see git-replace(1)).
 	 */
-	struct oidmap replace_map;
+	struct oidmap *replace_map;
 
 	/*
 	 * private data
diff --git a/replace-object.c b/replace-object.c
index afbdf2df25..953fa9cc40 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -25,7 +25,7 @@ static int register_replace_ref(const char *refname,
 	oidcpy(&repl_obj->replacement, oid);
 
 	/* Register new object */
-	if (oidmap_put(&the_repository->objects->replace_map, repl_obj))
+	if (oidmap_put(the_repository->objects->replace_map, repl_obj))
 		die("duplicate replace ref: %s", refname);
 
 	return 0;
@@ -33,14 +33,16 @@ static int register_replace_ref(const char *refname,
 
 static void prepare_replace_object(void)
 {
-	static int replace_object_prepared;
-
-	if (replace_object_prepared)
+	if (the_repository->objects->replace_map)
 		return;
 
+	the_repository->objects->replace_map =
+		xmalloc(sizeof(*the_repository->objects->replace_map));
+	oidmap_init(the_repository->objects->replace_map, 0);
+
 	for_each_replace_ref(register_replace_ref, NULL);
-	replace_object_prepared = 1;
-	if (!the_repository->objects->replace_map.map.tablesize)
+
+	if (!the_repository->objects->replace_map->map.tablesize)
 		check_replace_refs = 0;
 }
 
@@ -64,7 +66,7 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)
 	/* Try to recursively replace the object */
 	while (depth-- > 0) {
 		struct replace_object *repl_obj =
-			oidmap_get(&the_repository->objects->replace_map, cur);
+			oidmap_get(the_repository->objects->replace_map, cur);
 		if (!repl_obj)
 			return cur;
 		cur = &repl_obj->replacement;
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 05/16] replace-object: eliminate replace objects prepared flag

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> By making the oidmap a pointer, we eliminate the need for
> the global boolean variable 'replace_object_prepared'.

That is not quite a justification for this change, as making it a
pointer (and paying for the malloc(3) overhead) is not the only way
to remove the variable (i.e. the "has this been initialized?" bit
can be moved to "struct raw_object_store").

One possible advantage of this approach, I guess, is that we would
more quickly catch code that tries to access replace-map without
initializing it.

Re: [PATCH 05/16] replace-object: eliminate replace objects prepared flag

[René Scharfe]

Am 10.04.2018 um 00:45 schrieb Stefan Beller:
> By making the oidmap a pointer, we eliminate the need for
> the global boolean variable 'replace_object_prepared'.
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>   object-store.h   |  2 +-
>   replace-object.c | 16 +++++++++-------
>   2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/object-store.h b/object-store.h
> index c04b4c95eb..1ff862c7f9 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -99,7 +99,7 @@ struct raw_object_store {
>   	 * Objects that should be substituted by other objects
>   	 * (see git-replace(1)).
>   	 */
> -	struct oidmap replace_map;
> +	struct oidmap *replace_map;

This also allows the '#include "oidmap.h"' introduced in patch 3 to be
replaced by 'struct oidmap;' (forward declaration instead of include).
Keeping the type opaque discourages circumventing accessor functions;
not dragging in other headers avoids some compile time overhead.

René

[PATCH 06/16] replace-object: check_replace_refs is safe in multi repo environment

[Stefan Beller]

In e1111cef23 (inline lookup_replace_object() calls, 2011-05-15) a shortcut
for checking the object replacement was added by setting check_replace_refs
to 0 once the replacements were evaluated to not exist. This works fine in
with the assumption of only one repository in existence.

The assumption won't hold true any more when we work on multiple instances
of a repository structs (e.g. one struct per submodule), as the first
repository to be inspected may have no replacements and would set the
global variable. Other repositories would then completely omit their
evaluation of replacements.

This reverts back the meaning of the flag `check_replace_refs` of
"Do we need to check with the lookup table?" to "Do we need to read
the replacement definition?", adding the bypassing logic to
lookup_replace_object after the replacement definition was read.
As with the original patch, delay the renaming of the global variable

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 environment.c    | 2 +-
 replace-object.c | 3 ---
 replace-object.h | 5 ++++-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/environment.c b/environment.c
index 39b3d906c8..b991fc0a87 100644
--- a/environment.c
+++ b/environment.c
@@ -50,7 +50,7 @@ const char *editor_program;
 const char *askpass_program;
 const char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
-int check_replace_refs = 1;
+int check_replace_refs = 1; /* NEEDSWORK: rename to read_replace_refs */
 char *git_replace_ref_base;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
diff --git a/replace-object.c b/replace-object.c
index 953fa9cc40..b2405f6027 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -41,9 +41,6 @@ static void prepare_replace_object(void)
 	oidmap_init(the_repository->objects->replace_map, 0);
 
 	for_each_replace_ref(register_replace_ref, NULL);
-
-	if (!the_repository->objects->replace_map->map.tablesize)
-		check_replace_refs = 0;
 }
 
 /* We allow "recursive" replacement. Only within reason, though */
diff --git a/replace-object.h b/replace-object.h
index 15315311fb..dbc51265ec 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -3,6 +3,7 @@
 
 #include "oidmap.h"
 #include "repository.h"
+#include "object-store.h"
 
 struct replace_object {
 	struct oidmap_entry original;
@@ -23,7 +24,9 @@ extern const struct object_id *do_lookup_replace_object(const struct object_id *
  */
 static inline const struct object_id *lookup_replace_object(const struct object_id *oid)
 {
-	if (!check_replace_refs)
+	if (!check_replace_refs ||
+	    (the_repository->objects->replace_map &&
+	     the_repository->objects->replace_map->map.tablesize == 0))
 		return oid;
 	return do_lookup_replace_object(oid);
 }
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 06/16] replace-object: check_replace_refs is safe in multi repo environment

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> In e1111cef23 (inline lookup_replace_object() calls, 2011-05-15) a
> shortcut for checking the object replacement was added by setting
> check_replace_refs to 0 once the replacements were evaluated to
> not exist. This works fine in with the assumption of only one
> repository in existence.

"works fine in with the..."?  I guess s/ in with/ with/?

> The assumption won't hold true any more when we work on multiple
> instances of a repository structs (e.g. one struct per submodule),
> as the first repository to be inspected may have no replacements
> and would set the global variable. Other repositories would then
> completely omit their evaluation of replacements.
>
> This reverts back the meaning of the flag `check_replace_refs` of
> "Do we need to check with the lookup table?" to "Do we need to read
> the replacement definition?", adding the bypassing logic to
> lookup_replace_object after the replacement definition was read.
> As with the original patch, delay the renaming of the global variable

Hmph, if we decided that replace database is per repository
instance, shouldn't this variable also become per repository,
instead of staying to be a system-wide global?

Perhaps that will happpen in a later stage of the series that I
haven't seen yet, I guess.  And until that happens, we disable the
optimization and always call into do_lookup_replace_object() when
lookup_replace_object() is called, which is OK.

>  static inline const struct object_id *lookup_replace_object(const struct object_id *oid)
>  {
> -	if (!check_replace_refs)
> +	if (!check_replace_refs ||
> +	    (the_repository->objects->replace_map &&
> +	     the_repository->objects->replace_map->map.tablesize == 0))

Ah, we still have the same optimization, so this looks alright.  The
variable's name and semantics do need to be updated--I didn't check
but if this variable is exposed to the end users in any way, such a
fundamental sematic change may be hard to transition, though.

This series looks good so far.

[PATCH 07/16] refs: add repository argument to get_main_ref_store

[Stefan Beller]

Add a repository argument to allow the get_main_ref_store caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/pack-refs.c       |  3 +-
 refs.c                    | 67 ++++++++++++++++++++-------------------
 refs.h                    |  4 ++-
 revision.c                |  5 +--
 t/helper/test-ref-store.c |  3 +-
 5 files changed, 44 insertions(+), 38 deletions(-)

diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index b106a392a4..f3353564f9 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "parse-options.h"
 #include "refs.h"
+#include "repository.h"
 
 static char const * const pack_refs_usage[] = {
 	N_("git pack-refs [<options>]"),
@@ -17,5 +18,5 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
 	};
 	if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
 		usage_with_options(pack_refs_usage, opts);
-	return refs_pack_refs(get_main_ref_store(), flags);
+	return refs_pack_refs(get_main_ref_store(the_repository), flags);
 }
diff --git a/refs.c b/refs.c
index 8b7a77fe5e..74d4ed97cb 100644
--- a/refs.c
+++ b/refs.c
@@ -13,6 +13,7 @@
 #include "tag.h"
 #include "submodule.h"
 #include "worktree.h"
+#include "repository.h"
 
 /*
  * List of all available backends
@@ -206,7 +207,7 @@ char *refs_resolve_refdup(struct ref_store *refs,
 char *resolve_refdup(const char *refname, int resolve_flags,
 		     struct object_id *oid, int *flags)
 {
-	return refs_resolve_refdup(get_main_ref_store(),
+	return refs_resolve_refdup(get_main_ref_store(the_repository),
 				   refname, resolve_flags,
 				   oid, flags);
 }
@@ -228,7 +229,7 @@ int refs_read_ref_full(struct ref_store *refs, const char *refname,
 
 int read_ref_full(const char *refname, int resolve_flags, struct object_id *oid, int *flags)
 {
-	return refs_read_ref_full(get_main_ref_store(), refname,
+	return refs_read_ref_full(get_main_ref_store(the_repository), refname,
 				  resolve_flags, oid, flags);
 }
 
@@ -375,7 +376,7 @@ int refs_for_each_tag_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_tag_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_tag_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_tag_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
@@ -385,7 +386,7 @@ int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_da
 
 int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_branch_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_branch_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
@@ -395,7 +396,7 @@ int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_da
 
 int for_each_remote_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_remote_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_remote_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int head_ref_namespaced(each_ref_fn fn, void *cb_data)
@@ -730,7 +731,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
 	struct strbuf err = STRBUF_INIT;
 
 	if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
-		assert(refs == get_main_ref_store());
+		assert(refs == get_main_ref_store(the_repository));
 		return delete_pseudoref(refname, old_oid);
 	}
 
@@ -752,7 +753,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
 int delete_ref(const char *msg, const char *refname,
 	       const struct object_id *old_oid, unsigned int flags)
 {
-	return refs_delete_ref(get_main_ref_store(), msg, refname,
+	return refs_delete_ref(get_main_ref_store(the_repository), msg, refname,
 			       old_oid, flags);
 }
 
@@ -928,7 +929,7 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
 
 struct ref_transaction *ref_transaction_begin(struct strbuf *err)
 {
-	return ref_store_transaction_begin(get_main_ref_store(), err);
+	return ref_store_transaction_begin(get_main_ref_store(the_repository), err);
 }
 
 void ref_transaction_free(struct ref_transaction *transaction)
@@ -1060,7 +1061,7 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
 	int ret = 0;
 
 	if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
-		assert(refs == get_main_ref_store());
+		assert(refs == get_main_ref_store(the_repository));
 		ret = write_pseudoref(refname, new_oid, old_oid, &err);
 	} else {
 		t = ref_store_transaction_begin(refs, &err);
@@ -1099,7 +1100,7 @@ int update_ref(const char *msg, const char *refname,
 	       const struct object_id *old_oid,
 	       unsigned int flags, enum action_on_err onerr)
 {
-	return refs_update_ref(get_main_ref_store(), msg, refname, new_oid,
+	return refs_update_ref(get_main_ref_store(the_repository), msg, refname, new_oid,
 			       old_oid, flags, onerr);
 }
 
@@ -1320,7 +1321,7 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int head_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_head_ref(get_main_ref_store(), fn, cb_data);
+	return refs_head_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 struct ref_iterator *refs_ref_iterator_begin(
@@ -1379,7 +1380,7 @@ int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
@@ -1390,7 +1391,7 @@ int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
 
 int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_ref_in(get_main_ref_store(), prefix, fn, cb_data);
+	return refs_for_each_ref_in(get_main_ref_store(the_repository), prefix, fn, cb_data);
 }
 
 int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)
@@ -1399,7 +1400,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsig
 
 	if (broken)
 		flag = DO_FOR_EACH_INCLUDE_BROKEN;
-	return do_for_each_ref(get_main_ref_store(),
+	return do_for_each_ref(get_main_ref_store(the_repository),
 			       prefix, fn, 0, flag, cb_data);
 }
 
@@ -1416,7 +1417,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 
 int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(get_main_ref_store(),
+	return do_for_each_ref(get_main_ref_store(the_repository),
 			       git_replace_ref_base, fn,
 			       strlen(git_replace_ref_base),
 			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
@@ -1427,7 +1428,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_main_ref_store(),
+	ret = do_for_each_ref(get_main_ref_store(the_repository),
 			      buf.buf, fn, 0, 0, cb_data);
 	strbuf_release(&buf);
 	return ret;
@@ -1441,7 +1442,7 @@ int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_rawref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_rawref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_rawref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_read_raw_ref(struct ref_store *ref_store,
@@ -1547,7 +1548,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 /* backend functions */
 int refs_init_db(struct strbuf *err)
 {
-	struct ref_store *refs = get_main_ref_store();
+	struct ref_store *refs = get_main_ref_store(the_repository);
 
 	return refs->be->init_db(refs, err);
 }
@@ -1555,7 +1556,7 @@ int refs_init_db(struct strbuf *err)
 const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
 			       struct object_id *oid, int *flags)
 {
-	return refs_resolve_ref_unsafe(get_main_ref_store(), refname,
+	return refs_resolve_ref_unsafe(get_main_ref_store(the_repository), refname,
 				       resolve_flags, oid, flags);
 }
 
@@ -1651,7 +1652,7 @@ static struct ref_store *ref_store_init(const char *gitdir,
 	return refs;
 }
 
-struct ref_store *get_main_ref_store(void)
+struct ref_store *get_main_ref_store_the_repository(void)
 {
 	if (main_ref_store)
 		return main_ref_store;
@@ -1726,7 +1727,7 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)
 	const char *id;
 
 	if (wt->is_current)
-		return get_main_ref_store();
+		return get_main_ref_store(the_repository);
 
 	id = wt->id ? wt->id : "/";
 	refs = lookup_ref_store_map(&worktree_ref_stores, id);
@@ -1782,7 +1783,7 @@ int refs_peel_ref(struct ref_store *refs, const char *refname,
 
 int peel_ref(const char *refname, struct object_id *oid)
 {
-	return refs_peel_ref(get_main_ref_store(), refname, oid);
+	return refs_peel_ref(get_main_ref_store(the_repository), refname, oid);
 }
 
 int refs_create_symref(struct ref_store *refs,
@@ -1798,7 +1799,7 @@ int refs_create_symref(struct ref_store *refs,
 int create_symref(const char *ref_target, const char *refs_heads_master,
 		  const char *logmsg)
 {
-	return refs_create_symref(get_main_ref_store(), ref_target,
+	return refs_create_symref(get_main_ref_store(the_repository), ref_target,
 				  refs_heads_master, logmsg);
 }
 
@@ -2006,7 +2007,7 @@ int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_reflog(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_reflog(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_reflog(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_reflog_ent_reverse(struct ref_store *refs,
@@ -2021,7 +2022,7 @@ int refs_for_each_reflog_ent_reverse(struct ref_store *refs,
 int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
 				void *cb_data)
 {
-	return refs_for_each_reflog_ent_reverse(get_main_ref_store(),
+	return refs_for_each_reflog_ent_reverse(get_main_ref_store(the_repository),
 						refname, fn, cb_data);
 }
 
@@ -2034,7 +2035,7 @@ int refs_for_each_reflog_ent(struct ref_store *refs, const char *refname,
 int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn,
 			void *cb_data)
 {
-	return refs_for_each_reflog_ent(get_main_ref_store(), refname,
+	return refs_for_each_reflog_ent(get_main_ref_store(the_repository), refname,
 					fn, cb_data);
 }
 
@@ -2045,7 +2046,7 @@ int refs_reflog_exists(struct ref_store *refs, const char *refname)
 
 int reflog_exists(const char *refname)
 {
-	return refs_reflog_exists(get_main_ref_store(), refname);
+	return refs_reflog_exists(get_main_ref_store(the_repository), refname);
 }
 
 int refs_create_reflog(struct ref_store *refs, const char *refname,
@@ -2057,7 +2058,7 @@ int refs_create_reflog(struct ref_store *refs, const char *refname,
 int safe_create_reflog(const char *refname, int force_create,
 		       struct strbuf *err)
 {
-	return refs_create_reflog(get_main_ref_store(), refname,
+	return refs_create_reflog(get_main_ref_store(the_repository), refname,
 				  force_create, err);
 }
 
@@ -2068,7 +2069,7 @@ int refs_delete_reflog(struct ref_store *refs, const char *refname)
 
 int delete_reflog(const char *refname)
 {
-	return refs_delete_reflog(get_main_ref_store(), refname);
+	return refs_delete_reflog(get_main_ref_store(the_repository), refname);
 }
 
 int refs_reflog_expire(struct ref_store *refs,
@@ -2091,7 +2092,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,
 		  reflog_expiry_cleanup_fn cleanup_fn,
 		  void *policy_cb_data)
 {
-	return refs_reflog_expire(get_main_ref_store(),
+	return refs_reflog_expire(get_main_ref_store(the_repository),
 				  refname, oid, flags,
 				  prepare_fn, should_prune_fn,
 				  cleanup_fn, policy_cb_data);
@@ -2114,7 +2115,7 @@ int refs_delete_refs(struct ref_store *refs, const char *msg,
 int delete_refs(const char *msg, struct string_list *refnames,
 		unsigned int flags)
 {
-	return refs_delete_refs(get_main_ref_store(), msg, refnames, flags);
+	return refs_delete_refs(get_main_ref_store(the_repository), msg, refnames, flags);
 }
 
 int refs_rename_ref(struct ref_store *refs, const char *oldref,
@@ -2125,7 +2126,7 @@ int refs_rename_ref(struct ref_store *refs, const char *oldref,
 
 int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 {
-	return refs_rename_ref(get_main_ref_store(), oldref, newref, logmsg);
+	return refs_rename_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
 }
 
 int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
@@ -2136,5 +2137,5 @@ int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
 
 int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)
 {
-	return refs_copy_existing_ref(get_main_ref_store(), oldref, newref, logmsg);
+	return refs_copy_existing_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
 }
diff --git a/refs.h b/refs.h
index 01be5ae32f..0d013377ce 100644
--- a/refs.h
+++ b/refs.h
@@ -758,7 +758,9 @@ int reflog_expire(const char *refname, const struct object_id *oid,
 
 int ref_storage_backend_exists(const char *name);
 
-struct ref_store *get_main_ref_store(void);
+#define get_main_ref_store(r) \
+	get_main_ref_store_##r()
+struct ref_store *get_main_ref_store_the_repository(void);
 /*
  * Return the ref_store instance for the specified submodule. For the
  * main repository, use submodule==NULL; such a call cannot fail. For
diff --git a/revision.c b/revision.c
index b42c836d7a..1cff11833e 100644
--- a/revision.c
+++ b/revision.c
@@ -6,6 +6,7 @@
 #include "diff.h"
 #include "refs.h"
 #include "revision.h"
+#include "repository.h"
 #include "graph.h"
 #include "grep.h"
 #include "reflog-walk.h"
@@ -1285,7 +1286,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)
 
 	cb.all_revs = revs;
 	cb.all_flags = flags;
-	cb.refs = get_main_ref_store();
+	cb.refs = get_main_ref_store(the_repository);
 	for_each_reflog(handle_one_reflog, &cb);
 
 	if (!revs->single_worktree)
@@ -2176,7 +2177,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 			die("BUG: --single-worktree cannot be used together with submodule");
 		refs = get_submodule_ref_store(submodule);
 	} else
-		refs = get_main_ref_store();
+		refs = get_main_ref_store(the_repository);
 
 	/*
 	 * NOTE!
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 7314b5943e..e8c328ec0e 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -2,6 +2,7 @@
 #include "refs.h"
 #include "worktree.h"
 #include "object-store.h"
+#include "repository.h"
 
 static const char *notnull(const char *arg, const char *name)
 {
@@ -22,7 +23,7 @@ static const char **get_store(const char **argv, struct ref_store **refs)
 	if (!argv[0]) {
 		die("ref store required");
 	} else if (!strcmp(argv[0], "main")) {
-		*refs = get_main_ref_store();
+		*refs = get_main_ref_store(the_repository);
 	} else if (skip_prefix(argv[0], "submodule:", &gitdir)) {
 		struct strbuf sb = STRBUF_INIT;
 		int ret;
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 07/16] refs: add repository argument to get_main_ref_store

[Michael Haggerty]

On 04/10/2018 12:45 AM, Stefan Beller wrote:
> Add a repository argument to allow the get_main_ref_store caller
> to be more specific about which repository to handle. This is a small
> mechanical change; it doesn't change the implementation to handle
> repositories other than the_repository yet.
> 
> As with the previous commits, use a macro to catch callers passing a
> repository other than the_repository at compile time.

This seems OK to me from a refs perspective.

The macro trick is surprising. I guess it gets you a compile-time check,
under the assumption that nothing else is called `the_repository`. But
why actually commit the macro, as opposed to compiling once locally to
check for correctness, then maybe add something like `assert(r ==
the_repository)` for the actual commit?

But I don't care either way, since the macro disappears again soon.

Michael

Re: [PATCH 07/16] refs: add repository argument to get_main_ref_store

[Stefan Beller]

Hi Michael,

On Tue, Apr 10, 2018 at 6:36 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 04/10/2018 12:45 AM, Stefan Beller wrote:
>> Add a repository argument to allow the get_main_ref_store caller
>> to be more specific about which repository to handle. This is a small
>> mechanical change; it doesn't change the implementation to handle
>> repositories other than the_repository yet.
>>
>> As with the previous commits, use a macro to catch callers passing a
>> repository other than the_repository at compile time.
>
> This seems OK to me from a refs perspective.
>
> The macro trick is surprising. I guess it gets you a compile-time check,
> under the assumption that nothing else is called `the_repository`.

Yes. Credit goes to Jonathan Tan for this trick.

> But
> why actually commit the macro, as opposed to compiling once locally to
> check for correctness, then maybe add something like `assert(r ==
> the_repository)` for the actual commit?

The eternal struggle of contributing patches that are easy to review. ;)

With the assert we'll have a run time check, which is not desirable
compared to a compile time check. And from a reviewers point of view
running a "rebase -x make" on the series that Junio queued is easier
than to reason about the "assert(r = the_repository)" IMHO.

> But I don't care either way, since the macro disappears again soon.

Glad you're ok with this approach.

Thanks for looking at the refs specific code,
Stefan

[PATCH 08/16] refs: add repository argument to for_each_replace_ref

[Stefan Beller]

Add a repository argument to allow for_each_replace_ref callers to be
more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/replace.c | 4 +++-
 refs.c            | 2 +-
 refs.h            | 4 +++-
 replace-object.c  | 2 +-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 19006e52bc..ef8145e556 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -14,6 +14,8 @@
 #include "refs.h"
 #include "parse-options.h"
 #include "run-command.h"
+#include "object-store.h"
+#include "repository.h"
 #include "tag.h"
 
 static const char * const git_replace_usage[] = {
@@ -83,7 +85,7 @@ static int list_replace_refs(const char *pattern, const char *format)
 		    "valid formats are 'short', 'medium' and 'long'\n",
 		    format);
 
-	for_each_replace_ref(show_reference, (void *)&data);
+	for_each_replace_ref(the_repository, show_reference, (void *)&data);
 
 	return 0;
 }
diff --git a/refs.c b/refs.c
index 74d4ed97cb..f58b9fb7df 100644
--- a/refs.c
+++ b/refs.c
@@ -1415,7 +1415,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 	return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
 }
 
-int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(get_main_ref_store(the_repository),
 			       git_replace_ref_base, fn,
diff --git a/refs.h b/refs.h
index 0d013377ce..ab3d2bec2f 100644
--- a/refs.h
+++ b/refs.h
@@ -300,7 +300,9 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,
 int for_each_tag_ref(each_ref_fn fn, void *cb_data);
 int for_each_branch_ref(each_ref_fn fn, void *cb_data);
 int for_each_remote_ref(each_ref_fn fn, void *cb_data);
-int for_each_replace_ref(each_ref_fn fn, void *cb_data);
+#define for_each_replace_ref(r, fn, cb) \
+	for_each_replace_ref_##r(fn, cb)
+int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data);
 int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
 int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
 			 const char *prefix, void *cb_data);
diff --git a/replace-object.c b/replace-object.c
index b2405f6027..16a95ea416 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -40,7 +40,7 @@ static void prepare_replace_object(void)
 		xmalloc(sizeof(*the_repository->objects->replace_map));
 	oidmap_init(the_repository->objects->replace_map, 0);
 
-	for_each_replace_ref(register_replace_ref, NULL);
+	for_each_replace_ref(the_repository, register_replace_ref, NULL);
 }
 
 /* We allow "recursive" replacement. Only within reason, though */
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 09/16] replace-object: add repository argument to prepare_replace_object

[Stefan Beller]

Add a repository argument to allow the prepare_replace_object caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index 16a95ea416..567d9da708 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -31,7 +31,9 @@ static int register_replace_ref(const char *refname,
 	return 0;
 }
 
-static void prepare_replace_object(void)
+#define prepare_replace_object(r) \
+	prepare_replace_object_##r()
+static void prepare_replace_object_the_repository(void)
 {
 	if (the_repository->objects->replace_map)
 		return;
@@ -58,7 +60,7 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)
 	int depth = MAXREPLACEDEPTH;
 	const struct object_id *cur = oid;
 
-	prepare_replace_object();
+	prepare_replace_object(the_repository);
 
 	/* Try to recursively replace the object */
 	while (depth-- > 0) {
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 10/16] replace-object: add repository argument to do_lookup_replace_object

[Stefan Beller]

Add a repository argument to allow the do_lookup_replace_object caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 2 +-
 replace-object.h | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index 567d9da708..adfed78901 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -55,7 +55,7 @@ static void prepare_replace_object_the_repository(void)
  * permanently-allocated value.  This function always respects replace
  * references, regardless of the value of check_replace_refs.
  */
-const struct object_id *do_lookup_replace_object(const struct object_id *oid)
+const struct object_id *do_lookup_replace_object_the_repository(const struct object_id *oid)
 {
 	int depth = MAXREPLACEDEPTH;
 	const struct object_id *cur = oid;
diff --git a/replace-object.h b/replace-object.h
index dbc51265ec..ddeb0470bd 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -14,7 +14,8 @@ struct replace_object {
  * This internal function is only declared here for the benefit of
  * lookup_replace_object().  Please do not call it directly.
  */
-extern const struct object_id *do_lookup_replace_object(const struct object_id *oid);
+#define do_lookup_replace_object(r, s) do_lookup_replace_object_##r(s)
+extern const struct object_id *do_lookup_replace_object_the_repository(const struct object_id *oid);
 
 /*
  * If object sha1 should be replaced, return the replacement object's
@@ -28,7 +29,7 @@ static inline const struct object_id *lookup_replace_object(const struct object_
 	    (the_repository->objects->replace_map &&
 	     the_repository->objects->replace_map->map.tablesize == 0))
 		return oid;
-	return do_lookup_replace_object(oid);
+	return do_lookup_replace_object(the_repository, oid);
 }
 
 #endif /* REPLACE_OBJECT_H */
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 11/16] replace-object: add repository argument to lookup_replace_object

[Stefan Beller]

Add a repository argument to allow callers of lookup_replace_object
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/mktag.c  | 2 +-
 object.c         | 2 +-
 replace-object.h | 3 ++-
 sha1_file.c      | 6 +++---
 streaming.c      | 2 +-
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/mktag.c b/builtin/mktag.c
index e3d20a7722..82a6e86077 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -25,7 +25,7 @@ static int verify_object(const struct object_id *oid, const char *expected_type)
 	enum object_type type;
 	unsigned long size;
 	void *buffer = read_object_file(oid, &type, &size);
-	const struct object_id *repl = lookup_replace_object(oid);
+	const struct object_id *repl = lookup_replace_object(the_repository, oid);
 
 	if (buffer) {
 		if (type == type_from_string(expected_type))
diff --git a/object.c b/object.c
index 998ec2a25f..66cffaf6e5 100644
--- a/object.c
+++ b/object.c
@@ -247,7 +247,7 @@ struct object *parse_object(const struct object_id *oid)
 	unsigned long size;
 	enum object_type type;
 	int eaten;
-	const struct object_id *repl = lookup_replace_object(oid);
+	const struct object_id *repl = lookup_replace_object(the_repository, oid);
 	void *buffer;
 	struct object *obj;
 
diff --git a/replace-object.h b/replace-object.h
index ddeb0470bd..dff57bfa1e 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -23,7 +23,8 @@ extern const struct object_id *do_lookup_replace_object_the_repository(const str
  * either sha1 or a pointer to a permanently-allocated value.  When
  * object replacement is suppressed, always return sha1.
  */
-static inline const struct object_id *lookup_replace_object(const struct object_id *oid)
+#define lookup_replace_object(r, s) lookup_replace_object_##r(s)
+static inline const struct object_id *lookup_replace_object_the_repository(const struct object_id *oid)
 {
 	if (!check_replace_refs ||
 	    (the_repository->objects->replace_map &&
diff --git a/sha1_file.c b/sha1_file.c
index c38e41e49e..028a4357c5 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1240,7 +1240,7 @@ int oid_object_info_extended(const struct object_id *oid, struct object_info *oi
 	int already_retried = 0;
 
 	if (flags & OBJECT_INFO_LOOKUP_REPLACE)
-		real = lookup_replace_object(oid);
+		real = lookup_replace_object(the_repository, oid);
 
 	if (is_null_oid(real))
 		return -1;
@@ -1379,8 +1379,8 @@ void *read_object_file_extended(const struct object_id *oid,
 	const struct packed_git *p;
 	const char *path;
 	struct stat st;
-	const struct object_id *repl = lookup_replace ? lookup_replace_object(oid)
-						      : oid;
+	const struct object_id *repl = lookup_replace ?
+		lookup_replace_object(the_repository, oid) : oid;
 
 	errno = 0;
 	data = read_object(repl->hash, type, size);
diff --git a/streaming.c b/streaming.c
index a6e1162946..cce7b17ea7 100644
--- a/streaming.c
+++ b/streaming.c
@@ -140,7 +140,7 @@ struct git_istream *open_istream(const struct object_id *oid,
 {
 	struct git_istream *st;
 	struct object_info oi = OBJECT_INFO_INIT;
-	const struct object_id *real = lookup_replace_object(oid);
+	const struct object_id *real = lookup_replace_object(the_repository, oid);
 	enum input_source src = istream_source(real, type, &oi);
 
 	if (src < 0)
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 12/16] refs: store the main ref store inside the repository struct

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 refs.c       | 13 +++++--------
 refs.h       |  4 +---
 repository.h |  3 +++
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/refs.c b/refs.c
index f58b9fb7df..b5be754a97 100644
--- a/refs.c
+++ b/refs.c
@@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry(
 	return entry;
 }
 
-/* A pointer to the ref_store for the main repository: */
-static struct ref_store *main_ref_store;
-
 /* A hashmap of ref_stores, stored by submodule name: */
 static struct hashmap submodule_ref_stores;
 
@@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char *gitdir,
 	return refs;
 }
 
-struct ref_store *get_main_ref_store_the_repository(void)
+struct ref_store *get_main_ref_store(struct repository *r)
 {
-	if (main_ref_store)
-		return main_ref_store;
+	if (r->main_ref_store)
+		return r->main_ref_store;
 
-	main_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);
-	return main_ref_store;
+	r->main_ref_store = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
+	return r->main_ref_store;
 }
 
 /*
diff --git a/refs.h b/refs.h
index ab3d2bec2f..f5ab68c0ed 100644
--- a/refs.h
+++ b/refs.h
@@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,
 
 int ref_storage_backend_exists(const char *name);
 
-#define get_main_ref_store(r) \
-	get_main_ref_store_##r()
-struct ref_store *get_main_ref_store_the_repository(void);
+struct ref_store *get_main_ref_store(struct repository *r);
 /*
  * Return the ref_store instance for the specified submodule. For the
  * main repository, use submodule==NULL; such a call cannot fail. For
diff --git a/repository.h b/repository.h
index 09df94a472..7d0710b273 100644
--- a/repository.h
+++ b/repository.h
@@ -26,6 +26,9 @@ struct repository {
 	 */
 	struct raw_object_store *objects;
 
+	/* The store in which the refs are held. */
+	struct ref_store *main_ref_store;
+
 	/*
 	 * Path to the repository's graft file.
 	 * Cannot be NULL after initialization.
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH 12/16] refs: store the main ref store inside the repository struct

[Brandon Williams]

On 04/09, Stefan Beller wrote:
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  refs.c       | 13 +++++--------
>  refs.h       |  4 +---
>  repository.h |  3 +++
>  3 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index f58b9fb7df..b5be754a97 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry(
>  	return entry;
>  }
>  
> -/* A pointer to the ref_store for the main repository: */
> -static struct ref_store *main_ref_store;
> -
>  /* A hashmap of ref_stores, stored by submodule name: */
>  static struct hashmap submodule_ref_stores;
>  
> @@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char *gitdir,
>  	return refs;
>  }
>  
> -struct ref_store *get_main_ref_store_the_repository(void)
> +struct ref_store *get_main_ref_store(struct repository *r)
>  {
> -	if (main_ref_store)
> -		return main_ref_store;
> +	if (r->main_ref_store)
> +		return r->main_ref_store;
>  
> -	main_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);
> -	return main_ref_store;
> +	r->main_ref_store = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
> +	return r->main_ref_store;

I assume that since this takes in a git dir as a parameter
that the ref-store is in a good enough place to be embedded in a
repository struct (as in this would work with an arbitrary repo)?

>  }
>  
>  /*
> diff --git a/refs.h b/refs.h
> index ab3d2bec2f..f5ab68c0ed 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,
>  
>  int ref_storage_backend_exists(const char *name);
>  
> -#define get_main_ref_store(r) \
> -	get_main_ref_store_##r()
> -struct ref_store *get_main_ref_store_the_repository(void);
> +struct ref_store *get_main_ref_store(struct repository *r);
>  /*
>   * Return the ref_store instance for the specified submodule. For the
>   * main repository, use submodule==NULL; such a call cannot fail. For
> diff --git a/repository.h b/repository.h
> index 09df94a472..7d0710b273 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -26,6 +26,9 @@ struct repository {
>  	 */
>  	struct raw_object_store *objects;
>  
> +	/* The store in which the refs are held. */
> +	struct ref_store *main_ref_store;
> +
>  	/*
>  	 * Path to the repository's graft file.
>  	 * Cannot be NULL after initialization.
> -- 
> 2.17.0.484.g0c8726318c-goog
> 

-- 
Brandon Williams

Re: [PATCH 12/16] refs: store the main ref store inside the repository struct

[Stefan Beller]

Hi Brandon,

On Mon, Apr 9, 2018 at 4:24 PM, Brandon Williams <bmwill@google.com> wrote:

>> -     main_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);
>> -     return main_ref_store;
>> +     r->main_ref_store = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
>> +     return r->main_ref_store;
>
> I assume that since this takes in a git dir as a parameter
> that the ref-store is in a good enough place to be embedded in a
> repository struct (as in this would work with an arbitrary repo)?

That is my current understanding.

As the refs code can also take a path into a submodule and construct
a submodule ref store for the caller, we'd want to resolve the tension
between the ref store and the repository struct who is responsible for
the submodule ref store eventually by removing the submodule
functionality from the ref store and only relying on the ref stores created by
the repository struct.

Re: [PATCH 12/16] refs: store the main ref store inside the repository struct

[Brandon Williams]

On 04/09, Stefan Beller wrote:
> Hi Brandon,
> 
> On Mon, Apr 9, 2018 at 4:24 PM, Brandon Williams <bmwill@google.com> wrote:
> 
> >> -     main_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);
> >> -     return main_ref_store;
> >> +     r->main_ref_store = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
> >> +     return r->main_ref_store;
> >
> > I assume that since this takes in a git dir as a parameter
> > that the ref-store is in a good enough place to be embedded in a
> > repository struct (as in this would work with an arbitrary repo)?
> 
> That is my current understanding.
> 
> As the refs code can also take a path into a submodule and construct
> a submodule ref store for the caller, we'd want to resolve the tension
> between the ref store and the repository struct who is responsible for
> the submodule ref store eventually by removing the submodule
> functionality from the ref store and only relying on the ref stores created by
> the repository struct.

oh right, assuming it can already handle submodule ref-stores it should
work as-is then.

-- 
Brandon Williams

Re: [PATCH 12/16] refs: store the main ref store inside the repository struct

[Michael Haggerty]

On 04/10/2018 12:45 AM, Stefan Beller wrote:
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  refs.c       | 13 +++++--------
>  refs.h       |  4 +---
>  repository.h |  3 +++
>  3 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index f58b9fb7df..b5be754a97 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry(
>  	return entry;
>  }
>  
> -/* A pointer to the ref_store for the main repository: */
> -static struct ref_store *main_ref_store;
> -
>  /* A hashmap of ref_stores, stored by submodule name: */
>  static struct hashmap submodule_ref_stores;
>  
> @@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char *gitdir,
>  	return refs;
>  }
>  
> -struct ref_store *get_main_ref_store_the_repository(void)
> +struct ref_store *get_main_ref_store(struct repository *r)
>  {
> -	if (main_ref_store)
> -		return main_ref_store;
> +	if (r->main_ref_store)
> +		return r->main_ref_store;
>  
> -	main_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);
> -	return main_ref_store;
> +	r->main_ref_store = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
> +	return r->main_ref_store;
>  }
>  
>  /*
> diff --git a/refs.h b/refs.h
> index ab3d2bec2f..f5ab68c0ed 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,
>  
>  int ref_storage_backend_exists(const char *name);
>  
> -#define get_main_ref_store(r) \
> -	get_main_ref_store_##r()
> -struct ref_store *get_main_ref_store_the_repository(void);
> +struct ref_store *get_main_ref_store(struct repository *r);
>  /*
>   * Return the ref_store instance for the specified submodule. For the
>   * main repository, use submodule==NULL; such a call cannot fail. For
> diff --git a/repository.h b/repository.h
> index 09df94a472..7d0710b273 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -26,6 +26,9 @@ struct repository {
>  	 */
>  	struct raw_object_store *objects;
>  
> +	/* The store in which the refs are held. */
> +	struct ref_store *main_ref_store;
> +
>  	/*
>  	 * Path to the repository's graft file.
>  	 * Cannot be NULL after initialization.
> 

This also makes sense to me, as far as it goes. I have a few comments
and questions:

Why do you call the new member `main_ref_store`? Is there expected to be
some other `ref_store` associated with a repository?

I think the origin of the name `main_ref_store` was to distinguish it
from submodule ref stores. But presumably those will soon become the
"main" ref stores for their respective submodule repository objects,
right? So maybe calling things `repository.ref_store` and
`get_ref_store(repository)` would be appropriate.

There are some places in the reference code that only work with the main
repository. The ones that I can think of are:

* `ref_resolves_to_object()` depends on an object store.

* `peel_object()` and `ref_iterator_peel()` also have to look up objects
(in this case, tag objects).

* Anything that calls `files_assert_main_repository()` or depends on
`REF_STORE_MAIN` isn't implemented for other reference stores (usually,
I think, these are functions that depend on the object store).

Some of these things might be easy to generalize to non-main
repositories, but I didn't bother because AFAIK currently only the main
repository store is ever mutated.

You can move a now-obsolete comment above the definition of `struct
files_ref_store` if you haven't in some other patch (search for
"libification").

Hope that helps,
Michael

Re: [PATCH 12/16] refs: store the main ref store inside the repository struct

[Stefan Beller]

Hi Michael,

On Tue, Apr 10, 2018 at 7:02 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:

> This also makes sense to me, as far as it goes. I have a few comments
> and questions:
>
> Why do you call the new member `main_ref_store`? Is there expected to be
> some other `ref_store` associated with a repository?

I'll rename it in a reroll.

>
> I think the origin of the name `main_ref_store` was to distinguish it
> from submodule ref stores. But presumably those will soon become the
> "main" ref stores for their respective submodule repository objects,
> right?

I hope so.

> So maybe calling things `repository.ref_store` and
> `get_ref_store(repository)` would be appropriate.

ok.

>
> There are some places in the reference code that only work with the main
> repository. The ones that I can think of are:
>
> * `ref_resolves_to_object()` depends on an object store.
>
> * `peel_object()` and `ref_iterator_peel()` also have to look up objects
> (in this case, tag objects).
>
> * Anything that calls `files_assert_main_repository()` or depends on
> `REF_STORE_MAIN` isn't implemented for other reference stores (usually,
> I think, these are functions that depend on the object store).
>
> Some of these things might be easy to generalize to non-main
> repositories, but I didn't bother because AFAIK currently only the main
> repository store is ever mutated.
>
> You can move a now-obsolete comment above the definition of `struct
> files_ref_store` if you haven't in some other patch (search for
> "libification").

ok, I'll have a look at that.

My plan was to remove the submodule accessors from the refs API, and
mandate the access via

    repo_submodule_init(&submodule_repo, superproject_repo, path);
    sub_ref_store = get_ref_store(submodule_repo);

instead of also having

    sub_ref_store = get_submodule_ref_store(path);

as that would ease the refs API (and its internals potentially)
as well as avoiding errors with mixing up repositories. As the
construction of a submodule repository struct requires its
superproject repo, it helps avoiding pitfalls with nested
submodules IMO.

Thanks for the comments,
Stefan

>
> Hope that helps,
> Michael

[PATCH 13/16] refs: allow for_each_replace_ref to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 refs.c | 4 ++--
 refs.h | 4 +---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/refs.c b/refs.c
index b5be754a97..bed5f88405 100644
--- a/refs.c
+++ b/refs.c
@@ -1415,9 +1415,9 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 	return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
 }
 
-int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data)
+int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(get_main_ref_store(the_repository),
+	return do_for_each_ref(get_main_ref_store(r),
 			       git_replace_ref_base, fn,
 			       strlen(git_replace_ref_base),
 			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
diff --git a/refs.h b/refs.h
index f5ab68c0ed..15f3a91cc4 100644
--- a/refs.h
+++ b/refs.h
@@ -300,9 +300,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,
 int for_each_tag_ref(each_ref_fn fn, void *cb_data);
 int for_each_branch_ref(each_ref_fn fn, void *cb_data);
 int for_each_remote_ref(each_ref_fn fn, void *cb_data);
-#define for_each_replace_ref(r, fn, cb) \
-	for_each_replace_ref_##r(fn, cb)
-int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data);
+int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data);
 int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
 int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
 			 const char *prefix, void *cb_data);
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 14/16] replace-object: allow prepare_replace_object to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index adfed78901..eae52c66f3 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -31,18 +31,16 @@ static int register_replace_ref(const char *refname,
 	return 0;
 }
 
-#define prepare_replace_object(r) \
-	prepare_replace_object_##r()
-static void prepare_replace_object_the_repository(void)
+static void prepare_replace_object(struct repository *r)
 {
-	if (the_repository->objects->replace_map)
+	if (r->objects->replace_map)
 		return;
 
-	the_repository->objects->replace_map =
+	r->objects->replace_map =
 		xmalloc(sizeof(*the_repository->objects->replace_map));
-	oidmap_init(the_repository->objects->replace_map, 0);
+	oidmap_init(r->objects->replace_map, 0);
 
-	for_each_replace_ref(the_repository, register_replace_ref, NULL);
+	for_each_replace_ref(r, register_replace_ref, NULL);
 }
 
 /* We allow "recursive" replacement. Only within reason, though */
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 15/16] replace-object: allow do_lookup_replace_object to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.c | 7 ++++---
 replace-object.h | 4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/replace-object.c b/replace-object.c
index eae52c66f3..246b98cd4f 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -53,17 +53,18 @@ static void prepare_replace_object(struct repository *r)
  * permanently-allocated value.  This function always respects replace
  * references, regardless of the value of check_replace_refs.
  */
-const struct object_id *do_lookup_replace_object_the_repository(const struct object_id *oid)
+const struct object_id *do_lookup_replace_object(struct repository *r,
+						 const struct object_id *oid)
 {
 	int depth = MAXREPLACEDEPTH;
 	const struct object_id *cur = oid;
 
-	prepare_replace_object(the_repository);
+	prepare_replace_object(r);
 
 	/* Try to recursively replace the object */
 	while (depth-- > 0) {
 		struct replace_object *repl_obj =
-			oidmap_get(the_repository->objects->replace_map, cur);
+			oidmap_get(r->objects->replace_map, cur);
 		if (!repl_obj)
 			return cur;
 		cur = &repl_obj->replacement;
diff --git a/replace-object.h b/replace-object.h
index dff57bfa1e..3520fd7ff7 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -14,8 +14,8 @@ struct replace_object {
  * This internal function is only declared here for the benefit of
  * lookup_replace_object().  Please do not call it directly.
  */
-#define do_lookup_replace_object(r, s) do_lookup_replace_object_##r(s)
-extern const struct object_id *do_lookup_replace_object_the_repository(const struct object_id *oid);
+extern const struct object_id *do_lookup_replace_object(struct repository *r,
+						        const struct object_id *oid);
 
 /*
  * If object sha1 should be replaced, return the replacement object's
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 16/16] replace-object: allow lookup_replace_object to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/replace-object.h b/replace-object.h
index 3520fd7ff7..9f607a929b 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -23,14 +23,14 @@ extern const struct object_id *do_lookup_replace_object(struct repository *r,
  * either sha1 or a pointer to a permanently-allocated value.  When
  * object replacement is suppressed, always return sha1.
  */
-#define lookup_replace_object(r, s) lookup_replace_object_##r(s)
-static inline const struct object_id *lookup_replace_object_the_repository(const struct object_id *oid)
+static inline const struct object_id *lookup_replace_object(struct repository *r,
+							    const struct object_id *oid)
 {
 	if (!check_replace_refs ||
-	    (the_repository->objects->replace_map &&
-	     the_repository->objects->replace_map->map.tablesize == 0))
+	    (r->objects->replace_map &&
+	     r->objects->replace_map->map.tablesize == 0))
 		return oid;
-	return do_lookup_replace_object(the_repository, oid);
+	return do_lookup_replace_object(r, oid);
 }
 
 #endif /* REPLACE_OBJECT_H */
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCHv2 00/16] object-store refactoring 3 (replace objects, main ref store)

[Brandon Williams]

On 04/09, Stefan Beller wrote:

I've looked through the series and it looks good.  My only concern is
making sure that the ref-store is in a good place to be embedded into
the repository struct.

> v2:
> This applies on top of a merge of
> origin/bc/object-id and origin/sb/packfiles-in-repository,
> both of which are pending merge to master. It is also available at
> https://github.com/stefanbeller/git/tree/object-store-3
> 
> * removed whitespaces as noted by Stolee
> * incorporated Renes patch as the first patch of this series
>   (It may go independently if this series takes too long)
> * Adressed Erics concern regarding sloppy commit messages
>   (removed #Conflict markers), typo in comment
> * I did not drop the main_ from the ref store, yet, as asked by Duy.
> 
> Thanks,
> Stefan
> 
> v1:
> This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c
> (sb/packfiles-in-repository).
> It is also available at https://github.com/stefanbeller/git/tree/object-store-3
> 
> This series will bring the replacement mechanism (git replace)
> into the object store.
> 
> The first patches are cleaning up a bit, and patches 6-19 are converting
> one function at a time using the tick-tock pattern with the #define trick.
> See cfc62fc98c (sha1_file: add repository argument to link_alt_odb_entry,
> 2018-03-23) for explanation.
> 
> Thanks,
> Stefan
> 
> René Scharfe (1):
>   replace_object: use oidmap
> 
> Stefan Beller (15):
>   replace_object.c: rename to use dash in file name
>   replace-object: move replace_map to object store
>   object-store: move lookup_replace_object to replace-object.h
>   replace-object: eliminate replace objects prepared flag
>   replace-object: check_replace_refs is safe in multi repo environment
>   refs: add repository argument to get_main_ref_store
>   refs: add repository argument to for_each_replace_ref
>   replace-object: add repository argument to prepare_replace_object
>   replace-object: add repository argument to do_lookup_replace_object
>   replace-object: add repository argument to lookup_replace_object
>   refs: store the main ref store inside the repository struct
>   refs: allow for_each_replace_ref to handle arbitrary repositories
>   replace-object: allow prepare_replace_object to handle arbitrary
>     repositories
>   replace-object: allow do_lookup_replace_object to handle arbitrary
>     repositories
>   replace-object: allow lookup_replace_object to handle arbitrary
>     repositories
> 
>  Makefile                  |   2 +-
>  builtin/mktag.c           |   3 +-
>  builtin/pack-refs.c       |   3 +-
>  builtin/replace.c         |   4 +-
>  cache.h                   |  19 ------
>  environment.c             |   2 +-
>  object-store.h            |   8 +++
>  object.c                  |   3 +-
>  refs.c                    |  80 +++++++++++++------------
>  refs.h                    |   4 +-
>  replace-object.c          |  73 +++++++++++++++++++++++
>  replace-object.h          |  36 ++++++++++++
>  replace_object.c          | 120 --------------------------------------
>  repository.h              |   3 +
>  revision.c                |   5 +-
>  sha1_file.c               |   7 ++-
>  streaming.c               |   3 +-
>  t/helper/test-ref-store.c |   3 +-
>  18 files changed, 183 insertions(+), 195 deletions(-)
>  create mode 100644 replace-object.c
>  create mode 100644 replace-object.h
>  delete mode 100644 replace_object.c
> 
> -- 
> 2.17.0.484.g0c8726318c-goog
> 

-- 
Brandon Williams

Re: [PATCHv2 00/16] object-store refactoring 3 (replace objects, main ref store)

[Stefan Beller]

On Mon, Apr 9, 2018 at 4:25 PM, Brandon Williams <bmwill@google.com> wrote:
> On 04/09, Stefan Beller wrote:
>
> I've looked through the series and it looks good.  My only concern is
> making sure that the ref-store is in a good place to be embedded into
> the repository struct.
>

Michael knows more about the ref store.

Could you have a look at (the preparatory)
https://public-inbox.org/git/20180409224533.17764-8-sbeller@google.com/
and
https://public-inbox.org/git/20180409224533.17764-13-sbeller@google.com/

Thanks,
Stefan

[PATCHv3 00/15] replace_object.c: rename to use dash in file name

[Stefan Beller]

v3:
* interdiff below,
  the only changes are renaming the variable 
  -       struct ref_store *main_ref_store;
  +       struct ref_store *refs;
  in struct repository.
  as well as dropping the file rename patch.
* improved commit messages from discussion on the single patches.

v2:
This applies on top of a merge of
origin/bc/object-id and origin/sb/packfiles-in-repository,
both of which are pending merge to master. It is also available at
https://github.com/stefanbeller/git/tree/object-store-3

* removed whitespaces as noted by Stolee
* incorporated Renes patch as the first patch of this series
  (It may go independently if this series takes too long)
* Adressed Erics concern regarding sloppy commit messages
  (removed #Conflict markers), typo in comment
* I did not drop the main_ from the ref store, yet, as asked by Duy.

Thanks,
Stefan

v1:
This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c
(sb/packfiles-in-repository).
It is also available at https://github.com/stefanbeller/git/tree/object-store-3

This series will bring the replacement mechanism (git replace)
into the object store.

The first patches are cleaning up a bit, and patches 6-19 are converting
one function at a time using the tick-tock pattern with the #define trick.
See cfc62fc98c (sha1_file: add repository argument to link_alt_odb_entry,
2018-03-23) for explanation.

Thanks,
Stefan

René Scharfe (1):
  replace_object: use oidmap

Stefan Beller (14):
  replace-object: move replace_map to object store
  object-store: move lookup_replace_object to replace-object.h
  replace-object: eliminate replace objects prepared flag
  replace-object: check_replace_refs is safe in multi repo environment
  refs: add repository argument to get_main_ref_store
  refs: add repository argument to for_each_replace_ref
  replace-object: add repository argument to prepare_replace_object
  replace-object: add repository argument to do_lookup_replace_object
  replace-object: add repository argument to lookup_replace_object
  refs: store the main ref store inside the repository struct
  refs: allow for_each_replace_ref to handle arbitrary repositories
  replace-object: allow prepare_replace_object to handle arbitrary
    repositories
  replace-object: allow do_lookup_replace_object to handle arbitrary
    repositories
  replace-object: allow lookup_replace_object to handle arbitrary
    repositories

 builtin/mktag.c           |  3 +-
 builtin/pack-refs.c       |  3 +-
 builtin/replace.c         |  4 +-
 cache.h                   | 19 --------
 environment.c             |  2 +-
 object-store.h            |  8 ++++
 object.c                  |  3 +-
 refs.c                    | 80 ++++++++++++++++----------------
 refs.h                    |  4 +-
 refs/files-backend.c      |  4 --
 replace-object.h          | 36 +++++++++++++++
 replace_object.c          | 97 ++++++++++-----------------------------
 repository.h              |  3 ++
 revision.c                |  5 +-
 sha1_file.c               |  7 +--
 streaming.c               |  3 +-
 t/helper/test-ref-store.c |  3 +-
 17 files changed, 134 insertions(+), 150 deletions(-)
 create mode 100644 replace-object.h


$ git diff github/object-store-3
 diff --git c/Makefile w/Makefile
index 94e0bf47b1..de4b8f0c02 100644
--- c/Makefile
+++ w/Makefile
@@ -871,7 +871,7 @@ LIB_OBJS += refs/packed-backend.o
 LIB_OBJS += refs/ref-cache.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
-LIB_OBJS += replace-object.o
+LIB_OBJS += replace_object.o
 LIB_OBJS += repository.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
diff --git c/refs.c w/refs.c
index bed5f88405..9b56fa9b81 100644
--- c/refs.c
+++ w/refs.c
@@ -1651,11 +1651,11 @@ static struct ref_store *ref_store_init(const char *gitdir,
 
 struct ref_store *get_main_ref_store(struct repository *r)
 {
-       if (r->main_ref_store)
-               return r->main_ref_store;
+       if (r->refs)
+               return r->refs;
 
-       r->main_ref_store = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
-       return r->main_ref_store;
+       r->refs = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
+       return r->refs;
 }
 
 /*
diff --git c/refs/files-backend.c w/refs/files-backend.c
index bec8e30e9e..5c76a75817 100644
--- c/refs/files-backend.c
+++ w/refs/files-backend.c
@@ -61,10 +61,6 @@ struct ref_lock {
        struct object_id old_oid;
 };
 
-/*
- * Future: need to be in "struct repository"
- * when doing a full libification.
- */
 struct files_ref_store {
        struct ref_store base;
        unsigned int store_flags;
diff --git c/replace-object.c w/replace_object.c
similarity index 100%
rename from replace-object.c
rename to replace_object.c
diff --git c/repository.h w/repository.h
index 7d0710b273..e6e00f541b 100644
--- c/repository.h
+++ w/repository.h
@@ -27,7 +27,7 @@ struct repository {
        struct raw_object_store *objects;
 
        /* The store in which the refs are held. */
-       struct ref_store *main_ref_store;
+       struct ref_store *refs;
 
        /*
         * Path to the repository's graft file.

-- 
2.17.0.484.g0c8726318c-goog

[PATCH 01/15] replace_object: use oidmap

[Stefan Beller]

From: René Scharfe <l.s.r@web.de>

Load the replace objects into an oidmap to allow for easy lookups in
constant time.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace_object.c | 76 ++++++++++--------------------------------------
 1 file changed, 16 insertions(+), 60 deletions(-)

diff --git a/replace_object.c b/replace_object.c
index 336357394d..a757a5ebf2 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -1,54 +1,14 @@
 #include "cache.h"
-#include "sha1-lookup.h"
+#include "oidmap.h"
 #include "refs.h"
 #include "commit.h"
 
-/*
- * An array of replacements.  The array is kept sorted by the original
- * sha1.
- */
-static struct replace_object {
-	struct object_id original;
+struct replace_object {
+	struct oidmap_entry original;
 	struct object_id replacement;
-} **replace_object;
-
-static int replace_object_alloc, replace_object_nr;
+};
 
-static const unsigned char *replace_sha1_access(size_t index, void *table)
-{
-	struct replace_object **replace = table;
-	return replace[index]->original.hash;
-}
-
-static int replace_object_pos(const unsigned char *sha1)
-{
-	return sha1_pos(sha1, replace_object, replace_object_nr,
-			replace_sha1_access);
-}
-
-static int register_replace_object(struct replace_object *replace,
-				   int ignore_dups)
-{
-	int pos = replace_object_pos(replace->original.hash);
-
-	if (0 <= pos) {
-		if (ignore_dups)
-			free(replace);
-		else {
-			free(replace_object[pos]);
-			replace_object[pos] = replace;
-		}
-		return 1;
-	}
-	pos = -pos - 1;
-	ALLOC_GROW(replace_object, replace_object_nr + 1, replace_object_alloc);
-	replace_object_nr++;
-	if (pos < replace_object_nr)
-		MOVE_ARRAY(replace_object + pos + 1, replace_object + pos,
-			   replace_object_nr - pos - 1);
-	replace_object[pos] = replace;
-	return 0;
-}
+static struct oidmap replace_map = OIDMAP_INIT;
 
 static int register_replace_ref(const char *refname,
 				const struct object_id *oid,
@@ -59,7 +19,7 @@ static int register_replace_ref(const char *refname,
 	const char *hash = slash ? slash + 1 : refname;
 	struct replace_object *repl_obj = xmalloc(sizeof(*repl_obj));
 
-	if (get_oid_hex(hash, &repl_obj->original)) {
+	if (get_oid_hex(hash, &repl_obj->original.oid)) {
 		free(repl_obj);
 		warning("bad replace ref name: %s", refname);
 		return 0;
@@ -69,7 +29,7 @@ static int register_replace_ref(const char *refname,
 	oidcpy(&repl_obj->replacement, oid);
 
 	/* Register new object */
-	if (register_replace_object(repl_obj, 1))
+	if (oidmap_put(&replace_map, repl_obj))
 		die("duplicate replace ref: %s", refname);
 
 	return 0;
@@ -84,7 +44,7 @@ static void prepare_replace_object(void)
 
 	for_each_replace_ref(register_replace_ref, NULL);
 	replace_object_prepared = 1;
-	if (!replace_object_nr)
+	if (!replace_map.map.tablesize)
 		check_replace_refs = 0;
 }
 
@@ -100,21 +60,17 @@ static void prepare_replace_object(void)
  */
 const struct object_id *do_lookup_replace_object(const struct object_id *oid)
 {
-	int pos, depth = MAXREPLACEDEPTH;
+	int depth = MAXREPLACEDEPTH;
 	const struct object_id *cur = oid;
 
 	prepare_replace_object();
 
 	/* Try to recursively replace the object */
-	do {
-		if (--depth < 0)
-			die("replace depth too high for object %s",
-			    oid_to_hex(oid));
-
-		pos = replace_object_pos(cur->hash);
-		if (0 <= pos)
-			cur = &replace_object[pos]->replacement;
-	} while (0 <= pos);
-
-	return cur;
+	while (depth-- > 0) {
+		struct replace_object *repl_obj = oidmap_get(&replace_map, cur);
+		if (!repl_obj)
+			return cur;
+		cur = &repl_obj->replacement;
+	}
+	die("replace depth too high for object %s", oid_to_hex(oid));
 }
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 02/15] replace-object: move replace_map to object store

[Stefan Beller]

The relationship between an object X and another object Y that
replaces the object X is defined only within the scope of a
single repository.

The exception in reachability rule around these replacement objects
is also local to a repository (i.e. if traversal from refs reaches
X, then both X and Y are reachable and need to be kept from gc).

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 object-store.h   |  8 ++++++++
 replace-object.h |  9 +++++++++
 replace_object.c | 17 +++++++----------
 3 files changed, 24 insertions(+), 10 deletions(-)
 create mode 100644 replace-object.h

diff --git a/object-store.h b/object-store.h
index fef33f345f..c04b4c95eb 100644
--- a/object-store.h
+++ b/object-store.h
@@ -1,6 +1,8 @@
 #ifndef OBJECT_STORE_H
 #define OBJECT_STORE_H
 
+#include "oidmap.h"
+
 struct alternate_object_database {
 	struct alternate_object_database *next;
 
@@ -93,6 +95,12 @@ struct raw_object_store {
 	struct alternate_object_database *alt_odb_list;
 	struct alternate_object_database **alt_odb_tail;
 
+	/*
+	 * Objects that should be substituted by other objects
+	 * (see git-replace(1)).
+	 */
+	struct oidmap replace_map;
+
 	/*
 	 * private data
 	 *
diff --git a/replace-object.h b/replace-object.h
new file mode 100644
index 0000000000..f9a2b70eb8
--- /dev/null
+++ b/replace-object.h
@@ -0,0 +1,9 @@
+#ifndef REPLACE_OBJECT_H
+#define REPLACE_OBJECT_H
+
+struct replace_object {
+	struct oidmap_entry original;
+	struct object_id replacement;
+};
+
+#endif /* REPLACE_OBJECT_H */
diff --git a/replace_object.c b/replace_object.c
index a757a5ebf2..afbdf2df25 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -1,15 +1,11 @@
 #include "cache.h"
 #include "oidmap.h"
+#include "object-store.h"
+#include "replace-object.h"
 #include "refs.h"
+#include "repository.h"
 #include "commit.h"
 
-struct replace_object {
-	struct oidmap_entry original;
-	struct object_id replacement;
-};
-
-static struct oidmap replace_map = OIDMAP_INIT;
-
 static int register_replace_ref(const char *refname,
 				const struct object_id *oid,
 				int flag, void *cb_data)
@@ -29,7 +25,7 @@ static int register_replace_ref(const char *refname,
 	oidcpy(&repl_obj->replacement, oid);
 
 	/* Register new object */
-	if (oidmap_put(&replace_map, repl_obj))
+	if (oidmap_put(&the_repository->objects->replace_map, repl_obj))
 		die("duplicate replace ref: %s", refname);
 
 	return 0;
@@ -44,7 +40,7 @@ static void prepare_replace_object(void)
 
 	for_each_replace_ref(register_replace_ref, NULL);
 	replace_object_prepared = 1;
-	if (!replace_map.map.tablesize)
+	if (!the_repository->objects->replace_map.map.tablesize)
 		check_replace_refs = 0;
 }
 
@@ -67,7 +63,8 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)
 
 	/* Try to recursively replace the object */
 	while (depth-- > 0) {
-		struct replace_object *repl_obj = oidmap_get(&replace_map, cur);
+		struct replace_object *repl_obj =
+			oidmap_get(&the_repository->objects->replace_map, cur);
 		if (!repl_obj)
 			return cur;
 		cur = &repl_obj->replacement;
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 03/15] object-store: move lookup_replace_object to replace-object.h

[Stefan Beller]

lookup_replace_object is a low-level function that most users of the
object store do not need to use directly.

Move it to replace-object.h to avoid a dependency loop in an upcoming
change to its inline definition that will make use of repository.h.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/mktag.c  |  1 +
 cache.h          | 19 -------------------
 object.c         |  1 +
 replace-object.h | 22 ++++++++++++++++++++++
 sha1_file.c      |  1 +
 streaming.c      |  1 +
 6 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/builtin/mktag.c b/builtin/mktag.c
index 9f5a50a8fd..e3d20a7722 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "tag.h"
+#include "replace-object.h"
 
 /*
  * A signature file has a very simple fixed format: four lines
diff --git a/cache.h b/cache.h
index a5c4fddf77..e3c6cba514 100644
--- a/cache.h
+++ b/cache.h
@@ -1187,25 +1187,6 @@ static inline void *read_object_file(const struct object_id *oid, enum object_ty
 	return read_object_file_extended(oid, type, size, 1);
 }
 
-/*
- * This internal function is only declared here for the benefit of
- * lookup_replace_object().  Please do not call it directly.
- */
-extern const struct object_id *do_lookup_replace_object(const struct object_id *oid);
-
-/*
- * If object sha1 should be replaced, return the replacement object's
- * name (replaced recursively, if necessary).  The return value is
- * either sha1 or a pointer to a permanently-allocated value.  When
- * object replacement is suppressed, always return sha1.
- */
-static inline const struct object_id *lookup_replace_object(const struct object_id *oid)
-{
-	if (!check_replace_refs)
-		return oid;
-	return do_lookup_replace_object(oid);
-}
-
 /* Read and unpack an object file into memory, write memory to an object file */
 extern int oid_object_info(const struct object_id *, unsigned long *);
 
diff --git a/object.c b/object.c
index a0a756f24f..998ec2a25f 100644
--- a/object.c
+++ b/object.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "object.h"
+#include "replace-object.h"
 #include "blob.h"
 #include "tree.h"
 #include "commit.h"
diff --git a/replace-object.h b/replace-object.h
index f9a2b70eb8..15315311fb 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -1,9 +1,31 @@
 #ifndef REPLACE_OBJECT_H
 #define REPLACE_OBJECT_H
 
+#include "oidmap.h"
+#include "repository.h"
+
 struct replace_object {
 	struct oidmap_entry original;
 	struct object_id replacement;
 };
 
+/*
+ * This internal function is only declared here for the benefit of
+ * lookup_replace_object().  Please do not call it directly.
+ */
+extern const struct object_id *do_lookup_replace_object(const struct object_id *oid);
+
+/*
+ * If object sha1 should be replaced, return the replacement object's
+ * name (replaced recursively, if necessary).  The return value is
+ * either sha1 or a pointer to a permanently-allocated value.  When
+ * object replacement is suppressed, always return sha1.
+ */
+static inline const struct object_id *lookup_replace_object(const struct object_id *oid)
+{
+	if (!check_replace_refs)
+		return oid;
+	return do_lookup_replace_object(oid);
+}
+
 #endif /* REPLACE_OBJECT_H */
diff --git a/sha1_file.c b/sha1_file.c
index 3e0af41892..c38e41e49e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -23,6 +23,7 @@
 #include "sha1-lookup.h"
 #include "bulk-checkin.h"
 #include "repository.h"
+#include "replace-object.h"
 #include "streaming.h"
 #include "dir.h"
 #include "list.h"
diff --git a/streaming.c b/streaming.c
index 7d55ba64c7..a6e1162946 100644
--- a/streaming.c
+++ b/streaming.c
@@ -5,6 +5,7 @@
 #include "streaming.h"
 #include "repository.h"
 #include "object-store.h"
+#include "replace-object.h"
 #include "packfile.h"
 
 enum input_source {
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 04/15] replace-object: eliminate replace objects prepared flag

[Stefan Beller]

Make the oidmap a pointer.

That way we eliminate the need for the global boolean
variable 'replace_object_prepared' as we can put this information
into the pointer being NULL or not.

Another advantage of this is that we would more quickly catch
code that tries to access replace-map without initializing it.

This also allows the '#include "oidmap.h"' introduced in a previous
patch to be replaced by the forward declaration of 'struct oidmap;'.
Keeping the type opaque discourages circumventing accessor functions;
not dragging in other headers avoids some compile time overhead.

One disadvantage of this is change is performance as we need to
pay the overhead for a malloc. The alternative of moving the
global variable into the object store is less modular code.

Helped-by: René Scharfe <l.s.r@web.de>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 object-store.h   |  2 +-
 replace_object.c | 16 +++++++++-------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/object-store.h b/object-store.h
index c04b4c95eb..1ff862c7f9 100644
--- a/object-store.h
+++ b/object-store.h
@@ -99,7 +99,7 @@ struct raw_object_store {
 	 * Objects that should be substituted by other objects
 	 * (see git-replace(1)).
 	 */
-	struct oidmap replace_map;
+	struct oidmap *replace_map;
 
 	/*
 	 * private data
diff --git a/replace_object.c b/replace_object.c
index afbdf2df25..953fa9cc40 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -25,7 +25,7 @@ static int register_replace_ref(const char *refname,
 	oidcpy(&repl_obj->replacement, oid);
 
 	/* Register new object */
-	if (oidmap_put(&the_repository->objects->replace_map, repl_obj))
+	if (oidmap_put(the_repository->objects->replace_map, repl_obj))
 		die("duplicate replace ref: %s", refname);
 
 	return 0;
@@ -33,14 +33,16 @@ static int register_replace_ref(const char *refname,
 
 static void prepare_replace_object(void)
 {
-	static int replace_object_prepared;
-
-	if (replace_object_prepared)
+	if (the_repository->objects->replace_map)
 		return;
 
+	the_repository->objects->replace_map =
+		xmalloc(sizeof(*the_repository->objects->replace_map));
+	oidmap_init(the_repository->objects->replace_map, 0);
+
 	for_each_replace_ref(register_replace_ref, NULL);
-	replace_object_prepared = 1;
-	if (!the_repository->objects->replace_map.map.tablesize)
+
+	if (!the_repository->objects->replace_map->map.tablesize)
 		check_replace_refs = 0;
 }
 
@@ -64,7 +66,7 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)
 	/* Try to recursively replace the object */
 	while (depth-- > 0) {
 		struct replace_object *repl_obj =
-			oidmap_get(&the_repository->objects->replace_map, cur);
+			oidmap_get(the_repository->objects->replace_map, cur);
 		if (!repl_obj)
 			return cur;
 		cur = &repl_obj->replacement;
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 05/15] replace-object: check_replace_refs is safe in multi repo environment

[Stefan Beller]

In e1111cef23 (inline lookup_replace_object() calls, 2011-05-15) a shortcut
for checking the object replacement was added by setting check_replace_refs
to 0 once the replacements were evaluated to not exist. This works fine in
with the assumption of only one repository in existence.

The assumption won't hold true any more when we work on multiple instances
of a repository structs (e.g. one struct per submodule), as the first
repository to be inspected may have no replacements and would set the
global variable. Other repositories would then completely omit their
evaluation of replacements.

This reverts back the meaning of the flag `check_replace_refs` of
"Do we need to check with the lookup table?" to "Do we need to read
the replacement definition?", adding the bypassing logic to
lookup_replace_object after the replacement definition was read.
As with the original patch, delay the renaming of the global variable

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 environment.c    | 2 +-
 replace-object.h | 5 ++++-
 replace_object.c | 3 ---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/environment.c b/environment.c
index 39b3d906c8..b991fc0a87 100644
--- a/environment.c
+++ b/environment.c
@@ -50,7 +50,7 @@ const char *editor_program;
 const char *askpass_program;
 const char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
-int check_replace_refs = 1;
+int check_replace_refs = 1; /* NEEDSWORK: rename to read_replace_refs */
 char *git_replace_ref_base;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
diff --git a/replace-object.h b/replace-object.h
index 15315311fb..dbc51265ec 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -3,6 +3,7 @@
 
 #include "oidmap.h"
 #include "repository.h"
+#include "object-store.h"
 
 struct replace_object {
 	struct oidmap_entry original;
@@ -23,7 +24,9 @@ extern const struct object_id *do_lookup_replace_object(const struct object_id *
  */
 static inline const struct object_id *lookup_replace_object(const struct object_id *oid)
 {
-	if (!check_replace_refs)
+	if (!check_replace_refs ||
+	    (the_repository->objects->replace_map &&
+	     the_repository->objects->replace_map->map.tablesize == 0))
 		return oid;
 	return do_lookup_replace_object(oid);
 }
diff --git a/replace_object.c b/replace_object.c
index 953fa9cc40..b2405f6027 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -41,9 +41,6 @@ static void prepare_replace_object(void)
 	oidmap_init(the_repository->objects->replace_map, 0);
 
 	for_each_replace_ref(register_replace_ref, NULL);
-
-	if (!the_repository->objects->replace_map->map.tablesize)
-		check_replace_refs = 0;
 }
 
 /* We allow "recursive" replacement. Only within reason, though */
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 06/15] refs: add repository argument to get_main_ref_store

[Stefan Beller]

Add a repository argument to allow the get_main_ref_store caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/pack-refs.c       |  3 +-
 refs.c                    | 67 ++++++++++++++++++++-------------------
 refs.h                    |  4 ++-
 revision.c                |  5 +--
 t/helper/test-ref-store.c |  3 +-
 5 files changed, 44 insertions(+), 38 deletions(-)

diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index b106a392a4..f3353564f9 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "parse-options.h"
 #include "refs.h"
+#include "repository.h"
 
 static char const * const pack_refs_usage[] = {
 	N_("git pack-refs [<options>]"),
@@ -17,5 +18,5 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
 	};
 	if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
 		usage_with_options(pack_refs_usage, opts);
-	return refs_pack_refs(get_main_ref_store(), flags);
+	return refs_pack_refs(get_main_ref_store(the_repository), flags);
 }
diff --git a/refs.c b/refs.c
index 8b7a77fe5e..74d4ed97cb 100644
--- a/refs.c
+++ b/refs.c
@@ -13,6 +13,7 @@
 #include "tag.h"
 #include "submodule.h"
 #include "worktree.h"
+#include "repository.h"
 
 /*
  * List of all available backends
@@ -206,7 +207,7 @@ char *refs_resolve_refdup(struct ref_store *refs,
 char *resolve_refdup(const char *refname, int resolve_flags,
 		     struct object_id *oid, int *flags)
 {
-	return refs_resolve_refdup(get_main_ref_store(),
+	return refs_resolve_refdup(get_main_ref_store(the_repository),
 				   refname, resolve_flags,
 				   oid, flags);
 }
@@ -228,7 +229,7 @@ int refs_read_ref_full(struct ref_store *refs, const char *refname,
 
 int read_ref_full(const char *refname, int resolve_flags, struct object_id *oid, int *flags)
 {
-	return refs_read_ref_full(get_main_ref_store(), refname,
+	return refs_read_ref_full(get_main_ref_store(the_repository), refname,
 				  resolve_flags, oid, flags);
 }
 
@@ -375,7 +376,7 @@ int refs_for_each_tag_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_tag_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_tag_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_tag_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
@@ -385,7 +386,7 @@ int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_da
 
 int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_branch_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_branch_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
@@ -395,7 +396,7 @@ int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_da
 
 int for_each_remote_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_remote_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_remote_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int head_ref_namespaced(each_ref_fn fn, void *cb_data)
@@ -730,7 +731,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
 	struct strbuf err = STRBUF_INIT;
 
 	if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
-		assert(refs == get_main_ref_store());
+		assert(refs == get_main_ref_store(the_repository));
 		return delete_pseudoref(refname, old_oid);
 	}
 
@@ -752,7 +753,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg,
 int delete_ref(const char *msg, const char *refname,
 	       const struct object_id *old_oid, unsigned int flags)
 {
-	return refs_delete_ref(get_main_ref_store(), msg, refname,
+	return refs_delete_ref(get_main_ref_store(the_repository), msg, refname,
 			       old_oid, flags);
 }
 
@@ -928,7 +929,7 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
 
 struct ref_transaction *ref_transaction_begin(struct strbuf *err)
 {
-	return ref_store_transaction_begin(get_main_ref_store(), err);
+	return ref_store_transaction_begin(get_main_ref_store(the_repository), err);
 }
 
 void ref_transaction_free(struct ref_transaction *transaction)
@@ -1060,7 +1061,7 @@ int refs_update_ref(struct ref_store *refs, const char *msg,
 	int ret = 0;
 
 	if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
-		assert(refs == get_main_ref_store());
+		assert(refs == get_main_ref_store(the_repository));
 		ret = write_pseudoref(refname, new_oid, old_oid, &err);
 	} else {
 		t = ref_store_transaction_begin(refs, &err);
@@ -1099,7 +1100,7 @@ int update_ref(const char *msg, const char *refname,
 	       const struct object_id *old_oid,
 	       unsigned int flags, enum action_on_err onerr)
 {
-	return refs_update_ref(get_main_ref_store(), msg, refname, new_oid,
+	return refs_update_ref(get_main_ref_store(the_repository), msg, refname, new_oid,
 			       old_oid, flags, onerr);
 }
 
@@ -1320,7 +1321,7 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int head_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_head_ref(get_main_ref_store(), fn, cb_data);
+	return refs_head_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 struct ref_iterator *refs_ref_iterator_begin(
@@ -1379,7 +1380,7 @@ int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_ref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_ref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_ref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
@@ -1390,7 +1391,7 @@ int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
 
 int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_ref_in(get_main_ref_store(), prefix, fn, cb_data);
+	return refs_for_each_ref_in(get_main_ref_store(the_repository), prefix, fn, cb_data);
 }
 
 int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)
@@ -1399,7 +1400,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsig
 
 	if (broken)
 		flag = DO_FOR_EACH_INCLUDE_BROKEN;
-	return do_for_each_ref(get_main_ref_store(),
+	return do_for_each_ref(get_main_ref_store(the_repository),
 			       prefix, fn, 0, flag, cb_data);
 }
 
@@ -1416,7 +1417,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 
 int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(get_main_ref_store(),
+	return do_for_each_ref(get_main_ref_store(the_repository),
 			       git_replace_ref_base, fn,
 			       strlen(git_replace_ref_base),
 			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
@@ -1427,7 +1428,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_main_ref_store(),
+	ret = do_for_each_ref(get_main_ref_store(the_repository),
 			      buf.buf, fn, 0, 0, cb_data);
 	strbuf_release(&buf);
 	return ret;
@@ -1441,7 +1442,7 @@ int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_rawref(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_rawref(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_rawref(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_read_raw_ref(struct ref_store *ref_store,
@@ -1547,7 +1548,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 /* backend functions */
 int refs_init_db(struct strbuf *err)
 {
-	struct ref_store *refs = get_main_ref_store();
+	struct ref_store *refs = get_main_ref_store(the_repository);
 
 	return refs->be->init_db(refs, err);
 }
@@ -1555,7 +1556,7 @@ int refs_init_db(struct strbuf *err)
 const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
 			       struct object_id *oid, int *flags)
 {
-	return refs_resolve_ref_unsafe(get_main_ref_store(), refname,
+	return refs_resolve_ref_unsafe(get_main_ref_store(the_repository), refname,
 				       resolve_flags, oid, flags);
 }
 
@@ -1651,7 +1652,7 @@ static struct ref_store *ref_store_init(const char *gitdir,
 	return refs;
 }
 
-struct ref_store *get_main_ref_store(void)
+struct ref_store *get_main_ref_store_the_repository(void)
 {
 	if (main_ref_store)
 		return main_ref_store;
@@ -1726,7 +1727,7 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)
 	const char *id;
 
 	if (wt->is_current)
-		return get_main_ref_store();
+		return get_main_ref_store(the_repository);
 
 	id = wt->id ? wt->id : "/";
 	refs = lookup_ref_store_map(&worktree_ref_stores, id);
@@ -1782,7 +1783,7 @@ int refs_peel_ref(struct ref_store *refs, const char *refname,
 
 int peel_ref(const char *refname, struct object_id *oid)
 {
-	return refs_peel_ref(get_main_ref_store(), refname, oid);
+	return refs_peel_ref(get_main_ref_store(the_repository), refname, oid);
 }
 
 int refs_create_symref(struct ref_store *refs,
@@ -1798,7 +1799,7 @@ int refs_create_symref(struct ref_store *refs,
 int create_symref(const char *ref_target, const char *refs_heads_master,
 		  const char *logmsg)
 {
-	return refs_create_symref(get_main_ref_store(), ref_target,
+	return refs_create_symref(get_main_ref_store(the_repository), ref_target,
 				  refs_heads_master, logmsg);
 }
 
@@ -2006,7 +2007,7 @@ int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 
 int for_each_reflog(each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_reflog(get_main_ref_store(), fn, cb_data);
+	return refs_for_each_reflog(get_main_ref_store(the_repository), fn, cb_data);
 }
 
 int refs_for_each_reflog_ent_reverse(struct ref_store *refs,
@@ -2021,7 +2022,7 @@ int refs_for_each_reflog_ent_reverse(struct ref_store *refs,
 int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
 				void *cb_data)
 {
-	return refs_for_each_reflog_ent_reverse(get_main_ref_store(),
+	return refs_for_each_reflog_ent_reverse(get_main_ref_store(the_repository),
 						refname, fn, cb_data);
 }
 
@@ -2034,7 +2035,7 @@ int refs_for_each_reflog_ent(struct ref_store *refs, const char *refname,
 int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn,
 			void *cb_data)
 {
-	return refs_for_each_reflog_ent(get_main_ref_store(), refname,
+	return refs_for_each_reflog_ent(get_main_ref_store(the_repository), refname,
 					fn, cb_data);
 }
 
@@ -2045,7 +2046,7 @@ int refs_reflog_exists(struct ref_store *refs, const char *refname)
 
 int reflog_exists(const char *refname)
 {
-	return refs_reflog_exists(get_main_ref_store(), refname);
+	return refs_reflog_exists(get_main_ref_store(the_repository), refname);
 }
 
 int refs_create_reflog(struct ref_store *refs, const char *refname,
@@ -2057,7 +2058,7 @@ int refs_create_reflog(struct ref_store *refs, const char *refname,
 int safe_create_reflog(const char *refname, int force_create,
 		       struct strbuf *err)
 {
-	return refs_create_reflog(get_main_ref_store(), refname,
+	return refs_create_reflog(get_main_ref_store(the_repository), refname,
 				  force_create, err);
 }
 
@@ -2068,7 +2069,7 @@ int refs_delete_reflog(struct ref_store *refs, const char *refname)
 
 int delete_reflog(const char *refname)
 {
-	return refs_delete_reflog(get_main_ref_store(), refname);
+	return refs_delete_reflog(get_main_ref_store(the_repository), refname);
 }
 
 int refs_reflog_expire(struct ref_store *refs,
@@ -2091,7 +2092,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,
 		  reflog_expiry_cleanup_fn cleanup_fn,
 		  void *policy_cb_data)
 {
-	return refs_reflog_expire(get_main_ref_store(),
+	return refs_reflog_expire(get_main_ref_store(the_repository),
 				  refname, oid, flags,
 				  prepare_fn, should_prune_fn,
 				  cleanup_fn, policy_cb_data);
@@ -2114,7 +2115,7 @@ int refs_delete_refs(struct ref_store *refs, const char *msg,
 int delete_refs(const char *msg, struct string_list *refnames,
 		unsigned int flags)
 {
-	return refs_delete_refs(get_main_ref_store(), msg, refnames, flags);
+	return refs_delete_refs(get_main_ref_store(the_repository), msg, refnames, flags);
 }
 
 int refs_rename_ref(struct ref_store *refs, const char *oldref,
@@ -2125,7 +2126,7 @@ int refs_rename_ref(struct ref_store *refs, const char *oldref,
 
 int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 {
-	return refs_rename_ref(get_main_ref_store(), oldref, newref, logmsg);
+	return refs_rename_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
 }
 
 int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
@@ -2136,5 +2137,5 @@ int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
 
 int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)
 {
-	return refs_copy_existing_ref(get_main_ref_store(), oldref, newref, logmsg);
+	return refs_copy_existing_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
 }
diff --git a/refs.h b/refs.h
index 01be5ae32f..0d013377ce 100644
--- a/refs.h
+++ b/refs.h
@@ -758,7 +758,9 @@ int reflog_expire(const char *refname, const struct object_id *oid,
 
 int ref_storage_backend_exists(const char *name);
 
-struct ref_store *get_main_ref_store(void);
+#define get_main_ref_store(r) \
+	get_main_ref_store_##r()
+struct ref_store *get_main_ref_store_the_repository(void);
 /*
  * Return the ref_store instance for the specified submodule. For the
  * main repository, use submodule==NULL; such a call cannot fail. For
diff --git a/revision.c b/revision.c
index b42c836d7a..1cff11833e 100644
--- a/revision.c
+++ b/revision.c
@@ -6,6 +6,7 @@
 #include "diff.h"
 #include "refs.h"
 #include "revision.h"
+#include "repository.h"
 #include "graph.h"
 #include "grep.h"
 #include "reflog-walk.h"
@@ -1285,7 +1286,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)
 
 	cb.all_revs = revs;
 	cb.all_flags = flags;
-	cb.refs = get_main_ref_store();
+	cb.refs = get_main_ref_store(the_repository);
 	for_each_reflog(handle_one_reflog, &cb);
 
 	if (!revs->single_worktree)
@@ -2176,7 +2177,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 			die("BUG: --single-worktree cannot be used together with submodule");
 		refs = get_submodule_ref_store(submodule);
 	} else
-		refs = get_main_ref_store();
+		refs = get_main_ref_store(the_repository);
 
 	/*
 	 * NOTE!
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 7314b5943e..e8c328ec0e 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -2,6 +2,7 @@
 #include "refs.h"
 #include "worktree.h"
 #include "object-store.h"
+#include "repository.h"
 
 static const char *notnull(const char *arg, const char *name)
 {
@@ -22,7 +23,7 @@ static const char **get_store(const char **argv, struct ref_store **refs)
 	if (!argv[0]) {
 		die("ref store required");
 	} else if (!strcmp(argv[0], "main")) {
-		*refs = get_main_ref_store();
+		*refs = get_main_ref_store(the_repository);
 	} else if (skip_prefix(argv[0], "submodule:", &gitdir)) {
 		struct strbuf sb = STRBUF_INIT;
 		int ret;
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 07/15] refs: add repository argument to for_each_replace_ref

[Stefan Beller]

Add a repository argument to allow for_each_replace_ref callers to be
more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/replace.c | 4 +++-
 refs.c            | 2 +-
 refs.h            | 4 +++-
 replace_object.c  | 2 +-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 19006e52bc..ef8145e556 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -14,6 +14,8 @@
 #include "refs.h"
 #include "parse-options.h"
 #include "run-command.h"
+#include "object-store.h"
+#include "repository.h"
 #include "tag.h"
 
 static const char * const git_replace_usage[] = {
@@ -83,7 +85,7 @@ static int list_replace_refs(const char *pattern, const char *format)
 		    "valid formats are 'short', 'medium' and 'long'\n",
 		    format);
 
-	for_each_replace_ref(show_reference, (void *)&data);
+	for_each_replace_ref(the_repository, show_reference, (void *)&data);
 
 	return 0;
 }
diff --git a/refs.c b/refs.c
index 74d4ed97cb..f58b9fb7df 100644
--- a/refs.c
+++ b/refs.c
@@ -1415,7 +1415,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 	return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
 }
 
-int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(get_main_ref_store(the_repository),
 			       git_replace_ref_base, fn,
diff --git a/refs.h b/refs.h
index 0d013377ce..ab3d2bec2f 100644
--- a/refs.h
+++ b/refs.h
@@ -300,7 +300,9 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,
 int for_each_tag_ref(each_ref_fn fn, void *cb_data);
 int for_each_branch_ref(each_ref_fn fn, void *cb_data);
 int for_each_remote_ref(each_ref_fn fn, void *cb_data);
-int for_each_replace_ref(each_ref_fn fn, void *cb_data);
+#define for_each_replace_ref(r, fn, cb) \
+	for_each_replace_ref_##r(fn, cb)
+int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data);
 int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
 int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
 			 const char *prefix, void *cb_data);
diff --git a/replace_object.c b/replace_object.c
index b2405f6027..16a95ea416 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -40,7 +40,7 @@ static void prepare_replace_object(void)
 		xmalloc(sizeof(*the_repository->objects->replace_map));
 	oidmap_init(the_repository->objects->replace_map, 0);
 
-	for_each_replace_ref(register_replace_ref, NULL);
+	for_each_replace_ref(the_repository, register_replace_ref, NULL);
 }
 
 /* We allow "recursive" replacement. Only within reason, though */
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 08/15] replace-object: add repository argument to prepare_replace_object

[Stefan Beller]

Add a repository argument to allow the prepare_replace_object caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace_object.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/replace_object.c b/replace_object.c
index 16a95ea416..567d9da708 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -31,7 +31,9 @@ static int register_replace_ref(const char *refname,
 	return 0;
 }
 
-static void prepare_replace_object(void)
+#define prepare_replace_object(r) \
+	prepare_replace_object_##r()
+static void prepare_replace_object_the_repository(void)
 {
 	if (the_repository->objects->replace_map)
 		return;
@@ -58,7 +60,7 @@ const struct object_id *do_lookup_replace_object(const struct object_id *oid)
 	int depth = MAXREPLACEDEPTH;
 	const struct object_id *cur = oid;
 
-	prepare_replace_object();
+	prepare_replace_object(the_repository);
 
 	/* Try to recursively replace the object */
 	while (depth-- > 0) {
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 09/15] replace-object: add repository argument to do_lookup_replace_object

[Stefan Beller]

Add a repository argument to allow the do_lookup_replace_object caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.h | 5 +++--
 replace_object.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/replace-object.h b/replace-object.h
index dbc51265ec..ddeb0470bd 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -14,7 +14,8 @@ struct replace_object {
  * This internal function is only declared here for the benefit of
  * lookup_replace_object().  Please do not call it directly.
  */
-extern const struct object_id *do_lookup_replace_object(const struct object_id *oid);
+#define do_lookup_replace_object(r, s) do_lookup_replace_object_##r(s)
+extern const struct object_id *do_lookup_replace_object_the_repository(const struct object_id *oid);
 
 /*
  * If object sha1 should be replaced, return the replacement object's
@@ -28,7 +29,7 @@ static inline const struct object_id *lookup_replace_object(const struct object_
 	    (the_repository->objects->replace_map &&
 	     the_repository->objects->replace_map->map.tablesize == 0))
 		return oid;
-	return do_lookup_replace_object(oid);
+	return do_lookup_replace_object(the_repository, oid);
 }
 
 #endif /* REPLACE_OBJECT_H */
diff --git a/replace_object.c b/replace_object.c
index 567d9da708..adfed78901 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -55,7 +55,7 @@ static void prepare_replace_object_the_repository(void)
  * permanently-allocated value.  This function always respects replace
  * references, regardless of the value of check_replace_refs.
  */
-const struct object_id *do_lookup_replace_object(const struct object_id *oid)
+const struct object_id *do_lookup_replace_object_the_repository(const struct object_id *oid)
 {
 	int depth = MAXREPLACEDEPTH;
 	const struct object_id *cur = oid;
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 10/15] replace-object: add repository argument to lookup_replace_object

[Stefan Beller]

Add a repository argument to allow callers of lookup_replace_object
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/mktag.c  | 2 +-
 object.c         | 2 +-
 replace-object.h | 3 ++-
 sha1_file.c      | 6 +++---
 streaming.c      | 2 +-
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/mktag.c b/builtin/mktag.c
index e3d20a7722..82a6e86077 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -25,7 +25,7 @@ static int verify_object(const struct object_id *oid, const char *expected_type)
 	enum object_type type;
 	unsigned long size;
 	void *buffer = read_object_file(oid, &type, &size);
-	const struct object_id *repl = lookup_replace_object(oid);
+	const struct object_id *repl = lookup_replace_object(the_repository, oid);
 
 	if (buffer) {
 		if (type == type_from_string(expected_type))
diff --git a/object.c b/object.c
index 998ec2a25f..66cffaf6e5 100644
--- a/object.c
+++ b/object.c
@@ -247,7 +247,7 @@ struct object *parse_object(const struct object_id *oid)
 	unsigned long size;
 	enum object_type type;
 	int eaten;
-	const struct object_id *repl = lookup_replace_object(oid);
+	const struct object_id *repl = lookup_replace_object(the_repository, oid);
 	void *buffer;
 	struct object *obj;
 
diff --git a/replace-object.h b/replace-object.h
index ddeb0470bd..dff57bfa1e 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -23,7 +23,8 @@ extern const struct object_id *do_lookup_replace_object_the_repository(const str
  * either sha1 or a pointer to a permanently-allocated value.  When
  * object replacement is suppressed, always return sha1.
  */
-static inline const struct object_id *lookup_replace_object(const struct object_id *oid)
+#define lookup_replace_object(r, s) lookup_replace_object_##r(s)
+static inline const struct object_id *lookup_replace_object_the_repository(const struct object_id *oid)
 {
 	if (!check_replace_refs ||
 	    (the_repository->objects->replace_map &&
diff --git a/sha1_file.c b/sha1_file.c
index c38e41e49e..028a4357c5 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1240,7 +1240,7 @@ int oid_object_info_extended(const struct object_id *oid, struct object_info *oi
 	int already_retried = 0;
 
 	if (flags & OBJECT_INFO_LOOKUP_REPLACE)
-		real = lookup_replace_object(oid);
+		real = lookup_replace_object(the_repository, oid);
 
 	if (is_null_oid(real))
 		return -1;
@@ -1379,8 +1379,8 @@ void *read_object_file_extended(const struct object_id *oid,
 	const struct packed_git *p;
 	const char *path;
 	struct stat st;
-	const struct object_id *repl = lookup_replace ? lookup_replace_object(oid)
-						      : oid;
+	const struct object_id *repl = lookup_replace ?
+		lookup_replace_object(the_repository, oid) : oid;
 
 	errno = 0;
 	data = read_object(repl->hash, type, size);
diff --git a/streaming.c b/streaming.c
index a6e1162946..cce7b17ea7 100644
--- a/streaming.c
+++ b/streaming.c
@@ -140,7 +140,7 @@ struct git_istream *open_istream(const struct object_id *oid,
 {
 	struct git_istream *st;
 	struct object_info oi = OBJECT_INFO_INIT;
-	const struct object_id *real = lookup_replace_object(oid);
+	const struct object_id *real = lookup_replace_object(the_repository, oid);
 	enum input_source src = istream_source(real, type, &oi);
 
 	if (src < 0)
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 11/15] refs: store the main ref store inside the repository struct

[Stefan Beller]

This moves the 'main_ref_store', which was a global variable in refs.c
into the repository struct.

This patch does not deal with the parts in the refs subsystem which deal
with the submodules there. A later patch needs to get rid of the submodule
exposure in the refs API, such as 'get_submodule_ref_store(path)'.

Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 refs.c               | 13 +++++--------
 refs.h               |  4 +---
 refs/files-backend.c |  4 ----
 repository.h         |  3 +++
 4 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/refs.c b/refs.c
index f58b9fb7df..36df1bc73a 100644
--- a/refs.c
+++ b/refs.c
@@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry(
 	return entry;
 }
 
-/* A pointer to the ref_store for the main repository: */
-static struct ref_store *main_ref_store;
-
 /* A hashmap of ref_stores, stored by submodule name: */
 static struct hashmap submodule_ref_stores;
 
@@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char *gitdir,
 	return refs;
 }
 
-struct ref_store *get_main_ref_store_the_repository(void)
+struct ref_store *get_main_ref_store(struct repository *r)
 {
-	if (main_ref_store)
-		return main_ref_store;
+	if (r->refs)
+		return r->refs;
 
-	main_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);
-	return main_ref_store;
+	r->refs = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
+	return r->refs;
 }
 
 /*
diff --git a/refs.h b/refs.h
index ab3d2bec2f..f5ab68c0ed 100644
--- a/refs.h
+++ b/refs.h
@@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,
 
 int ref_storage_backend_exists(const char *name);
 
-#define get_main_ref_store(r) \
-	get_main_ref_store_##r()
-struct ref_store *get_main_ref_store_the_repository(void);
+struct ref_store *get_main_ref_store(struct repository *r);
 /*
  * Return the ref_store instance for the specified submodule. For the
  * main repository, use submodule==NULL; such a call cannot fail. For
diff --git a/refs/files-backend.c b/refs/files-backend.c
index bec8e30e9e..5c76a75817 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -61,10 +61,6 @@ struct ref_lock {
 	struct object_id old_oid;
 };
 
-/*
- * Future: need to be in "struct repository"
- * when doing a full libification.
- */
 struct files_ref_store {
 	struct ref_store base;
 	unsigned int store_flags;
diff --git a/repository.h b/repository.h
index 09df94a472..e6e00f541b 100644
--- a/repository.h
+++ b/repository.h
@@ -26,6 +26,9 @@ struct repository {
 	 */
 	struct raw_object_store *objects;
 
+	/* The store in which the refs are held. */
+	struct ref_store *refs;
+
 	/*
 	 * Path to the repository's graft file.
 	 * Cannot be NULL after initialization.
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 12/15] refs: allow for_each_replace_ref to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 refs.c | 4 ++--
 refs.h | 4 +---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/refs.c b/refs.c
index 36df1bc73a..9b56fa9b81 100644
--- a/refs.c
+++ b/refs.c
@@ -1415,9 +1415,9 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 	return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
 }
 
-int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data)
+int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref(get_main_ref_store(the_repository),
+	return do_for_each_ref(get_main_ref_store(r),
 			       git_replace_ref_base, fn,
 			       strlen(git_replace_ref_base),
 			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
diff --git a/refs.h b/refs.h
index f5ab68c0ed..15f3a91cc4 100644
--- a/refs.h
+++ b/refs.h
@@ -300,9 +300,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,
 int for_each_tag_ref(each_ref_fn fn, void *cb_data);
 int for_each_branch_ref(each_ref_fn fn, void *cb_data);
 int for_each_remote_ref(each_ref_fn fn, void *cb_data);
-#define for_each_replace_ref(r, fn, cb) \
-	for_each_replace_ref_##r(fn, cb)
-int for_each_replace_ref_the_repository(each_ref_fn fn, void *cb_data);
+int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data);
 int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
 int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
 			 const char *prefix, void *cb_data);
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 13/15] replace-object: allow prepare_replace_object to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace_object.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/replace_object.c b/replace_object.c
index adfed78901..eae52c66f3 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -31,18 +31,16 @@ static int register_replace_ref(const char *refname,
 	return 0;
 }
 
-#define prepare_replace_object(r) \
-	prepare_replace_object_##r()
-static void prepare_replace_object_the_repository(void)
+static void prepare_replace_object(struct repository *r)
 {
-	if (the_repository->objects->replace_map)
+	if (r->objects->replace_map)
 		return;
 
-	the_repository->objects->replace_map =
+	r->objects->replace_map =
 		xmalloc(sizeof(*the_repository->objects->replace_map));
-	oidmap_init(the_repository->objects->replace_map, 0);
+	oidmap_init(r->objects->replace_map, 0);
 
-	for_each_replace_ref(the_repository, register_replace_ref, NULL);
+	for_each_replace_ref(r, register_replace_ref, NULL);
 }
 
 /* We allow "recursive" replacement. Only within reason, though */
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 14/15] replace-object: allow do_lookup_replace_object to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.h | 4 ++--
 replace_object.c | 7 ++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/replace-object.h b/replace-object.h
index dff57bfa1e..3520fd7ff7 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -14,8 +14,8 @@ struct replace_object {
  * This internal function is only declared here for the benefit of
  * lookup_replace_object().  Please do not call it directly.
  */
-#define do_lookup_replace_object(r, s) do_lookup_replace_object_##r(s)
-extern const struct object_id *do_lookup_replace_object_the_repository(const struct object_id *oid);
+extern const struct object_id *do_lookup_replace_object(struct repository *r,
+						        const struct object_id *oid);
 
 /*
  * If object sha1 should be replaced, return the replacement object's
diff --git a/replace_object.c b/replace_object.c
index eae52c66f3..246b98cd4f 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -53,17 +53,18 @@ static void prepare_replace_object(struct repository *r)
  * permanently-allocated value.  This function always respects replace
  * references, regardless of the value of check_replace_refs.
  */
-const struct object_id *do_lookup_replace_object_the_repository(const struct object_id *oid)
+const struct object_id *do_lookup_replace_object(struct repository *r,
+						 const struct object_id *oid)
 {
 	int depth = MAXREPLACEDEPTH;
 	const struct object_id *cur = oid;
 
-	prepare_replace_object(the_repository);
+	prepare_replace_object(r);
 
 	/* Try to recursively replace the object */
 	while (depth-- > 0) {
 		struct replace_object *repl_obj =
-			oidmap_get(the_repository->objects->replace_map, cur);
+			oidmap_get(r->objects->replace_map, cur);
 		if (!repl_obj)
 			return cur;
 		cur = &repl_obj->replacement;
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 15/15] replace-object: allow lookup_replace_object to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 replace-object.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/replace-object.h b/replace-object.h
index 3520fd7ff7..9f607a929b 100644
--- a/replace-object.h
+++ b/replace-object.h
@@ -23,14 +23,14 @@ extern const struct object_id *do_lookup_replace_object(struct repository *r,
  * either sha1 or a pointer to a permanently-allocated value.  When
  * object replacement is suppressed, always return sha1.
  */
-#define lookup_replace_object(r, s) lookup_replace_object_##r(s)
-static inline const struct object_id *lookup_replace_object_the_repository(const struct object_id *oid)
+static inline const struct object_id *lookup_replace_object(struct repository *r,
+							    const struct object_id *oid)
 {
 	if (!check_replace_refs ||
-	    (the_repository->objects->replace_map &&
-	     the_repository->objects->replace_map->map.tablesize == 0))
+	    (r->objects->replace_map &&
+	     r->objects->replace_map->map.tablesize == 0))
 		return oid;
-	return do_lookup_replace_object(the_repository, oid);
+	return do_lookup_replace_object(r, oid);
 }
 
 #endif /* REPLACE_OBJECT_H */
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCHv3 00/15] replace_object.c: rename to use dash in file name

[Derrick Stolee]

On 4/11/2018 8:21 PM, Stefan Beller wrote:
> v3:
> * interdiff below,
>    the only changes are renaming the variable
>    -       struct ref_store *main_ref_store;
>    +       struct ref_store *refs;
>    in struct repository.
>    as well as dropping the file rename patch.
> * improved commit messages from discussion on the single patches.
Looks good to me. Thanks!
-Stolee