git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Forcing git top-level
@ 2015-03-31  9:25 Cedric Gava
  2015-03-31 18:15 ` Jeff King
  0 siblings, 1 reply; 11+ messages in thread
From: Cedric Gava @ 2015-03-31  9:25 UTC (permalink / raw)
  To: git


Hello

I’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 "/"...
I would like to change the top-level to point to /home/mydir.

I’ve also tried to bare clone the original .git folder, but the new git repo stills points it top-level to "/".

Thank you for your answer

Best regards
Cedric

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Forcing git top-level
  2015-03-31  9:25 Forcing git top-level Cedric Gava
@ 2015-03-31 18:15 ` Jeff King
  2015-03-31 18:34   ` [PATCH] init: don't set core.worktree when initializing /.git Jeff King
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff King @ 2015-03-31 18:15 UTC (permalink / raw)
  To: Cedric Gava; +Cc: git

On Tue, Mar 31, 2015 at 11:25:58AM +0200, Cedric Gava wrote:

> I’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 "/"...
> I would like to change the top-level to point to /home/mydir.

Try running "git config --unset core.worktree" in the .git dir.

It looks like "git init" will write a core.worktree entry in this case,
even though it isn't technically needed. I think it is due to these
lines in builtin/init-db.c:

                if (!starts_with(git_dir, work_tree) ||
                    strcmp(git_dir + strlen(work_tree), "/.git")) {
                        git_config_set("core.worktree", work_tree);
                }

The check returns a false positive for the root directory because we
only have one slash (i.e., appending "/.git" to our worktree would be
"//.git" in this case).

-Peff

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] init: don't set core.worktree when initializing /.git
  2015-03-31 18:15 ` Jeff King
@ 2015-03-31 18:34   ` Jeff King
  2015-03-31 19:14     ` Jonathan Nieder
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff King @ 2015-03-31 18:34 UTC (permalink / raw)
  To: Cedric Gava; +Cc: git

If you create a git repository in the root directory with
"git init /", we erroneously write a core.worktree entry.
That can be surprising if you later move the .git directory
to another path (which usually moves the relative working
tree, but is foiled if there is an explicit worktree set).

The problem is that we check whether core.worktree is
necessary by seeing if we can make the git_dir by
concatenating "/.git" onto the working tree. That would lead
to "//.git" in this instance, but we actually have "/.git"
(without the doubled slash).

We can fix this by special-casing the root directory. I also
split the logic out into its own function to make the
conditional a bit more readable (and used skip_prefix, which
I think makes it a little more obvious what is going on).

No tests, as we would need to be able to write to "/" to do
so. I did manually confirm that:

  sudo git init /
  cd /
  git rev-parse --show-toplevel
  git config core.worktree

Still finds the top-level correctly (as "/"), and does not
set any core.worktree variable.

Signed-off-by: Jeff King <peff@peff.net>
---
The current behavior isn't _wrong_, in the sense that it's OK to set
core.worktree when we don't need to. But I think it is unnecessarily
confusing to users who expect to be able to move .git directories
around, because it usually just works. So I'd call this an extremely
minor bug.

 builtin/init-db.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 6723d39..6efe2df 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -182,6 +182,21 @@ static int git_init_db_config(const char *k, const char *v, void *cb)
 	return 0;
 }
 
