* [PATCH v2 1/9] refs: reorder some function definitions
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
@ 2017-02-10 11:16 ` Michael Haggerty
2017-02-10 19:14 ` Junio C Hamano
2017-02-10 11:16 ` [PATCH v2 2/9] refs: make some ref_store lookup functions private Michael Haggerty
` (10 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
This avoids the need to add forward declarations in the next step.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 64 ++++++++++++++++++++++++++++++++--------------------------------
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/refs.c b/refs.c
index 9bd0bc1..707092f 100644
--- a/refs.c
+++ b/refs.c
@@ -1358,27 +1358,19 @@ static struct ref_store *main_ref_store;
/* A linked list of ref_stores for submodules: */
static struct ref_store *submodule_ref_stores;
-void base_ref_store_init(struct ref_store *refs,
- const struct ref_storage_be *be,
- const char *submodule)
+struct ref_store *lookup_ref_store(const char *submodule)
{
- refs->be = be;
- if (!submodule) {
- if (main_ref_store)
- die("BUG: main_ref_store initialized twice");
+ struct ref_store *refs;
- refs->submodule = "";
- refs->next = NULL;
- main_ref_store = refs;
- } else {
- if (lookup_ref_store(submodule))
- die("BUG: ref_store for submodule '%s' initialized twice",
- submodule);
+ if (!submodule || !*submodule)
+ return main_ref_store;
- refs->submodule = xstrdup(submodule);
- refs->next = submodule_ref_stores;
- submodule_ref_stores = refs;
+ for (refs = submodule_ref_stores; refs; refs = refs->next) {
+ if (!strcmp(submodule, refs->submodule))
+ return refs;
}
+
+ return NULL;
}
struct ref_store *ref_store_init(const char *submodule)
@@ -1395,21 +1387,6 @@ struct ref_store *ref_store_init(const char *submodule)
return be->init(submodule);
}
-struct ref_store *lookup_ref_store(const char *submodule)
-{
- struct ref_store *refs;
-
- if (!submodule || !*submodule)
- return main_ref_store;
-
- for (refs = submodule_ref_stores; refs; refs = refs->next) {
- if (!strcmp(submodule, refs->submodule))
- return refs;
- }
-
- return NULL;
-}
-
struct ref_store *get_ref_store(const char *submodule)
{
struct ref_store *refs;
@@ -1435,6 +1412,29 @@ struct ref_store *get_ref_store(const char *submodule)
return refs;
}
+void base_ref_store_init(struct ref_store *refs,
+ const struct ref_storage_be *be,
+ const char *submodule)
+{
+ refs->be = be;
+ if (!submodule) {
+ if (main_ref_store)
+ die("BUG: main_ref_store initialized twice");
+
+ refs->submodule = "";
+ refs->next = NULL;
+ main_ref_store = refs;
+ } else {
+ if (lookup_ref_store(submodule))
+ die("BUG: ref_store for submodule '%s' initialized twice",
+ submodule);
+
+ refs->submodule = xstrdup(submodule);
+ refs->next = submodule_ref_stores;
+ submodule_ref_stores = refs;
+ }
+}
+
void assert_main_repository(struct ref_store *refs, const char *caller)
{
if (*refs->submodule)
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 1/9] refs: reorder some function definitions
2017-02-10 11:16 ` [PATCH v2 1/9] refs: reorder some function definitions Michael Haggerty
@ 2017-02-10 19:14 ` Junio C Hamano
0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2017-02-10 19:14 UTC (permalink / raw)
To: Michael Haggerty
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git
Michael Haggerty <mhagger@alum.mit.edu> writes:
> This avoids the need to add forward declarations in the next step.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs.c | 64 ++++++++++++++++++++++++++++++++--------------------------------
> 1 file changed, 32 insertions(+), 32 deletions(-)
Makes sense, but the patch itself looks like ... unreadble ;-)
>
> diff --git a/refs.c b/refs.c
> index 9bd0bc1..707092f 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1358,27 +1358,19 @@ static struct ref_store *main_ref_store;
> /* A linked list of ref_stores for submodules: */
> static struct ref_store *submodule_ref_stores;
>
> -void base_ref_store_init(struct ref_store *refs,
> - const struct ref_storage_be *be,
> - const char *submodule)
> +struct ref_store *lookup_ref_store(const char *submodule)
> {
> - refs->be = be;
> - if (!submodule) {
> - if (main_ref_store)
> - die("BUG: main_ref_store initialized twice");
> + struct ref_store *refs;
>
> - refs->submodule = "";
> - refs->next = NULL;
> - main_ref_store = refs;
> - } else {
> - if (lookup_ref_store(submodule))
> - die("BUG: ref_store for submodule '%s' initialized twice",
> - submodule);
> + if (!submodule || !*submodule)
> + return main_ref_store;
>
> - refs->submodule = xstrdup(submodule);
> - refs->next = submodule_ref_stores;
> - submodule_ref_stores = refs;
> + for (refs = submodule_ref_stores; refs; refs = refs->next) {
> + if (!strcmp(submodule, refs->submodule))
> + return refs;
> }
> +
> + return NULL;
> }
>
> struct ref_store *ref_store_init(const char *submodule)
> @@ -1395,21 +1387,6 @@ struct ref_store *ref_store_init(const char *submodule)
> return be->init(submodule);
> }
>
> -struct ref_store *lookup_ref_store(const char *submodule)
> -{
> - struct ref_store *refs;
> -
> - if (!submodule || !*submodule)
> - return main_ref_store;
> -
> - for (refs = submodule_ref_stores; refs; refs = refs->next) {
> - if (!strcmp(submodule, refs->submodule))
> - return refs;
> - }
> -
> - return NULL;
> -}
> -
> struct ref_store *get_ref_store(const char *submodule)
> {
> struct ref_store *refs;
> @@ -1435,6 +1412,29 @@ struct ref_store *get_ref_store(const char *submodule)
> return refs;
> }
>
> +void base_ref_store_init(struct ref_store *refs,
> + const struct ref_storage_be *be,
> + const char *submodule)
> +{
> + refs->be = be;
> + if (!submodule) {
> + if (main_ref_store)
> + die("BUG: main_ref_store initialized twice");
> +
> + refs->submodule = "";
> + refs->next = NULL;
> + main_ref_store = refs;
> + } else {
> + if (lookup_ref_store(submodule))
> + die("BUG: ref_store for submodule '%s' initialized twice",
> + submodule);
> +
> + refs->submodule = xstrdup(submodule);
> + refs->next = submodule_ref_stores;
> + submodule_ref_stores = refs;
> + }
> +}
> +
> void assert_main_repository(struct ref_store *refs, const char *caller)
> {
> if (*refs->submodule)
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 2/9] refs: make some ref_store lookup functions private
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
2017-02-10 11:16 ` [PATCH v2 1/9] refs: reorder some function definitions Michael Haggerty
@ 2017-02-10 11:16 ` Michael Haggerty
2017-02-10 11:16 ` [PATCH v2 3/9] refs: remove some unnecessary handling of submodule == "" Michael Haggerty
` (9 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
The following functions currently don't need to be exposed:
* ref_store_init()
* lookup_ref_store()
That might change in the future, but for now make them private.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 19 +++++++++++++++++--
refs/refs-internal.h | 19 -------------------
2 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/refs.c b/refs.c
index 707092f..d7265cc 100644
--- a/refs.c
+++ b/refs.c
@@ -1358,7 +1358,15 @@ static struct ref_store *main_ref_store;
/* A linked list of ref_stores for submodules: */
static struct ref_store *submodule_ref_stores;
-struct ref_store *lookup_ref_store(const char *submodule)
+/*
+ * Return the ref_store instance for the specified submodule (or the
+ * main repository if submodule is NULL). If that ref_store hasn't
+ * been initialized yet, return NULL.
+ *
+ * For backwards compatibility, submodule=="" is treated the same as
+ * submodule==NULL.
+ */
+static struct ref_store *lookup_ref_store(const char *submodule)
{
struct ref_store *refs;
@@ -1373,7 +1381,14 @@ struct ref_store *lookup_ref_store(const char *submodule)
return NULL;
}
-struct ref_store *ref_store_init(const char *submodule)
+/*
+ * Create, record, and return a ref_store instance for the specified
+ * submodule (or the main repository if submodule is NULL).
+ *
+ * For backwards compatibility, submodule=="" is treated the same as
+ * submodule==NULL.
+ */
+static struct ref_store *ref_store_init(const char *submodule)
{
const char *be_name = "files";
struct ref_storage_be *be = find_ref_storage_backend(be_name);
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 708b260..d8a7eb1 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -653,25 +653,6 @@ void base_ref_store_init(struct ref_store *refs,
const char *submodule);
/*
- * Create, record, and return a ref_store instance for the specified
- * submodule (or the main repository if submodule is NULL).
- *
- * For backwards compatibility, submodule=="" is treated the same as
- * submodule==NULL.
- */
-struct ref_store *ref_store_init(const char *submodule);
-
-/*
- * Return the ref_store instance for the specified submodule (or the
- * main repository if submodule is NULL). If that ref_store hasn't
- * been initialized yet, return NULL.
- *
- * For backwards compatibility, submodule=="" is treated the same as
- * submodule==NULL.
- */
-struct ref_store *lookup_ref_store(const char *submodule);
-
-/*
* Return the ref_store instance for the specified submodule. For the
* main repository, use submodule==NULL; such a call cannot fail. For
* a submodule, the submodule must exist and be a nonbare repository,
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 3/9] refs: remove some unnecessary handling of submodule == ""
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
2017-02-10 11:16 ` [PATCH v2 1/9] refs: reorder some function definitions Michael Haggerty
2017-02-10 11:16 ` [PATCH v2 2/9] refs: make some ref_store lookup functions private Michael Haggerty
@ 2017-02-10 11:16 ` Michael Haggerty
2017-02-10 11:16 ` [PATCH v2 4/9] register_ref_store(): new function Michael Haggerty
` (8 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
The only external entry point to the ref_store lookup functions is
get_ref_store(), which ensures that submodule == "" is passed along as
NULL. So ref_store_init() and lookup_ref_store() don't have to handle
submodule being specified as the empty string.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/refs.c b/refs.c
index d7265cc..6348414 100644
--- a/refs.c
+++ b/refs.c
@@ -1362,15 +1362,12 @@ static struct ref_store *submodule_ref_stores;
* Return the ref_store instance for the specified submodule (or the
* main repository if submodule is NULL). If that ref_store hasn't
* been initialized yet, return NULL.
- *
- * For backwards compatibility, submodule=="" is treated the same as
- * submodule==NULL.
*/
static struct ref_store *lookup_ref_store(const char *submodule)
{
struct ref_store *refs;
- if (!submodule || !*submodule)
+ if (!submodule)
return main_ref_store;
for (refs = submodule_ref_stores; refs; refs = refs->next) {
@@ -1384,9 +1381,6 @@ static struct ref_store *lookup_ref_store(const char *submodule)
/*
* Create, record, and return a ref_store instance for the specified
* submodule (or the main repository if submodule is NULL).
- *
- * For backwards compatibility, submodule=="" is treated the same as
- * submodule==NULL.
*/
static struct ref_store *ref_store_init(const char *submodule)
{
@@ -1396,10 +1390,7 @@ static struct ref_store *ref_store_init(const char *submodule)
if (!be)
die("BUG: reference backend %s is unknown", be_name);
- if (!submodule || !*submodule)
- return be->init(NULL);
- else
- return be->init(submodule);
+ return be->init(submodule);
}
struct ref_store *get_ref_store(const char *submodule)
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 4/9] register_ref_store(): new function
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
` (2 preceding siblings ...)
2017-02-10 11:16 ` [PATCH v2 3/9] refs: remove some unnecessary handling of submodule == "" Michael Haggerty
@ 2017-02-10 11:16 ` Michael Haggerty
2017-02-10 11:16 ` [PATCH v2 5/9] refs: store submodule ref stores in a hashmap Michael Haggerty
` (7 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
Move the responsibility for registering the ref_store for a submodule
from base_ref_store_init() to a new function, register_ref_store(). Call
the latter from ref_store_init().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 43 +++++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/refs.c b/refs.c
index 6348414..d7158b6 100644
--- a/refs.c
+++ b/refs.c
@@ -1379,6 +1379,29 @@ static struct ref_store *lookup_ref_store(const char *submodule)
}
/*
+ * Register the specified ref_store to be the one that should be used
+ * for submodule (or the main repository if submodule is NULL). It is
+ * a fatal error to call this function twice for the same submodule.
+ */
+static void register_ref_store(struct ref_store *refs, const char *submodule)
+{
+ if (!submodule) {
+ if (main_ref_store)
+ die("BUG: main_ref_store initialized twice");
+
+ refs->next = NULL;
+ main_ref_store = refs;
+ } else {
+ if (lookup_ref_store(submodule))
+ die("BUG: ref_store for submodule '%s' initialized twice",
+ submodule);
+
+ refs->next = submodule_ref_stores;
+ submodule_ref_stores = refs;
+ }
+}
+
+/*
* Create, record, and return a ref_store instance for the specified
* submodule (or the main repository if submodule is NULL).
*/
@@ -1386,11 +1409,14 @@ static struct ref_store *ref_store_init(const char *submodule)
{
const char *be_name = "files";
struct ref_storage_be *be = find_ref_storage_backend(be_name);
+ struct ref_store *refs;
if (!be)
die("BUG: reference backend %s is unknown", be_name);
- return be->init(submodule);
+ refs = be->init(submodule);
+ register_ref_store(refs, submodule);
+ return refs;
}
struct ref_store *get_ref_store(const char *submodule)
@@ -1423,22 +1449,11 @@ void base_ref_store_init(struct ref_store *refs,
const char *submodule)
{
refs->be = be;
- if (!submodule) {
- if (main_ref_store)
- die("BUG: main_ref_store initialized twice");
+ if (!submodule)
refs->submodule = "";
- refs->next = NULL;
- main_ref_store = refs;
- } else {
- if (lookup_ref_store(submodule))
- die("BUG: ref_store for submodule '%s' initialized twice",
- submodule);
-
+ else
refs->submodule = xstrdup(submodule);
- refs->next = submodule_ref_stores;
- submodule_ref_stores = refs;
- }
}
void assert_main_repository(struct ref_store *refs, const char *caller)
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 5/9] refs: store submodule ref stores in a hashmap
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
` (3 preceding siblings ...)
2017-02-10 11:16 ` [PATCH v2 4/9] register_ref_store(): new function Michael Haggerty
@ 2017-02-10 11:16 ` Michael Haggerty
2017-02-10 11:16 ` [PATCH v2 6/9] refs: push the submodule attribute down Michael Haggerty
` (6 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
Aside from scaling better, this means that the submodule name needn't be
stored in the ref_store instance anymore (which will be changed in a
moment). This, in turn, will help loosen the strict 1:1 relationship
between ref_stores and submodules.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 58 ++++++++++++++++++++++++++++++++++++++++------------
refs/refs-internal.h | 6 ------
2 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/refs.c b/refs.c
index d7158b6..5121c57 100644
--- a/refs.c
+++ b/refs.c
@@ -3,6 +3,7 @@
*/
#include "cache.h"
+#include "hashmap.h"
#include "lockfile.h"
#include "refs.h"
#include "refs/refs-internal.h"
@@ -1352,11 +1353,41 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,
return 0;
}
+struct submodule_hash_entry
+{
+ struct hashmap_entry ent; /* must be the first member! */
+
+ struct ref_store *refs;
+
+ /* NUL-terminated name of submodule: */
+ char submodule[FLEX_ARRAY];
+};
+
+static int submodule_hash_cmp(const void *entry, const void *entry_or_key,
+ const void *keydata)
+{
+ const struct submodule_hash_entry *e1 = entry, *e2 = entry_or_key;
+ const char *submodule = keydata ? keydata : e2->submodule;
+
+ return strcmp(e1->submodule, submodule);
+}
+
+static struct submodule_hash_entry *alloc_submodule_hash_entry(
+ const char *submodule, struct ref_store *refs)
+{
+ struct submodule_hash_entry *entry;
+
+ FLEX_ALLOC_STR(entry, submodule, submodule);
+ hashmap_entry_init(entry, strhash(submodule));
+ entry->refs = refs;
+ return entry;
+}
+
/* A pointer to the ref_store for the main repository: */
static struct ref_store *main_ref_store;
-/* A linked list of ref_stores for submodules: */
-static struct ref_store *submodule_ref_stores;
+/* A hashmap of ref_stores, stored by submodule name: */
+static struct hashmap submodule_ref_stores;
/*
* Return the ref_store instance for the specified submodule (or the
@@ -1365,17 +1396,18 @@ static struct ref_store *submodule_ref_stores;
*/
static struct ref_store *lookup_ref_store(const char *submodule)
{
- struct ref_store *refs;
+ struct submodule_hash_entry *entry;
if (!submodule)
return main_ref_store;
- for (refs = submodule_ref_stores; refs; refs = refs->next) {
- if (!strcmp(submodule, refs->submodule))
- return refs;
- }
+ if (!submodule_ref_stores.tablesize)
+ /* It's initialized on demand in register_ref_store(). */
+ return NULL;
- return NULL;
+ entry = hashmap_get_from_hash(&submodule_ref_stores,
+ strhash(submodule), submodule);
+ return entry ? entry->refs : NULL;
}
/*
@@ -1389,15 +1421,15 @@ static void register_ref_store(struct ref_store *refs, const char *submodule)
if (main_ref_store)
die("BUG: main_ref_store initialized twice");
- refs->next = NULL;
main_ref_store = refs;
} else {
- if (lookup_ref_store(submodule))
+ if (!submodule_ref_stores.tablesize)
+ hashmap_init(&submodule_ref_stores, submodule_hash_cmp, 0);
+
+ if (hashmap_put(&submodule_ref_stores,
+ alloc_submodule_hash_entry(submodule, refs)))
die("BUG: ref_store for submodule '%s' initialized twice",
submodule);
-
- refs->next = submodule_ref_stores;
- submodule_ref_stores = refs;
}
}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index d8a7eb1..07fd208 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -636,12 +636,6 @@ struct ref_store {
* reference store:
*/
const char *submodule;
-
- /*
- * Submodule reference store instances are stored in a linked
- * list using this pointer.
- */
- struct ref_store *next;
};
/*
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 6/9] refs: push the submodule attribute down
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
` (4 preceding siblings ...)
2017-02-10 11:16 ` [PATCH v2 5/9] refs: store submodule ref stores in a hashmap Michael Haggerty
@ 2017-02-10 11:16 ` Michael Haggerty
2017-02-10 11:16 ` [PATCH v2 7/9] base_ref_store_init(): remove submodule argument Michael Haggerty
` (5 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
Push the submodule attribute down from ref_store to files_ref_store.
This is another step towards loosening the 1:1 connection between
ref_stores and submodules.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 11 ----------
refs/files-backend.c | 61 ++++++++++++++++++++++++++++++++++++----------------
refs/refs-internal.h | 13 -----------
3 files changed, 43 insertions(+), 42 deletions(-)
diff --git a/refs.c b/refs.c
index 5121c57..07959ff 100644
--- a/refs.c
+++ b/refs.c
@@ -1481,17 +1481,6 @@ void base_ref_store_init(struct ref_store *refs,
const char *submodule)
{
refs->be = be;
-
- if (!submodule)
- refs->submodule = "";
- else
- refs->submodule = xstrdup(submodule);
-}
-
-void assert_main_repository(struct ref_store *refs, const char *caller)
-{
- if (*refs->submodule)
- die("BUG: %s called for a submodule", caller);
}
/* backend functions */
diff --git a/refs/files-backend.c b/refs/files-backend.c
index f902393..5e135a4 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -912,6 +912,14 @@ struct packed_ref_cache {
*/
struct files_ref_store {
struct ref_store base;
+
+ /*
+ * The name of the submodule represented by this object, or
+ * the empty string if it represents the main repository's
+ * reference store:
+ */
+ const char *submodule;
+
struct ref_entry *loose;
struct packed_ref_cache *packed;
};
@@ -974,10 +982,23 @@ static struct ref_store *files_ref_store_create(const char *submodule)
base_ref_store_init(ref_store, &refs_be_files, submodule);
+ refs->submodule = submodule ? xstrdup(submodule) : "";
+
return ref_store;
}
/*
+ * Die if refs is for a submodule (i.e., not for the main repository).
+ * caller is used in any necessary error messages.
+ */
+static void files_assert_main_repository(struct files_ref_store *refs,
+ const char *caller)
+{
+ if (*refs->submodule)
+ die("BUG: %s called for a submodule", caller);
+}
+
+/*
* Downcast ref_store to files_ref_store. Die if ref_store is not a
* files_ref_store. If submodule_allowed is not true, then also die if
* files_ref_store is for a submodule (i.e., not for the main
@@ -987,14 +1008,18 @@ static struct files_ref_store *files_downcast(
struct ref_store *ref_store, int submodule_allowed,
const char *caller)
{
+ struct files_ref_store *refs;
+
if (ref_store->be != &refs_be_files)
die("BUG: ref_store is type \"%s\" not \"files\" in %s",
ref_store->be->name, caller);
+ refs = (struct files_ref_store *)ref_store;
+
if (!submodule_allowed)
- assert_main_repository(ref_store, caller);
+ files_assert_main_repository(refs, caller);
- return (struct files_ref_store *)ref_store;
+ return refs;
}
/* The length of a peeled reference line in packed-refs, including EOL: */
@@ -1133,8 +1158,8 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref
{
char *packed_refs_file;
- if (*refs->base.submodule)
- packed_refs_file = git_pathdup_submodule(refs->base.submodule,
+ if (*refs->submodule)
+ packed_refs_file = git_pathdup_submodule(refs->submodule,
"packed-refs");
else
packed_refs_file = git_pathdup("packed-refs");
@@ -1203,8 +1228,8 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
size_t path_baselen;
int err = 0;
- if (*refs->base.submodule)
- err = strbuf_git_path_submodule(&path, refs->base.submodule, "%s", dirname);
+ if (*refs->submodule)
+ err = strbuf_git_path_submodule(&path, refs->submodule, "%s", dirname);
else
strbuf_git_path(&path, "%s", dirname);
path_baselen = path.len;
@@ -1244,10 +1269,10 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
} else {
int read_ok;
- if (*refs->base.submodule) {
+ if (*refs->submodule) {
hashclr(sha1);
flag = 0;
- read_ok = !resolve_gitlink_ref(refs->base.submodule,
+ read_ok = !resolve_gitlink_ref(refs->submodule,
refname.buf, sha1);
} else {
read_ok = !read_ref_full(refname.buf,
@@ -1358,8 +1383,8 @@ static int files_read_raw_ref(struct ref_store *ref_store,
*type = 0;
strbuf_reset(&sb_path);
- if (*refs->base.submodule)
- strbuf_git_path_submodule(&sb_path, refs->base.submodule, "%s", refname);
+ if (*refs->submodule)
+ strbuf_git_path_submodule(&sb_path, refs->submodule, "%s", refname);
else
strbuf_git_path(&sb_path, "%s", refname);
@@ -1540,7 +1565,7 @@ static int lock_raw_ref(struct files_ref_store *refs,
int ret = TRANSACTION_GENERIC_ERROR;
assert(err);
- assert_main_repository(&refs->base, "lock_raw_ref");
+ files_assert_main_repository(refs, "lock_raw_ref");
*type = 0;
@@ -2006,7 +2031,7 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,
int attempts_remaining = 3;
int resolved;
- assert_main_repository(&refs->base, "lock_ref_sha1_basic");
+ files_assert_main_repository(refs, "lock_ref_sha1_basic");
assert(err);
lock = xcalloc(1, sizeof(struct ref_lock));
@@ -2152,7 +2177,7 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
static int timeout_value = 1000;
struct packed_ref_cache *packed_ref_cache;
- assert_main_repository(&refs->base, "lock_packed_refs");
+ files_assert_main_repository(refs, "lock_packed_refs");
if (!timeout_configured) {
git_config_get_int("core.packedrefstimeout", &timeout_value);
@@ -2190,7 +2215,7 @@ static int commit_packed_refs(struct files_ref_store *refs)
int save_errno = 0;
FILE *out;
- assert_main_repository(&refs->base, "commit_packed_refs");
+ files_assert_main_repository(refs, "commit_packed_refs");
if (!packed_ref_cache->lock)
die("internal error: packed-refs not locked");
@@ -2223,7 +2248,7 @@ static void rollback_packed_refs(struct files_ref_store *refs)
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(refs);
- assert_main_repository(&refs->base, "rollback_packed_refs");
+ files_assert_main_repository(refs, "rollback_packed_refs");
if (!packed_ref_cache->lock)
die("internal error: packed-refs not locked");
@@ -2397,7 +2422,7 @@ static int repack_without_refs(struct files_ref_store *refs,
struct string_list_item *refname;
int ret, needs_repacking = 0, removed = 0;
- assert_main_repository(&refs->base, "repack_without_refs");
+ files_assert_main_repository(refs, "repack_without_refs");
assert(err);
/* Look for a packed ref */
@@ -2930,7 +2955,7 @@ static int commit_ref_update(struct files_ref_store *refs,
const unsigned char *sha1, const char *logmsg,
struct strbuf *err)
{
- assert_main_repository(&refs->base, "commit_ref_update");
+ files_assert_main_repository(refs, "commit_ref_update");
clear_loose_ref_cache(refs);
if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg, 0, err)) {
@@ -3560,7 +3585,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
int ret;
struct ref_lock *lock;
- assert_main_repository(&refs->base, "lock_ref_for_update");
+ files_assert_main_repository(refs, "lock_ref_for_update");
if ((update->flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1))
update->flags |= REF_DELETING;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 07fd208..008822d 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -629,13 +629,6 @@ extern struct ref_storage_be refs_be_files;
struct ref_store {
/* The backend describing this ref_store's storage scheme: */
const struct ref_storage_be *be;
-
- /*
- * The name of the submodule represented by this object, or
- * the empty string if it represents the main repository's
- * reference store:
- */
- const char *submodule;
};
/*
@@ -658,10 +651,4 @@ void base_ref_store_init(struct ref_store *refs,
*/
struct ref_store *get_ref_store(const char *submodule);
-/*
- * Die if refs is for a submodule (i.e., not for the main repository).
- * caller is used in any necessary error messages.
- */
-void assert_main_repository(struct ref_store *refs, const char *caller);
-
#endif /* REFS_REFS_INTERNAL_H */
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 7/9] base_ref_store_init(): remove submodule argument
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
` (5 preceding siblings ...)
2017-02-10 11:16 ` [PATCH v2 6/9] refs: push the submodule attribute down Michael Haggerty
@ 2017-02-10 11:16 ` Michael Haggerty
2017-02-10 11:16 ` [PATCH v2 8/9] files_ref_store::submodule: use NULL for the main repository Michael Haggerty
` (4 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
This is another step towards weakening the 1:1 relationship between
ref_stores and submodules.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 +--
refs/files-backend.c | 2 +-
refs/refs-internal.h | 7 +++----
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 07959ff..05af56b 100644
--- a/refs.c
+++ b/refs.c
@@ -1477,8 +1477,7 @@ struct ref_store *get_ref_store(const char *submodule)
}
void base_ref_store_init(struct ref_store *refs,
- const struct ref_storage_be *be,
- const char *submodule)
+ const struct ref_storage_be *be)
{
refs->be = be;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 5e135a4..c9d2d28 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -980,7 +980,7 @@ static struct ref_store *files_ref_store_create(const char *submodule)
struct files_ref_store *refs = xcalloc(1, sizeof(*refs));
struct ref_store *ref_store = (struct ref_store *)refs;
- base_ref_store_init(ref_store, &refs_be_files, submodule);
+ base_ref_store_init(ref_store, &refs_be_files);
refs->submodule = submodule ? xstrdup(submodule) : "";
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 008822d..793c850 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -632,12 +632,11 @@ struct ref_store {
};
/*
- * Fill in the generic part of refs for the specified submodule and
- * add it to our collection of reference stores.
+ * Fill in the generic part of refs and add it to our collection of
+ * reference stores.
*/
void base_ref_store_init(struct ref_store *refs,
- const struct ref_storage_be *be,
- const char *submodule);
+ const struct ref_storage_be *be);
/*
* Return the ref_store instance for the specified submodule. For the
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 8/9] files_ref_store::submodule: use NULL for the main repository
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
` (6 preceding siblings ...)
2017-02-10 11:16 ` [PATCH v2 7/9] base_ref_store_init(): remove submodule argument Michael Haggerty
@ 2017-02-10 11:16 ` Michael Haggerty
2017-02-10 11:16 ` [PATCH v2 9/9] read_loose_refs(): read refs using resolve_ref_recursively() Michael Haggerty
` (3 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
The old practice of storing the empty string in this member for the main
repository was a holdover from before 00eebe3 (refs: create a base class
"ref_store" for files_ref_store, 2016-09-04), when the submodule was
stored in a flex array at the end of `struct files_ref_store`. Storing
NULL for this case is more idiomatic and a tiny bit less code.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs/files-backend.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c9d2d28..4fe92f0 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -915,8 +915,8 @@ struct files_ref_store {
/*
* The name of the submodule represented by this object, or
- * the empty string if it represents the main repository's
- * reference store:
+ * NULL if it represents the main repository's reference
+ * store:
*/
const char *submodule;
@@ -982,7 +982,7 @@ static struct ref_store *files_ref_store_create(const char *submodule)
base_ref_store_init(ref_store, &refs_be_files);
- refs->submodule = submodule ? xstrdup(submodule) : "";
+ refs->submodule = xstrdup_or_null(submodule);
return ref_store;
}
@@ -994,7 +994,7 @@ static struct ref_store *files_ref_store_create(const char *submodule)
static void files_assert_main_repository(struct files_ref_store *refs,
const char *caller)
{
- if (*refs->submodule)
+ if (refs->submodule)
die("BUG: %s called for a submodule", caller);
}
@@ -1158,7 +1158,7 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref
{
char *packed_refs_file;
- if (*refs->submodule)
+ if (refs->submodule)
packed_refs_file = git_pathdup_submodule(refs->submodule,
"packed-refs");
else
@@ -1228,7 +1228,7 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
size_t path_baselen;
int err = 0;
- if (*refs->submodule)
+ if (refs->submodule)
err = strbuf_git_path_submodule(&path, refs->submodule, "%s", dirname);
else
strbuf_git_path(&path, "%s", dirname);
@@ -1269,7 +1269,7 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
} else {
int read_ok;
- if (*refs->submodule) {
+ if (refs->submodule) {
hashclr(sha1);
flag = 0;
read_ok = !resolve_gitlink_ref(refs->submodule,
@@ -1383,7 +1383,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
*type = 0;
strbuf_reset(&sb_path);
- if (*refs->submodule)
+ if (refs->submodule)
strbuf_git_path_submodule(&sb_path, refs->submodule, "%s", refname);
else
strbuf_git_path(&sb_path, "%s", refname);
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 9/9] read_loose_refs(): read refs using resolve_ref_recursively()
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
` (7 preceding siblings ...)
2017-02-10 11:16 ` [PATCH v2 8/9] files_ref_store::submodule: use NULL for the main repository Michael Haggerty
@ 2017-02-10 11:16 ` Michael Haggerty
2017-02-10 19:22 ` Junio C Hamano
2017-02-10 15:56 ` [PATCH v2 0/9] Store submodules in a hash, not a linked list Jeff King
` (2 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
There is no need to call read_ref_full() or resolve_gitlink_ref() from
read_loose_refs(), because we already have a ref_store object in hand.
So we can call resolve_ref_recursively() ourselves. Happily, this
unifies the code for the submodule vs. non-submodule cases.
This requires resolve_ref_recursively() to be exposed to the refs
subsystem, though not to non-refs code.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 50 +++++++++++++++++++++++++-------------------------
refs/files-backend.c | 18 ++++--------------
refs/refs-internal.h | 5 +++++
3 files changed, 34 insertions(+), 39 deletions(-)
diff --git a/refs.c b/refs.c
index 05af56b..f03dcf5 100644
--- a/refs.c
+++ b/refs.c
@@ -1230,10 +1230,10 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
}
/* This function needs to return a meaningful errno on failure */
-static const char *resolve_ref_recursively(struct ref_store *refs,
- const char *refname,
- int resolve_flags,
- unsigned char *sha1, int *flags)
+const char *resolve_ref_recursively(struct ref_store *refs,
+ const char *refname,
+ int resolve_flags,
+ unsigned char *sha1, int *flags)
{
static struct strbuf sb_refname = STRBUF_INIT;
int unused_flags;
@@ -1390,27 +1390,6 @@ static struct ref_store *main_ref_store;
static struct hashmap submodule_ref_stores;
/*
- * Return the ref_store instance for the specified submodule (or the
- * main repository if submodule is NULL). If that ref_store hasn't
- * been initialized yet, return NULL.
- */
-static struct ref_store *lookup_ref_store(const char *submodule)
-{
- struct submodule_hash_entry *entry;
-
- if (!submodule)
- return main_ref_store;
-
- if (!submodule_ref_stores.tablesize)
- /* It's initialized on demand in register_ref_store(). */
- return NULL;
-
- entry = hashmap_get_from_hash(&submodule_ref_stores,
- strhash(submodule), submodule);
- return entry ? entry->refs : NULL;
-}
-
-/*
* Register the specified ref_store to be the one that should be used
* for submodule (or the main repository if submodule is NULL). It is
* a fatal error to call this function twice for the same submodule.
@@ -1451,6 +1430,27 @@ static struct ref_store *ref_store_init(const char *submodule)
return refs;
}
+/*
+ * Return the ref_store instance for the specified submodule (or the
+ * main repository if submodule is NULL). If that ref_store hasn't
+ * been initialized yet, return NULL.
+ */
+static struct ref_store *lookup_ref_store(const char *submodule)
+{
+ struct submodule_hash_entry *entry;
+
+ if (!submodule)
+ return main_ref_store;
+
+ if (!submodule_ref_stores.tablesize)
+ /* It's initialized on demand in register_ref_store(). */
+ return NULL;
+
+ entry = hashmap_get_from_hash(&submodule_ref_stores,
+ strhash(submodule), submodule);
+ return entry ? entry->refs : NULL;
+}
+
struct ref_store *get_ref_store(const char *submodule)
{
struct ref_store *refs;
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 4fe92f0..cdb6b8f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1267,20 +1267,10 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
create_dir_entry(refs, refname.buf,
refname.len, 1));
} else {
- int read_ok;
-
- if (refs->submodule) {
- hashclr(sha1);
- flag = 0;
- read_ok = !resolve_gitlink_ref(refs->submodule,
- refname.buf, sha1);
- } else {
- read_ok = !read_ref_full(refname.buf,
- RESOLVE_REF_READING,
- sha1, &flag);
- }
-
- if (!read_ok) {
+ if (!resolve_ref_recursively(&refs->base,
+ refname.buf,
+ RESOLVE_REF_READING,
+ sha1, &flag)) {
hashclr(sha1);
flag |= REF_ISBROKEN;
} else if (is_null_sha1(sha1)) {
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 793c850..33adbf9 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -650,4 +650,9 @@ void base_ref_store_init(struct ref_store *refs,
*/
struct ref_store *get_ref_store(const char *submodule);
+const char *resolve_ref_recursively(struct ref_store *refs,
+ const char *refname,
+ int resolve_flags,
+ unsigned char *sha1, int *flags);
+
#endif /* REFS_REFS_INTERNAL_H */
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 9/9] read_loose_refs(): read refs using resolve_ref_recursively()
2017-02-10 11:16 ` [PATCH v2 9/9] read_loose_refs(): read refs using resolve_ref_recursively() Michael Haggerty
@ 2017-02-10 19:22 ` Junio C Hamano
2017-02-13 6:40 ` Michael Haggerty
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2017-02-10 19:22 UTC (permalink / raw)
To: Michael Haggerty
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git
Michael Haggerty <mhagger@alum.mit.edu> writes:
> There is no need to call read_ref_full() or resolve_gitlink_ref() from
> read_loose_refs(), because we already have a ref_store object in hand.
> So we can call resolve_ref_recursively() ourselves. Happily, this
> unifies the code for the submodule vs. non-submodule cases.
>
> This requires resolve_ref_recursively() to be exposed to the refs
> subsystem, though not to non-refs code.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs.c | 50 +++++++++++++++++++++++++-------------------------
> refs/files-backend.c | 18 ++++--------------
> refs/refs-internal.h | 5 +++++
> 3 files changed, 34 insertions(+), 39 deletions(-)
OK, but one thing puzzles me...
> @@ -1390,27 +1390,6 @@ static struct ref_store *main_ref_store;
> static struct hashmap submodule_ref_stores;
>
> /*
> - * Return the ref_store instance for the specified submodule (or the
> - * main repository if submodule is NULL). If that ref_store hasn't
> - * been initialized yet, return NULL.
> - */
> -static struct ref_store *lookup_ref_store(const char *submodule)
> -{
> - struct submodule_hash_entry *entry;
> -
> - if (!submodule)
> - return main_ref_store;
> -
> - if (!submodule_ref_stores.tablesize)
> - /* It's initialized on demand in register_ref_store(). */
> - return NULL;
> -
> - entry = hashmap_get_from_hash(&submodule_ref_stores,
> - strhash(submodule), submodule);
> - return entry ? entry->refs : NULL;
> -}
> -
> -/*
> * Register the specified ref_store to be the one that should be used
> * for submodule (or the main repository if submodule is NULL). It is
> * a fatal error to call this function twice for the same submodule.
> @@ -1451,6 +1430,27 @@ static struct ref_store *ref_store_init(const char *submodule)
> return refs;
> }
>
> +/*
> + * Return the ref_store instance for the specified submodule (or the
> + * main repository if submodule is NULL). If that ref_store hasn't
> + * been initialized yet, return NULL.
> + */
> +static struct ref_store *lookup_ref_store(const char *submodule)
> +{
> + struct submodule_hash_entry *entry;
> +
> + if (!submodule)
> + return main_ref_store;
> +
> + if (!submodule_ref_stores.tablesize)
> + /* It's initialized on demand in register_ref_store(). */
> + return NULL;
> +
> + entry = hashmap_get_from_hash(&submodule_ref_stores,
> + strhash(submodule), submodule);
> + return entry ? entry->refs : NULL;
> +}
> +
I somehow thought that we had an early "reorder the code" step to
avoid hunks like these? Am I missing some subtle changes made while
moving the function down?
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 9/9] read_loose_refs(): read refs using resolve_ref_recursively()
2017-02-10 19:22 ` Junio C Hamano
@ 2017-02-13 6:40 ` Michael Haggerty
2017-02-13 23:18 ` Junio C Hamano
0 siblings, 1 reply; 18+ messages in thread
From: Michael Haggerty @ 2017-02-13 6:40 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git
On 02/10/2017 08:22 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>> [...]
>
> OK, but one thing puzzles me...
>
>> @@ -1390,27 +1390,6 @@ static struct ref_store *main_ref_store;
>> static struct hashmap submodule_ref_stores;
>>
>> /*
>> - * Return the ref_store instance for the specified submodule (or the
>> - * main repository if submodule is NULL). If that ref_store hasn't
>> - * been initialized yet, return NULL.
>> - */
>> -static struct ref_store *lookup_ref_store(const char *submodule)
>> -{
>> - struct submodule_hash_entry *entry;
>> -
>> - if (!submodule)
>> - return main_ref_store;
>> -
>> - if (!submodule_ref_stores.tablesize)
>> - /* It's initialized on demand in register_ref_store(). */
>> - return NULL;
>> -
>> - entry = hashmap_get_from_hash(&submodule_ref_stores,
>> - strhash(submodule), submodule);
>> - return entry ? entry->refs : NULL;
>> -}
>> -
>> -/*
>> * Register the specified ref_store to be the one that should be used
>> * for submodule (or the main repository if submodule is NULL). It is
>> * a fatal error to call this function twice for the same submodule.
>> @@ -1451,6 +1430,27 @@ static struct ref_store *ref_store_init(const char *submodule)
>> return refs;
>> }
>>
>> +/*
>> + * Return the ref_store instance for the specified submodule (or the
>> + * main repository if submodule is NULL). If that ref_store hasn't
>> + * been initialized yet, return NULL.
>> + */
>> +static struct ref_store *lookup_ref_store(const char *submodule)
>> +{
>> + struct submodule_hash_entry *entry;
>> +
>> + if (!submodule)
>> + return main_ref_store;
>> +
>> + if (!submodule_ref_stores.tablesize)
>> + /* It's initialized on demand in register_ref_store(). */
>> + return NULL;
>> +
>> + entry = hashmap_get_from_hash(&submodule_ref_stores,
>> + strhash(submodule), submodule);
>> + return entry ? entry->refs : NULL;
>> +}
>> +
>
> I somehow thought that we had an early "reorder the code" step to
> avoid hunks like these? Am I missing some subtle changes made while
> moving the function down?
You are quite right; thanks for noticing. I forgot to un-move this
function when re-rolling. These two hunks can be discarded (the function
text is unchanged).
I pushed the fixed commit to branch `submodule-hash` in my fork [1]. If
you'd like me to send it to the mailing list again, please let me know.
Michael
[1] https://github.com/mhagger/git
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 9/9] read_loose_refs(): read refs using resolve_ref_recursively()
2017-02-13 6:40 ` Michael Haggerty
@ 2017-02-13 23:18 ` Junio C Hamano
0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2017-02-13 23:18 UTC (permalink / raw)
To: Michael Haggerty
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git
Michael Haggerty <mhagger@alum.mit.edu> writes:
> I pushed the fixed commit to branch `submodule-hash` in my fork [1]. If
> you'd like me to send it to the mailing list again, please let me know.
I was tempted to ask you to send it again, because fetching,
comparing and then cherry-picking is a lot more work than just
replacing one, but just to make sure my intuition about the
work required is correct, I did it anyway and yes, fetching,
comparing, cherry-picking and then amending the sign-off in was a
lot more work ;-)
So no need to resend. Thanks.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/9] Store submodules in a hash, not a linked list
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
` (8 preceding siblings ...)
2017-02-10 11:16 ` [PATCH v2 9/9] read_loose_refs(): read refs using resolve_ref_recursively() Michael Haggerty
@ 2017-02-10 15:56 ` Jeff King
2017-02-10 16:44 ` Stefan Beller
2017-02-10 19:23 ` Junio C Hamano
2017-02-13 3:09 ` David Turner
11 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2017-02-10 15:56 UTC (permalink / raw)
To: Michael Haggerty
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Stefan Beller, Johannes Schindelin, David Turner, git
On Fri, Feb 10, 2017 at 12:16:10PM +0100, Michael Haggerty wrote:
> This is v2 of the patch series, considerably reorganized but not that
> different codewise. Thanks to Stefan, Junio, and Peff for their
> feedback about v1 [1]. I think I have addressed all of your comments.
>
> Changes since v1:
I read through these and didn't didn't see any problems.
The reordering and new commits made sense to me.
-Peff
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/9] Store submodules in a hash, not a linked list
2017-02-10 15:56 ` [PATCH v2 0/9] Store submodules in a hash, not a linked list Jeff King
@ 2017-02-10 16:44 ` Stefan Beller
0 siblings, 0 replies; 18+ messages in thread
From: Stefan Beller @ 2017-02-10 16:44 UTC (permalink / raw)
To: Jeff King
Cc: Michael Haggerty, Junio C Hamano,
Nguyễn Thái Ngọc Duy, Johannes Schindelin,
David Turner, git@vger.kernel.org
On Fri, Feb 10, 2017 at 7:56 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Feb 10, 2017 at 12:16:10PM +0100, Michael Haggerty wrote:
>
>> This is v2 of the patch series, considerably reorganized but not that
>> different codewise. Thanks to Stefan, Junio, and Peff for their
>> feedback about v1 [1]. I think I have addressed all of your comments.
>>
>> Changes since v1:
>
> I read through these and didn't didn't see any problems.
>
> The reordering and new commits made sense to me.
Same here.
Stefan
>
> -Peff
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/9] Store submodules in a hash, not a linked list
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
` (9 preceding siblings ...)
2017-02-10 15:56 ` [PATCH v2 0/9] Store submodules in a hash, not a linked list Jeff King
@ 2017-02-10 19:23 ` Junio C Hamano
2017-02-13 3:09 ` David Turner
11 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2017-02-10 19:23 UTC (permalink / raw)
To: Michael Haggerty
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git
Michael Haggerty <mhagger@alum.mit.edu> writes:
> This is v2 of the patch series, considerably reorganized but not that
> different codewise.
Thanks. The way the series loses "!*submodule and !submodule are
the same thing, sometimes" is easier to follow when presented in
this order, at least to me.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 0/9] Store submodules in a hash, not a linked list
2017-02-10 11:16 [PATCH v2 0/9] Store submodules in a hash, not a linked list Michael Haggerty
` (10 preceding siblings ...)
2017-02-10 19:23 ` Junio C Hamano
@ 2017-02-13 3:09 ` David Turner
11 siblings, 0 replies; 18+ messages in thread
From: David Turner @ 2017-02-13 3:09 UTC (permalink / raw)
To: Michael Haggerty, Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, Jeff King, git
On Fri, 2017-02-10 at 12:16 +0100, Michael Haggerty wrote:
> This is v2 of the patch series, considerably reorganized but not that
> different codewise. Thanks to Stefan, Junio, and Peff for their
> feedback about v1 [1]. I think I have addressed all of your comments.
LGTM.
^ permalink raw reply [flat|nested] 18+ messages in thread