Re: [PATCH v3 10/25] Add new environment variable $GIT_COMMON_DIR

[Junio C Hamano <gitster@pobox.com>]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Note that logs/refs/.tmp-renamed-log is used to prepare new reflog
> entry and it's supposed to be on the same filesystem as the target
> reflog file. This is not guaranteed true for logs/HEAD when it's
> mapped to repos/xx/logs/HEAD because the user can put "repos"
> directory on different filesystem. Don't mess with .git unless you
> know what you're doing.

Hmph.  I am puzzled.

We use TMP_RENAMED_LOG in rename_ref() in this sequence:

 * First move refs/logs/$oldname to TMP_RENAMED_LOG;
 * Delete refs/$oldname;
 * Delete refs/$newname if exists;
 * Move TMP_RENAMED_LOG to refs/logs/$newname;
 * Create refs/$newname.

in rename_ref(), and TMP_RENAMED_LOG or the helper function
rename_tmp_log() that does the actual rename do not seem to be
called by any other codepath.

How would logs/HEAD get in the picture?  Specifically, I am not sure
if we ever allow renaming the HEAD (which typically is a symref but
could be detached) via rename_ref() at all.

> The redirection is done by git_path(), git_pathdup() and
> strbuf_git_path(). The selected list of paths goes to $GIT_COMMON_DIR,
> not the other way around in case a developer adds a new
> worktree-specific file and it's accidentally promoted to be shared
> across repositories (this includes unknown files added by third party
> commands)
>
> The list of known files that belong to $GIT_DIR are:
>
> ADD_EDIT.patch BISECT_ANCESTORS_OK BISECT_EXPECTED_REV BISECT_LOG
> BISECT_NAMES CHERRY_PICK_HEAD COMMIT_MSG FETCH_HEAD HEAD MERGE_HEAD
> MERGE_MODE MERGE_RR NOTES_EDITMSG NOTES_MERGE_WORKTREE ORIG_HEAD
> REVERT_HEAD SQUASH_MSG TAG_EDITMSG fast_import_crash_* logs/HEAD
> next-index-* rebase-apply rebase-merge rsync-refs-* sequencer/*
> shallow_*
>
> Path mapping is NOT done for git_path_submodule(). Multi-checkouts are
> not supported as submodules.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

Other than the "I do not understand why logs/HEAD is an issue", the
other aspect of the above design feels sound to me.

> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 02bbc08..2c4a055 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -773,6 +773,14 @@ Git so take care if using Cogito etc.
>  	an explicit repository directory set via 'GIT_DIR' or on the
>  	command line.
>  
> +'GIT_COMMON_DIR'::
> +	If this variable is set to a path, non-worktree files that are
> +	normally in $GIT_DIR will be taken from this path
> +	instead. Worktree-specific files such as HEAD or index are
> +	taken from $GIT_DIR. This variable has lower precedence than
> +	other path variables such as GIT_INDEX_FILE,
> +	GIT_OBJECT_DIRECTORY...
> +

We may want to add something (not necessarily with this commit) to
repository-layout and glossary to describe this new "officially
supported" way to implement what contrib/workdir wanted to do.

> diff --git a/path.c b/path.c
> index 0f8c3dc..2d757dc 100644
> --- a/path.c
> +++ b/path.c
> @@ -90,6 +90,32 @@ static void replace_dir(struct strbuf *buf, int len, const char *newdir)
>  		buf->buf[newlen] = '/';
>  }
>  
> +static void update_common_dir(struct strbuf *buf, int git_dir_len)
> +{
> +	const char *common_dir_list[] = {
> +		"branches", "hooks", "info", "logs", "lost-found", "modules",
> +		"objects", "refs", "remotes", "rr-cache", "svn",
> +		NULL
> +	};
> +	const char *common_top_file_list[] = {
> +		"config", "gc.pid", "packed-refs", "shallow", NULL
> +	};
> +	char *base = buf->buf + git_dir_len;
> +	const char **p;
> +	if (is_dir_file(base, "logs", "HEAD"))

Just a style, but could you have a blank line between the series of
decls and the first statement?  It would be easier to read that way.

> +		return;	/* keep this in $GIT_DIR */
> +	for (p = common_dir_list; *p; p++)
> +		if (dir_prefix(base, *p)) {
> +			replace_dir(buf, git_dir_len, get_git_common_dir());
> +			return;
> +		}
> +	for (p = common_top_file_list; *p; p++)
> +		if (!strcmp(base, *p)) {
> +			replace_dir(buf, git_dir_len, get_git_common_dir());
> +			return;
> +		}
> +}
> +
>  static void adjust_git_path(struct strbuf *buf, int git_dir_len)
>  {
>  	const char *base = buf->buf + git_dir_len;
> @@ -101,6 +127,8 @@ static void adjust_git_path(struct strbuf *buf, int git_dir_len)
>  			      get_index_file(), strlen(get_index_file()));
>  	else if (git_db_env && dir_prefix(base, "objects"))
>  		replace_dir(buf, git_dir_len + 7, get_object_directory());
> +	else if (git_common_dir_env)
> +		update_common_dir(buf, git_dir_len);
>  }
>  
>  static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)
> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
> index 1d29901..f9a77e4 100755
> --- a/t/t0060-path-utils.sh
> +++ b/t/t0060-path-utils.sh
> @@ -241,5 +241,20 @@ test_expect_success 'setup fake objects directory foo' 'mkdir foo'
>  test_git_path GIT_OBJECT_DIRECTORY=foo objects foo
>  test_git_path GIT_OBJECT_DIRECTORY=foo objects/foo foo/foo
>  test_git_path GIT_OBJECT_DIRECTORY=foo objects2 .git/objects2
> +test_expect_success 'setup common repository' 'git --git-dir=bar init'
> +test_git_path GIT_COMMON_DIR=bar index                    .git/index
> +test_git_path GIT_COMMON_DIR=bar HEAD                     .git/HEAD
> +test_git_path GIT_COMMON_DIR=bar logs/HEAD                .git/logs/HEAD
> +test_git_path GIT_COMMON_DIR=bar objects                  bar/objects
> +test_git_path GIT_COMMON_DIR=bar objects/bar              bar/objects/bar
> +test_git_path GIT_COMMON_DIR=bar info/exclude             bar/info/exclude
> +test_git_path GIT_COMMON_DIR=bar remotes/bar              bar/remotes/bar
> +test_git_path GIT_COMMON_DIR=bar branches/bar             bar/branches/bar
> +test_git_path GIT_COMMON_DIR=bar logs/refs/heads/master   bar/logs/refs/heads/master
> +test_git_path GIT_COMMON_DIR=bar refs/heads/master        bar/refs/heads/master
> +test_git_path GIT_COMMON_DIR=bar hooks/me                 bar/hooks/me
> +test_git_path GIT_COMMON_DIR=bar config                   bar/config
> +test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs
> +test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow
>  
>  test_done