Re: [PATCH/POC 2/7] Add new environment variable $GIT_SUPER_DIR
Duy Nguyen pclo...@gmail.com writes: The exception list could be equally long (most of them are *_HEAD). .git is also used for temporary files with mkstemp, but I think that's safe for sharing. What could break is when people add a new local *_HEAD and forget to update the exception list. Sensible; it may be worth having the above to describe why we choose to go that share only the selected ones route in one of the log message and/or in-code comment. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/POC 2/7] Add new environment variable $GIT_SUPER_DIR
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: This is the base for git-new-workdir integration. The git-new-workdir script creates a separate worktree that shares everything except worktree-related stuff. The sharing is eanbled by this new env variable. In the new worktree, both variables are set at the same time, GIT_DIR and GIT_SUPER_DIR. Shared paths are checked at adjust_git_path() and rewritten to use $GIT_SUPER_DIR instead of $GIT_DIR. Let's call this split-repo setup to distinguish from $GIT_DIR-only one. The ln -s is avoided because Windows probably does not have the support, and symlinks don't survive operations that delete the old file, then create a new one. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- cache.h | 2 ++ environment.c | 11 +++-- path.c| 10 t/t0060-path-utils.sh | 67 +++ 4 files changed, 88 insertions(+), 2 deletions(-) diff --git a/cache.h b/cache.h index cdafbd7..823582f 100644 --- a/cache.h +++ b/cache.h @@ -347,6 +347,7 @@ static inline enum object_type object_type(unsigned int mode) /* Double-check local_repo_env below if you add to this list. */ #define GIT_DIR_ENVIRONMENT GIT_DIR +#define GIT_SUPER_DIR_ENVIRONMENT GIT_SUPER_DIR #define GIT_NAMESPACE_ENVIRONMENT GIT_NAMESPACE #define GIT_WORK_TREE_ENVIRONMENT GIT_WORK_TREE #define GIT_PREFIX_ENVIRONMENT GIT_PREFIX @@ -399,6 +400,7 @@ extern int is_inside_git_dir(void); extern char *git_work_tree_cfg; extern int is_inside_work_tree(void); extern const char *get_git_dir(void); +extern const char *get_git_super_dir(void); extern int is_git_directory(const char *path); extern char *get_object_directory(void); extern char *get_index_file(void); diff --git a/environment.c b/environment.c index 1d74dde..d5ae7a3 100644 --- a/environment.c +++ b/environment.c @@ -79,7 +79,7 @@ static char *work_tree; static const char *namespace; static size_t namespace_len; -static const char *git_dir; +static const char *git_dir, *git_super_dir; static char *git_object_dir, *git_index_file, *git_graft_file; int git_db_env, git_index_env, git_graft_env; @@ -131,10 +131,12 @@ static void setup_git_env(void) git_dir = DEFAULT_GIT_DIR_ENVIRONMENT; gitfile = read_gitfile(git_dir); git_dir = xstrdup(gitfile ? gitfile : git_dir); + git_super_dir = getenv(GIT_SUPER_DIR_ENVIRONMENT); git_object_dir = getenv(DB_ENVIRONMENT); if (!git_object_dir) { git_object_dir = xmalloc(strlen(git_dir) + 9); - sprintf(git_object_dir, %s/objects, git_dir); + sprintf(git_object_dir, %s/objects, + git_super_dir ? git_super_dir : git_dir); } else git_db_env = 1; git_index_file = getenv(INDEX_ENVIRONMENT); @@ -167,6 +169,11 @@ const char *get_git_dir(void) return git_dir; } +const char *get_git_super_dir(void) +{ + return git_super_dir; +} + const char *get_git_namespace(void) { if (!namespace) diff --git a/path.c b/path.c index eda9176..86a7c15 100644 --- a/path.c +++ b/path.c @@ -75,6 +75,16 @@ static void adjust_git_path(char *buf, int git_dir_len) strcpy(buf, get_index_file()); else if (git_db_env dir_prefix(base, objects)) replace_dir(buf, git_dir_len + 7, get_object_directory()); + else if (get_git_super_dir()) { + if (dir_prefix(base, objects) || dir_prefix(base, info) || + dir_prefix(base, refs) || + (dir_prefix(base, logs) strcmp(base, logs/HEAD)) || + dir_prefix(base, rr-cache) || dir_prefix(base, hooks) || + dir_prefix(base, svn) || + !strcmp(base, config) || !strcmp(base, packed-refs) || + !strcmp(base, shallow)) + replace_dir(buf, git_dir_len, get_git_super_dir()); + } This is essentially the list of what are shared across workdirs, right? I wonder if it is a bad idea to make everything under .git of the borrowed repository shared by default, with selected exceptions. Granted, not sharing by default is definitely safer than blindly sharing by default, so that alone may be a good argument to use a set of known-to-be-safe-to-share paths, like this code does. Don't we want .git/branches and .git/remotes shared? After all, their moral equivalents from .git/config are shared with the code. The name super might need to be rethought, but getting the mechanism and the semantics right is the more important first step, and I think this is going in a good direction. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/POC 2/7] Add new environment variable $GIT_SUPER_DIR
On Sat, Dec 14, 2013 at 1:12 AM, Junio C Hamano gits...@pobox.com wrote: diff --git a/path.c b/path.c index eda9176..86a7c15 100644 --- a/path.c +++ b/path.c @@ -75,6 +75,16 @@ static void adjust_git_path(char *buf, int git_dir_len) strcpy(buf, get_index_file()); else if (git_db_env dir_prefix(base, objects)) replace_dir(buf, git_dir_len + 7, get_object_directory()); + else if (get_git_super_dir()) { + if (dir_prefix(base, objects) || dir_prefix(base, info) || + dir_prefix(base, refs) || + (dir_prefix(base, logs) strcmp(base, logs/HEAD)) || + dir_prefix(base, rr-cache) || dir_prefix(base, hooks) || + dir_prefix(base, svn) || + !strcmp(base, config) || !strcmp(base, packed-refs) || + !strcmp(base, shallow)) + replace_dir(buf, git_dir_len, get_git_super_dir()); + } This is essentially the list of what are shared across workdirs, right? I wonder if it is a bad idea to make everything under .git of the borrowed repository shared by default, with selected exceptions. Granted, not sharing by default is definitely safer than blindly sharing by default, so that alone may be a good argument to use a set of known-to-be-safe-to-share paths, like this code does. The exception list could be equally long (most of them are *_HEAD). .git is also used for temporary files with mkstemp, but I think that's safe for sharing. What could break is when people add a new local *_HEAD and forget to update the exception list. Don't we want .git/branches and .git/remotes shared? After all, their moral equivalents from .git/config are shared with the code. Yes. I missed them. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/POC 2/7] Add new environment variable $GIT_SUPER_DIR
This is the base for git-new-workdir integration. The git-new-workdir script creates a separate worktree that shares everything except worktree-related stuff. The sharing is eanbled by this new env variable. In the new worktree, both variables are set at the same time, GIT_DIR and GIT_SUPER_DIR. Shared paths are checked at adjust_git_path() and rewritten to use $GIT_SUPER_DIR instead of $GIT_DIR. Let's call this split-repo setup to distinguish from $GIT_DIR-only one. The ln -s is avoided because Windows probably does not have the support, and symlinks don't survive operations that delete the old file, then create a new one. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- cache.h | 2 ++ environment.c | 11 +++-- path.c| 10 t/t0060-path-utils.sh | 67 +++ 4 files changed, 88 insertions(+), 2 deletions(-) diff --git a/cache.h b/cache.h index cdafbd7..823582f 100644 --- a/cache.h +++ b/cache.h @@ -347,6 +347,7 @@ static inline enum object_type object_type(unsigned int mode) /* Double-check local_repo_env below if you add to this list. */ #define GIT_DIR_ENVIRONMENT GIT_DIR +#define GIT_SUPER_DIR_ENVIRONMENT GIT_SUPER_DIR #define GIT_NAMESPACE_ENVIRONMENT GIT_NAMESPACE #define GIT_WORK_TREE_ENVIRONMENT GIT_WORK_TREE #define GIT_PREFIX_ENVIRONMENT GIT_PREFIX @@ -399,6 +400,7 @@ extern int is_inside_git_dir(void); extern char *git_work_tree_cfg; extern int is_inside_work_tree(void); extern const char *get_git_dir(void); +extern const char *get_git_super_dir(void); extern int is_git_directory(const char *path); extern char *get_object_directory(void); extern char *get_index_file(void); diff --git a/environment.c b/environment.c index 1d74dde..d5ae7a3 100644 --- a/environment.c +++ b/environment.c @@ -79,7 +79,7 @@ static char *work_tree; static const char *namespace; static size_t namespace_len; -static const char *git_dir; +static const char *git_dir, *git_super_dir; static char *git_object_dir, *git_index_file, *git_graft_file; int git_db_env, git_index_env, git_graft_env; @@ -131,10 +131,12 @@ static void setup_git_env(void) git_dir = DEFAULT_GIT_DIR_ENVIRONMENT; gitfile = read_gitfile(git_dir); git_dir = xstrdup(gitfile ? gitfile : git_dir); + git_super_dir = getenv(GIT_SUPER_DIR_ENVIRONMENT); git_object_dir = getenv(DB_ENVIRONMENT); if (!git_object_dir) { git_object_dir = xmalloc(strlen(git_dir) + 9); - sprintf(git_object_dir, %s/objects, git_dir); + sprintf(git_object_dir, %s/objects, + git_super_dir ? git_super_dir : git_dir); } else git_db_env = 1; git_index_file = getenv(INDEX_ENVIRONMENT); @@ -167,6 +169,11 @@ const char *get_git_dir(void) return git_dir; } +const char *get_git_super_dir(void) +{ + return git_super_dir; +} + const char *get_git_namespace(void) { if (!namespace) diff --git a/path.c b/path.c index eda9176..86a7c15 100644 --- a/path.c +++ b/path.c @@ -75,6 +75,16 @@ static void adjust_git_path(char *buf, int git_dir_len) strcpy(buf, get_index_file()); else if (git_db_env dir_prefix(base, objects)) replace_dir(buf, git_dir_len + 7, get_object_directory()); + else if (get_git_super_dir()) { + if (dir_prefix(base, objects) || dir_prefix(base, info) || + dir_prefix(base, refs) || + (dir_prefix(base, logs) strcmp(base, logs/HEAD)) || + dir_prefix(base, rr-cache) || dir_prefix(base, hooks) || + dir_prefix(base, svn) || + !strcmp(base, config) || !strcmp(base, packed-refs) || + !strcmp(base, shallow)) + replace_dir(buf, git_dir_len, get_git_super_dir()); + } } static char *vsnpath(char *buf, size_t n, const char *fmt, va_list args) diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index 7ad2730..45f114c 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -271,4 +271,71 @@ test_expect_success 'git_path objects2' ' test_cmp expect actual ' +test_expect_success 'git_path super index' ' + GIT_SUPER_DIR=foo test-path-utils git_path index actual + echo .git/index expect + test_cmp expect actual +' + +test_expect_success 'git_path super HEAD' ' + GIT_SUPER_DIR=foo test-path-utils git_path HEAD actual + echo .git/HEAD expect + test_cmp expect actual +' + +test_expect_success 'git_path super objects/*' ' + GIT_SUPER_DIR=foo test-path-utils git_path objects/foo actual + echo foo/objects/foo expect + test_cmp expect actual +' + +test_expect_success 'git_path super info/*' ' + GIT_SUPER_DIR=foo test-path-utils git_path info/exclude actual + echo foo/info/exclude expect + test_cmp expect