[PATCH] checkout-index needs a working tree

[PATCH] checkout-index needs a working tree

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	This fixes "git --work-tree=/some/where/else checkout-index".

 git.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git.c b/git.c
index 25b8274..f8c4545 100644
--- a/git.c
+++ b/git.c
@@ -315,7 +315,8 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "branch", cmd_branch, RUN_SETUP },
 		{ "bundle", cmd_bundle },
 		{ "cat-file", cmd_cat_file, RUN_SETUP },
-		{ "checkout-index", cmd_checkout_index, RUN_SETUP },
+		{ "checkout-index", cmd_checkout_index,
+			RUN_SETUP | NEED_WORK_TREE},
 		{ "check-ref-format", cmd_check_ref_format },
 		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
 		{ "cherry", cmd_cherry, RUN_SETUP },
-- 
1.5.3.rc4.1.g7805

Re: [PATCH] checkout-index needs a working tree

[Junio C Hamano]

Hmmph.  I was trying to come up with a better commit log message
for this change.

	Paths given from the command line of checkout-index name
	files relative to the cwd, whose implication is that it
	is relative to where you are in relation with the top of
	the working tree.  For doing that, you need to have the
	work tree to begin with.

Does this mean that any command that uses its prefix parameter
to cmd_xxx() needs NEED_WORK_TREE?

I wonder if it would help us to catch similar breakages if we
change git.c::run_command() so that we do not pass prefix (or
pass a bogus pointer ((const char *)1)) for commands that do not
ask for NEED_WORK_TREE.

Re: [PATCH] checkout-index needs a working tree

[Johannes Schindelin]

Hi,

On Sat, 4 Aug 2007, Junio C Hamano wrote:

> I wonder if it would help us to catch similar breakages if we change 
> git.c::run_command() so that we do not pass prefix (or pass a bogus 
> pointer ((const char *)1)) for commands that do not ask for 
> NEED_WORK_TREE.

I tried that, but we have some places where we ask "if (prefix && 
*prefix)", for example in ls-tree.  It does not _require_ a work tree, but 
it certainly uses it when it is available -- which is fine.

Other users are more tricky, such as ls-files and update-index, which need 
a working tree only in some cases.

I'll probably write a patch on Monday (if nobody else is faster) to 
provide a function "require_work_tree()" in environment.c, which does the 
obvious (with caching).  Then comes the tedious part: identifying all 
those options that need a working tree...

Ciao,
Dscho

Re: [PATCH] checkout-index needs a working tree

[Uwe Kleine-König]

Johannes Schindelin wrote:
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
> 	This fixes "git --work-tree=/some/where/else checkout-index".
> 
>  git.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/git.c b/git.c
> index 25b8274..f8c4545 100644
> --- a/git.c
> +++ b/git.c
> @@ -315,7 +315,8 @@ static void handle_internal_command(int argc, const char **argv)
>  		{ "branch", cmd_branch, RUN_SETUP },
>  		{ "bundle", cmd_bundle },
>  		{ "cat-file", cmd_cat_file, RUN_SETUP },
> -		{ "checkout-index", cmd_checkout_index, RUN_SETUP },
> +		{ "checkout-index", cmd_checkout_index,
> +			RUN_SETUP | NEED_WORK_TREE},
>  		{ "check-ref-format", cmd_check_ref_format },
>  		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
>  		{ "cherry", cmd_cherry, RUN_SETUP },
With this patch I'm not able to do

	git checkout-index --prefix=/tmp/exportdir -a

to export an entire tree as described in git-checkout-index(1) in a bare
repo.