+/*
+ * If the git_dir is not directly inside the working tree, then git will not
+ * find it by default, and we need to set the worktree explicitly.
+ */
+static int needs_work_tree_config(const char *git_dir, const char *work_tree)
+{
+	/* special case that is missed by the general rule below */
+	if (!strcmp(work_tree, "/") && !strcmp(git_dir, "/.git"))
+		return 0;
+	if (skip_prefix(git_dir, work_tree, &git_dir) &&
+	    !strcmp(git_dir, "/.git"))
+		return 0;
+	return 1;
+}
+
 static int create_default_files(const char *template_path)
 {
 	const char *git_dir = get_git_dir();
@@ -274,10 +289,8 @@ static int create_default_files(const char *template_path)
 		/* allow template config file to override the default */
 		if (log_all_ref_updates == -1)
 		    git_config_set("core.logallrefupdates", "true");
-		if (!starts_with(git_dir, work_tree) ||
-		    strcmp(git_dir + strlen(work_tree), "/.git")) {
+		if (needs_work_tree_config(git_dir, work_tree))
 			git_config_set("core.worktree", work_tree);
-		}
 	}
 
 	if (!reinit) {
-- 
2.4.0.rc0.363.gf9f328b

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] init: don't set core.worktree when initializing /.git
  2015-03-31 18:34   ` [PATCH] init: don't set core.worktree when initializing /.git Jeff King
@ 2015-03-31 19:14     ` Jonathan Nieder
  2015-04-02 18:37       ` [PATCH v2] " Jeff King
  2015-04-03 10:08       ` [PATCH] t1509: update prepare script to be able to run t1509 in chroot again Nguyễn Thái Ngọc Duy
  0 siblings, 2 replies; 11+ messages in thread
From: Jonathan Nieder @ 2015-03-31 19:14 UTC (permalink / raw)
  To: Jeff King; +Cc: Cedric Gava, git, Nguyễn Thái Ngọc Duy

Jeff King wrote:

> No tests, as we would need to be able to write to "/" to do
> so.

t1509-root-worktree.sh is supposed to test the repository-at-/ case.
But I wouldn't be surprised if it's bitrotted, since people don't set
up a throwaway chroot or VM for tests too often.

[...]
> The current behavior isn't _wrong_, in the sense that it's OK to set
> core.worktree when we don't need to. But I think it is unnecessarily
> confusing to users who expect to be able to move .git directories
> around, because it usually just works. So I'd call this an extremely
> minor bug.

This belongs in the commit message.

[...]
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -182,6 +182,21 @@ static int git_init_db_config(const char *k, const char *v, void *cb)
>  	return 0;
>  }
>  
> +/*
> + * If the git_dir is not directly inside the working tree, then git will not
> + * find it by default, and we need to set the worktree explicitly.
> + */
> +static int needs_work_tree_config(const char *git_dir, const char *work_tree)
> +{
> +	/* special case that is missed by the general rule below */

(optional) I'd leave out this comment --- it seems obvious enough in
context and the purpose of the comment is unobvious without looking
at the history.

> +	if (!strcmp(work_tree, "/") && !strcmp(git_dir, "/.git"))
> +		return 0;
> +	if (skip_prefix(git_dir, work_tree, &git_dir) &&
> +	    !strcmp(git_dir, "/.git"))
> +		return 0;

work_tree has been cleaned up with real_path, so in the normal case it
contains getcwd() output which will not end with a / unless it has to.
The only exception I can see is when git hits the MAXDEPTH limit for
symlink resolution (5 nested symlinks), in which case we take what we
find instead of erroring out, which looks like a bug.

We have called set_git_dir_init so git_dir has been cleaned up by
real_path in the same way.  Good.

With or without the commit message, comment, and test improvements
mentioned above,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] init: don't set core.worktree when initializing /.git
  2015-03-31 19:14     ` Jonathan Nieder
@ 2015-04-02 18:37       ` Jeff King
  2015-04-02 18:49         ` Jonathan Nieder
  2015-04-03 10:08       ` [PATCH] t1509: update prepare script to be able to run t1509 in chroot again Nguyễn Thái Ngọc Duy
  1 sibling, 1 reply; 11+ messages in thread
From: Jeff King @ 2015-04-02 18:37 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Cedric Gava, git, Nguyễn Thái Ngọc Duy

On Tue, Mar 31, 2015 at 12:14:20PM -0700, Jonathan Nieder wrote:

> > No tests, as we would need to be able to write to "/" to do
> > so.
> 
> t1509-root-worktree.sh is supposed to test the repository-at-/ case.
> But I wouldn't be surprised if it's bitrotted, since people don't set
> up a throwaway chroot or VM for tests too often.

Thanks, I had thought we had such a test script, but for some reason had
trouble finding it earlier. I couldn't get it to run using the included
t1509/prepare-chroot.sh helper; the resulting chroot was missing several
tools we rely on in test-lib.sh (perl, expr, tr, and mv).

