{"thread":{"id":"10824","subject":"[PATCH] setup_git_directory: Setup cwd properly if worktree is found","startedAt":"2007-11-12T11:24:09Z","lastAt":"2007-11-27T14:46:39Z","messageCount":6,"participants":["Nguyễn Thái Ngọc Duy","Johannes Schindelin","Johannes Sixt","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"59468","messageId":"20071112112408.GA5420@laptop","threadId":"10824","inReplyTo":null,"subject":"[PATCH] setup_git_directory: Setup cwd properly if worktree is found","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2007-11-12T11:24:09Z","receivedAt":"2007-11-12T11:24:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This makes sure (if possible) the current working directory is at\nroot worktree. \"git rev-parse --show-cwd\" is added to aid the tests.\nAlso the first test is for \"Add missing inside_work_tree\" commit.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin-rev-parse.c |    6 ++++++\n setup.c             |    7 ++++++-\n t/t1501-worktree.sh |   33 +++++++++++++++++++++++++++++++++\n 3 files changed, 45 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-rev-parse.c b/builtin-rev-parse.c\nindex 8d78b69..933875c 100644\n--- a/builtin-rev-parse.c\n+++ b/builtin-rev-parse.c\n@@ -319,6 +319,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tputs(prefix);\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--show-cwd\")) {\n+\t\t\t\tchar cwd[PATH_MAX+1];\n+\t\t\t\tgetcwd(cwd, sizeof(cwd));\n+\t\t\t\tprintf(\"%s\\n\", cwd);\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--show-cdup\")) {\n \t\t\t\tconst char *pfx = prefix;\n \t\t\t\tif (!is_inside_work_tree()) {\ndiff --git a/setup.c b/setup.c\nindex 6f8f769..d90f65e 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -360,7 +360,12 @@ const char *setup_git_directory(void)\n \t\tif (retval && chdir(retval))\n \t\t\tdie (\"Could not jump back into original cwd\");\n \t\trel = get_relative_cwd(buffer, PATH_MAX, get_git_work_tree());\n-\t\treturn rel && *rel ? strcat(rel, \"/\") : NULL;\n+\t\tif (rel && *rel) {\n+\t\t\tif (chdir(get_git_work_tree()))\n+\t\t\t\tdie (\"Could not chdir to %s\", get_git_work_tree());\n+\t\t\treturn strcat(rel, \"/\");\n+\t\t}\n+\t\treturn NULL;\n \t}\n \n \treturn retval;\ndiff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh\nindex 7ee3820..a4e1f72 100755\n--- a/t/t1501-worktree.sh\n+++ b/t/t1501-worktree.sh\n@@ -30,6 +30,18 @@ test_rev_parse() {\n \n mkdir -p work/sub/dir || exit 1\n mv .git repo.git || exit 1\n+export SAVED_WORK_DIR=$(pwd)/work\n+\n+say \"GIT_WORK_TREE without core.worktree\"\n+\n+export GIT_DIR=\"$(pwd)\"/repo.git\n+export GIT_CONFIG=\"$GIT_DIR\"/config\n+export GIT_WORK_TREE=\"$(pwd)\"/work\n+\n+test_expect_success 'cwd is set properly' '\n+\t(cd work/sub &&\n+\t test \"$(git rev-parse --show-cwd)\" = \"$SAVED_WORK_DIR\")\n+'\n \n say \"core.worktree = relative path\"\n export GIT_DIR=repo.git\n@@ -47,6 +59,11 @@ export GIT_CONFIG=\"$(pwd)\"/$GIT_DIR/config\n test_rev_parse 'subdirectory' false false true sub/dir/\n cd ../../.. || exit 1\n \n+test_expect_success 'cwd is set properly' '\n+\t(cd work/sub &&\n+\t test \"$(GIT_DIR=../../repo.git git rev-parse --show-cwd)\" = \"$SAVED_WORK_DIR\")\n+'\n+\n say \"core.worktree = absolute path\"\n export GIT_DIR=$(pwd)/repo.git\n export GIT_CONFIG=$GIT_DIR/config\n@@ -58,6 +75,11 @@ cd sub/dir || exit 1\n test_rev_parse 'subdirectory' false false true sub/dir/\n cd ../../.. || exit 1\n \n+test_expect_success 'cwd is set properly' '\n+\t(cd work/sub &&\n+\t test \"$(git rev-parse --show-cwd)\" = \"$SAVED_WORK_DIR\")\n+'\n+\n say \"GIT_WORK_TREE=relative path (override core.worktree)\"\n export GIT_DIR=$(pwd)/repo.git\n export GIT_CONFIG=$GIT_DIR/config\n@@ -72,7 +94,13 @@ export GIT_WORK_TREE=../..\n test_rev_parse 'subdirectory' false false true sub/dir/\n cd ../../.. || exit 1\n \n+test_expect_success 'cwd is set properly' '\n+\t(cd work/sub &&\n+\t test \"$(GIT_WORK_TREE=.. git rev-parse --show-cwd)\" = \"$SAVED_WORK_DIR\")\n+'\n+\n mv work repo.git/work\n+export SAVED_WORK_DIR=$(pwd)/repo.git/work\n \n say \"GIT_WORK_TREE=absolute path, work tree below git dir\"\n export GIT_DIR=$(pwd)/repo.git\n@@ -89,6 +117,11 @@ cd sub/dir || exit 1\n test_rev_parse 'in repo.git/sub/dir' false true true sub/dir/\n cd ../../../.. || exit 1\n \n+test_expect_success 'cwd is set properly' '\n+\t(cd repo.git/work/sub &&\n+\t test \"$(git rev-parse --show-cwd)\" = \"$SAVED_WORK_DIR\")\n+'\n+\n test_expect_success 'repo finds its work tree' '\n \t(cd repo.git &&\n \t : > work/sub/dir/untracked &&\n-- \n1.5.3.5.475.gd7a30\n"},{"id":"59469","messageId":"Pine.LNX.4.64.0711121139010.4362@racer.site","threadId":"10824","inReplyTo":"20071112112408.GA5420@laptop","subject":"Re: [PATCH] setup_git_directory: Setup cwd properly if worktree is found","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-12T11:57:23Z","receivedAt":"2007-11-12T11:57:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 12 Nov 2007, Nguyễn Thái Ngọc Duy wrote:\n\n> diff --git a/setup.c b/setup.c\n> index 6f8f769..d90f65e 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -360,7 +360,12 @@ const char *setup_git_directory(void)\n>  \t\tif (retval && chdir(retval))\n>  \t\t\tdie (\"Could not jump back into original cwd\");\n>  \t\trel = get_relative_cwd(buffer, PATH_MAX, get_git_work_tree());\n> -\t\treturn rel && *rel ? strcat(rel, \"/\") : NULL;\n> +\t\tif (rel && *rel) {\n> +\t\t\tif (chdir(get_git_work_tree()))\n> +\t\t\t\tdie (\"Could not chdir to %s\", get_git_work_tree());\n> +\t\t\treturn strcat(rel, \"/\");\n> +\t\t}\n> +\t\treturn NULL;\n\nHmm.  Maybe this needs a bit more clarification?\n\nWhen setup_git_directory() returns, the cwd is supposed to be the current \nworking tree's root.  Your patch fixes that nicely when the worktree \nsetting was overridden by the config (which is read in \ncheck_repository_format()).\n\nBut what about setup_git_directory_gently()?  If the working tree is \noverridden by the config, this function is still bogus, right?\n\nAs far as I see, setup_git_directory_gently() only works correctly when \ncore.worktree is _not_ set, unless GIT_WORK_TREE is set (which is supposed \nto override the config setting).  Note: I treat GIT_WORK_TREE the same as \n--work-tree, since at that time they are identical.\n\nMaybe the config stuff has to move into _gently()?\n\nCiao,\nDscho\n\nP.S.: Thanks for picking up the ball.  At some stage I got so fed up with \nthe work-tree stuff that I had to take a long break from it.  It is a \nmessy concept, and the implementation is messy, partly because of it, and \npartly because I wrote it."},{"id":"59476","messageId":"Pine.LNX.4.64.0711121224430.4362@racer.site","threadId":"10824","inReplyTo":"fcaeb9bf0711120413w180c07e1qbf1b186753593d7@mail.gmail.com","subject":"Re: [PATCH] setup_git_directory: Setup cwd properly if worktree is found","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-12T12:31:22Z","receivedAt":"2007-11-12T12:31:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 12 Nov 2007, Nguyen Thai Ngoc Duy wrote:\n\n> On Nov 12, 2007 6:57 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n>\n> > But what about setup_git_directory_gently()?  If the working tree is \n> > overridden by the config, this function is still bogus, right?\n> \n> Hmm.. thinking a little bit more. I guess you're right because \n> GIT_WORK_TREE takes precedence over core.worktree. Maybe some more bits \n> for check_repository_format_version(). Tough decision because, from the \n> value of inside_work_tree, we don't know if we can safely skip \n> overriding inside_work_tree.\n\nI was thinking about adding check_repository_format_version() and a check \nfor inside_work_tree < 0 with obvious handling in two places, probably as \na function:  first, when we have a gitdirenv but no work_tree_env, and \nsecond, at the end of _gently() when we found a git dir but only if \nwork_tree_env was not set.\n\n> > As far as I see, setup_git_directory_gently() only works correctly \n> > when core.worktree is _not_ set, unless GIT_WORK_TREE is set (which is \n> > supposed to override the config setting).  Note: I treat GIT_WORK_TREE \n> > the same as --work-tree, since at that time they are identical.\n> >\n> > Maybe the config stuff has to move into _gently()?\n> \n> Well, it could be a bit more complicated because you need to know\n> GIT_DIR first before reading config. I'd rather not move as _gently()\n> is complicated already.\n\nAFAICT it is not a question of complexity, but of correctness.  Wouldn't \nyou agree that the prefix _gently() returns is wrong if we don't fix it?\n\nBesides, it might be needed anyway if we are serious about the version \ncheck.  This check, however, would have to be done _whenever_ we found a \ngit directory, not only when work_tree_env is NULL.\n\nBut we could break down _gently() quite nicely.  ATM it both handles the \ncases when gitdirenv was set, and when it was unset.  Even if those are \nreally two pretty different situations.\n\nCiao,\nDscho\n"},{"id":"59478","messageId":"47384CDD.5000209@viscovery.net","threadId":"10824","inReplyTo":"20071112112408.GA5420@laptop","subject":"Re: [PATCH] setup_git_directory: Setup cwd properly if worktree is found","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-11-12T12:53:49Z","receivedAt":"2007-11-12T12:53:49Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Nguyễn Thái Ngọc Duy schrieb:\n> +export SAVED_WORK_DIR=$(pwd)/work\n> +export GIT_DIR=\"$(pwd)\"/repo.git\n> +export GIT_CONFIG=\"$GIT_DIR\"/config\n> +export GIT_WORK_TREE=\"$(pwd)\"/work\n\nThese are not very portable; use:\n\n    GIT_DIR=\"$(pwd)\"/repo.git\n    GIT_CONFIG=\"$GIT_DIR\"/config\n    GIT_WORK_TREE=\"$(pwd)\"/work\n    export GIT_DIR GIT_CONFIG GIT_WORK_TREE\n\nThe first one also needs $(pwd) quoted.\n\n-- Hannes\n"},{"id":"61075","messageId":"fcaeb9bf0711270612p52ce20eaue39eac1d529c3fd3@mail.gmail.com","threadId":"10824","inReplyTo":"Pine.LNX.4.64.0711121224430.4362@racer.site","subject":"Re: [PATCH] setup_git_directory: Setup cwd properly if worktree is found","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2007-11-27T14:12:27Z","receivedAt":"2007-11-27T14:12:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Nov 12, 2007 7:31 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Mon, 12 Nov 2007, Nguyen Thai Ngoc Duy wrote:\n>\n> > On Nov 12, 2007 6:57 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> >\n> > > But what about setup_git_directory_gently()?  If the working tree is\n> > > overridden by the config, this function is still bogus, right?\n> >\n> > Hmm.. thinking a little bit more. I guess you're right because\n> > GIT_WORK_TREE takes precedence over core.worktree. Maybe some more bits\n> > for check_repository_format_version(). Tough decision because, from the\n> > value of inside_work_tree, we don't know if we can safely skip\n> > overriding inside_work_tree.\n>\n> I was thinking about adding check_repository_format_version() and a check\n> for inside_work_tree < 0 with obvious handling in two places, probably as\n> a function:  first, when we have a gitdirenv but no work_tree_env, and\n> second, at the end of _gently() when we found a git dir but only if\n> work_tree_env was not set.\n>\n> > > As far as I see, setup_git_directory_gently() only works correctly\n> > > when core.worktree is _not_ set, unless GIT_WORK_TREE is set (which is\n> > > supposed to override the config setting).  Note: I treat GIT_WORK_TREE\n> > > the same as --work-tree, since at that time they are identical.\n> > >\n> > > Maybe the config stuff has to move into _gently()?\n> >\n> > Well, it could be a bit more complicated because you need to know\n> > GIT_DIR first before reading config. I'd rather not move as _gently()\n> > is complicated already.\n>\n> AFAICT it is not a question of complexity, but of correctness.  Wouldn't\n> you agree that the prefix _gently() returns is wrong if we don't fix it?\n>\n> Besides, it might be needed anyway if we are serious about the version\n> check.  This check, however, would have to be done _whenever_ we found a\n> git directory, not only when work_tree_env is NULL.\n\nQuestion time. setup_git_directory_gently() can be happy even if there\nis no repository. Now if we move version check into setup_..._gently\nand it finds git program is too old to handle the repository, what\nwould we do? die() like in check_repository_format() or tell the\ncaller there is no repository?\n-- \nDuy\n"},{"id":"61082","messageId":"Pine.LNX.4.64.0711271446080.27959@racer.site","threadId":"10824","inReplyTo":"fcaeb9bf0711270612p52ce20eaue39eac1d529c3fd3@mail.gmail.com","subject":"Re: [PATCH] setup_git_directory: Setup cwd properly if worktree is found","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-27T14:46:39Z","receivedAt":"2007-11-27T14:46:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Nov 2007, Nguyen Thai Ngoc Duy wrote:\n\n> Question time. setup_git_directory_gently() can be happy even if there \n> is no repository. Now if we move version check into setup_..._gently and \n> it finds git program is too old to handle the repository, what would we \n> do? die() like in check_repository_format() or tell the caller there is \n> no repository?\n\nIn the interest of least surprise, die().\n\nCiao,\nDscho\n \n"}]}