(Not that I need it, but I claimed on #git that it works, ... )

BTW: I didn't try if reverting this patch helps, but probably it does.

Best regards
Uwe

-- 
Uwe Kleine-König

http://www.google.com/search?q=72+PS+point+in+inch

Re: [PATCH] checkout-index needs a working tree

[Johannes Schindelin]

Hi,

On Fri, 10 Aug 2007, Uwe Kleine-K?nig wrote:

> Johannes Schindelin wrote:
> > 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > 
> > 	This fixes "git --work-tree=/some/where/else checkout-index".
> > 
> >  git.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/git.c b/git.c
> > index 25b8274..f8c4545 100644
> > --- a/git.c
> > +++ b/git.c
> > @@ -315,7 +315,8 @@ static void handle_internal_command(int argc, const char **argv)
> >  		{ "branch", cmd_branch, RUN_SETUP },
> >  		{ "bundle", cmd_bundle },
> >  		{ "cat-file", cmd_cat_file, RUN_SETUP },
> > -		{ "checkout-index", cmd_checkout_index, RUN_SETUP },
> > +		{ "checkout-index", cmd_checkout_index,
> > +			RUN_SETUP | NEED_WORK_TREE},
> >  		{ "check-ref-format", cmd_check_ref_format },
> >  		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
> >  		{ "cherry", cmd_cherry, RUN_SETUP },
> With this patch I'm not able to do
> 
> 	git checkout-index --prefix=/tmp/exportdir -a
> 
> to export an entire tree as described in git-checkout-index(1) in a bare
> repo.

That is _completely_ expected.  If it is a bare repository, you can _only_ 
override that check by GIT_WORK_TREE or --work-tree.

But I have to wonder: if you want to use git checkout-index, which is a 
work-tree operation, why did you mark it as bare to begin with?

Ciao,
Dscho

Re: [PATCH] checkout-index needs a working tree

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> That is _completely_ expected.  If it is a bare repository, you can _only_ 
> override that check by GIT_WORK_TREE or --work-tree.
>
> But I have to wonder: if you want to use git checkout-index, which is a 
> work-tree operation, why did you mark it as bare to begin with?

I do not necessarily think --prefix=untar/it/here/ is a work
tree operation.

Perhaps we probably are better off if we add something that says
specifying GIT_DIR alone means you are at the top of work tree
(to resurrect the traditional behaviour), to alleviate fallouts
like this and the other cvsserver one?  If one does not like
that traditional behaviour, the new GIT_WORK_TREE support can
be used override it.

Re: [PATCH] checkout-index needs a working tree

[Johannes Schindelin]

Hi,

On Thu, 9 Aug 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > That is _completely_ expected.  If it is a bare repository, you can _only_ 
> > override that check by GIT_WORK_TREE or --work-tree.
> >
> > But I have to wonder: if you want to use git checkout-index, which is a 
> > work-tree operation, why did you mark it as bare to begin with?
> 
> I do not necessarily think --prefix=untar/it/here/ is a work
> tree operation.
> 
> Perhaps we probably are better off if we add something that says
> specifying GIT_DIR alone means you are at the top of work tree
> (to resurrect the traditional behaviour), to alleviate fallouts
> like this and the other cvsserver one?  If one does not like
> that traditional behaviour, the new GIT_WORK_TREE support can
> be used override it.

I think I sent a patch for that, but was negative about it, even if I 
promised not to question your decision.

Ciao,
Dscho

[PATCH] Reinstate the old behaviour when GIT_DIR is set and GIT_WORK_TREE is unset

[Junio C Hamano]

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Sun, 5 Aug 2007 14:12:53 +0100

The old behaviour was to unilaterally default to the cwd is the work tree
when GIT_DIR was set, but GIT_WORK_TREE wasn't, no matter if we are inside
the GIT_DIR, or if GIT_DIR is actually something like ../../../.git.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

 > I think I sent a patch for that, but was negative about it, even if I 
 > promised not to question your decision.

 Yes you did.  Here is a refresher with an affected test
 adjusted.

 We already have a few changes that worked around the semantics
 change by either setting GIT_WORK_TREE or cd'ing up, but I
 think they should not need to be reverted.

 It makes more sense to keep the old semantics -- people who use
 unusual GIT_DIR setting should know what they are doing, and
 the new GIT_WORK_TREE feature (and core.worktree) would give
 them better control.  We just should not break existing users
 that set GIT_DIR and nothing else.  Which means I need another
 rewrite on the Release Notes, and probably yet another rc
 cycle.

 setup.c              |   52 +++++++++----------------------------------------
 t/t1500-rev-parse.sh |    6 +++-
 2 files changed, 14 insertions(+), 44 deletions(-)

diff --git a/setup.c b/setup.c
index b55b82c..06004f1 100644
--- a/setup.c
+++ b/setup.c
@@ -189,53 +189,21 @@ int is_inside_work_tree(void)
 }
 
 /*
- * If no worktree was given, and we are outside of a default work tree,
- * now is the time to set it.
- *
- * In other words, if the user calls git with something like
- *
- *	git --git-dir=/some/where/else/.git bla
- *
- * default to /some/where/else as working directory; if the specified
- * git-dir does not end in "/.git", the cwd is used as working directory.
+ * set_work_tree() is only ever called if you set GIT_DIR explicitely.
+ * The old behaviour (which we retain here) is to set the work tree root
+ * to the cwd, unless overridden by the config, the command line, or
+ * GIT_WORK_TREE.
  */