I gave up for now; I don't think rescuing that script is worth the
effort for this minor bug.

> > The current behavior isn't _wrong_, in the sense that it's OK to set
> > core.worktree when we don't need to. But I think it is unnecessarily
> > confusing to users who expect to be able to move .git directories
> > around, because it usually just works. So I'd call this an extremely
> > minor bug.
> 
> This belongs in the commit message.

It's mostly a restatement of what is already in the first paragraph of
the commit message. I folded it into that paragraph.

> > +static int needs_work_tree_config(const char *git_dir, const char *work_tree)
> > +{
> > +	/* special case that is missed by the general rule below */
> 
> (optional) I'd leave out this comment --- it seems obvious enough in
> context and the purpose of the comment is unobvious without looking
> at the history.

OK, dropped. Here's the re-roll.

-- >8 --
Subject: [PATCH] init: don't set core.worktree when initializing /.git

If you create a git repository in the root directory with
"git init /", we erroneously write a core.worktree entry.
This isn't _wrong_, in the sense that it's OK to set
core.worktree when we don't need to. But it is unnecessarily
surprising if you later move the .git directory to another
path (which usually moves the relative working tree, but is
foiled if there is an explicit worktree set).

The problem is that we check whether core.worktree is
necessary by seeing if we can make the git_dir by
concatenating "/.git" onto the working tree. That would lead
to "//.git" in this instance, but we actually have "/.git"
(without the doubled slash).

We can fix this by special-casing the root directory. I also
split the logic out into its own function to make the
conditional a bit more readable (and used skip_prefix, which
I think makes it a little more obvious what is going on).

No tests, as we would need to be able to write to "/" to do
so. I did manually confirm that:

  sudo git init /
  cd /
  git rev-parse --show-toplevel
  git config core.worktree

still finds the top-level correctly (as "/"), and does not
set any core.worktree variable.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/init-db.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 6723d39..ab9f86b 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -182,6 +182,20 @@ static int git_init_db_config(const char *k, const char *v, void *cb)
 	return 0;
 }
 
+/*
+ * If the git_dir is not directly inside the working tree, then git will not
+ * find it by default, and we need to set the worktree explicitly.
+ */
+static int needs_work_tree_config(const char *git_dir, const char *work_tree)
+{
+	if (!strcmp(work_tree, "/") && !strcmp(git_dir, "/.git"))
+		return 0;
+	if (skip_prefix(git_dir, work_tree, &git_dir) &&
+	    !strcmp(git_dir, "/.git"))
+		return 0;
+	return 1;
+}
+
 static int create_default_files(const char *template_path)
 {
 	const char *git_dir = get_git_dir();
@@ -274,10 +288,8 @@ static int create_default_files(const char *template_path)
 		/* allow template config file to override the default */
 		if (log_all_ref_updates == -1)
 		    git_config_set("core.logallrefupdates", "true");
-		if (!starts_with(git_dir, work_tree) ||
-		    strcmp(git_dir + strlen(work_tree), "/.git")) {
+		if (needs_work_tree_config(git_dir, work_tree))
 			git_config_set("core.worktree", work_tree);
-		}
 	}
 
 	if (!reinit) {
-- 
2.4.0.rc0.363.gf9f328b

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] init: don't set core.worktree when initializing /.git
  2015-04-02 18:37       ` [PATCH v2] " Jeff King
@ 2015-04-02 18:49         ` Jonathan Nieder
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Nieder @ 2015-04-02 18:49 UTC (permalink / raw)
  To: Jeff King; +Cc: Cedric Gava, git, Nguyễn Thái Ngọc Duy

Jeff King wrote:

> OK, dropped. Here's the re-roll.

Thanks.

> -- >8 --
> Subject: [PATCH] init: don't set core.worktree when initializing /.git
[...]
> No tests, as we would need to be able to write to "/" to do
> so.

This is confusing in the light of t1509-root-worktree.sh existing, but
anyone curious can dig up this mailing list thread so I don't mind. :)

[...]
> Signed-off-by: Jeff King <peff@peff.net>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Yep - still looks good.

