Thread (103 messages) flat view 103 messages, 6 authors, 2018-02-28

Re: [PATCH 1/4] repository.c: move env-related setup code back to environment.c

From: Eric Sunshine <hidden>
Date: 2018-02-26 18:57:29

On Mon, Feb 26, 2018 at 5:30 AM, Nguyễn Thái Ngọc Duy [off-list ref] wrote:
quoted hunk ↗ jump to hunk
It does not make sense that generic repository code contains handling
of environment variables, which are specific for the main repository
only. Refactor repo_set_gitdir() function to take $GIT_DIR and
optionally _all_ other customizable paths. These optional paths can be
NULL and will be calculated according to the default directory layout.

Note that some dead functions are left behind to reduce diff
noise. They will be deleted in the next patch.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
diff --git a/environment.c b/environment.c
@@ -148,10 +148,17 @@ static char *expand_namespace(const char *raw_namespace)
-void setup_git_env(void)
+void setup_git_env(const char *git_dir)
 {
        const char *shallow_file;
        const char *replace_ref_base;
+       struct set_gitdir_args args = { NULL };
+
+       args.shared_root = getenv(GIT_COMMON_DIR_ENVIRONMENT);
+       args.object_dir = getenv(DB_ENVIRONMENT);
+       args.graft_file = getenv(GRAFT_ENVIRONMENT);
+       args.index_file = getenv(INDEX_ENVIRONMENT);
According to POSIX[1], the result of getenv() may be invalidated by
another call to getenv() (or setenv() or unsetenv() or putenv()).

[1]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html
quoted hunk ↗ jump to hunk
+       repo_set_gitdir(the_repository, git_dir, &args);
diff --git a/repository.c b/repository.c
@@ -61,15 +61,50 @@ static void repo_setup_env(struct repository *repo)
+void repo_set_gitdir(struct repository *repo,
+                    const char *root,
+                    const struct set_gitdir_args *o)
+{
+       const char *gitfile = read_gitfile(root);
+       char *old_gitdir = repo->gitdir;
+
+       repo->gitdir = xstrdup(gitfile ? gitfile : root);
        free(old_gitdir);
Can:

    char *old_gitdir = repo->gitdir;
    repo->gitdir = xstrdup(...);
    free(old_gitdir);

be simplified to:

    free(repo->gitdir);
    repo->gitdir = xstrdup(...);

?
+       repo_set_commondir(repo, o->shared_root);
The repo_set_gitdir() prototype (below) makes it seem as if the last
argument ('o', in this case) is optional, presumably by passing in
NULL, however, this code does not appear to be prepared to deal with
NULL.
quoted hunk ↗ jump to hunk
+       expand_base_dir(&repo->objects.objectdir, o->object_dir,
+                       repo->commondir, "objects");
+       expand_base_dir(&repo->graft_file, o->graft_file,
+                       repo->commondir, "info/grafts");
+       expand_base_dir(&repo->index_file, o->index_file,
+                       repo->gitdir, "index");
 }
diff --git a/repository.h b/repository.h
@@ -89,7 +89,16 @@ struct repository {
+struct set_gitdir_args {
+       [...]
+};
+
+extern void repo_set_gitdir(struct repository *repo,
+                           const char *root,
+                           const struct set_gitdir_args *optional);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help