threads / patch / 9385

patchcheckout-index needs a working tree

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

## tl;dr

12 messages between Aug 4, 2007 and Aug 11, 2007. Diffs are folded; open one to read it.

replies: 11people: 3as markdown or json

Johannes Schindelin· Aug 4, 2007, 22:20 UTC · lore
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(-)
Show changes to git.c +2 −1
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
Junio C Hamano· Aug 4, 2007, 22:50 UTC · re: Johannes Schindelin · lore

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

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.

Johannes Schindelin· Aug 5, 2007, 01:33 UTC · re: Junio C Hamano · lore

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

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

Uwe Kleine-König· Aug 9, 2007, 22:35 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin wrote:
Show 23 quoted lines
> 
> 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
Johannes Schindelin· Aug 10, 2007, 00:31 UTC · re: Uwe Kleine-König · lore

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

Hi,
On Fri, 10 Aug 2007, Uwe Kleine-K?nig wrote:
Show 30 quoted lines
> 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

Junio C Hamano· Aug 10, 2007, 00:55 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 5 quoted lines
> 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.

Johannes Schindelin· Aug 10, 2007, 01:11 UTC · re: Junio C Hamano · lore

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

Hi,
On Thu, 9 Aug 2007, Junio C Hamano wrote:
Show 17 quoted lines
> 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

Junio C Hamano· Aug 10, 2007, 07:57 UTC · re: Johannes Schindelin · lore

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

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(-)
Show changes to 2 files +14 −44

setup.c, t/t1500-rev-parse.sh

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
Uwe Kleine-König· Aug 10, 2007, 11:28 UTC · re: Junio C Hamano · lore

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

Hello Junio,
Junio C Hamano wrote:
Show 30 quoted lines
> 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");}
/* */
Junio C Hamano· Aug 10, 2007, 19:26 UTC · re: Uwe Kleine-König · lore

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

Uwe Kleine-König  <ukleinek@informatik.uni-freiburg.de> writes:
Show 6 quoted lines
> 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.

Junio C Hamano· Aug 11, 2007, 05:17 UTC · re: Junio C Hamano · lore

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

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
Show changes to 2 files +79 −2

git.c, 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
Uwe Kleine-König· Aug 11, 2007, 18:15 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to Documentation/git-checkout-index.txt +6 −9
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

← back to recent threads