Regards,
Jonathan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] t1509: update prepare script to be able to run t1509 in chroot again
  2015-03-31 19:14     ` Jonathan Nieder
  2015-04-02 18:37       ` [PATCH v2] " Jeff King
@ 2015-04-03 10:08       ` Nguyễn Thái Ngọc Duy
  2015-04-03 12:01         ` Jeff King
  1 sibling, 1 reply; 11+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2015-04-03 10:08 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Jonathan Niedier,
	Nguyễn Thái Ngọc Duy

Tested on Gentoo and OpenSUSE 13.1, both x86-64

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 On Wed, Apr 1, 2015 at 2:14 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
 > Jeff King wrote:
 >
 >> No tests, as we would need to be able to write to "/" to do
 >> so.
 >
 > t1509-root-worktree.sh is supposed to test the repository-at-/ case.
 > But I wouldn't be surprised if it's bitrotted, since people don't set
 > up a throwaway chroot or VM for tests too often.

 Can't leave it rotting. Either fix it or kill it. This is the first
 option. Good news is the test passes, nothing else is broken. Bad
 news is it does not detect the core.worktree breakage, but this on
 top would verify that Jeff's patch works

  diff --git a/t/t1509-root-worktree.sh b/t/t1509-root-worktree.sh
  index b6977d4..17ff4ce 100755
  --- a/t/t1509-root-worktree.sh
  +++ b/t/t1509-root-worktree.sh
  @@ -224,6 +224,10 @@ test_expect_success 'setup' '
   	test_cmp expected result
   '
   
  +test_expect_success 'no core.worktree after git init' '
  +	test "`git config core.worktree`" = ""
  +'
  +
   test_vars 'auto gitdir, root' ".git" "/" ""
   test_foobar_root
 


 t/t1509/prepare-chroot.sh | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/t/t1509/prepare-chroot.sh b/t/t1509/prepare-chroot.sh