-const char *set_work_tree(const char *dir)
+static const char *set_work_tree(const char *dir)
 {
-	char dir_buffer[PATH_MAX], *rel = NULL;
-	static char buffer[PATH_MAX + 1];
-	int len, suffix_len = strlen(DEFAULT_GIT_DIR_ENVIRONMENT) + 1;
-
-	/* strip the variable 'dir' of the postfix "/.git" if it has it */
-	len = strlen(dir);
-	if (len > suffix_len &&
-	    !strcmp(dir + len - suffix_len, "/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
-		if ((len - suffix_len) >= sizeof(dir_buffer))
-			die("directory name too long");
-		memcpy(dir_buffer, dir, len - suffix_len);
-		dir_buffer[len - suffix_len] = '\0';
-
-		/* are we inside the default work tree? */
-		rel = get_relative_cwd(buffer, sizeof(buffer), dir_buffer);
-	}
+	char buffer[PATH_MAX + 1];
 
-	/* if rel is set, the cwd is _not_ the current working tree */
-	if (rel && *rel) {
-		if (!is_absolute_path(dir))
-			set_git_dir(make_absolute_path(dir));
-		dir = dir_buffer;
-		if (chdir(dir))
-			die("cannot chdir to %s: %s", dir, strerror(errno));
-		else
-			strcat(rel, "/");
-		inside_git_dir = 0;
-	} else {
-		rel = NULL;
-		dir = getcwd(buffer, sizeof(buffer));
-	}
-	git_work_tree_cfg = xstrdup(dir);
+	if (!getcwd(buffer, sizeof(buffer)))
+		die ("Could not get the current working directory");
+	git_work_tree_cfg = xstrdup(buffer);
 	inside_work_tree = 1;
 
-	return rel;
+	return NULL;
 }
 
 /*
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index bea40cb..e474b3f 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -28,6 +28,8 @@ test_rev_parse() {
 	[ $# -eq 0 ] && return
 }
 
+# label is-bare is-inside-git is-inside-work prefix
+
 test_rev_parse toplevel false false true ''
 
 cd .git || exit 1
@@ -53,13 +55,13 @@ export GIT_DIR=../.git
 export GIT_CONFIG="$(pwd)"/../.git/config
 
 git config core.bare false
-test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true work/
+test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true ''
 
 git config core.bare true
 test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false false ''
 
 git config --unset core.bare
-test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true work/
+test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true ''
 
 mv ../.git ../repo.git || exit 1
 export GIT_DIR=../repo.git
-- 
1.5.3.rc4.29.g74276

Re: [PATCH] Reinstate the old behaviour when GIT_DIR is set and GIT_WORK_TREE is unset

[Uwe Kleine-König]

Hello Junio,

Junio C Hamano wrote:
> From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Date: Sun, 5 Aug 2007 14:12:53 +0100
> 
> The old behaviour was to unilaterally default to the cwd is the work tree
> when GIT_DIR was set, but GIT_WORK_TREE wasn't, no matter if we are inside
> the GIT_DIR, or if GIT_DIR is actually something like ../../../.git.
> 
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>  > I think I sent a patch for that, but was negative about it, even if I 
>  > promised not to question your decision.
> 
>  Yes you did.  Here is a refresher with an affected test
>  adjusted.
> 
>  We already have a few changes that worked around the semantics
>  change by either setting GIT_WORK_TREE or cd'ing up, but I
>  think they should not need to be reverted.
> 
>  It makes more sense to keep the old semantics -- people who use
>  unusual GIT_DIR setting should know what they are doing, and
>  the new GIT_WORK_TREE feature (and core.worktree) would give
>  them better control.  We just should not break existing users
>  that set GIT_DIR and nothing else.  Which means I need another
>  rewrite on the Release Notes, and probably yet another rc
>  cycle.

I don't know if you planed to make 

	git checkout-index --prefix=/tmp/tra -a

work (again) in a bare repo.  Probably not, so it's no surprise that it
still doesn't work.

Best regards
Uwe

-- 
Uwe Kleine-König

cat /*dev/null; echo 'Hello World!';
cat > /dev/null <<*/ 
() { } int main() { printf("Hello World!\n");}
/* */

Re: [PATCH] Reinstate the old behaviour when GIT_DIR is set and GIT_WORK_TREE is unset

[Junio C Hamano]

Uwe Kleine-König  <ukleinek@informatik.uni-freiburg.de> writes:

> I don't know if you planed to make 
>
> 	git checkout-index --prefix=/tmp/tra -a
>
> work (again) in a bare repo.  Probably not, so it's no surprise that it
> still doesn't work.

I think it is now safe to revert that NEED_WORK_TREE change
in git.c for checkout-index.

Re: [PATCH] Reinstate the old behaviour when GIT_DIR is set and GIT_WORK_TREE is unset

[Junio C Hamano]

Subject: [PATCH] Revert 4465f410 and add tests

checkout-index --prefix=<path> does not need a working tree,
even when run from inside a bare repository.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 Junio C Hamano <gitster@pobox.com> writes:

 > Uwe Kleine-König  <ukleinek@informatik.uni-freiburg.de> writes:
 >
 >> I don't know if you planed to make 
 >>
 >> 	git checkout-index --prefix=/tmp/tra -a
 >>
 >> work (again) in a bare repo.  Probably not, so it's no surprise that it
 >> still doesn't work.
 >
 > I think it is now safe to revert that NEED_WORK_TREE change
 > in git.c for checkout-index.

 How does this look?  I think the second test (no GIT_DIR, just
 being in a bare repository and specifying --prefix to redirect
 the output elsewhere) never worked even before the current
 work-tree feature was started (none of v1.4.4.4, v1.5.0, nor
 v1.5.2 worked that way), so this is not even "work again", but
 more like "now works".  Later tests with exported GIT_DIR would
 work with or without reverting the NEED_WORK_TREE change in
 git.c, so in that sense reverting is probably not such a good
 idea to begin with.  I dunno.

 git.c                            |    3 +-
 t/t2008-checkout-index-prefix.sh |   78 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100755 t/t2008-checkout-index-prefix.sh

diff --git a/git.c b/git.c
index f8c4545..bafbb4b 100644
--- a/git.c
+++ b/git.c
@@ -315,8 +315,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "branch", cmd_branch, RUN_SETUP },
 		{ "bundle", cmd_bundle },
 		{ "cat-file", cmd_cat_file, RUN_SETUP },
