Re: [PATCH/POC 2/7] Add new environment variable $GIT_SUPER_DIR

2013-12-14 Thread Junio C Hamano
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

2013-12-13 Thread Junio C Hamano
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

2013-12-13 Thread Duy Nguyen
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

2013-12-11 Thread Nguyễn Thái Ngọc Duy
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