{"thread":{"id":"9233","subject":"[PATCH 3/5] Clean up work-tree handling","startedAt":"2007-07-26T06:30:09Z","lastAt":"2007-07-27T19:32:27Z","messageCount":10,"participants":["Johannes Schindelin","Shawn O. Pearce","Junio C Hamano","Matthias Lederhofer"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"48651","messageId":"Pine.LNX.4.64.0707260729150.14781@racer.site","threadId":"9233","inReplyTo":null,"subject":"[PATCH 3/5] Clean up work-tree handling","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-26T06:30:09Z","receivedAt":"2007-07-26T06:30:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThe old version of work-tree support was an unholy mess, barely readable,\nand not to the point.\n\nFor example, why do you have to provide a worktree, when it is not used?\nAs in \"git status\".  Now it works.\n\nAnother riddle was: if you can have work trees inside the git dir, why\nare some programs complaining that they need a work tree?\n\nIOW when inside repo.git/work, where GIT_DIR points to repo.git\nand GIT_WORK_TREE to work, and cwd is work, --is-inside-git-dir _must_\nreturn true, because it is _in the git dir_, but scripts _must_ test\nfor the right thing.\n\nIn related news, there is a long standing bug fixed: when in\n.git/bla/x.git/, which is a bare repository, git formerly assumed\n../.. to be the appropriate git dir.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tOkay, this diffstat looks so good also because I refactored some\n\tuseful stuff out.  So it is only half cheating.\n\n\tHowever, from the diff you only can see how horrible the code\n\tlooked like.  The new version of setup_git_directory_gently()\n\tactually resembles the pre-work-tree version  very much,\n\twith the notable exception that it is tested for bare repo\n\tright after testing .git/, and only after both failed, continue\n\tto assume that we're in a subdirectory.\n\n cache.h       |    2 +\n environment.c |   31 +++++--\n git.c         |    2 +-\n setup.c       |  242 +++++++++++++++++++-------------------------------------\n 4 files changed, 107 insertions(+), 170 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex b242147..7c55a1d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -208,12 +208,14 @@ enum object_type {\n extern int is_bare_repository_cfg;\n extern int is_bare_repository(void);\n extern int is_inside_git_dir(void);\n+extern char *git_work_tree_cfg;\n extern int is_inside_work_tree(void);\n extern const char *get_git_dir(void);\n extern char *get_object_directory(void);\n extern char *get_refs_directory(void);\n extern char *get_index_file(void);\n extern char *get_graft_file(void);\n+extern const char *get_git_work_tree(void);\n \n #define ALTERNATE_DB_ENVIRONMENT \"GIT_ALTERNATE_OBJECT_DIRECTORIES\"\n \ndiff --git a/environment.c b/environment.c\nindex f83fb9e..fa177ac 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -35,6 +35,10 @@ int pager_in_use;\n int pager_use_color = 1;\n int auto_crlf = 0;\t/* 1: both ways, -1: only when adding git objects */\n \n+/* This is set by setup_git_dir_gently() and/or git_default_config() */\n+char *git_work_tree_cfg;\n+static const char *work_tree;\n+\n static const char *git_dir;\n static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;\n \n@@ -62,15 +66,7 @@ static void setup_git_env(void)\n \n int is_bare_repository(void)\n {\n-\tconst char *dir, *s;\n-\tif (0 <= is_bare_repository_cfg)\n-\t\treturn is_bare_repository_cfg;\n-\n-\tdir = get_git_dir();\n-\tif (!strcmp(dir, DEFAULT_GIT_DIR_ENVIRONMENT))\n-\t\treturn 0;\n-\ts = strrchr(dir, '/');\n-\treturn !s || strcmp(s + 1, DEFAULT_GIT_DIR_ENVIRONMENT);\n+\treturn !get_git_work_tree();\n }\n \n const char *get_git_dir(void)\n@@ -80,6 +76,23 @@ const char *get_git_dir(void)\n \treturn git_dir;\n }\n \n+const char *get_git_work_tree(void)\n+{\n+\tstatic int initialized = 0;\n+\tif (!initialized) {\n+\t\twork_tree = getenv(GIT_WORK_TREE_ENVIRONMENT);\n+\t\tif (!work_tree) {\n+\t\t\twork_tree = git_work_tree_cfg;\n+\t\t\tif (work_tree && !is_absolute_path(work_tree))\n+\t\t\twork_tree = git_path(work_tree);\n+\t\t}\n+\t\tif (work_tree && !is_absolute_path(work_tree))\n+\t\t\twork_tree = xstrdup(make_absolute_path(work_tree));\n+\t\tinitialized = 1;\n+\t}\n+\treturn work_tree;\n+}\n+\n char *get_object_directory(void)\n {\n \tif (!git_object_dir)\ndiff --git a/git.c b/git.c\nindex a647f9c..5df10d3 100644\n--- a/git.c\n+++ b/git.c\n@@ -75,7 +75,7 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)\n \t\t\t\t*envchanged = 1;\n \t\t} else if (!strcmp(cmd, \"--work-tree\")) {\n \t\t\tif (*argc < 2) {\n-\t\t\t\tfprintf(stderr, \"No directory given for --work-tree.\\n\" );\n+\t\t\t\terror(\"No directory given for --work-tree.\\n\");\n \t\t\t\tusage(git_usage_string);\n \t\t\t}\n \t\t\tsetenv(GIT_WORK_TREE_ENVIRONMENT, (*argv)[1], 1);\ndiff --git a/setup.c b/setup.c\nindex 7b07144..9c0f5f6 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -1,4 +1,8 @@\n #include \"cache.h\"\n+#include \"dir.h\"\n+\n+static int inside_git_dir = -1;\n+static int inside_work_tree = -1;\n \n const char *prefix_path(const char *prefix, int len, const char *path)\n {\n@@ -170,100 +174,41 @@ static int is_git_directory(const char *suspect)\n \treturn 1;\n }\n \n-static int inside_git_dir = -1;\n-\n int is_inside_git_dir(void)\n {\n-\tif (inside_git_dir >= 0)\n-\t\treturn inside_git_dir;\n-\tdie(\"BUG: is_inside_git_dir called before setup_git_directory\");\n+\tif (inside_git_dir < 0)\n+\t\tinside_git_dir = is_inside_dir(get_git_dir());\n+\treturn inside_git_dir;\n }\n \n-static int inside_work_tree = -1;\n-\n int is_inside_work_tree(void)\n {\n-\tif (inside_git_dir >= 0)\n-\t\treturn inside_work_tree;\n-\tdie(\"BUG: is_inside_work_tree called before setup_git_directory\");\n-}\n-\n-static char *gitworktree_config;\n-\n-static int git_setup_config(const char *var, const char *value)\n-{\n-\tif (!strcmp(var, \"core.worktree\")) {\n-\t\tif (gitworktree_config)\n-\t\t\tstrlcpy(gitworktree_config, value, PATH_MAX);\n-\t\treturn 0;\n-\t}\n-\treturn git_default_config(var, value);\n+\tif (inside_work_tree < 0)\n+\t\tinside_work_tree = is_inside_dir(get_git_work_tree());\n+\treturn inside_work_tree;\n }\n \n+/*\n+ * We cannot decide in this function whether we are in the work tree or\n+ * not, since the config can only be read _after_ this function was called.\n+ */\n const char *setup_git_directory_gently(int *nongit_ok)\n {\n \tstatic char cwd[PATH_MAX+1];\n-\tchar worktree[PATH_MAX+1], gitdir[PATH_MAX+1];\n-\tconst char *gitdirenv, *gitworktree;\n-\tint wt_rel_gitdir = 0;\n+\tconst char *gitdirenv;\n+\tint len, offset;\n \n+\t/*\n+\t * If GIT_DIR is set explicitly, we're not going\n+\t * to do any discovery, but we still do repository\n+\t * validation.\n+\t */\n \tgitdirenv = getenv(GIT_DIR_ENVIRONMENT);\n-\tif (!gitdirenv) {\n-\t\tint len, offset;\n-\n-\t\tif (!getcwd(cwd, sizeof(cwd)-1))\n-\t\t\tdie(\"Unable to read current working directory\");\n-\n-\t\toffset = len = strlen(cwd);\n-\t\tfor (;;) {\n-\t\t\tif (is_git_directory(\".git\"))\n-\t\t\t\tbreak;\n-\t\t\tif (offset == 0) {\n-\t\t\t\toffset = -1;\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t\tchdir(\"..\");\n-\t\t\twhile (cwd[--offset] != '/')\n-\t\t\t\t; /* do nothing */\n-\t\t}\n-\n-\t\tif (offset >= 0) {\n-\t\t\tinside_work_tree = 1;\n-\t\t\tgit_config(git_default_config);\n-\t\t\tif (offset == len) {\n-\t\t\t\tinside_git_dir = 0;\n-\t\t\t\treturn NULL;\n-\t\t\t}\n-\n-\t\t\tcwd[len++] = '/';\n-\t\t\tcwd[len] = '\\0';\n-\t\t\tinside_git_dir = !prefixcmp(cwd + offset + 1, \".git/\");\n-\t\t\treturn cwd + offset + 1;\n-\t\t}\n-\n-\t\tif (chdir(cwd))\n-\t\t\tdie(\"Cannot come back to cwd\");\n-\t\tif (!is_git_directory(\".\")) {\n-\t\t\tif (nongit_ok) {\n-\t\t\t\t*nongit_ok = 1;\n-\t\t\t\treturn NULL;\n-\t\t\t}\n-\t\t\tdie(\"Not a git repository\");\n-\t\t}\n-\t\tsetenv(GIT_DIR_ENVIRONMENT, cwd, 1);\n-\t\tgitdirenv = getenv(GIT_DIR_ENVIRONMENT);\n-\t\tif (!gitdirenv)\n-\t\t\tdie(\"getenv after setenv failed\");\n-\t}\n-\n-\tif (PATH_MAX - 40 < strlen(gitdirenv)) {\n-\t\tif (nongit_ok) {\n-\t\t\t*nongit_ok = 1;\n+\tif (gitdirenv) {\n+\t\tif (PATH_MAX - 40 < strlen(gitdirenv))\n+\t\t\tdie(\"'$%s' too big\", GIT_DIR_ENVIRONMENT);\n+\t\tif (is_git_directory(gitdirenv))\n \t\t\treturn NULL;\n-\t\t}\n-\t\tdie(\"$%s too big\", GIT_DIR_ENVIRONMENT);\n-\t}\n-\tif (!is_git_directory(gitdirenv)) {\n \t\tif (nongit_ok) {\n \t\t\t*nongit_ok = 1;\n \t\t\treturn NULL;\n@@ -273,92 +218,58 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \n \tif (!getcwd(cwd, sizeof(cwd)-1))\n \t\tdie(\"Unable to read current working directory\");\n-\tif (chdir(gitdirenv)) {\n-\t\tif (nongit_ok) {\n-\t\t\t*nongit_ok = 1;\n-\t\t\treturn NULL;\n-\t\t}\n-\t\tdie(\"Cannot change directory to $%s '%s'\",\n-\t\t\tGIT_DIR_ENVIRONMENT, gitdirenv);\n-\t}\n-\tif (!getcwd(gitdir, sizeof(gitdir)-1))\n-\t\tdie(\"Unable to read current working directory\");\n-\tif (chdir(cwd))\n-\t\tdie(\"Cannot come back to cwd\");\n \n \t/*\n-\t * In case there is a work tree we may change the directory,\n-\t * therefore make GIT_DIR an absolute path.\n+\t * Test in the following order (relative to the cwd):\n+\t * - .git/\n+\t * - . (bare)\n+\t * - ../.git/\n+\t * - ../../.git/\n+\t *   etc.\n \t */\n-\tif (gitdirenv[0] != '/') {\n-\t\tsetenv(GIT_DIR_ENVIRONMENT, gitdir, 1);\n-\t\tgitdirenv = getenv(GIT_DIR_ENVIRONMENT);\n-\t\tif (!gitdirenv)\n-\t\t\tdie(\"getenv after setenv failed\");\n-\t\tif (PATH_MAX - 40 < strlen(gitdirenv)) {\n-\t\t\tif (nongit_ok) {\n-\t\t\t\t*nongit_ok = 1;\n-\t\t\t\treturn NULL;\n-\t\t\t}\n-\t\t\tdie(\"$%s too big after expansion to absolute path\",\n-\t\t\t\tGIT_DIR_ENVIRONMENT);\n-\t\t}\n-\t}\n-\n-\tstrcat(cwd, \"/\");\n-\tstrcat(gitdir, \"/\");\n-\tinside_git_dir = !prefixcmp(cwd, gitdir);\n-\n-\tgitworktree = getenv(GIT_WORK_TREE_ENVIRONMENT);\n-\tif (!gitworktree) {\n-\t\tgitworktree_config = worktree;\n-\t\tworktree[0] = '\\0';\n-\t}\n-\tgit_config(git_setup_config);\n-\tif (!gitworktree) {\n-\t\tgitworktree_config = NULL;\n-\t\tif (worktree[0])\n-\t\t\tgitworktree = worktree;\n-\t\tif (gitworktree && gitworktree[0] != '/')\n-\t\t\twt_rel_gitdir = 1;\n+\toffset = len = strlen(cwd);\n+\tif (is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT)) {\n+\t\tinside_git_dir = 0;\n+\t\tgit_work_tree_cfg = xstrdup(cwd);\n+\t\treturn NULL;\n \t}\n-\n-\tif (wt_rel_gitdir && chdir(gitdirenv))\n-\t\tdie(\"Cannot change directory to $%s '%s'\",\n-\t\t\tGIT_DIR_ENVIRONMENT, gitdirenv);\n-\tif (gitworktree && chdir(gitworktree)) {\n-\t\tif (nongit_ok) {\n-\t\t\tif (wt_rel_gitdir && chdir(cwd))\n-\t\t\t\tdie(\"Cannot come back to cwd\");\n-\t\t\t*nongit_ok = 1;\n-\t\t\treturn NULL;\n-\t\t}\n-\t\tif (wt_rel_gitdir)\n-\t\t\tdie(\"Cannot change directory to working tree '%s'\"\n-\t\t\t\t\" from $%s\", gitworktree, GIT_DIR_ENVIRONMENT);\n-\t\telse\n-\t\t\tdie(\"Cannot change directory to working tree '%s'\",\n-\t\t\t\tgitworktree);\n+\tif (is_git_directory(\".\")) {\n+\t\tsetenv(GIT_DIR_ENVIRONMENT, \".\", 1);\n+\t\tinside_git_dir = 1;\n+\t\tinside_work_tree = 0;\n+\t\treturn NULL;\n \t}\n-\tif (!getcwd(worktree, sizeof(worktree)-1))\n-\t\tdie(\"Unable to read current working directory\");\n-\tstrcat(worktree, \"/\");\n-\tinside_work_tree = !prefixcmp(cwd, worktree);\n \n-\tif (gitworktree && inside_work_tree && !prefixcmp(worktree, gitdir) &&\n-\t    strcmp(worktree, gitdir)) {\n-\t\tinside_git_dir = 0;\n+\tchdir(\"..\");\n+\tfor (;;) {\n+\t\tif (is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))\n+\t\t\tbreak;\n+\t\tchdir(\"..\");\n+\t\tdo {\n+\t\t\tif (!offset) {\n+\t\t\t\tif (nongit_ok) {\n+\t\t\t\t\tif (chdir(cwd))\n+\t\t\t\t\t\tdie(\"Cannot come back to cwd\");\n+\t\t\t\t\t*nongit_ok = 1;\n+\t\t\t\t\treturn NULL;\n+\t\t\t\t}\n+\t\t\t\tdie(\"Not a git repository\");\n+\t\t\t}\n+\t\t} while (cwd[--offset] != '/');\n \t}\n \n-\tif (!inside_work_tree) {\n-\t\tif (chdir(cwd))\n-\t\t\tdie(\"Cannot come back to cwd\");\n+\tif (offset == len)\n \t\treturn NULL;\n-\t}\n \n-\tif (!strcmp(cwd, worktree))\n-\t\treturn NULL;\n-\treturn cwd+strlen(worktree);\n+\t/* Make \"offset\" point to past the '/', and add a '/' at the end */\n+\toffset++;\n+\tcwd[len++] = '/';\n+\tcwd[len] = 0;\n+\tinside_git_dir = !prefixcmp(cwd + offset,\n+\t\t\tDEFAULT_GIT_DIR_ENVIRONMENT \"/\");\n+\tif (!inside_git_dir)\n+\t\tgit_work_tree_cfg = xstrdup(cwd);\n+\treturn cwd + offset;\n }\n \n int git_config_perm(const char *var, const char *value)\n@@ -382,11 +293,16 @@ int git_config_perm(const char *var, const char *value)\n \n int check_repository_format_version(const char *var, const char *value)\n {\n-       if (strcmp(var, \"core.repositoryformatversion\") == 0)\n-               repository_format_version = git_config_int(var, value);\n+\tif (strcmp(var, \"core.repositoryformatversion\") == 0)\n+\t\trepository_format_version = git_config_int(var, value);\n \telse if (strcmp(var, \"core.sharedrepository\") == 0)\n \t\tshared_repository = git_config_perm(var, value);\n-       return 0;\n+\telse if (strcmp(var, \"core.worktree\") == 0) {\n+\t\tif (git_work_tree_cfg)\n+\t\t\tfree(git_work_tree_cfg);\n+\t\tgit_work_tree_cfg = xstrdup(value);\n+\t}\n+\treturn 0;\n }\n \n int check_repository_format(void)\n@@ -400,7 +316,13 @@ int check_repository_format(void)\n \n const char *setup_git_directory(void)\n {\n-\tconst char *retval = setup_git_directory_gently(NULL);\n+\tconst char *retval = setup_git_directory_gently(NULL), *work_tree;\n \tcheck_repository_format();\n+\twork_tree = get_git_work_tree();\n+\tif (work_tree) {\n+\t\tstatic char buffer[PATH_MAX + 1];\n+\t\tchar *rel = get_relative_cwd(buffer, PATH_MAX, work_tree);\n+\t\treturn rel && *rel ? strcat(normalize_path(rel), \"/\") : NULL;\n+\t}\n \treturn retval;\n }\n-- \n1.5.3.rc2.42.gda8d-dirty\n"},{"id":"48664","messageId":"20070726065650.GC18114@spearce.org","threadId":"9233","inReplyTo":"Pine.LNX.4.64.0707260729150.14781@racer.site","subject":"Re: [PATCH 3/5] Clean up work-tree handling","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-07-26T06:56:50Z","receivedAt":"2007-07-26T06:56:50Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> In related news, there is a long standing bug fixed: when in\n> .git/bla/x.git/, which is a bare repository, git formerly assumed\n> ../.. to be the appropriate git dir.\n\nHeh.  I spent about 10 minutes last week trying to understand why\ngit-new-workdir was failing horribly on one user's system.\n\nTurns out they once managed to do:\n\n  cd /\n  git init\n\na long, long, long time ago and just didn't notice it until now.  git\nkept finding /.git rather than the .git it was supposed to locate,\nwhich was a bare repository in $HOME/.storage-pool/dayjob.git.\n \n-- \nShawn.\n"},{"id":"48667","messageId":"7vmyxj8wza.fsf@assigned-by-dhcp.cox.net","threadId":"9233","inReplyTo":"Pine.LNX.4.64.0707260729150.14781@racer.site","subject":"Re: [PATCH 3/5] Clean up work-tree handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-26T07:06:49Z","receivedAt":"2007-07-26T07:06:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This seems to break t1020 and t1500, probably among other\nthings.\n"},{"id":"48704","messageId":"Pine.LNX.4.64.0707261459570.14781@racer.site","threadId":"9233","inReplyTo":"7vmyxj8wza.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/5] Clean up work-tree handling","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-26T14:01:21Z","receivedAt":"2007-07-26T14:01:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 26 Jul 2007, Junio C Hamano wrote:\n\n> This seems to break t1020 and t1500, probably among other\n> things.\n\nIt does, yes.  I only realised that when I woke up.\n\nWill fix.  In the meantime, you guys can discuss the merits of my patch \nseries.  Am I the only one who finds the new version much easier to read, \nmuch like the pre-work-tree version?\n\nCiao,\nDscho\n"},{"id":"48730","messageId":"7vzm1j6lkf.fsf@assigned-by-dhcp.cox.net","threadId":"9233","inReplyTo":"Pine.LNX.4.64.0707261459570.14781@racer.site","subject":"Re: [PATCH 3/5] Clean up work-tree handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-26T18:56:16Z","receivedAt":"2007-07-26T18:56:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> ...  Am I the only one who finds the new version much easier to read, \n> much like the pre-work-tree version?\n\nOh there is (modulo bugs to be worked on further) no question\nabout that.  The changes to environment.c alone would convince\nanybody, I would say.  ;-).\n"},{"id":"48743","messageId":"20070726220949.GA4420@moooo.ath.cx","threadId":"9233","inReplyTo":"Pine.LNX.4.64.0707260729150.14781@racer.site","subject":"Re: [PATCH 3/5] Clean up work-tree handling","fromName":"Matthias Lederhofer","fromEmail":"matled@gmx.net","sentAt":"2007-07-26T22:09:49Z","receivedAt":"2007-07-26T22:09:49Z","isPatch":true,"sender":{"key":"matled@gmx.net","avatar":null},"body":"I try to take a closer look at your changes tomorrow evening.  Here\nare just two short things I saw while taking a short look at the\npatch.\n\nJohannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> +const char *get_git_work_tree(void)\n> +{\n> +\tstatic int initialized = 0;\n> +\tif (!initialized) {\n> +\t\twork_tree = getenv(GIT_WORK_TREE_ENVIRONMENT);\n> +\t\tif (!work_tree) {\n> +\t\t\twork_tree = git_work_tree_cfg;\n> +\t\t\tif (work_tree && !is_absolute_path(work_tree))\n> +\t\t\twork_tree = git_path(work_tree);\n\nA tab is missing here.\n\n> -\t\t\t\tfprintf(stderr, \"No directory given for --work-tree.\\n\" );\n> +\t\t\t\terror(\"No directory given for --work-tree.\\n\");\n\nThere should probably be no '\\n' at the end when the 'error' function\nis used.  There are two other calls to fprintf(stderr, <error message>)\nnext to the one you changed, why did you change this one but not the\nother ones?\n"},{"id":"48745","messageId":"7vfy3a7q5p.fsf@assigned-by-dhcp.cox.net","threadId":"9233","inReplyTo":"20070726220949.GA4420@moooo.ath.cx","subject":"Re: [PATCH 3/5] Clean up work-tree handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-26T22:31:46Z","receivedAt":"2007-07-26T22:31:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthias Lederhofer <matled@gmx.net> writes:\n\n> I try to take a closer look at your changes tomorrow evening.  Here\n> are just two short things I saw while taking a short look at the\n> patch.\n>\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n>> +const char *get_git_work_tree(void)\n>> +{\n>> +\tstatic int initialized = 0;\n>> +\tif (!initialized) {\n>> +\t\twork_tree = getenv(GIT_WORK_TREE_ENVIRONMENT);\n>> +\t\tif (!work_tree) {\n>> +\t\t\twork_tree = git_work_tree_cfg;\n>> +\t\t\tif (work_tree && !is_absolute_path(work_tree))\n>> +\t\t\twork_tree = git_path(work_tree);\n>\n> A tab is missing here.\n\nI think xstrdup() is missing here, too.\n"},{"id":"48786","messageId":"Pine.LNX.4.64.0707271146290.14781@racer.site","threadId":"9233","inReplyTo":"20070726220949.GA4420@moooo.ath.cx","subject":"Re: [PATCH 3/5] Clean up work-tree handling","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-27T10:50:57Z","receivedAt":"2007-07-27T10:50:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 27 Jul 2007, Matthias Lederhofer wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> > +const char *get_git_work_tree(void)\n> > +{\n> > +\tstatic int initialized = 0;\n> > +\tif (!initialized) {\n> > +\t\twork_tree = getenv(GIT_WORK_TREE_ENVIRONMENT);\n> > +\t\tif (!work_tree) {\n> > +\t\t\twork_tree = git_work_tree_cfg;\n> > +\t\t\tif (work_tree && !is_absolute_path(work_tree))\n> > +\t\t\twork_tree = git_path(work_tree);\n> \n> A tab is missing here.\n\nRight.  And as Junio pointed out, an xstrdup().\n\n> > -\t\t\t\tfprintf(stderr, \"No directory given for --work-tree.\\n\" );\n> > +\t\t\t\terror(\"No directory given for --work-tree.\\n\");\n> \n> There should probably be no '\\n' at the end when the 'error' function\n> is used.  There are two other calls to fprintf(stderr, <error message>)\n> next to the one you changed, why did you change this one but not the\n> other ones?\n\nWell, that is a left over of some unrelated editing.\n\nThe patch series that I sent out was deficient in many ways, but I was \ntired, and wanted to show where I am heading.\n\nATM I am trying to finish up this series, with quite a few changes to the \ncode I sent out.\n\nBut there is a fundamental question I have to ask: Is there any reason why \n\n\t$ git --git-dir=/some/where/else.git bla\n\nshould pretend that the repo is bare if core.bare == 1?  I mean, we are \nimplicitely setting the work tree to the cwd, no?\n\nIOW I see the merits of \"core.bare = false\" (to prevent harm when calling \ngit inside the git directory), but I cannot see the merits of \"core.bare = \ntrue\".  Someone enlighten me?\n\nCiao,\nDscho\n"},{"id":"48824","messageId":"7v8x911wn6.fsf@assigned-by-dhcp.cox.net","threadId":"9233","inReplyTo":"Pine.LNX.4.64.0707271146290.14781@racer.site","subject":"Re: [PATCH 3/5] Clean up work-tree handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-27T19:20:29Z","receivedAt":"2007-07-27T19:20:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> But there is a fundamental question I have to ask: Is there any reason why \n>\n> \t$ git --git-dir=/some/where/else.git bla\n>\n> should pretend that the repo is bare if core.bare == 1?  I mean, we are \n> implicitely setting the work tree to the cwd, no?\n\nI have two repositories at the primary k.org machine.\n\n - /home/junio/git.git --- this is with a worktree so that I can\n   build and test on a FC machine (my primary development\n   machine at home is a Debian).\n\n - /pub/scm/git/git.git/ --- this is a bare repository that is\n   mirrored out to git://git.kernel.org/ and friends.\n\nAnd I usually am in the former.  From time to time, I do this:\n\n $ GIT_DIR=/pub/scm/git/git.git/ git fsck\n $ GIT_DIR=/pub/scm/git/git.git/ git repack\n\nbecause I am old fashioned, but I would expect these to be\nequivalent to the above:\n\n $ git --git-dir=/pub/scm/git/git.git/ fsck\n $ git --git-dir=/pub/scm/git/git.git/ repack\n\nI do not think these imply that the repository is with worktree.\n"},{"id":"48827","messageId":"Pine.LNX.4.64.0707272029140.14781@racer.site","threadId":"9233","inReplyTo":"7v8x911wn6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/5] Clean up work-tree handling","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-27T19:32:27Z","receivedAt":"2007-07-27T19:32:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 27 Jul 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > But there is a fundamental question I have to ask: Is there any reason why \n> >\n> > \t$ git --git-dir=/some/where/else.git bla\n> >\n> > should pretend that the repo is bare if core.bare == 1?  I mean, we are \n> > implicitely setting the work tree to the cwd, no?\n> \n> I have two repositories at the primary k.org machine.\n> \n>  - /home/junio/git.git --- this is with a worktree so that I can\n>    build and test on a FC machine (my primary development\n>    machine at home is a Debian).\n> \n>  - /pub/scm/git/git.git/ --- this is a bare repository that is\n>    mirrored out to git://git.kernel.org/ and friends.\n> \n> And I usually am in the former.  From time to time, I do this:\n> \n>  $ GIT_DIR=/pub/scm/git/git.git/ git fsck\n>  $ GIT_DIR=/pub/scm/git/git.git/ git repack\n> \n> because I am old fashioned, but I would expect these to be\n> equivalent to the above:\n> \n>  $ git --git-dir=/pub/scm/git/git.git/ fsck\n>  $ git --git-dir=/pub/scm/git/git.git/ repack\n> \n> I do not think these imply that the repository is with worktree.\n\nBut in your use cases it does not matter, since neither fsck nor repack \nneed a worktree.\n\nHere is one of _my_ scenarios: I want to track a directory I have no write \naccess to (or better put: I should have no write access to).  So I created \na bare repository, and when I add new files I use the \"GIT_DIR=... git...\" \nmantra.\n\nBut you're right, I could just set core.worktree=... and unset core.bare.\n\nI can change that easily enough.  Other comments on the series?\n\nCiao,\nDscho\n"}]}