threads / patch / 9263
Subject: [PATCH 0/8 REVISION2] work-tree cleanups
Johannes SchindelinHi,
the patch series I sent was almost as bogus as the original work-tree patch series. However, thanks to the comments, and a night's sleep, here comes a revised patch series which is not only readable (it was readable before, too), but which actually works as expected.
Not only does it pass the tests, but it also adds some tests of its own.
And yes, you can "GIT_DIR=/some/where git add <file>..." now. Even if you are not at your working tree's root.
Ciao, Dscho
Johannes Schindelin[PATCH 1/8] Add is_absolute_path() and make_absolute_path()
This patch adds convenience functions to work with absolute paths. The function is_absolute_path() should help the efforts to integrate the MinGW fork.
Note that make_absolute_path() returns a pointer to a static buffer.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> ---
Okay, gone is normalize_path(). In contrast, "make_absolute_path()" does the getcwd() && chdir() && getcwd() && chdir(back) mantra, to follow symlinks.
AFAICT, make_absolute_path() would be a Good Thing to use for the recent lockfile stuff.
Makefile | 2 +- cache.h | 5 ++++ path.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ t/t0000-basic.sh | 16 +++++++++++++ test-absolute-path.c | 11 +++++++++ 5 files changed, 95 insertions(+), 1 deletions(-) create mode 100644 test-absolute-path.c
Makefile, cache.h, path.c, t/t0000-basic.sh, test-absolute-path.c
diff --git a/Makefile b/Makefile index d8100ad..546e008 100644 --- a/Makefile +++ b/Makefile @@ -936,7 +936,7 @@ endif ### Testing rules -TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X +TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X test-absolute-path$X all:: $(TEST_PROGRAMS) diff --git a/cache.h b/cache.h index 53801b8..98af530 100644 --- a/cache.h +++ b/cache.h @@ -358,6 +358,11 @@ int git_config_perm(const char *var, const char *value); int adjust_shared_perm(const char *path); int safe_create_leading_directories(char *path); char *enter_repo(char *path, int strict); +static inline int is_absolute_path(const char *path) +{ + return path[0] == '/'; +} +const char *make_absolute_path(const char *path); /* Read and unpack a sha1 file into memory, write memory to a sha1 file */ extern int sha1_object_info(const unsigned char *, unsigned long *); diff --git a/path.c b/path.c index c4ce962..0f7012f 100644 --- a/path.c +++ b/path.c @@ -292,3 +292,65 @@ int adjust_shared_perm(const char *path) return -2; return 0; } + +/* We allow "recursive" symbolic links. Only within reason, though. */ +#define MAXDEPTH 5 + +const char *make_absolute_path(const char *path) +{ + static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1]; + char cwd[1024] = ""; + int buf_index = 1, len; + + int depth = MAXDEPTH; + char *last_elem = NULL; + struct stat st; + + if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) + die ("Too long path: %.*s", 60, path); + + while (depth--) { + if (stat(buf, &st) || !S_ISDIR(st.st_mode)) { + char *last_slash = strrchr(buf, '/'); + *last_slash = '\0'; + last_elem = xstrdup(last_slash + 1); + } + + if (*buf) { + if (!*cwd && getcwd(cwd, sizeof(cwd)) < 0) + die ("Could not get current working directory"); + + if (chdir(buf)) + die ("Could not switch to '%s'", buf); + } + if (getcwd(buf, PATH_MAX) < 0) + die ("Could not get current working directory"); + + if (last_elem) { + int len = strlen(buf); + if (len + strlen(last_elem) + 2 > PATH_MAX) + die ("Too long path name: '%s/%s'", + buf, last_elem); + buf[len] = '/'; + strcpy(buf + len + 1, last_elem); + free(last_elem); + last_elem = NULL; + } + + if (!lstat(buf, &st) && S_ISLNK(st.st_mode)) { + len = readlink(buf, next_buf, PATH_MAX); + if (len < 0) + die ("Invalid symlink: %s", buf); + next_buf[len] = '\0'; + buf = next_buf; + buf_index = 1 - buf_index; + next_buf = bufs[buf_index]; + } else + break; + } + + if (*cwd && chdir(cwd)) + die ("Could not change back to '%s'", cwd); + + return buf; +} diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index 4bba9c0..4e49d59 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -281,4 +281,20 @@ test_expect_success 'update-index D/F conflict' ' test $numpath0 = 1 ' +test_expect_success 'absolute path works as expected' ' + mkdir first && + ln -s ../.git first/.git && + mkdir second && + ln -s ../first second/other && + mkdir third && + dir="$(cd .git; pwd -P)" && + dir2=third/../second/other/.git && + test "$dir" = "$(test-absolute-path $dir2)" && + file="$dir"/index && + test "$file" = "$(test-absolute-path $dir2/index)" && + ln -s ../first/file .git/syml && + sym="$(cd first; pwd -P)"/file && + test "$sym" = "$(test-absolute-path $dir2/syml)" +' + test_done diff --git a/test-absolute-path.c b/test-absolute-path.c new file mode 100644 index 0000000..c959ea2 --- /dev/null +++ b/test-absolute-path.c @@ -0,0 +1,11 @@ +#include "cache.h" + +int main(int argc, char **argv) +{ + while (argc > 1) { + puts(make_absolute_path(argv[1])); + argc--; + argv++; + } + return 0; +}
-- 1.5.3.rc3.18.g49a1
Junio C HamanoRe: [PATCH 1/8] Add is_absolute_path() and make_absolute_path()
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> diff --git a/path.c b/path.c
> index c4ce962..0f7012f 100644
> --- a/path.c
> +++ b/path.c
> @@ -292,3 +292,65 @@ int adjust_shared_perm(const char *path)
> return -2;
> return 0;
> }
> +
> +/* We allow "recursive" symbolic links. Only within reason, though. */
> +#define MAXDEPTH 5
> +
> +const char *make_absolute_path(const char *path)
> +{
> + static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
> + char cwd[1024] = "";
> + int buf_index = 1, len;
> +
> + int depth = MAXDEPTH;
> + char *last_elem = NULL;
> + struct stat st;
> +
> + if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
> + die ("Too long path: %.*s", 60, path);
> +
> + while (depth--) {
> + if (stat(buf, &st) || !S_ISDIR(st.st_mode)) {
> + char *last_slash = strrchr(buf, '/');
> + *last_slash = '\0';
> + last_elem = xstrdup(last_slash + 1);What happens when incoming path is just "abc"? Does your test script checks that case?
Johannes SchindelinRe: [PATCH 1/8] Add is_absolute_path() and make_absolute_path()
Hi,
On Fri, 27 Jul 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > diff --git a/path.c b/path.c
> > index c4ce962..0f7012f 100644
> > --- a/path.c
> > +++ b/path.c
> > @@ -292,3 +292,65 @@ int adjust_shared_perm(const char *path)
> > return -2;
> > return 0;
> > }
> > +
> > +/* We allow "recursive" symbolic links. Only within reason, though. */
> > +#define MAXDEPTH 5
> > +
> > +const char *make_absolute_path(const char *path)
> > +{
> > + static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
> > + char cwd[1024] = "";
> > + int buf_index = 1, len;
> > +
> > + int depth = MAXDEPTH;
> > + char *last_elem = NULL;
> > + struct stat st;
> > +
> > + if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
> > + die ("Too long path: %.*s", 60, path);
> > +
> > + while (depth--) {
> > + if (stat(buf, &st) || !S_ISDIR(st.st_mode)) {
> > + char *last_slash = strrchr(buf, '/');
> > + *last_slash = '\0';
> > + last_elem = xstrdup(last_slash + 1);
>
> What happens when incoming path is just "abc"? Does your test
> script checks that case?No, and you already guessed it: there will be a segmentation fault.
Will fix.
Ciao, Dscho
Johannes Schindelin[PATCH 2/8] Add functions get_relative_cwd() and is_inside_dir()
The function get_relative_cwd() works just as getcwd(), only that it takes an absolute path as additional parameter, returning the prefix of the current working directory relative to the given path. If the cwd is no subdirectory of the given path, it returns NULL.
is_inside_dir() is just a trivial wrapper over get_relative_cwd().
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- This patch should be the same as I sent out earlier. Just to make sure that you get the correct version, I send it again.
dir.c | 30 ++++++++++++++++++++++++++++++ dir.h | 3 +++ 2 files changed, 33 insertions(+), 0 deletions(-)
dir.c, dir.h
diff --git a/dir.c b/dir.c index 8d8faf5..c16a004 100644 --- a/dir.c +++ b/dir.c @@ -642,3 +642,33 @@ file_exists(const char *f) struct stat sb; return stat(f, &sb) == 0; } + +char *get_relative_cwd(char *buffer, int size, const char *dir) +{ + char *cwd = buffer; + + if (!dir) + return 0; + + if (!getcwd(buffer, PATH_MAX)) + return 0; + + if (!is_absolute_path(dir)) + dir = make_absolute_path(dir); + + while (*dir && *dir == *cwd) { + dir++; + cwd++; + } + if (*dir) + return NULL; + if (*cwd == '/') + return cwd + 1; + return cwd; +} + +int is_inside_dir(const char *dir) +{ + char buffer[PATH_MAX]; + return get_relative_cwd(buffer, sizeof(buffer), dir) != NULL; +} diff --git a/dir.h b/dir.h index ec0e8ab..f55a87b 100644 --- a/dir.h +++ b/dir.h @@ -61,4 +61,7 @@ extern void add_exclude(const char *string, const char *base, extern int file_exists(const char *); extern struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len); +extern char *get_relative_cwd(char *buffer, int size, const char *dir); +extern int is_inside_dir(const char *dir); + #endif
-- 1.5.3.rc3.18.g49a1
Junio C HamanoRe: [PATCH 2/8] Add functions get_relative_cwd() and is_inside_dir()
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> +char *get_relative_cwd(char *buffer, int size, const char *dir)
> +{
> + char *cwd = buffer;
> +
> + if (!dir)
> + return 0;
> +
> + if (!getcwd(buffer, PATH_MAX))
> + return 0;Can size be less than PATH_MAX?
Johannes SchindelinRe: [PATCH 2/8] Add functions get_relative_cwd() and is_inside_dir()
Hi,
On Fri, 27 Jul 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > +char *get_relative_cwd(char *buffer, int size, const char *dir)
> > +{
> > + char *cwd = buffer;
> > +
> > + if (!dir)
> > + return 0;
> > +
> > + if (!getcwd(buffer, PATH_MAX))
> > + return 0;
>
> Can size be less than PATH_MAX?Thanks. Will fix.
Ciao, Dscho
Johannes Schindelin[PATCH 3/8] Clean up work-tree handling
The old version of work-tree support was an unholy mess, barely readable, and not to the point.
For example, why do you have to provide a worktree, when it is not used? As in "git status". Now it works.
Another riddle was: if you can have work trees inside the git dir, why are some programs complaining that they need a work tree?
IOW when inside repo.git/work, where GIT_DIR points to repo.git and GIT_WORK_TREE to work, and cwd is work, --is-inside-git-dir _must_ return true, because it is _in the git dir_, but scripts _must_ test for the right thing.
Also, GIT_DIR=../.git should behave the same as if no GIT_DIR was specified, unless there is a repository in the current working directory. It does now.
In related news, a long standing bug was fixed: when in .git/bla/x.git/, which is a bare repository, git formerly assumed ../.. to be the appropriate git dir. This problem was reported by Shawn Pearce to have caused much pain, where a colleague mistakenly ran "git init" in "/" a long time ago, and bare repositories just would not work.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Not only because of ohloh am I proud that in spite of removing more lines than I added, there were more comments added than removed...
builtin-rev-parse.c | 12 +-- cache.h | 2 + environment.c | 32 +++++-- setup.c | 270 ++++++++++++++++++++++----------------------------- 4 files changed, 143 insertions(+), 173 deletions(-)
builtin-rev-parse.c, cache.h, environment.c, setup.c
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c index 497903a..3f787a8 100644 --- a/builtin-rev-parse.c +++ b/builtin-rev-parse.c @@ -320,15 +320,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, "--show-cdup")) { - const char *pfx = prefix; - while (pfx) { - pfx = strchr(pfx, '/'); - if (pfx) { - pfx++; - printf("../"); - } - } - putchar('\n'); + const char *work_tree = get_git_work_tree(); + if (work_tree) + printf("%s\n", work_tree); continue; } if (!strcmp(arg, "--git-dir")) { diff --git a/cache.h b/cache.h index 98af530..c0cab34 100644 --- a/cache.h +++ b/cache.h @@ -208,12 +208,14 @@ enum object_type { extern int is_bare_repository_cfg; extern int is_bare_repository(void); 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 char *get_object_directory(void); extern char *get_refs_directory(void); extern char *get_index_file(void); extern char *get_graft_file(void); +extern const char *get_git_work_tree(void); #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES" diff --git a/environment.c b/environment.c index f83fb9e..cb112a4 100644 --- a/environment.c +++ b/environment.c @@ -35,6 +35,10 @@ int pager_in_use; int pager_use_color = 1; int auto_crlf = 0; /* 1: both ways, -1: only when adding git objects */ +/* This is set by setup_git_dir_gently() and/or git_default_config() */ +char *git_work_tree_cfg; +static const char *work_tree; + static const char *git_dir; static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file; @@ -62,15 +66,8 @@ static void setup_git_env(void) int is_bare_repository(void) { - const char *dir, *s; - if (0 <= is_bare_repository_cfg) - return is_bare_repository_cfg; - - dir = get_git_dir(); - if (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT)) - return 0; - s = strrchr(dir, '/'); - return !s || strcmp(s + 1, DEFAULT_GIT_DIR_ENVIRONMENT); + /* if core.bare is not 'false', let's see if there is a work tree */ + return is_bare_repository_cfg && !get_git_work_tree(); } const char *get_git_dir(void) @@ -80,6 +77,23 @@ const char *get_git_dir(void) return git_dir; } +const char *get_git_work_tree(void) +{ + static int initialized = 0; + if (!initialized) { + work_tree = getenv(GIT_WORK_TREE_ENVIRONMENT); + if (!work_tree) { + work_tree = git_work_tree_cfg; + if (work_tree && !is_absolute_path(work_tree)) + work_tree = xstrdup(git_path(work_tree)); + } + if (work_tree && !is_absolute_path(work_tree)) + work_tree = xstrdup(make_absolute_path(work_tree)); + initialized = 1; + } + return work_tree; +} + char *get_object_directory(void) { if (!git_object_dir) diff --git a/setup.c b/setup.c index 7b07144..0323d25 100644 --- a/setup.c +++ b/setup.c @@ -1,4 +1,8 @@ #include "cache.h" +#include "dir.h" + +static int inside_git_dir = -1; +static int inside_work_tree = -1; const char *prefix_path(const char *prefix, int len, const char *path) { @@ -170,100 +174,80 @@ static int is_git_directory(const char *suspect) return 1; } -static int inside_git_dir = -1; - int is_inside_git_dir(void) { - if (inside_git_dir >= 0) - return inside_git_dir; - die("BUG: is_inside_git_dir called before setup_git_directory"); + if (inside_git_dir < 0) + inside_git_dir = is_inside_dir(get_git_dir()); + return inside_git_dir; } -static int inside_work_tree = -1; - int is_inside_work_tree(void) { - if (inside_git_dir >= 0) - return inside_work_tree; - die("BUG: is_inside_work_tree called before setup_git_directory"); + if (inside_work_tree < 0) + inside_work_tree = is_inside_dir(get_git_work_tree()); + return inside_work_tree; } -static char *gitworktree_config; - -static int git_setup_config(const char *var, const char *value) +/* + * If no worktree was given, and we are outside of a default work tree, + * now is the time to set it. + * + * In other words, if the user calls git with something like + * + * git --git-dir=/some/where/else bla + * + * default to the current working directory for the work-tree root. + */ +const char* set_work_tree(const char *dir) { - if (!strcmp(var, "core.worktree")) { - if (gitworktree_config) - strlcpy(gitworktree_config, value, PATH_MAX); - return 0; + char dir_buffer[PATH_MAX]; + static char buffer[PATH_MAX + 1], *rel; + int len, postfix_len = strlen(DEFAULT_GIT_DIR_ENVIRONMENT); + + len = strlen(dir); + if (len > postfix_len + 1 && dir[len - postfix_len - 1] == '/' && + !strcmp(dir + len - postfix_len, + DEFAULT_GIT_DIR_ENVIRONMENT)) + dir = strncpy(dir_buffer, dir, len - postfix_len - 1); + + rel = get_relative_cwd(buffer, sizeof(buffer), dir); + if (rel && *rel) + strcat(rel, "/"); + else { + rel = NULL; + dir = getcwd(buffer, sizeof(buffer)); } - return git_default_config(var, value); + git_work_tree_cfg = xstrdup(dir); + inside_work_tree = 1; + + return rel; } +/* + * We cannot decide in this function whether we are in the work tree or + * not, since the config can only be read _after_ this function was called. + */ const char *setup_git_directory_gently(int *nongit_ok) { + const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT); static char cwd[PATH_MAX+1]; - char worktree[PATH_MAX+1], gitdir[PATH_MAX+1]; - const char *gitdirenv, *gitworktree; - int wt_rel_gitdir = 0; + const char *gitdirenv; + int len, offset; + /* + * If GIT_DIR is set explicitly, we're not going + * to do any discovery, but we still do repository + * validation. + */ gitdirenv = getenv(GIT_DIR_ENVIRONMENT); - if (!gitdirenv) { - int len, offset; - - if (!getcwd(cwd, sizeof(cwd)-1)) - die("Unable to read current working directory"); - - offset = len = strlen(cwd); - for (;;) { - if (is_git_directory(".git")) - break; - if (offset == 0) { - offset = -1; - break; - } - chdir(".."); - while (cwd[--offset] != '/') - ; /* do nothing */ - } - - if (offset >= 0) { - inside_work_tree = 1; - git_config(git_default_config); - if (offset == len) { - inside_git_dir = 0; - return NULL; - } - - cwd[len++] = '/'; - cwd[len] = '\0'; - inside_git_dir = !prefixcmp(cwd + offset + 1, ".git/"); - return cwd + offset + 1; - } - - if (chdir(cwd)) - die("Cannot come back to cwd"); - if (!is_git_directory(".")) { - if (nongit_ok) { - *nongit_ok = 1; - return NULL; - } - die("Not a git repository"); - } - setenv(GIT_DIR_ENVIRONMENT, cwd, 1); - gitdirenv = getenv(GIT_DIR_ENVIRONMENT); - if (!gitdirenv) - die("getenv after setenv failed"); - } - - if (PATH_MAX - 40 < strlen(gitdirenv)) { - if (nongit_ok) { - *nongit_ok = 1; + if (gitdirenv) { + if (PATH_MAX - 40 < strlen(gitdirenv)) + die("'$%s' too big", GIT_DIR_ENVIRONMENT); + if (is_git_directory(gitdirenv)) { + if (!work_tree_env) + return set_work_tree(gitdirenv); return NULL; } - die("$%s too big", GIT_DIR_ENVIRONMENT); - } - if (!is_git_directory(gitdirenv)) { if (nongit_ok) { *nongit_ok = 1; return NULL; @@ -273,92 +257,53 @@ const char *setup_git_directory_gently(int *nongit_ok) if (!getcwd(cwd, sizeof(cwd)-1)) die("Unable to read current working directory"); - if (chdir(gitdirenv)) { - if (nongit_ok) { - *nongit_ok = 1; - return NULL; - } - die("Cannot change directory to $%s '%s'", - GIT_DIR_ENVIRONMENT, gitdirenv); - } - if (!getcwd(gitdir, sizeof(gitdir)-1)) - die("Unable to read current working directory"); - if (chdir(cwd)) - die("Cannot come back to cwd"); /* - * In case there is a work tree we may change the directory, - * therefore make GIT_DIR an absolute path. + * Test in the following order (relative to the cwd): + * - .git/ + * - ./ (bare) + * - ../.git/ + * - ../ (bare) + * - ../../.git/ + * etc. */ - if (gitdirenv[0] != '/') { - setenv(GIT_DIR_ENVIRONMENT, gitdir, 1); - gitdirenv = getenv(GIT_DIR_ENVIRONMENT); - if (!gitdirenv) - die("getenv after setenv failed"); - if (PATH_MAX - 40 < strlen(gitdirenv)) { - if (nongit_ok) { - *nongit_ok = 1; - return NULL; - } - die("$%s too big after expansion to absolute path", - GIT_DIR_ENVIRONMENT); - } - } - - strcat(cwd, "/"); - strcat(gitdir, "/"); - inside_git_dir = !prefixcmp(cwd, gitdir); - - gitworktree = getenv(GIT_WORK_TREE_ENVIRONMENT); - if (!gitworktree) { - gitworktree_config = worktree; - worktree[0] = '\0'; - } - git_config(git_setup_config); - if (!gitworktree) { - gitworktree_config = NULL; - if (worktree[0]) - gitworktree = worktree; - if (gitworktree && gitworktree[0] != '/') - wt_rel_gitdir = 1; - } - - if (wt_rel_gitdir && chdir(gitdirenv)) - die("Cannot change directory to $%s '%s'", - GIT_DIR_ENVIRONMENT, gitdirenv); - if (gitworktree && chdir(gitworktree)) { - if (nongit_ok) { - if (wt_rel_gitdir && chdir(cwd)) - die("Cannot come back to cwd"); - *nongit_ok = 1; + offset = len = strlen(cwd); + for (;;) { + if (is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT)) + break; + if (is_git_directory(".")) { + inside_git_dir = 1; + if (!work_tree_env) + inside_work_tree = 0; + setenv(GIT_DIR_ENVIRONMENT, ".", 1); return NULL; } - if (wt_rel_gitdir) - die("Cannot change directory to working tree '%s'" - " from $%s", gitworktree, GIT_DIR_ENVIRONMENT); - else - die("Cannot change directory to working tree '%s'", - gitworktree); - } - if (!getcwd(worktree, sizeof(worktree)-1)) - die("Unable to read current working directory"); - strcat(worktree, "/"); - inside_work_tree = !prefixcmp(cwd, worktree); - - if (gitworktree && inside_work_tree && !prefixcmp(worktree, gitdir) && - strcmp(worktree, gitdir)) { - inside_git_dir = 0; + chdir(".."); + do { + if (!offset) { + if (nongit_ok) { + if (chdir(cwd)) + die("Cannot come back to cwd"); + *nongit_ok = 1; + return NULL; + } + die("Not a git repository"); + } + } while (cwd[--offset] != '/'); } - if (!inside_work_tree) { - if (chdir(cwd)) - die("Cannot come back to cwd"); + inside_git_dir = 0; + if (!work_tree_env) + inside_work_tree = 1; + git_work_tree_cfg = xstrndup(cwd, offset); + if (offset == len) return NULL; - } - if (!strcmp(cwd, worktree)) - return NULL; - return cwd+strlen(worktree); + /* Make "offset" point to past the '/', and add a '/' at the end */ + offset++; + cwd[len++] = '/'; + cwd[len] = 0; + return cwd + offset; } int git_config_perm(const char *var, const char *value) @@ -382,11 +327,17 @@ int git_config_perm(const char *var, const char *value) int check_repository_format_version(const char *var, const char *value) { - if (strcmp(var, "core.repositoryformatversion") == 0) - repository_format_version = git_config_int(var, value); + if (strcmp(var, "core.repositoryformatversion") == 0) + repository_format_version = git_config_int(var, value); else if (strcmp(var, "core.sharedrepository") == 0) shared_repository = git_config_perm(var, value); - return 0; + else if (strcmp(var, "core.worktree") == 0) { + if (git_work_tree_cfg) + free(git_work_tree_cfg); + git_work_tree_cfg = xstrdup(value); + inside_work_tree = -1; + } + return 0; } int check_repository_format(void) @@ -402,5 +353,14 @@ const char *setup_git_directory(void) { const char *retval = setup_git_directory_gently(NULL); check_repository_format(); + + /* If the work tree is not the default one, recompute prefix */ + if (inside_work_tree < 0) { + static char buffer[PATH_MAX + 1]; + char *rel = get_relative_cwd(buffer, PATH_MAX, + get_git_work_tree()); + return rel && *rel ? strcat(rel, "/") : NULL; + } + return retval; }
-- 1.5.3.rc3.18.g49a1
Junio C HamanoRe: [PATCH 3/8] Clean up work-tree handling
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Not only because of ohloh am I proud that in spite of removing > more lines than I added, there were more comments added than > removed...
> diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
> index 497903a..3f787a8 100644
> --- a/builtin-rev-parse.c
> +++ b/builtin-rev-parse.c
> @@ -320,15 +320,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> continue;
> }
> if (!strcmp(arg, "--show-cdup")) {
> - const char *pfx = prefix;
> - while (pfx) {
> - pfx = strchr(pfx, '/');
> - if (pfx) {
> - pfx++;
> - printf("../");
> - }
> - }
> - putchar('\n');
> + const char *work_tree = get_git_work_tree();
> + if (work_tree)
> + printf("%s\n", work_tree);
> continue;This changes semantics, I think.
It used to be relative "up" path when no funny work-tree stuff is used, but get_git_work_tree() now seems to return absolute, hence this option as well. If it introduces regression to existing callers is up to what the caller does to the resulting path, though. If it only is used to prefix other things (i.e. path="$(git rev-parse --show-cdup)$1"), the caller would be safe, but if the caller counted number of ../ in the return value to see how deep it is, or if the caller expected to see empty string in order to see if the process is at the toplevel, this change would become a regression.
> @@ -62,15 +66,8 @@ static void setup_git_env(void)
>
> int is_bare_repository(void)
> {
> + /* if core.bare is not 'false', let's see if there is a work tree */
> + return is_bare_repository_cfg && !get_git_work_tree();
> }I thought about making core.bare a tertiary, true/false/depends, but I think this makes more sense.
Johannes SchindelinRe: [PATCH 3/8] Clean up work-tree handling
Hi,
On Fri, 27 Jul 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Not only because of ohloh am I proud that in spite of removing
> > more lines than I added, there were more comments added than
> > removed...
>
> > diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
> > index 497903a..3f787a8 100644
> > --- a/builtin-rev-parse.c
> > +++ b/builtin-rev-parse.c
> > @@ -320,15 +320,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> > continue;
> > }
> > if (!strcmp(arg, "--show-cdup")) {
> > - const char *pfx = prefix;
> > - while (pfx) {
> > - pfx = strchr(pfx, '/');
> > - if (pfx) {
> > - pfx++;
> > - printf("../");
> > - }
> > - }
> > - putchar('\n');
> > + const char *work_tree = get_git_work_tree();
> > + if (work_tree)
> > + printf("%s\n", work_tree);
> > continue;
>
> This changes semantics, I think.
>
> It used to be relative "up" path when no funny work-tree stuff
> is used, but get_git_work_tree() now seems to return absolute,
> hence this option as well. If it introduces regression to
> existing callers is up to what the caller does to the resulting
> path, though. If it only is used to prefix other things
> (i.e. path="$(git rev-parse --show-cdup)$1"), the caller would
> be safe, but if the caller counted number of ../ in the return
> value to see how deep it is, or if the caller expected to see
> empty string in order to see if the process is at the toplevel,
> this change would become a regression.I am somewhat negative on keeping _that_ much backwards compatibility. Scripts which depend on show-cdup being a relative path _will_ be broken by work-tree. Is it worth it to detect those errors late?
> > @@ -62,15 +66,8 @@ static void setup_git_env(void)
> >
> > int is_bare_repository(void)
> > {
> > + /* if core.bare is not 'false', let's see if there is a work tree */
> > + return is_bare_repository_cfg && !get_git_work_tree();
> > }
>
> I thought about making core.bare a tertiary, true/false/depends,
> but I think this makes more sense.Actually, you made me think again, and I am more along those lines now:
return is_bare_repository_cfg >= 0 ? is_bare_repository_cfg : !get_git_work_tree();
and according patch to get_git_work_tree to return NULL if is_bare_repository_cfg == 1.
Ciao, Dscho
Junio C HamanoRe: [PATCH 3/8] Clean up work-tree handling
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> This changes semantics, I think. >> >> It used to be relative "up" path when no funny work-tree stuff >> is used, but get_git_work_tree() now seems to return absolute, >> hence this option as well. If it introduces regression to >> existing callers is up to what the caller does to the resulting >> path, though. If it only is used to prefix other things >> (i.e. path="$(git rev-parse --show-cdup)$1"), the caller would >> be safe, but if the caller counted number of ../ in the return >> value to see how deep it is, or if the caller expected to see >> empty string in order to see if the process is at the toplevel, >> this change would become a regression. > > I am somewhat negative on keeping _that_ much backwards compatibility. > Scripts which depend on show-cdup being a relative path _will_ be broken > by work-tree. Is it worth it to detect those errors late?
Well, one of the conditions to accept the worktree stuff was not to break anybody who never ever uses worktree. So if we can keep the UP-ness of cdup, it would be much better.
Johannes SchindelinRe: [PATCH 3/8] Clean up work-tree handling
Hi,
On Fri, 27 Jul 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > >> This changes semantics, I think. > >> > >> It used to be relative "up" path when no funny work-tree stuff > >> is used, but get_git_work_tree() now seems to return absolute, > >> hence this option as well. If it introduces regression to > >> existing callers is up to what the caller does to the resulting > >> path, though. If it only is used to prefix other things > >> (i.e. path="$(git rev-parse --show-cdup)$1"), the caller would > >> be safe, but if the caller counted number of ../ in the return > >> value to see how deep it is, or if the caller expected to see > >> empty string in order to see if the process is at the toplevel, > >> this change would become a regression. > > > > I am somewhat negative on keeping _that_ much backwards compatibility. > > Scripts which depend on show-cdup being a relative path _will_ be broken > > by work-tree. Is it worth it to detect those errors late? > > Well, one of the conditions to accept the worktree stuff was not > to break anybody who never ever uses worktree. So if we can > keep the UP-ness of cdup, it would be much better.
One could record if the work tree was changed from the default one, and do the old thing in that case.
But I really have to wonder what other use people concocted for "--show-cdup"? Potentially some directory-counting? But --show-prefix would be much better at that.
I'll try to flange something into the code to detect unchanged working tree, but that is rather ugly, so I'd prefer not to.
Ciao, Dscho
Junio C HamanoRe: [PATCH 3/8] Clean up work-tree handling
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Well, one of the conditions to accept the worktree stuff was not >> to break anybody who never ever uses worktree. So if we can >> keep the UP-ness of cdup, it would be much better. > > One could record if the work tree was changed from the default one, and > do the old thing in that case.
This really should not be something you have to bend over with special code to support. One thing I think people's scripts expect is that cdup is what the name suggests, "how much do I go *up* in order to go to the top of the working tree", and if the answer is "nothing", they can tell that they are already at the top of the tree.
Now, I think it is fair to say that if your worktree is somewhere totally unrelated to your cwd, no amount of going up will take you to the top. IOW, you have to come down after going up some levels. In such a case, it is easier to code the implementation of --show-cdup to give an absolute path.
But in that case you are not even in the working tree to begin with, aren't you? Does git need to support that? What should "prefix = setup_git_dir()" do and return, in order to come up with the string to prepend to paths relative to the current directory the users give us? I would say if your cwd is not inside your working tree, then --show-cdup and --show-prefix should error out, saying "Not in the working tree". These options are about translating paths relative to your cwd to relative to the top of the working tree, so if you are outside a working tree, they do not make much sense.
Also I think the implementation of --show-prefix needs to work in terms of relative relationship between cwd and the top already, so I am not sure where your aversion of keeping the current cdup behaviour is coming from.
I was hoping that --show-cdup is rarely used, but unfortunately cd_to_toplevel() in git-sh-setup.sh is not even the only in-tree user of --show-cdup; it checks if it is empty to avoid chdir() unnecessarily. There are others:
- emacs/git.el uses it as part of expand-file-name, so I think giving an absolute path back is Ok.
- contrib/fast-import/git-p4 gives it to os.chdir() after checking if the length is positive; this should be Ok although it would involve an unnecessary chdir() if the process is already at the top with this change.
- contrib/p4import/git-p4import.py: the same story as git-p4.
- git-svn.perl: the same story as above.
Johannes SchindelinRe: [PATCH 3/8] Clean up work-tree handling
Hi,
On Fri, 27 Jul 2007, Junio C Hamano wrote:
> Now, I think it is fair to say that if your worktree is somewhere > totally unrelated to your cwd, no amount of going up will take you to > the top. IOW, you have to come down after going up some levels. In > such a case, it is easier to code the implementation of --show-cdup to > give an absolute path. > > But in that case you are not even in the working tree to begin > with, aren't you? Does git need to support that?
I'd say yes.
It is utterly _inconvenient_ to have to cd to the working tree when you just want to check the status, for example. And git already knows about the work-tree!
But you got me convinced about the relative path: it is true that nobody who has not set core.worktree should be affected.
So I will do something like
if (!inside_work_tree()) {
puts(get_git_work_tree());
continue;
}
[do the old thing of outputting ../../[...]]In fact, I had this in an unpublished version of the patch, and decided that I could remove more lines without breaking the test suite.
Heck, I'll even add a test case to make sure that behavior is maintained. Okay?
Ciao, Dscho
P.S.: I'll be offline for a few hours, but then come back to finish it up.
Junio C HamanoRe: [PATCH 3/8] Clean up work-tree handling
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Now, I think it is fair to say that if your worktree is somewhere >> totally unrelated to your cwd, no amount of going up will take you to >> the top. IOW, you have to come down after going up some levels. In >> such a case, it is easier to code the implementation of --show-cdup to >> give an absolute path. >> >> But in that case you are not even in the working tree to begin >> with, aren't you? Does git need to support that? > > I'd say yes. > > It is utterly _inconvenient_ to have to cd to the working tree when you > just want to check the status, for example. And git already knows about > the work-tree!
That is a bit different issue. In such a case, I would agree it would still be sensible for "git diff" to show the whole tree diff, for example. In other words, my "does git need to support that" is not about "even when the location of a worktree is known, operations that require a work tree should fail when invoked from outside of it".
Think about what the sensible thing to do is for "git diff ." Within a work tree, it instructs git to behave as if the user issued the command from the toplevel with prefix as the argument. And scripts that want to mimick that behaviour would use --show-prefix to obtain the prefix string, and --show-cdup to obtain how to move to the toplevel.
Outside of a work tree, I think the only two semi-sensible behaviours exist. Either tell the user that we cannot understand what "." should mean in that context and error out (IOW, "Not inside a work tree"), or assume that the user meant "from the top". So in such a case, if we do not want to error out to make things more "convenient", one possibility would be:
* give empty as prefix;
* give absolute or cwd relative path to the work tree for cdup (it would not be a sequence of ../ anyway in this case).
I think this is in line with the traditional behaviour when GIT_DIR is explicitly given. We assume the cwd is the toplevel, and return empty prefix and empty cdup.
Having said that, I am not convinced that "assume toplevel outside of a work tree" is a win for the end users. If the command errors out with a message telling the user that relative path does not make sense from outside a work tree, the user would understand. If the command does not error out but always works relative to the toplevel without explanation, it might confuse the user more until the he realizes "assume toplevel outside of a work tree" is the rule that is applying to his case.
Johannes SchindelinRe: [PATCH 3/8] Clean up work-tree handling
Hi,
On Sat, 28 Jul 2007, Junio C Hamano wrote:
> Outside of a work tree, I think the only two semi-sensible behaviours > exist. Either tell the user that we cannot understand what "." should > mean in that context and error out (IOW, "Not inside a work tree"), or > assume that the user meant "from the top". So in such a case, if we do > not want to error out to make things more "convenient", one possibility > would be: > > * give empty as prefix; > > * give absolute or cwd relative path to the work tree for cdup > (it would not be a sequence of ../ anyway in this case).
I am happy that you agree with the way I implemented it...
> I think this is in line with the traditional behaviour when GIT_DIR is > explicitly given. We assume the cwd is the toplevel, and return empty > prefix and empty cdup.
Yes, I want that behaviour.
> Having said that, I am not convinced that "assume toplevel outside of a > work tree" is a win for the end users. If the command errors out with a > message telling the user that relative path does not make sense from > outside a work tree, the user would understand. If the command does not > error out but always works relative to the toplevel without explanation, > it might confuse the user more until the he realizes "assume toplevel > outside of a work tree" is the rule that is applying to his case.
There are two reasons against that...
- it is really convenient to be able to say "git add .vimrc" (think Martin Krafft's desired setup). Let's just put some uppercase warning at the side of --work-tree in git-init.txt to tell the user about the behaviour.
- the implementation of "you are not in the worktree" would not be elegant: "git log" should succeed, "git log dir/file" should not.
Ciao, Dscho
Johannes SchindelinRe: [PATCH 3/8] Clean up work-tree handling
Hi,
I still have a problem with what should happen if both "core.bare == true" and "core.worktree = /some/where/over/the/rainbow". Should it be bare, or should it have a working tree?
So here is what I plan to support so far:
--work-tree= overrides GIT_WORK_TREE, which overrides core.worktree, which overrides core.bare, which overrides GIT_DIR/.. when GIT_DIR ends in /.git, which overrides the directory in which .git/ was found.
Does that look okay?
Ciao, Dscho
Junio C HamanoRe: [PATCH 3/8] Clean up work-tree handling
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> I still have a problem with what should happen if both "core.bare == true" > and "core.worktree = /some/where/over/the/rainbow". Should it be bare, or > should it have a working tree?
They sound contradicting with each other to me. Isn't that a clear configuration error?
Johannes SchindelinRe: [PATCH 3/8] Clean up work-tree handling
Hi,
On Sun, 29 Jul 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > I still have a problem with what should happen if both "core.bare == true" > > and "core.worktree = /some/where/over/the/rainbow". Should it be bare, or > > should it have a working tree? > > They sound contradicting with each other to me. Isn't that a > clear configuration error?
Yes. But why not play nice?
Okay, the real reason I do not want to catch this error is because it complicates my code.
But really, why not say "worktree takes precedence"?
BTW I realised that callers of setup_git_directory_gently() might forget the check for the repository format version...
Ciao, Dscho
Johannes Schindelin[PATCH 4/8] Add set_git_dir() function
With the function set_git_dir() you can reset the path that will be used for git_path(), git_dir() and friends.
The responsibility to close files and throw away information from the old git_dir lies with the caller.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> ---
This is just ripped out from the branch--new-workdir topic I was working on, just that it has a shorter name now.
Ah, and just to be sure, it xstrdup()s the path.
cache.h | 1 + environment.c | 8 ++++++++ 2 files changed, 9 insertions(+), 0 deletions(-)
cache.h, environment.c
diff --git a/cache.h b/cache.h index c0cab34..36963f2 100644 --- a/cache.h +++ b/cache.h @@ -216,6 +216,7 @@ extern char *get_refs_directory(void); extern char *get_index_file(void); extern char *get_graft_file(void); extern const char *get_git_work_tree(void); +extern int set_git_dir(const char *path); #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES" diff --git a/environment.c b/environment.c index cb112a4..33c0f90 100644 --- a/environment.c +++ b/environment.c @@ -121,3 +121,11 @@ char *get_graft_file(void) setup_git_env(); return git_graft_file; } + +int set_git_dir(const char *path) +{ + if (setenv(GIT_DIR_ENVIRONMENT, xstrdup(path), 1)) + return error("Could not set GIT_DIR to '%s'", path); + setup_git_env(); + return 0; +}
-- 1.5.3.rc3.18.g49a1
Johannes Schindelin[PATCH 5/8] work-trees are allowed inside a git-dir
It is allowed to call
$ git --git-dir=../ --work-tree=. bla
when you really want to. In this case, you are both in the git directory and in the working tree.
The earlier handling of this situation was seriously bogus. For regular working tree operations, it checked if inside git dir. That makes no sense, of course, since the check should be for a work tree, and nothing else.
Fix that.
Coincidently, you can omit the "--work-tree" now, if your repository knows about the location already.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- builtin-ls-files.c | 8 +++++--- git-sh-setup.sh | 3 +-- git.c | 11 ++++++++--- 3 files changed, 14 insertions(+), 8 deletions(-)
builtin-ls-files.c, git-sh-setup.sh, git.c
diff --git a/builtin-ls-files.c b/builtin-ls-files.c index 61577ea..d36181a 100644 --- a/builtin-ls-files.c +++ b/builtin-ls-files.c @@ -469,9 +469,11 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix) break; } - if (require_work_tree && - (!is_inside_work_tree() || is_inside_git_dir())) - die("This operation must be run in a work tree"); + if (require_work_tree && !is_inside_work_tree()) { + const char *work_tree = get_git_work_tree(); + if (!work_tree || chdir(work_tree)) + die("This operation must be run in a work tree"); + } pathspec = get_pathspec(prefix, argv + i); diff --git a/git-sh-setup.sh b/git-sh-setup.sh index c51985e..7bef43f 100755 --- a/git-sh-setup.sh +++ b/git-sh-setup.sh @@ -59,8 +59,7 @@ cd_to_toplevel () { } require_work_tree () { - test $(git rev-parse --is-inside-work-tree) = true && - test $(git rev-parse --is-inside-git-dir) = false || + test $(git rev-parse --is-inside-work-tree) = true || die "fatal: $0 cannot be used without a working tree." } diff --git a/git.c b/git.c index a647f9c..2433355 100644 --- a/git.c +++ b/git.c @@ -272,9 +272,14 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv) prefix = setup_git_directory(); if (p->option & USE_PAGER) setup_pager(); - if ((p->option & NEED_WORK_TREE) && - (!is_inside_work_tree() || is_inside_git_dir())) - die("%s must be run in a work tree", p->cmd); + if (p->option & NEED_WORK_TREE) { + const char *work_tree = get_git_work_tree(); + const char *git_dir = get_git_dir(); + if (!is_absolute_path(git_dir)) + set_git_dir(make_absolute_path(git_dir)); + if (!work_tree || chdir(work_tree)) + die("%s must be run in a work tree", p->cmd); + } trace_argv_printf(argv, argc, "trace: built-in: git"); status = p->fn(argc, argv, prefix);
-- 1.5.3.rc3.18.g49a1
Junio C HamanoRe: [PATCH 5/8] work-trees are allowed inside a git-dir
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> It is allowed to call > > $ git --git-dir=../ --work-tree=. bla > > when you really want to. In this case, you are both in the git directory > and in the working tree. > > The earlier handling of this situation was seriously bogus. For regular > working tree operations, it checked if inside git dir. That makes no > sense, of course, since the check should be for a work tree, and nothing > else. > > Fix that.
I do not doubt this patch makes the above command line to work better, but I have to wonder how that layout is useful. Care to give a use case or two in the commit log message?
Johannes SchindelinRe: [PATCH 5/8] work-trees are allowed inside a git-dir
Hi,
On Fri, 27 Jul 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > It is allowed to call > > > > $ git --git-dir=../ --work-tree=. bla > > > > when you really want to. In this case, you are both in the git directory > > and in the working tree. > > > > The earlier handling of this situation was seriously bogus. For regular > > working tree operations, it checked if inside git dir. That makes no > > sense, of course, since the check should be for a work tree, and nothing > > else. > > > > Fix that. > > I do not doubt this patch makes the above command line to work > better, but I have to wonder how that layout is useful. Care to > give a use case or two in the commit log message?
In the commit log message? Better somewhere else. Only git developers read the commit message.
But yes, I can point to a use case. AFAIR Martin Krafft brought up the issue to track different components of the home directory in different repositories.
I have a similar scenario here, which does not involve a home directory, but rather a directory where I should not put anything into (I could, but if the admin was anything akin to competent, I could not).
There are files in that directory (and all of its subdirectories) of a certain type, which are the only ones which are human generated, and therefore precious. I like to add them, and inspect them, with
git --git-dir=$HOME/x.git add
and
git --git-dir=$HOME/x.git diff
Another similar scenario is a network drive on Losedows, where the locking always fails. So I do not _want_ a repo there, even if I _could_.
Ciao, Dscho
Junio C HamanoRe: [PATCH 5/8] work-trees are allowed inside a git-dir
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> >> > It is allowed to call >> > >> > $ git --git-dir=../ --work-tree=. bla >> > >> > when you really want to. In this case, you are both in the git directory >> > and in the working tree. > ... > There are files in that directory (and all of its subdirectories) of a > certain type, which are the only ones which are human generated, and > therefore precious. I like to add them, and inspect them, with > > git --git-dir=$HOME/x.git add > > and > > git --git-dir=$HOME/x.git diff
I understand the --git-dir=$HOME/x.git to keep track of something in $HOME/foo/bar example.
But that is not the issue you described in the original message. I was asking about this (which is the way I read your original message):
$ GIT_DIR=$HOME/x.git git init
$ mkdir $HOME/x.git/workroot
$ cd $HOME/x.git/workroot
$ git --git-dir=../ --work-tree=. That is, $HOME/x.git/ is the GIT_DIR that has HEAD, index and refs/, and you are keeping track of contents whose rootlevel is at $HOME/x.git/workroot
Johannes SchindelinRe: [PATCH 5/8] work-trees are allowed inside a git-dir
Hi,
On Fri, 27 Jul 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > >> > >> > It is allowed to call > >> > > >> > $ git --git-dir=../ --work-tree=. bla > >> > > >> > when you really want to. In this case, you are both in the git directory > >> > and in the working tree. > > ... > > There are files in that directory (and all of its subdirectories) of a > > certain type, which are the only ones which are human generated, and > > therefore precious. I like to add them, and inspect them, with > > > > git --git-dir=$HOME/x.git add > > > > and > > > > git --git-dir=$HOME/x.git diff > > I understand the --git-dir=$HOME/x.git to keep track of > something in $HOME/foo/bar example. > > But that is not the issue you described in the original message. > I was asking about this (which is the way I read your original > message): > > $ GIT_DIR=$HOME/x.git git init > $ mkdir $HOME/x.git/workroot > $ cd $HOME/x.git/workroot > $ git --git-dir=../ --work-tree=. > > That is, $HOME/x.git/ is the GIT_DIR that has HEAD, index and > refs/, and you are keeping track of contents whose rootlevel is > at $HOME/x.git/workroot
Ah! But I have a really nice use case for that, too. I track a .git/refs/exclude in one of my branches, because I do not want anybody else to have those excludes. They only apply to me.
Another example would be a temporary checkout+change+checkin to some branch that is not currently checked out in the default working tree. (I do that, too, and had to work around that by cloning with "-l -n -s") What I do there is to keep a checkout of some often-rsync'ed (actually wget'ed) state in the current working tree, automatically committing when upstream changes, and tracking upstream _releases_ in a different branch.
Ciao, Dscho
Johannes Schindelin[PATCH 6/8] init: use get_git_work_tree() instead of rolling our own
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> ---
This is no change in function, but in readability.
builtin-init-db.c | 48 +++++++++++------------------------------------- 1 files changed, 11 insertions(+), 37 deletions(-)
diff --git a/builtin-init-db.c b/builtin-init-db.c index 66ddaeb..0d9b1e0 100644 --- a/builtin-init-db.c +++ b/builtin-init-db.c @@ -174,36 +174,7 @@ static void copy_templates(const char *git_dir, int len, const char *template_di closedir(dir); } -/* - * Get the full path to the working tree specified in $GIT_WORK_TREE - * or NULL if no working tree is specified. - */ -static const char *get_work_tree(void) -{ - const char *git_work_tree; - char cwd[PATH_MAX]; - static char worktree[PATH_MAX]; - - git_work_tree = getenv(GIT_WORK_TREE_ENVIRONMENT); - if (!git_work_tree) - return NULL; - if (!getcwd(cwd, sizeof(cwd))) - die("Unable to read current working directory"); - if (chdir(git_work_tree)) - die("Cannot change directory to specified working tree '%s'", - git_work_tree); - if (git_work_tree[0] != '/') { - if (!getcwd(worktree, sizeof(worktree))) - die("Unable to read current working directory"); - git_work_tree = worktree; - } - if (chdir(cwd)) - die("Cannot come back to cwd"); - return git_work_tree; -} - -static int create_default_files(const char *git_dir, const char *git_work_tree, - const char *template_path) +static int create_default_files(const char *git_dir, const char *template_path) { unsigned len = strlen(git_dir); static char path[PATH_MAX]; @@ -282,16 +253,16 @@ static int create_default_files(const char *git_dir, const char *git_work_tree, } git_config_set("core.filemode", filemode ? "true" : "false"); - if (is_bare_repository() && !git_work_tree) { + if (is_bare_repository()) git_config_set("core.bare", "true"); - } else { + const char *work_tree = get_git_work_tree(); git_config_set("core.bare", "false"); /* allow template config file to override the default */ if (log_all_ref_updates == -1) git_config_set("core.logallrefupdates", "true"); - if (git_work_tree) - git_config_set("core.worktree", git_work_tree); + if (work_tree != git_work_tree_cfg) + git_config_set("core.worktree", work_tree); } return reinit; } @@ -308,7 +279,6 @@ static const char init_db_usage[] = int cmd_init_db(int argc, const char **argv, const char *prefix) { const char *git_dir; - const char *git_work_tree; const char *sha1_dir; const char *template_dir = NULL; char *path; @@ -329,7 +299,11 @@ int cmd_init_db(int argc, const char **argv, const char *prefix) usage(init_db_usage); } - git_work_tree = get_work_tree(); + git_work_tree_cfg = xcalloc(PATH_MAX, 1); + if (!getcwd(git_work_tree_cfg, PATH_MAX)) + die ("Cannot access current working directory."); + if (access(get_git_work_tree(), X_OK)) + die ("Cannot access work tree '%s'", get_git_work_tree()); /* * Set up the default .git directory contents @@ -346,7 +320,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix) */ check_repository_format(); - reinit = create_default_files(git_dir, git_work_tree, template_dir); + reinit = create_default_files(git_dir, template_dir); /* * And set up the object store.
-- 1.5.3.rc3.18.g49a1
Johannes Schindelin[PATCH 7/8] Make t1501 a little saner, and fix it
t1501 is still a PITA to debug, but less so than before. This patch fixes also the tests which tested for the wrong (old) work-tree behavior.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t1501-worktree.sh | 161 +++++++++++++++++++++++++++++---------------------- 1 files changed, 92 insertions(+), 69 deletions(-)
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh index aadeeab..fad9a9e 100755 --- a/t/t1501-worktree.sh +++ b/t/t1501-worktree.sh @@ -4,89 +4,112 @@ test_description='test separate work tree' . ./test-lib.sh test_rev_parse() { - name=$1 - shift - - test_expect_success "$name: is-bare-repository" \ - "test '$1' = \"\$(git rev-parse --is-bare-repository)\"" - shift - [ $# -eq 0 ] && return - - test_expect_success "$name: is-inside-git-dir" \ - "test '$1' = \"\$(git rev-parse --is-inside-git-dir)\"" - shift - [ $# -eq 0 ] && return - - test_expect_success "$name: is-inside-work-tree" \ - "test '$1' = \"\$(git rev-parse --is-inside-work-tree)\"" - shift - [ $# -eq 0 ] && return - - test_expect_success "$name: prefix" \ - "test '$1' = \"\$(git rev-parse --show-prefix)\"" - shift - [ $# -eq 0 ] && return + name="$1" + for option in --is-bare-repository --is-inside-git-dir \ + --is-inside-work-tree --show-prefix + do + shift + test_expect_success "$name: $option" \ + "test '$1' = \"\$(git rev-parse $option)\"" + test $# -eq 1 && return + done } -mkdir -p work/sub/dir || exit 1 -mv .git repo.git || exit 1 +# usage: set_repo <working directory> [<git dir> [<work tree> [env]]] +set_repo () { + cd "$1" + say "switching to $(pwd) with GIT_DIR $2" + + test -z "$2" || { + GIT_DIR="$2" + GIT_CONFIG="$2"/config + export GIT_DIR GIT_CONFIG + } + + test -z "$3" || + case "$4" in + env) + GIT_WORK_TREE="$3" + export GIT_WORK_TREE + git config core.workTree non-existent + ;; + *) + git config core.workTree "$3" + esac +} + +test_expect_success 'setup' ' + mkdir -p work/sub/dir && + mv .git repo.git +' say "core.worktree = relative path" -export GIT_DIR=repo.git -export GIT_CONFIG=$GIT_DIR/config -unset GIT_WORK_TREE -git config core.worktree ../work -test_rev_parse 'outside' false false false -cd work || exit 1 -export GIT_DIR=../repo.git -export GIT_CONFIG=$GIT_DIR/config -test_rev_parse 'inside' false false true '' -cd sub/dir || exit 1 -export GIT_DIR=../../../repo.git -export GIT_CONFIG=$GIT_DIR/config + +set_repo . repo.git ../work +test_rev_parse 'outside git dir' false false false + +set_repo work ../repo.git +test_rev_parse 'inside git dir' false false true '' + +set_repo sub/dir ../../../repo.git test_rev_parse 'subdirectory' false false true sub/dir/ -cd ../../.. || exit 1 +cd ../../.. say "core.worktree = absolute path" -export GIT_DIR=$(pwd)/repo.git -export GIT_CONFIG=$GIT_DIR/config -git config core.worktree "$(pwd)/work" -test_rev_parse 'outside' false false false -cd work || exit 1 -test_rev_parse 'inside' false false true '' -cd sub/dir || exit 1 +set_repo . $(pwd)/repo.git $(pwd)/work +test_rev_parse 'outside git dir' false false false + +set_repo work +test_rev_parse 'inside git dir' false false true '' + +set_repo sub/dir test_rev_parse 'subdirectory' false false true sub/dir/ -cd ../../.. || exit 1 +cd ../../.. say "GIT_WORK_TREE=relative path (override core.worktree)" -export GIT_DIR=$(pwd)/repo.git -export GIT_CONFIG=$GIT_DIR/config -git config core.worktree non-existent -export GIT_WORK_TREE=work -test_rev_parse 'outside' false false false -cd work || exit 1 -export GIT_WORK_TREE=. -test_rev_parse 'inside' false false true '' -cd sub/dir || exit 1 -export GIT_WORK_TREE=../.. + +set_repo . $(pwd)/repo.git work env +test_rev_parse 'outside git dir' false false false + +set_repo work "" . env +test_rev_parse 'inside git dir' false false true '' + +set_repo sub/dir "" ../../ env test_rev_parse 'subdirectory' false false true sub/dir/ cd ../../.. || exit 1 +say "GIT_WORK_TREE=absolute path, work tree below git dir" + mv work repo.git/work -say "GIT_WORK_TREE=absolute path, work tree below git dir" -export GIT_DIR=$(pwd)/repo.git -export GIT_CONFIG=$GIT_DIR/config -export GIT_WORK_TREE=$(pwd)/repo.git/work -test_rev_parse 'outside' false false false -cd repo.git || exit 1 -test_rev_parse 'in repo.git' false true false -cd objects || exit 1 -test_rev_parse 'in repo.git/objects' false true false -cd ../work || exit 1 -test_rev_parse 'in repo.git/work' false false true '' -cd sub/dir || exit 1 -test_rev_parse 'in repo.git/sub/dir' false false true sub/dir/ -cd ../../../.. || exit 1 +set_repo . $(pwd)/repo.git $(pwd)/repo.git/work env +test_rev_parse 'outside git dir' false false false + +set_repo repo.git +test_rev_parse 'in repo.git' false true false + +set_repo objects +test_rev_parse 'in repo.git/objects' false true false + +set_repo ../work +test_rev_parse 'in repo.git/work' false true true '' + +set_repo sub/dir +test_rev_parse 'in repo.git/sub/dir' false true true sub/dir/ +cd ../../../.. + +test_expect_success 'repo finds its work tree' ' + (cd repo.git && + : > work/sub/dir/untracked && + test sub/dir/untracked = "$(git ls-files --others)") +' + +test_expect_success 'repo finds its work tree from work tree, too' ' + (cd repo.git/work/sub/dir && + : > tracked && + git --git-dir=../../.. add tracked && + cd ../../.. && + test sub/dir/tracked = "$(git ls-files)") +' test_done
-- 1.5.3.rc3.18.g49a1
Johannes Schindelin[PATCH 8/8] Fix t1500 for sane work-tree behavior
When GIT_DIR=../.git, and no worktree is specified, it is reasonable to assume that the repository is not bare, that the work tree is ".." and that the prefix is the basename of the current directory.
This is the sane behavior.
t1500 tested for the old behavior, which was plain wrong. And this patch fixes it minimally.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Yeah, I broke down. But fixing t1501's style was painful enough.
t/t1500-rev-parse.sh | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh index ec49966..6e792b6 100755 --- a/t/t1500-rev-parse.sh +++ b/t/t1500-rev-parse.sh @@ -31,9 +31,9 @@ test_rev_parse() { test_rev_parse toplevel false false true '' cd .git || exit 1 -test_rev_parse .git/ false true true .git/ +test_rev_parse .git/ true true false '' cd objects || exit 1 -test_rev_parse .git/objects/ false true true .git/objects/ +test_rev_parse .git/objects/ true true false '' cd ../.. || exit 1 mkdir -p sub/dir || exit 1 @@ -42,7 +42,7 @@ test_rev_parse subdirectory false false true sub/dir/ cd ../.. || exit 1 git config core.bare true -test_rev_parse 'core.bare = true' true false true +test_rev_parse 'core.bare = true' false false true git config --unset core.bare test_rev_parse 'core.bare undefined' false false true @@ -53,13 +53,13 @@ export GIT_DIR=../.git export GIT_CONFIG="$GIT_DIR"/config git config core.bare false -test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true '' +test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true work/ git config core.bare true -test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false true '' +test_rev_parse 'GIT_DIR=../.git, core.bare = true' false false true work/ git config --unset core.bare -test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true '' +test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true work/ mv ../.git ../repo.git || exit 1 export GIT_DIR=../repo.git @@ -69,9 +69,9 @@ git config core.bare false test_rev_parse 'GIT_DIR=../repo.git, core.bare = false' false false true '' git config core.bare true -test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true false true '' +test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' false false true '' git config --unset core.bare -test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' true false true '' +test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' false false true '' test_done
-- 1.5.3.rc3.18.g49a1
Junio C HamanoRe: [PATCH 8/8] Fix t1500 for sane work-tree behavior
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> When GIT_DIR=../.git, and no worktree is specified, it is reasonable > to assume that the repository is not bare, that the work tree is ".." > and that the prefix is the basename of the current directory. > > This is the sane behavior.
That is a bit too strong blanket statement, while being weak on exact conditions, giving only one example.
It makes me wonder...
* When GIT_DIR=../../.git, and no worktree is specified, the
same holds true, with worktree is "../.."? (probably yes)* "GIT_DIR=../../foo/.git"? (I dunno)
* "GIT_DIR=../foo.git"? (probably not)
I am assuming that you meant something like this:
When no worktree is specified, and GIT_DIR (or --git-dir=) is
zero or more "../" followed by ".git" after stripping trailing
and/or redundant slashes, it is reasonable to assume that the
repository is not bare, and the work tree is the parent
directory of the GIT_DIR directory.but that requires guesswork if you give only one example and let the readers to guess.
Johannes SchindelinRe: [PATCH 8/8] Fix t1500 for sane work-tree behavior
Hi,
On Fri, 27 Jul 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > When GIT_DIR=../.git, and no worktree is specified, it is reasonable > > to assume that the repository is not bare, that the work tree is ".." > > and that the prefix is the basename of the current directory. > > > > This is the sane behavior. > > That is a bit too strong blanket statement, while being weak on > exact conditions, giving only one example.
Okay, let me defend it.
> It makes me wonder... > > * When GIT_DIR=../../.git, and no worktree is specified, the > same holds true, with worktree is "../.."? (probably yes)
You meant with "GIT_DIR=../.."? No. In that case, I'd assume a bare repository, and we're inside the git directory, and unless the user specified a working tree, assume that we have none.
> * "GIT_DIR=../../foo/.git"? (I dunno)
Unless ../../foo == .. no. When we're outside, we're outside.
> * "GIT_DIR=../foo.git"? (probably not)
Unless "$(basename "$(pwd)")" == foo.git, no.
> I am assuming that you meant something like this: > > When no worktree is specified, and GIT_DIR (or --git-dir=) is > zero or more "../" followed by ".git" after stripping trailing > and/or redundant slashes, it is reasonable to assume that the > repository is not bare, and the work tree is the parent > directory of the GIT_DIR directory. > > but that requires guesswork if you give only one example and let > the readers to guess.
Your explanation is really much more coherent than mine. Please replace mine.
Ciao, Dscho