-		{ "checkout-index", cmd_checkout_index,
-			RUN_SETUP | NEED_WORK_TREE},
+		{ "checkout-index", cmd_checkout_index, RUN_SETUP},
 		{ "check-ref-format", cmd_check_ref_format },
 		{ "check-attr", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },
 		{ "cherry", cmd_cherry, RUN_SETUP },
diff --git a/t/t2008-checkout-index-prefix.sh b/t/t2008-checkout-index-prefix.sh
new file mode 100755
index 0000000..7727f07
--- /dev/null
+++ b/t/t2008-checkout-index-prefix.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='checkout-index --prefix=foo/'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	mkdir filfre &&
+	>frotz &&
+	>filfre/nitfol &&
+	git update-index --add frotz filfre/nitfol
+
+'
+
+rm -fr f*
+
+mv .git repo.git || exit ;# very bad
+GIT_DIR="$(pwd)/repo.git"
+GIT_CONFIG="$GIT_DIR/config" git config core.bare true
+
+test_expect_success 'checkout with --prefix' '
+
+	(
+		cd repo.git &&
+		git checkout-index --prefix=../f- -a
+	) &&
+	test -f f-frotz &&
+	test -d f-filfre &&
+	test -f f-filfre/nitfol
+
+'
+
+export GIT_DIR
+
+rm -fr f*
+
+test_expect_success 'checkout with --prefix and GIT_DIR (0)' '
+
+	(
+		cd repo.git &&
+		git checkout-index --prefix=../f/ -a
+	) &&
+	test -d f &&
+	test -f f/frotz &&
+	test -d f/filfre &&
+	test -f f/filfre/nitfol
+
+'
+
+rm -fr f*
+
+test_expect_success 'checkout with --prefix and GIT_DIR (1)' '
+
+	git checkout-index --prefix=f/ -a &&
+	test -d f &&
+	test -f f/frotz &&
+	test -d f/filfre &&
+	test -f f/filfre/nitfol
+
+'
+
+rm -fr f*
+
+test_expect_success 'checkout with --prefix and GIT_DIR (2)' '
+
+	mkdir f &&
+	(
+		cd f && git checkout-index --prefix=../f1/ -a
+	) &&
+	test -d f1 &&
+	test -f f1/frotz &&
+	test -d f1/filfre &&
+	test -f f1/filfre/nitfol
+
+'
+
+test_done