index 6269117..c61afbf 100755
--- a/t/t1509/prepare-chroot.sh
+++ b/t/t1509/prepare-chroot.sh
@@ -14,25 +14,42 @@ xmkdir() {
 
 R="$1"
 
+[ "$UID" -eq 0 ] && die "This script should not be run as root, what if it does rm -rf /?"
 [ -n "$R" ] || die "usage: prepare-chroot.sh <root>"
 [ -x git ] || die "This script needs to be executed at git source code's top directory"
-[ -x /bin/busybox ] || die "You need busybox"
+if [ -x /bin/busybox ]; then
+	BB=/bin/busybox
+elif [ -x /usr/bin/busybox ]; then
+	BB=/usr/bin/busybox
+else
+	die "You need busybox"
+fi
 
 xmkdir "$R" "$R/bin" "$R/etc" "$R/lib" "$R/dev"
-[ -c "$R/dev/null" ] || die "/dev/null is missing. Do mknod $R/dev/null c 1 3 && chmod 666 $R/dev/null"
+touch "$R/dev/null"
 echo "root:x:0:0:root:/:/bin/sh" > "$R/etc/passwd"
 echo "$(id -nu):x:$(id -u):$(id -g)::$(pwd)/t:/bin/sh" >> "$R/etc/passwd"
 echo "root::0:root" > "$R/etc/group"
 echo "$(id -ng)::$(id -g):$(id -nu)" >> "$R/etc/group"
 
-[ -x "$R/bin/busybox" ] || cp /bin/busybox "$R/bin/busybox"
-[ -x "$R/bin/sh" ] || ln -s /bin/busybox "$R/bin/sh"
-[ -x "$R/bin/su" ] || ln -s /bin/busybox "$R/bin/su"
+[ -x "$R$BB" ] || cp $BB "$R/bin/busybox"
+for cmd in sh su ls expr tr basename rm mkdir mv id uname dirname cat true sed diff; do
+	ln -f -s /bin/busybox "$R/bin/$cmd"
+done
 
 mkdir -p "$R$(pwd)"
 rsync --exclude-from t/t1509/excludes -Ha . "$R$(pwd)"
-ldd git | grep '/' | sed 's,.*\s\(/[^ ]*\).*,\1,' | while read i; do
-	mkdir -p "$R$(dirname $i)"
-	cp "$i" "$R/$i"
+# Fake perl to reduce dependency, t1509 does not use perl, but some
+# env might slip through, see test-lib.sh, unset.*PERL_PATH
+sed 's|^PERL_PATH=*|PERL_PATH=/bin/true|' GIT-BUILD-OPTIONS > "$R$(pwd)/GIT-BUILD-OPTIONS"
+for cmd in git $BB;do 
+	ldd $cmd | grep '/' | sed 's,.*\s\(/[^ ]*\).*,\1,' | while read i; do
+		mkdir -p "$R$(dirname $i)"
+		cp "$i" "$R/$i"
+	done
 done
-echo "Execute this in root: 'chroot $R /bin/su - $(id -nu)'"
+cat <<EOF
+Execute this in root:
+chroot $R /bin/su - $(id -nu)
+IKNOWWHATIAMDOING=YES ./t1509-root-worktree.sh -v -i
+EOF
-- 
2.3.0.rc1.137.g477eb31

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] t1509: update prepare script to be able to run t1509 in chroot again
  2015-04-03 10:08       ` [PATCH] t1509: update prepare script to be able to run t1509 in chroot again Nguyễn Thái Ngọc Duy
@ 2015-04-03 12:01         ` Jeff King
  2015-04-03 12:14           ` Duy Nguyen
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff King @ 2015-04-03 12:01 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Jonathan Niedier

On Fri, Apr 03, 2015 at 05:08:57PM +0700, Nguyễn Thái Ngọc Duy wrote:

> diff --git a/t/t1509/prepare-chroot.sh b/t/t1509/prepare-chroot.sh
> index 6269117..c61afbf 100755
> --- a/t/t1509/prepare-chroot.sh
> +++ b/t/t1509/prepare-chroot.sh
> @@ -14,25 +14,42 @@ xmkdir() {
>  
>  R="$1"
>  
> +[ "$UID" -eq 0 ] && die "This script should not be run as root, what if it does rm -rf /?"

This line complains for me. $UID is set in my bash login shell, but not
in the dash shell for this script. Maybe "id -u" instead?

>  xmkdir "$R" "$R/bin" "$R/etc" "$R/lib" "$R/dev"
> -[ -c "$R/dev/null" ] || die "/dev/null is missing. Do mknod $R/dev/null c 1 3 && chmod 666 $R/dev/null"
> +touch "$R/dev/null"

It is nice not to mknod here, as it requires root. But making /dev/null
a regular file seems a bit flaky. We will constantly overwrite it with
the output of each test, and then pipe that output back into the next
test. The current set of tests in t1509 does not seem to mind, though.

> +# Fake perl to reduce dependency, t1509 does not use perl, but some
> +# env might slip through, see test-lib.sh, unset.*PERL_PATH
> +sed 's|^PERL_PATH=*|PERL_PATH=/bin/true|' GIT-BUILD-OPTIONS > "$R$(pwd)/GIT-BUILD-OPTIONS"

Re-preparing an already-made chroot makes this:

  PERL_PATH=/bin/true'/usr/bin/perl'

Did you want "PERL_PATH=.*" as your regex?

> -echo "Execute this in root: 'chroot $R /bin/su - $(id -nu)'"
> +cat <<EOF
> +Execute this in root:
> +chroot $R /bin/su - $(id -nu)
> +IKNOWWHATIAMDOING=YES ./t1509-root-worktree.sh -v -i
> +EOF

I found the "in root" here (and in the original) confusing. Do you mean
"as root"? I wonder if it would make sense to just show:

  sudo chroot $R /bin/su - $(id -nu)

as the sample command.

Aside from the nits above, I did get it to run t1509 with this. I can't
say I'm incredibly excited about the whole thing, if only because it is
clear that nobody is going to run it regularly (so regressions will
likely go unnoticed, which is the whole point of the test script).  But
perhaps it is better than nothing, and it is not hurting anyone to sit
there and bitrot again. ;)

-Peff

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] t1509: update prepare script to be able to run t1509 in chroot again
  2015-04-03 12:01         ` Jeff King
