{"thread":{"id":"38957","subject":"Forcing git top-level","startedAt":"2015-03-31T09:25:58Z","lastAt":"2015-04-18T11:22:53Z","messageCount":11,"participants":["Cedric Gava","Jeff King","Jonathan Nieder","Nguyễn Thái Ngọc Duy","Duy Nguyen","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"258736","messageId":"0FD999DB-3DBF-40D4-8128-715BDC49EAAB@free.fr","threadId":"38957","inReplyTo":null,"subject":"Forcing git top-level","fromName":"Cedric Gava","fromEmail":"gava.c@free.fr","sentAt":"2015-03-31T09:25:58Z","receivedAt":"2015-03-31T09:25:58Z","isPatch":false,"sender":{"key":"gava.c@free.fr","avatar":null},"body":"\nHello\n\nI’ve copied a .git folder located at the root (/) of a filesystem, into another directory (/home/mydir). If I issue a git rev-parse —show-toplevel I got \"/\"...\nI would like to change the top-level to point to /home/mydir.\n\nI’ve also tried to bare clone the original .git folder, but the new git repo stills points it top-level to \"/\".\n\nThank you for your answer\n\nBest regards\nCedric"},{"id":"258762","messageId":"20150331181552.GC19206@peff.net","threadId":"38957","inReplyTo":"0FD999DB-3DBF-40D4-8128-715BDC49EAAB@free.fr","subject":"Re: Forcing git top-level","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-31T18:15:52Z","receivedAt":"2015-03-31T18:15:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 31, 2015 at 11:25:58AM +0200, Cedric Gava wrote:\n\n> I’ve copied a .git folder located at the root (/) of a filesystem,\n> into another directory (/home/mydir). If I issue a git rev-parse\n> —show-toplevel I got \"/\"...\n> I would like to change the top-level to point to /home/mydir.\n\nTry running \"git config --unset core.worktree\" in the .git dir.\n\nIt looks like \"git init\" will write a core.worktree entry in this case,\neven though it isn't technically needed. I think it is due to these\nlines in builtin/init-db.c:\n\n if (!starts_with(git_dir, work_tree) ||\n strcmp(git_dir + strlen(work_tree), \"/.git\")) {\n git_config_set(\"core.worktree\", work_tree);\n }\n\nThe check returns a false positive for the root directory because we\nonly have one slash (i.e., appending \"/.git\" to our worktree would be\n\"//.git\" in this case).\n\n-Peff\n"},{"id":"258763","messageId":"20150331183423.GD19206@peff.net","threadId":"38957","inReplyTo":"20150331181552.GC19206@peff.net","subject":"[PATCH] init: don't set core.worktree when initializing /.git","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-31T18:34:23Z","receivedAt":"2015-03-31T18:34:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If you create a git repository in the root directory with\n\"git init /\", we erroneously write a core.worktree entry.\nThat can be surprising if you later move the .git directory\nto another path (which usually moves the relative working\ntree, but is foiled if there is an explicit worktree set).\n\nThe problem is that we check whether core.worktree is\nnecessary by seeing if we can make the git_dir by\nconcatenating \"/.git\" onto the working tree. That would lead\nto \"//.git\" in this instance, but we actually have \"/.git\"\n(without the doubled slash).\n\nWe can fix this by special-casing the root directory. I also\nsplit the logic out into its own function to make the\nconditional a bit more readable (and used skip_prefix, which\nI think makes it a little more obvious what is going on).\n\nNo tests, as we would need to be able to write to \"/\" to do\nso. I did manually confirm that:\n\n sudo git init /\n cd /\n git rev-parse --show-toplevel\n git config core.worktree\n\nStill finds the top-level correctly (as \"/\"), and does not\nset any core.worktree variable.\n\nSigned-off-by: Jeff King \n---\nThe current behavior isn't _wrong_, in the sense that it's OK to set\ncore.worktree when we don't need to. But I think it is unnecessarily\nconfusing to users who expect to be able to move .git directories\naround, because it usually just works. So I'd call this an extremely\nminor bug.\n\n builtin/init-db.c | 19 ++++++++++++++++---\n 1 file changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 6723d39..6efe2df 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -182,6 +182,21 @@ static int git_init_db_config(const char *k, const char *v, void *cb)\n \treturn 0;\n }\n \n+/*\n+ * If the git_dir is not directly inside the working tree, then git will not\n+ * find it by default, and we need to set the worktree explicitly.\n+ */\n+static int needs_work_tree_config(const char *git_dir, const char *work_tree)\n+{\n+\t/* special case that is missed by the general rule below */\n+\tif (!strcmp(work_tree, \"/\") && !strcmp(git_dir, \"/.git\"))\n+\t\treturn 0;\n+\tif (skip_prefix(git_dir, work_tree, &git_dir) &&\n+\t !strcmp(git_dir, \"/.git\"))\n+\t\treturn 0;\n+\treturn 1;\n+}\n+\n static int create_default_files(const char *template_path)\n {\n \tconst char *git_dir = get_git_dir();\n@@ -274,10 +289,8 @@ static int create_default_files(const char *template_path)\n \t\t/* allow template config file to override the default */\n \t\tif (log_all_ref_updates == -1)\n \t\t git_config_set(\"core.logallrefupdates\", \"true\");\n-\t\tif (!starts_with(git_dir, work_tree) ||\n-\t\t strcmp(git_dir + strlen(work_tree), \"/.git\")) {\n+\t\tif (needs_work_tree_config(git_dir, work_tree))\n \t\t\tgit_config_set(\"core.worktree\", work_tree);\n-\t\t}\n \t}\n \n \tif (!reinit) {\n-- \n2.4.0.rc0.363.gf9f328b\n"},{"id":"258767","messageId":"20150331191420.GE22844@google.com","threadId":"38957","inReplyTo":"20150331183423.GD19206@peff.net","subject":"Re: [PATCH] init: don't set core.worktree when initializing /.git","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2015-03-31T19:14:20Z","receivedAt":"2015-03-31T19:14:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> No tests, as we would need to be able to write to \"/\" to do\n> so.\n\nt1509-root-worktree.sh is supposed to test the repository-at-/ case.\nBut I wouldn't be surprised if it's bitrotted, since people don't set\nup a throwaway chroot or VM for tests too often.\n\n[...]\n> The current behavior isn't _wrong_, in the sense that it's OK to set\n> core.worktree when we don't need to. But I think it is unnecessarily\n> confusing to users who expect to be able to move .git directories\n> around, because it usually just works. So I'd call this an extremely\n> minor bug.\n\nThis belongs in the commit message.\n\n[...]\n> --- a/builtin/init-db.c\n> +++ b/builtin/init-db.c\n> @@ -182,6 +182,21 @@ static int git_init_db_config(const char *k, const char *v, void *cb)\n> \treturn 0;\n> }\n> \n> +/*\n> + * If the git_dir is not directly inside the working tree, then git will not\n> + * find it by default, and we need to set the worktree explicitly.\n> + */\n> +static int needs_work_tree_config(const char *git_dir, const char *work_tree)\n> +{\n> +\t/* special case that is missed by the general rule below */\n\n(optional) I'd leave out this comment --- it seems obvious enough in\ncontext and the purpose of the comment is unobvious without looking\nat the history.\n\n> +\tif (!strcmp(work_tree, \"/\") && !strcmp(git_dir, \"/.git\"))\n> +\t\treturn 0;\n> +\tif (skip_prefix(git_dir, work_tree, &git_dir) &&\n> +\t !strcmp(git_dir, \"/.git\"))\n> +\t\treturn 0;\n\nwork_tree has been cleaned up with real_path, so in the normal case it\ncontains getcwd() output which will not end with a / unless it has to.\nThe only exception I can see is when git hits the MAXDEPTH limit for\nsymlink resolution (5 nested symlinks), in which case we take what we\nfind instead of erroring out, which looks like a bug.\n\nWe have called set_git_dir_init so git_dir has been cleaned up by\nreal_path in the same way. Good.\n\nWith or without the commit message, comment, and test improvements\nmentioned above,\nReviewed-by: Jonathan Nieder \n\nThanks.\n"},{"id":"258871","messageId":"20150402183739.GA19911@peff.net","threadId":"38957","inReplyTo":"20150331191420.GE22844@google.com","subject":"[PATCH v2] init: don't set core.worktree when initializing /.git","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-04-02T18:37:40Z","receivedAt":"2015-04-02T18:37:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 31, 2015 at 12:14:20PM -0700, Jonathan Nieder wrote:\n\n> > No tests, as we would need to be able to write to \"/\" to do\n> > so.\n> \n> t1509-root-worktree.sh is supposed to test the repository-at-/ case.\n> But I wouldn't be surprised if it's bitrotted, since people don't set\n> up a throwaway chroot or VM for tests too often.\n\nThanks, I had thought we had such a test script, but for some reason had\ntrouble finding it earlier. I couldn't get it to run using the included\nt1509/prepare-chroot.sh helper; the resulting chroot was missing several\ntools we rely on in test-lib.sh (perl, expr, tr, and mv).\n\nI gave up for now; I don't think rescuing that script is worth the\neffort for this minor bug.\n\n> > The current behavior isn't _wrong_, in the sense that it's OK to set\n> > core.worktree when we don't need to. But I think it is unnecessarily\n> > confusing to users who expect to be able to move .git directories\n> > around, because it usually just works. So I'd call this an extremely\n> > minor bug.\n> \n> This belongs in the commit message.\n\nIt's mostly a restatement of what is already in the first paragraph of\nthe commit message. I folded it into that paragraph.\n\n> > +static int needs_work_tree_config(const char *git_dir, const char *work_tree)\n> > +{\n> > +\t/* special case that is missed by the general rule below */\n> \n> (optional) I'd leave out this comment --- it seems obvious enough in\n> context and the purpose of the comment is unobvious without looking\n> at the history.\n\nOK, dropped. Here's the re-roll.\n\n-- >8 --\nSubject: [PATCH] init: don't set core.worktree when initializing /.git\n\nIf you create a git repository in the root directory with\n\"git init /\", we erroneously write a core.worktree entry.\nThis isn't _wrong_, in the sense that it's OK to set\ncore.worktree when we don't need to. But it is unnecessarily\nsurprising if you later move the .git directory to another\npath (which usually moves the relative working tree, but is\nfoiled if there is an explicit worktree set).\n\nThe problem is that we check whether core.worktree is\nnecessary by seeing if we can make the git_dir by\nconcatenating \"/.git\" onto the working tree. That would lead\nto \"//.git\" in this instance, but we actually have \"/.git\"\n(without the doubled slash).\n\nWe can fix this by special-casing the root directory. I also\nsplit the logic out into its own function to make the\nconditional a bit more readable (and used skip_prefix, which\nI think makes it a little more obvious what is going on).\n\nNo tests, as we would need to be able to write to \"/\" to do\nso. I did manually confirm that:\n\n sudo git init /\n cd /\n git rev-parse --show-toplevel\n git config core.worktree\n\nstill finds the top-level correctly (as \"/\"), and does not\nset any core.worktree variable.\n\nSigned-off-by: Jeff King \nReviewed-by: Jonathan Nieder \n---\n builtin/init-db.c | 18 +++++++++++++++---\n 1 file changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 6723d39..ab9f86b 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -182,6 +182,20 @@ static int git_init_db_config(const char *k, const char *v, void *cb)\n \treturn 0;\n }\n \n+/*\n+ * If the git_dir is not directly inside the working tree, then git will not\n+ * find it by default, and we need to set the worktree explicitly.\n+ */\n+static int needs_work_tree_config(const char *git_dir, const char *work_tree)\n+{\n+\tif (!strcmp(work_tree, \"/\") && !strcmp(git_dir, \"/.git\"))\n+\t\treturn 0;\n+\tif (skip_prefix(git_dir, work_tree, &git_dir) &&\n+\t !strcmp(git_dir, \"/.git\"))\n+\t\treturn 0;\n+\treturn 1;\n+}\n+\n static int create_default_files(const char *template_path)\n {\n \tconst char *git_dir = get_git_dir();\n@@ -274,10 +288,8 @@ static int create_default_files(const char *template_path)\n \t\t/* allow template config file to override the default */\n \t\tif (log_all_ref_updates == -1)\n \t\t git_config_set(\"core.logallrefupdates\", \"true\");\n-\t\tif (!starts_with(git_dir, work_tree) ||\n-\t\t strcmp(git_dir + strlen(work_tree), \"/.git\")) {\n+\t\tif (needs_work_tree_config(git_dir, work_tree))\n \t\t\tgit_config_set(\"core.worktree\", work_tree);\n-\t\t}\n \t}\n \n \tif (!reinit) {\n-- \n2.4.0.rc0.363.gf9f328b\n"},{"id":"258872","messageId":"20150402184905.GB5467@google.com","threadId":"38957","inReplyTo":"20150402183739.GA19911@peff.net","subject":"Re: [PATCH v2] init: don't set core.worktree when initializing /.git","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2015-04-02T18:49:05Z","receivedAt":"2015-04-02T18:49:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> OK, dropped. Here's the re-roll.\n\nThanks.\n\n> -- >8 --\n> Subject: [PATCH] init: don't set core.worktree when initializing /.git\n[...]\n> No tests, as we would need to be able to write to \"/\" to do\n> so.\n\nThis is confusing in the light of t1509-root-worktree.sh existing, but\nanyone curious can dig up this mailing list thread so I don't mind. :)\n\n[...]\n> Signed-off-by: Jeff King \n> Reviewed-by: Jonathan Nieder \n\nYep - still looks good.\n\nRegards,\nJonathan\n"},{"id":"258918","messageId":"1428055737-8943-1-git-send-email-pclouds@gmail.com","threadId":"38957","inReplyTo":"20150331191420.GE22844@google.com","subject":"[PATCH] t1509: update prepare script to be able to run t1509 in chroot again","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-04-03T10:08:57Z","receivedAt":"2015-04-03T10:08:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Tested on Gentoo and OpenSUSE 13.1, both x86-64\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n On Wed, Apr 1, 2015 at 2:14 AM, Jonathan Nieder wrote:\n > Jeff King wrote:\n >\n >> No tests, as we would need to be able to write to \"/\" to do\n >> so.\n >\n > t1509-root-worktree.sh is supposed to test the repository-at-/ case.\n > But I wouldn't be surprised if it's bitrotted, since people don't set\n > up a throwaway chroot or VM for tests too often.\n\n Can't leave it rotting. Either fix it or kill it. This is the first\n option. Good news is the test passes, nothing else is broken. Bad\n news is it does not detect the core.worktree breakage, but this on\n top would verify that Jeff's patch works\n\n diff --git a/t/t1509-root-worktree.sh b/t/t1509-root-worktree.sh\n index b6977d4..17ff4ce 100755\n --- a/t/t1509-root-worktree.sh\n +++ b/t/t1509-root-worktree.sh\n @@ -224,6 +224,10 @@ test_expect_success 'setup' '\n \ttest_cmp expected result\n '\n \n +test_expect_success 'no core.worktree after git init' '\n +\ttest \"`git config core.worktree`\" = \"\"\n +'\n +\n test_vars 'auto gitdir, root' \".git\" \"/\" \"\"\n test_foobar_root\n \n\n\n t/t1509/prepare-chroot.sh | 35 ++++++++++++++++++++++++++---------\n 1 file changed, 26 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t1509/prepare-chroot.sh b/t/t1509/prepare-chroot.sh\nindex 6269117..c61afbf 100755\n--- a/t/t1509/prepare-chroot.sh\n+++ b/t/t1509/prepare-chroot.sh\n@@ -14,25 +14,42 @@ xmkdir() {\n \n R=\"$1\"\n \n+[ \"$UID\" -eq 0 ] && die \"This script should not be run as root, what if it does rm -rf /?\"\n [ -n \"$R\" ] || die \"usage: prepare-chroot.sh \"\n [ -x git ] || die \"This script needs to be executed at git source code's top directory\"\n-[ -x /bin/busybox ] || die \"You need busybox\"\n+if [ -x /bin/busybox ]; then\n+\tBB=/bin/busybox\n+elif [ -x /usr/bin/busybox ]; then\n+\tBB=/usr/bin/busybox\n+else\n+\tdie \"You need busybox\"\n+fi\n \n xmkdir \"$R\" \"$R/bin\" \"$R/etc\" \"$R/lib\" \"$R/dev\"\n-[ -c \"$R/dev/null\" ] || die \"/dev/null is missing. Do mknod $R/dev/null c 1 3 && chmod 666 $R/dev/null\"\n+touch \"$R/dev/null\"\n echo \"root:x:0:0:root:/:/bin/sh\" > \"$R/etc/passwd\"\n echo \"$(id -nu):x:$(id -u):$(id -g)::$(pwd)/t:/bin/sh\" >> \"$R/etc/passwd\"\n echo \"root::0:root\" > \"$R/etc/group\"\n echo \"$(id -ng)::$(id -g):$(id -nu)\" >> \"$R/etc/group\"\n \n-[ -x \"$R/bin/busybox\" ] || cp /bin/busybox \"$R/bin/busybox\"\n-[ -x \"$R/bin/sh\" ] || ln -s /bin/busybox \"$R/bin/sh\"\n-[ -x \"$R/bin/su\" ] || ln -s /bin/busybox \"$R/bin/su\"\n+[ -x \"$R$BB\" ] || cp $BB \"$R/bin/busybox\"\n+for cmd in sh su ls expr tr basename rm mkdir mv id uname dirname cat true sed diff; do\n+\tln -f -s /bin/busybox \"$R/bin/$cmd\"\n+done\n \n mkdir -p \"$R$(pwd)\"\n rsync --exclude-from t/t1509/excludes -Ha . \"$R$(pwd)\"\n-ldd git | grep '/' | sed 's,.*\\s\\(/[^ ]*\\).*,\\1,' | while read i; do\n-\tmkdir -p \"$R$(dirname $i)\"\n-\tcp \"$i\" \"$R/$i\"\n+# Fake perl to reduce dependency, t1509 does not use perl, but some\n+# env might slip through, see test-lib.sh, unset.*PERL_PATH\n+sed 's|^PERL_PATH=*|PERL_PATH=/bin/true|' GIT-BUILD-OPTIONS > \"$R$(pwd)/GIT-BUILD-OPTIONS\"\n+for cmd in git $BB;do \n+\tldd $cmd | grep '/' | sed 's,.*\\s\\(/[^ ]*\\).*,\\1,' | while read i; do\n+\t\tmkdir -p \"$R$(dirname $i)\"\n+\t\tcp \"$i\" \"$R/$i\"\n+\tdone\n done\n-echo \"Execute this in root: 'chroot $R /bin/su - $(id -nu)'\"\n+cat < diff --git a/t/t1509/prepare-chroot.sh b/t/t1509/prepare-chroot.sh\n> index 6269117..c61afbf 100755\n> --- a/t/t1509/prepare-chroot.sh\n> +++ b/t/t1509/prepare-chroot.sh\n> @@ -14,25 +14,42 @@ xmkdir() {\n> \n> R=\"$1\"\n> \n> +[ \"$UID\" -eq 0 ] && die \"This script should not be run as root, what if it does rm -rf /?\"\n\nThis line complains for me. $UID is set in my bash login shell, but not\nin the dash shell for this script. Maybe \"id -u\" instead?\n\n> xmkdir \"$R\" \"$R/bin\" \"$R/etc\" \"$R/lib\" \"$R/dev\"\n> -[ -c \"$R/dev/null\" ] || die \"/dev/null is missing. Do mknod $R/dev/null c 1 3 && chmod 666 $R/dev/null\"\n> +touch \"$R/dev/null\"\n\nIt is nice not to mknod here, as it requires root. But making /dev/null\na regular file seems a bit flaky. We will constantly overwrite it with\nthe output of each test, and then pipe that output back into the next\ntest. The current set of tests in t1509 does not seem to mind, though.\n\n> +# Fake perl to reduce dependency, t1509 does not use perl, but some\n> +# env might slip through, see test-lib.sh, unset.*PERL_PATH\n> +sed 's|^PERL_PATH=*|PERL_PATH=/bin/true|' GIT-BUILD-OPTIONS > \"$R$(pwd)/GIT-BUILD-OPTIONS\"\n\nRe-preparing an already-made chroot makes this:\n\n PERL_PATH=/bin/true'/usr/bin/perl'\n\nDid you want \"PERL_PATH=.*\" as your regex?\n\n> -echo \"Execute this in root: 'chroot $R /bin/su - $(id -nu)'\"\n> +cat < +Execute this in root:\n> +chroot $R /bin/su - $(id -nu)\n> +IKNOWWHATIAMDOING=YES ./t1509-root-worktree.sh -v -i\n> +EOF\n\nI found the \"in root\" here (and in the original) confusing. Do you mean\n\"as root\"? I wonder if it would make sense to just show:\n\n sudo chroot $R /bin/su - $(id -nu)\n\nas the sample command.\n\nAside from the nits above, I did get it to run t1509 with this. I can't\nsay I'm incredibly excited about the whole thing, if only because it is\nclear that nobody is going to run it regularly (so regressions will\nlikely go unnoticed, which is the whole point of the test script). But\nperhaps it is better than nothing, and it is not hurting anyone to sit\nthere and bitrot again. ;)\n\n-Peff\n"},{"id":"258921","messageId":"CACsJy8D3Rk51JVvwivj8BwTjprhYC7aEbfY4vF+4Mo-cn5taLw@mail.gmail.com","threadId":"38957","inReplyTo":"20150403120109.GB22170@peff.net","subject":"Re: [PATCH] t1509: update prepare script to be able to run t1509 in chroot again","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-04-03T12:14:18Z","receivedAt":"2015-04-03T12:14:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Apr 3, 2015 at 7:01 PM, Jeff King wrote:\n> Aside from the nits above, I did get it to run t1509 with this. I can't\n> say I'm incredibly excited about the whole thing, if only because it is\n> clear that nobody is going to run it regularly (so regressions will\n> likely go unnoticed, which is the whole point of the test script). But\n> perhaps it is better than nothing, and it is not hurting anyone to sit\n> there and bitrot again. ;)\n\nTo be honest, I didn't run it often in the last 5 years. The code it\nprotects seems not broken during this time and probably so for a\nforeseeable future. So I don't mind if you just kill t1509 and related\nscripts.\n-- \nDuy\n"},{"id":"259243","messageId":"xmqqr3rqa131.fsf@gitster.dls.corp.google.com","threadId":"38957","inReplyTo":"CACsJy8D3Rk51JVvwivj8BwTjprhYC7aEbfY4vF+4Mo-cn5taLw@mail.gmail.com","subject":"Re: [PATCH] t1509: update prepare script to be able to run t1509 in chroot again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-11T19:58:58Z","receivedAt":"2015-04-11T19:58:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Fri, Apr 3, 2015 at 7:01 PM, Jeff King wrote:\n>> Aside from the nits above, I did get it to run t1509 with this. I can't\n>> say I'm incredibly excited about the whole thing, if only because it is\n>> clear that nobody is going to run it regularly (so regressions will\n>> likely go unnoticed, which is the whole point of the test script). But\n>> perhaps it is better than nothing, and it is not hurting anyone to sit\n>> there and bitrot again. ;)\n>\n> To be honest, I didn't run it often in the last 5 years. The code it\n> protects seems not broken during this time and probably so for a\n> foreseeable future. So I don't mind if you just kill t1509 and related\n> scripts.\n\nYeah, but as long as you two worked out to make it run again on at\nleast two systems (i.e. yours), it would be good not to let that\neffort go to waste. Care to throw a v2 at me with $(id -u) and other\nchanges Peff mentioned?\n\nThanks.\n"},{"id":"259624","messageId":"1429356173-5161-1-git-send-email-pclouds@gmail.com","threadId":"38957","inReplyTo":"xmqqr3rqa131.fsf@gitster.dls.corp.google.com","subject":"[PATCH v2] t1509: update prepare script to be able to run t1509 in chroot again","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-04-18T11:22:53Z","receivedAt":"2015-04-18T11:22:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Tested on Gentoo and OpenSUSE 13.1, both x86-64\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n On Sun, Apr 12, 2015 at 2:58 AM, Junio C Hamano wrote:\n > Duy Nguyen writes:\n >\n >> On Fri, Apr 3, 2015 at 7:01 PM, Jeff King wrote:\n >>> Aside from the nits above, I did get it to run t1509 with this. I can't\n >>> say I'm incredibly excited about the whole thing, if only because it is\n >>> clear that nobody is going to run it regularly (so regressions will\n >>> likely go unnoticed, which is the whole point of the test script).  But\n >>> perhaps it is better than nothing, and it is not hurting anyone to sit\n >>> there and bitrot again. ;)\n >>\n >> To be honest, I didn't run it often in the last 5 years. The code it\n >> protects seems not broken during this time and probably so for a\n >> foreseeable future. So I don't mind if you just kill t1509 and related\n >> scripts.\n >\n > Yeah, but as long as you two worked out to make it run again on at\n > least two systems (i.e. yours), it would be good not to let that\n > effort go to waste. Care to throw a v2 at me with $(id -u) and other\n > changes Peff mentioned?\n\n thrown (a little bit late because $DAYJOB consumed much of my energy)\n\n t/t1509/prepare-chroot.sh | 38 +++++++++++++++++++++++++++++---------\n 1 file changed, 29 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t1509/prepare-chroot.sh b/t/t1509/prepare-chroot.sh\nindex 6269117..6d47e2c 100755\n--- a/t/t1509/prepare-chroot.sh\n+++ b/t/t1509/prepare-chroot.sh\n@@ -14,25 +14,45 @@ xmkdir() {\n \n R=\"$1\"\n \n+[ \"$(id -u)\" -eq 0 ] && die \"This script should not be run as root, what if it does rm -rf /?\"\n [ -n \"$R\" ] || die \"usage: prepare-chroot.sh \"\n [ -x git ] || die \"This script needs to be executed at git source code's top directory\"\n-[ -x /bin/busybox ] || die \"You need busybox\"\n+if [ -x /bin/busybox ]; then\n+\tBB=/bin/busybox\n+elif [ -x /usr/bin/busybox ]; then\n+\tBB=/usr/bin/busybox\n+else\n+\tdie \"You need busybox\"\n+fi\n \n xmkdir \"$R\" \"$R/bin\" \"$R/etc\" \"$R/lib\" \"$R/dev\"\n-[ -c \"$R/dev/null\" ] || die \"/dev/null is missing. Do mknod $R/dev/null c 1 3 && chmod 666 $R/dev/null\"\n+touch \"$R/dev/null\"\n echo \"root:x:0:0:root:/:/bin/sh\" > \"$R/etc/passwd\"\n echo \"$(id -nu):x:$(id -u):$(id -g)::$(pwd)/t:/bin/sh\" >> \"$R/etc/passwd\"\n echo \"root::0:root\" > \"$R/etc/group\"\n echo \"$(id -ng)::$(id -g):$(id -nu)\" >> \"$R/etc/group\"\n \n-[ -x \"$R/bin/busybox\" ] || cp /bin/busybox \"$R/bin/busybox\"\n-[ -x \"$R/bin/sh\" ] || ln -s /bin/busybox \"$R/bin/sh\"\n-[ -x \"$R/bin/su\" ] || ln -s /bin/busybox \"$R/bin/su\"\n+[ -x \"$R$BB\" ] || cp $BB \"$R/bin/busybox\"\n+for cmd in sh su ls expr tr basename rm mkdir mv id uname dirname cat true sed diff; do\n+\tln -f -s /bin/busybox \"$R/bin/$cmd\"\n+done\n \n mkdir -p \"$R$(pwd)\"\n rsync --exclude-from t/t1509/excludes -Ha . \"$R$(pwd)\"\n-ldd git | grep '/' | sed 's,.*\\s\\(/[^ ]*\\).*,\\1,' | while read i; do\n-\tmkdir -p \"$R$(dirname $i)\"\n-\tcp \"$i\" \"$R/$i\"\n+# Fake perl to reduce dependency, t1509 does not use perl, but some\n+# env might slip through, see test-lib.sh, unset.*PERL_PATH\n+sed 's|^PERL_PATH=.*|PERL_PATH=/bin/true|' GIT-BUILD-OPTIONS > \"$R$(pwd)/GIT-BUILD-OPTIONS\"\n+for cmd in git $BB;do \n+\tldd $cmd | grep '/' | sed 's,.*\\s\\(/[^ ]*\\).*,\\1,' | while read i; do\n+\t\tmkdir -p \"$R$(dirname $i)\"\n+\t\tcp \"$i\" \"$R/$i\"\n+\tdone\n done\n-echo \"Execute this in root: 'chroot $R /bin/su - $(id -nu)'\"\n+cat <