[PATCH] checkout-index doc: use --work-dir in the export example

[Uwe Kleine-König]

The old example used --prefix=.../ to teach the reader that the trailing
slash is important.  This lesson just moved down to the next example,
without a real example though.

Signed-off-by: Uwe Kleine-König <ukleinek@informatik.uni-freiburg.de>
---
Hello Junio,

even if you revert 4465f410 (and add the tests) I'd prefer using
--work-tree.

Best regards
Uwe

 Documentation/git-checkout-index.txt |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-checkout-index.txt b/Documentation/git-checkout-index.txt
index b1a8ce1..7c19351 100644
--- a/Documentation/git-checkout-index.txt
+++ b/Documentation/git-checkout-index.txt
@@ -143,21 +143,16 @@ $ git-checkout-index -n -f -a && git-update-index --ignore-missing --refresh
 ----------------
 
 Using `git-checkout-index` to "export an entire tree"::
-	The prefix ability basically makes it trivial to use
-	`git-checkout-index` as an "export as tree" function.
-	Just read the desired tree into the index, and do:
+	Just read the desired tree into the index, make sure
+	git-export-dir exists, and do:
 +
 ----------------
-$ git-checkout-index --prefix=git-export-dir/ -a
+$ git --work-dir=git-export-dir checkout-index -a
 ----------------
 +
 `git-checkout-index` will "export" the index into the specified
 directory.
 +
-The final "/" is important. The exported name is literally just
-prefixed with the specified string.  Contrast this with the
-following example.
-
 Export files with a prefix::
 +
 ----------------
@@ -165,7 +160,9 @@ $ git-checkout-index --prefix=.merged- Makefile
 ----------------
 +
 This will check out the currently cached copy of `Makefile`
-into the file `.merged-Makefile`.
+into the file `.merged-Makefile`.  The argument of --prefix is
+literally just prefixed with the specified string, so if you want to
+export to a directory you must end the prefix with a slash.
 
 
 Author
-- 
1.5.3.rc4.857.ge22ff