@ 2015-04-03 12:14           ` Duy Nguyen
  2015-04-11 19:58             ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Duy Nguyen @ 2015-04-03 12:14 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List, Jonathan Niedier

On Fri, Apr 3, 2015 at 7:01 PM, Jeff King <peff@peff.net> wrote:
> Aside from the nits above, I did get it to run t1509 with this. I can't
> say I'm incredibly excited about the whole thing, if only because it is
> clear that nobody is going to run it regularly (so regressions will
> likely go unnoticed, which is the whole point of the test script).  But
> perhaps it is better than nothing, and it is not hurting anyone to sit
> there and bitrot again. ;)

To be honest, I didn't run it often in the last 5 years. The code it
protects seems not broken during this time and probably so for a
foreseeable future. So I don't mind if you just kill t1509 and related
scripts.
-- 
Duy

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] t1509: update prepare script to be able to run t1509 in chroot again
  2015-04-03 12:14           ` Duy Nguyen
@ 2015-04-11 19:58             ` Junio C Hamano
  2015-04-18 11:22               ` [PATCH v2] " Nguyễn Thái Ngọc Duy
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2015-04-11 19:58 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Jeff King, Git Mailing List, Jonathan Niedier

Duy Nguyen <pclouds@gmail.com> writes:

> On Fri, Apr 3, 2015 at 7:01 PM, Jeff King <peff@peff.net> wrote:
>> Aside from the nits above, I did get it to run t1509 with this. I can't
>> say I'm incredibly excited about the whole thing, if only because it is
>> clear that nobody is going to run it regularly (so regressions will
>> likely go unnoticed, which is the whole point of the test script).  But
>> perhaps it is better than nothing, and it is not hurting anyone to sit
>> there and bitrot again. ;)
>
> To be honest, I didn't run it often in the last 5 years. The code it
> protects seems not broken during this time and probably so for a
> foreseeable future. So I don't mind if you just kill t1509 and related
> scripts.

Yeah, but as long as you two worked out to make it run again on at
least two systems (i.e. yours), it would be good not to let that
effort go to waste. Care to throw a v2 at me with $(id -u) and other
changes Peff mentioned?

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] t1509: update prepare script to be able to run t1509 in chroot again
  2015-04-11 19:58             ` Junio C Hamano
@ 2015-04-18 11:22               ` Nguyễn Thái Ngọc Duy
  0 siblings, 0 replies; 11+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2015-04-18 11:22 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Jonathan Niedier,
	Nguyễn Thái Ngọc Duy

Tested on Gentoo and OpenSUSE 13.1, both x86-64

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 On Sun, Apr 12, 2015 at 2:58 AM, Junio C Hamano <gitster@pobox.com> wrote:
 > Duy Nguyen <pclouds@gmail.com> writes:
 >
 >> On Fri, Apr 3, 2015 at 7:01 PM, Jeff King <peff@peff.net> wrote:
 >>> Aside from the nits above, I did get it to run t1509 with this. I can't
 >>> say I'm incredibly excited about the whole thing, if only because it is
 >>> clear that nobody is going to run it regularly (so regressions will
 >>> likely go unnoticed, which is the whole point of the test script).  But
 >>> perhaps it is better than nothing, and it is not hurting anyone to sit
 >>> there and bitrot again. ;)
 >>
 >> To be honest, I didn't run it often in the last 5 years. The code it
 >> protects seems not broken during this time and probably so for a
 >> foreseeable future. So I don't mind if you just kill t1509 and related
 >> scripts.
 >
 > Yeah, but as long as you two worked out to make it run again on at
 > least two systems (i.e. yours), it would be good not to let that
 > effort go to waste. Care to throw a v2 at me with $(id -u) and other
 > changes Peff mentioned?

 thrown (a little bit late because $DAYJOB consumed much of my energy)

 t/t1509/prepare-chroot.sh | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/t/t1509/prepare-chroot.sh b/t/t1509/prepare-chroot.sh
