Re: [PATCH v2 3/4] sha1_file.c: move delayed getenv(altdb) back to setup_git_env()
On Wed, Feb 28, 2018 at 3:12 AM, Brandon Williams wrote: > On 02/27, Nguyễn Thái Ngọc Duy wrote: >> diff --git a/repository.c b/repository.c >> index 7654b8ada9..e326f0fcbc 100644 >> --- a/repository.c >> +++ b/repository.c >> @@ -61,6 +61,8 @@ void repo_set_gitdir(struct repository *repo, >> repo_set_commondir(repo, o->shared_root); >> expand_base_dir(&repo->objects.objectdir, o->object_dir, >> repo->commondir, "objects"); >> + free(repo->objects.alternate_db); >> + repo->objects.alternate_db = xstrdup_or_null(o->alternate_db); > > Just a note that this only works because the object store is embedded in > the repository struct, it isn't a pointer to an object store. It is possible to make it work even if object store is not embedded though. From my point of view, this function is about "give me $GIT_DIR and optionally the where all other parts are, if you ignore default repo layout and tear the repository apart". We could initialize the object store later when it's created and pass the relevant paths to it. Exactly where it's safe to "create and initialize object store" is harder to see now because the whole main repo setup is spread out over many many functions. -- Duy
Re: [PATCH v2 3/4] sha1_file.c: move delayed getenv(altdb) back to setup_git_env()
On 02/27, Nguyễn Thái Ngọc Duy wrote:
> getenv() is supposed to work on the main repository only. This delayed
> getenv() code in sha1_file.c makes it more difficult to convert
> sha1_file.c to a generic object store that could be used by both
> submodule and main repositories.
>
> Move the getenv() back in setup_git_env() where other env vars are
> also fetched.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> environment.c | 1 +
> object-store.h | 5 -
> object.c | 1 +
> repository.c | 2 ++
> repository.h | 1 +
> sha1_file.c| 6 +-
> 6 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/environment.c b/environment.c
> index 74a2900ddf..47c6e31559 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -174,6 +174,7 @@ void setup_git_env(const char *git_dir)
> args.object_dir = getenv_safe(&to_free, DB_ENVIRONMENT);
> args.graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT);
> args.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);
> + args.alternate_db = getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT);
> repo_set_gitdir(the_repository, git_dir, &args);
> argv_array_clear(&to_free);
>
> diff --git a/object-store.h b/object-store.h
> index afe2f93459..9b1303549b 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -87,6 +87,9 @@ struct raw_object_store {
>*/
> char *objectdir;
>
> + /* Path to extra alternate object database if not NULL */
> + char *alternate_db;
> +
> struct packed_git *packed_git;
> /* A most-recently-used ordered version of the packed_git list. */
> struct list_head packed_git_mru;
> @@ -109,7 +112,7 @@ struct raw_object_store {
> unsigned packed_git_initialized : 1;
> };
>
> -#define RAW_OBJECT_STORE_INIT(o) { NULL, NULL,
> LIST_HEAD_INIT(o.packed_git_mru), NULL, NULL, 0, 0, 0 }
> +#define RAW_OBJECT_STORE_INIT(o) { NULL, NULL, NULL,
> LIST_HEAD_INIT(o.packed_git_mru), NULL, NULL, 0, 0, 0 }
>
> void raw_object_store_clear(struct raw_object_store *o);
>
> diff --git a/object.c b/object.c
> index a7c238339b..5317cfc390 100644
> --- a/object.c
> +++ b/object.c
> @@ -464,6 +464,7 @@ static void free_alt_odbs(struct raw_object_store *o)
> void raw_object_store_clear(struct raw_object_store *o)
> {
> FREE_AND_NULL(o->objectdir);
> + FREE_AND_NULL(o->alternate_db);
>
> free_alt_odbs(o);
> o->alt_odb_tail = NULL;
> diff --git a/repository.c b/repository.c
> index 7654b8ada9..e326f0fcbc 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -61,6 +61,8 @@ void repo_set_gitdir(struct repository *repo,
> repo_set_commondir(repo, o->shared_root);
> expand_base_dir(&repo->objects.objectdir, o->object_dir,
> repo->commondir, "objects");
> + free(repo->objects.alternate_db);
> + repo->objects.alternate_db = xstrdup_or_null(o->alternate_db);
Just a note that this only works because the object store is embedded in
the repository struct, it isn't a pointer to an object store.
Patch looks good.
> expand_base_dir(&repo->graft_file, o->graft_file,
> repo->commondir, "info/grafts");
> expand_base_dir(&repo->index_file, o->index_file,
> diff --git a/repository.h b/repository.h
> index b5b5d138aa..b1da2a6384 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -94,6 +94,7 @@ struct set_gitdir_args {
> const char *object_dir;
> const char *graft_file;
> const char *index_file;
> + const char *alternate_db;
> };
>
> extern void repo_set_gitdir(struct repository *repo,
> diff --git a/sha1_file.c b/sha1_file.c
> index dfc8deec38..ad1cd441e6 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -673,15 +673,11 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
>
> void prepare_alt_odb(struct repository *r)
> {
> - const char *alt;
> -
> if (r->objects.alt_odb_tail)
> return;
>
> - alt = getenv(ALTERNATE_DB_ENVIRONMENT);
> -
> r->objects.alt_odb_tail = &r->objects.alt_odb_list;
> - link_alt_odb_entries(r, alt, PATH_SEP, NULL, 0);
> + link_alt_odb_entries(r, r->objects.alternate_db, PATH_SEP, NULL, 0);
>
> read_info_alternates(r, r->objects.objectdir, 0);
> }
> --
> 2.16.1.435.g8f24da2e1a
>
--
Brandon Williams
[PATCH v2 3/4] sha1_file.c: move delayed getenv(altdb) back to setup_git_env()
getenv() is supposed to work on the main repository only. This delayed
getenv() code in sha1_file.c makes it more difficult to convert
sha1_file.c to a generic object store that could be used by both
submodule and main repositories.
Move the getenv() back in setup_git_env() where other env vars are
also fetched.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
environment.c | 1 +
object-store.h | 5 -
object.c | 1 +
repository.c | 2 ++
repository.h | 1 +
sha1_file.c| 6 +-
6 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/environment.c b/environment.c
index 74a2900ddf..47c6e31559 100644
--- a/environment.c
+++ b/environment.c
@@ -174,6 +174,7 @@ void setup_git_env(const char *git_dir)
args.object_dir = getenv_safe(&to_free, DB_ENVIRONMENT);
args.graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT);
args.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);
+ args.alternate_db = getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT);
repo_set_gitdir(the_repository, git_dir, &args);
argv_array_clear(&to_free);
diff --git a/object-store.h b/object-store.h
index afe2f93459..9b1303549b 100644
--- a/object-store.h
+++ b/object-store.h
@@ -87,6 +87,9 @@ struct raw_object_store {
*/
char *objectdir;
+ /* Path to extra alternate object database if not NULL */
+ char *alternate_db;
+
struct packed_git *packed_git;
/* A most-recently-used ordered version of the packed_git list. */
struct list_head packed_git_mru;
@@ -109,7 +112,7 @@ struct raw_object_store {
unsigned packed_git_initialized : 1;
};
-#define RAW_OBJECT_STORE_INIT(o) { NULL, NULL,
LIST_HEAD_INIT(o.packed_git_mru), NULL, NULL, 0, 0, 0 }
+#define RAW_OBJECT_STORE_INIT(o) { NULL, NULL, NULL,
LIST_HEAD_INIT(o.packed_git_mru), NULL, NULL, 0, 0, 0 }
void raw_object_store_clear(struct raw_object_store *o);
diff --git a/object.c b/object.c
index a7c238339b..5317cfc390 100644
--- a/object.c
+++ b/object.c
@@ -464,6 +464,7 @@ static void free_alt_odbs(struct raw_object_store *o)
void raw_object_store_clear(struct raw_object_store *o)
{
FREE_AND_NULL(o->objectdir);
+ FREE_AND_NULL(o->alternate_db);
free_alt_odbs(o);
o->alt_odb_tail = NULL;
diff --git a/repository.c b/repository.c
index 7654b8ada9..e326f0fcbc 100644
--- a/repository.c
+++ b/repository.c
@@ -61,6 +61,8 @@ void repo_set_gitdir(struct repository *repo,
repo_set_commondir(repo, o->shared_root);
expand_base_dir(&repo->objects.objectdir, o->object_dir,
repo->commondir, "objects");
+ free(repo->objects.alternate_db);
+ repo->objects.alternate_db = xstrdup_or_null(o->alternate_db);
expand_base_dir(&repo->graft_file, o->graft_file,
repo->commondir, "info/grafts");
expand_base_dir(&repo->index_file, o->index_file,
diff --git a/repository.h b/repository.h
index b5b5d138aa..b1da2a6384 100644
--- a/repository.h
+++ b/repository.h
@@ -94,6 +94,7 @@ struct set_gitdir_args {
const char *object_dir;
const char *graft_file;
const char *index_file;
+ const char *alternate_db;
};
extern void repo_set_gitdir(struct repository *repo,
diff --git a/sha1_file.c b/sha1_file.c
index dfc8deec38..ad1cd441e6 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -673,15 +673,11 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
void prepare_alt_odb(struct repository *r)
{
- const char *alt;
-
if (r->objects.alt_odb_tail)
return;
- alt = getenv(ALTERNATE_DB_ENVIRONMENT);
-
r->objects.alt_odb_tail = &r->objects.alt_odb_list;
- link_alt_odb_entries(r, alt, PATH_SEP, NULL, 0);
+ link_alt_odb_entries(r, r->objects.alternate_db, PATH_SEP, NULL, 0);
read_info_alternates(r, r->objects.objectdir, 0);
}
--
2.16.1.435.g8f24da2e1a