index 6269117..6d47e2c 100755
--- a/t/t1509/prepare-chroot.sh
+++ b/t/t1509/prepare-chroot.sh
@@ -14,25 +14,45 @@ xmkdir() {
 
 R="$1"
 
+[ "$(id -u)" -eq 0 ] && die "This script should not be run as root, what if it does rm -rf /?"
 [ -n "$R" ] || die "usage: prepare-chroot.sh <root>"
 [ -x git ] || die "This script needs to be executed at git source code's top directory"
-[ -x /bin/busybox ] || die "You need busybox"
+if [ -x /bin/busybox ]; then
+	BB=/bin/busybox
+elif [ -x /usr/bin/busybox ]; then
+	BB=/usr/bin/busybox
+else
+	die "You need busybox"
+fi
 
 xmkdir "$R" "$R/bin" "$R/etc" "$R/lib" "$R/dev"
-[ -c "$R/dev/null" ] || die "/dev/null is missing. Do mknod $R/dev/null c 1 3 && chmod 666 $R/dev/null"
+touch "$R/dev/null"
 echo "root:x:0:0:root:/:/bin/sh" > "$R/etc/passwd"
 echo "$(id -nu):x:$(id -u):$(id -g)::$(pwd)/t:/bin/sh" >> "$R/etc/passwd"
 echo "root::0:root" > "$R/etc/group"
 echo "$(id -ng)::$(id -g):$(id -nu)" >> "$R/etc/group"
 
-[ -x "$R/bin/busybox" ] || cp /bin/busybox "$R/bin/busybox"
-[ -x "$R/bin/sh" ] || ln -s /bin/busybox "$R/bin/sh"
-[ -x "$R/bin/su" ] || ln -s /bin/busybox "$R/bin/su"
+[ -x "$R$BB" ] || cp $BB "$R/bin/busybox"
+for cmd in sh su ls expr tr basename rm mkdir mv id uname dirname cat true sed diff; do
+	ln -f -s /bin/busybox "$R/bin/$cmd"
+done
 
 mkdir -p "$R$(pwd)"
 rsync --exclude-from t/t1509/excludes -Ha . "$R$(pwd)"
-ldd git | grep '/' | sed 's,.*\s\(/[^ ]*\).*,\1,' | while read i; do
-	mkdir -p "$R$(dirname $i)"
-	cp "$i" "$R/$i"
+# Fake perl to reduce dependency, t1509 does not use perl, but some
+# env might slip through, see test-lib.sh, unset.*PERL_PATH
+sed 's|^PERL_PATH=.*|PERL_PATH=/bin/true|' GIT-BUILD-OPTIONS > "$R$(pwd)/GIT-BUILD-OPTIONS"
+for cmd in git $BB;do 
+	ldd $cmd | grep '/' | sed 's,.*\s\(/[^ ]*\).*,\1,' | while read i; do
+		mkdir -p "$R$(dirname $i)"
+		cp "$i" "$R/$i"
+	done
 done
-echo "Execute this in root: 'chroot $R /bin/su - $(id -nu)'"
+cat <<EOF
+All is set up in $R, execute t1509 with the following commands:
+
+sudo chroot $R /bin/su - $(id -nu)
+IKNOWWHATIAMDOING=YES ./t1509-root-worktree.sh -v -i
+
+When you are done, simply delete $R to clean up
+EOF
-- 
2.3.0.rc1.137.g477eb31

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-04-18 11:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-31  9:25 Forcing git top-level Cedric Gava
2015-03-31 18:15 ` Jeff King
2015-03-31 18:34   ` [PATCH] init: don't set core.worktree when initializing /.git Jeff King
2015-03-31 19:14     ` Jonathan Nieder
2015-04-02 18:37       ` [PATCH v2] " Jeff King
2015-04-02 18:49         ` Jonathan Nieder
2015-04-03 10:08       ` [PATCH] t1509: update prepare script to be able to run t1509 in chroot again Nguyễn Thái Ngọc Duy
2015-04-03 12:01         ` Jeff King
2015-04-03 12:14           ` Duy Nguyen
2015-04-11 19:58             ` Junio C Hamano
2015-04-18 11:22               ` [PATCH v2] " Nguyễn Thái Ngọc Duy

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).