All of lore.kernel.org
 help / color / mirror / Atom feed
* git-reset does not seem to respect GIT_WORK_TREE
@ 2014-02-14 18:57 Patrick Palka
  2014-02-15  9:14 ` [PATCH] reset: setup worktree on --mixed Nguyễn Thái Ngọc Duy
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick Palka @ 2014-02-14 18:57 UTC (permalink / raw)
  To: git

Hi everyone,

I noticed that git-reset does not seem to respect GIT_WORK_TREE.  Here
is a simplified test case:

$ mkdir src_dir && cd src_dir
$ git init
$ touch A && git add A && git commit -m "Dummy commit."
$ mkdir ../build_dir && cd ../build_dir
$ export GIT_WORK_TREE=../src_dir
$ export GIT_DIR=../src_dir/.git
$ git reset
Unstaged changes after reset:
D       A

The final command "git reset" erroneously suggests that the file "A"
does not exist in the working tree.  Does anybody know why git-reset
behaves this way?

Thanks,
Patrick

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] reset: setup worktree on --mixed
  2014-02-14 18:57 git-reset does not seem to respect GIT_WORK_TREE Patrick Palka
@ 2014-02-15  9:14 ` Nguyễn Thái Ngọc Duy
  2014-02-15  9:38   ` Duy Nguyen
  2014-02-16  2:28   ` [PATCH v2] reset: optionally setup worktree and refresh index " Nguyễn Thái Ngọc Duy
  0 siblings, 2 replies; 6+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2014-02-15  9:14 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, patrick, Nguyễn Thái Ngọc Duy

--mixed does mainly two things: read_from_tree(), which does not
require a worktree, and refresh_index(), which does.

Reported-by: Patrick Palka <patrick@parcs.ath.cx>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/reset.c  |  2 +-
 t/t7102-reset.sh | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 6004803..9928c28 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -320,7 +320,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (reset_type == NONE)
 		reset_type = MIXED; /* by default */
 
-	if (reset_type != SOFT && reset_type != MIXED)
+	if (reset_type != SOFT)
 		setup_work_tree();
 
 	if (reset_type == MIXED && is_bare_repository())
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 8d4b50d..ee117e2 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -535,4 +535,15 @@ test_expect_success 'reset with paths accepts tree' '
 	git diff HEAD --exit-code
 '
 
+test_expect_success 'reset --mixed sets up work tree' '
+	git init mixed_worktree &&
+	(
+		cd mixed_worktree &&
+		test_commit dummy
+	) &&
+	: >expect &&
+	git --git-dir=mixed_worktree/.git --work-tree=mixed_worktree reset >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.8.5.2.240.g8478abd

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] reset: setup worktree on --mixed
  2014-02-15  9:14 ` [PATCH] reset: setup worktree on --mixed Nguyễn Thái Ngọc Duy
@ 2014-02-15  9:38   ` Duy Nguyen
  2014-02-16  2:28   ` [PATCH v2] reset: optionally setup worktree and refresh index " Nguyễn Thái Ngọc Duy
  1 sibling, 0 replies; 6+ messages in thread
From: Duy Nguyen @ 2014-02-15  9:38 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, patrick, Nguyễn Thái Ngọc Duy

On Sat, Feb 15, 2014 at 4:14 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> --mixed does mainly two things: read_from_tree(), which does not
> require a worktree, and refresh_index(), which does.

.. and I should have run the entire test suite before sending this. It
breaks t7103.5 (reset --mixed in bare repository). That test seems
wrong though, in my opinon..

>
> Reported-by: Patrick Palka <patrick@parcs.ath.cx>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/reset.c  |  2 +-
>  t/t7102-reset.sh | 11 +++++++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 6004803..9928c28 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -320,7 +320,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>         if (reset_type == NONE)
>                 reset_type = MIXED; /* by default */
>
> -       if (reset_type != SOFT && reset_type != MIXED)
> +       if (reset_type != SOFT)
>                 setup_work_tree();
>
>         if (reset_type == MIXED && is_bare_repository())
> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
> index 8d4b50d..ee117e2 100755
> --- a/t/t7102-reset.sh
> +++ b/t/t7102-reset.sh
> @@ -535,4 +535,15 @@ test_expect_success 'reset with paths accepts tree' '
>         git diff HEAD --exit-code
>  '
>
> +test_expect_success 'reset --mixed sets up work tree' '
> +       git init mixed_worktree &&
> +       (
> +               cd mixed_worktree &&
> +               test_commit dummy
> +       ) &&
> +       : >expect &&
> +       git --git-dir=mixed_worktree/.git --work-tree=mixed_worktree reset >actual &&
> +       test_cmp expect actual
> +'
> +
>  test_done
> --
> 1.8.5.2.240.g8478abd
>



-- 
Duy

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2] reset: optionally setup worktree and refresh index on --mixed
  2014-02-15  9:14 ` [PATCH] reset: setup worktree on --mixed Nguyễn Thái Ngọc Duy
  2014-02-15  9:38   ` Duy Nguyen
@ 2014-02-16  2:28   ` Nguyễn Thái Ngọc Duy
  2014-02-16  2:29     ` Eric Sunshine
  2014-02-16 18:19     ` Patrick Palka
  1 sibling, 2 replies; 6+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2014-02-16  2:28 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, patrick, Nguyễn Thái Ngọc Duy

Refreshing index requires work tree. So we have to options: always set
up work tree (and refuse to reset if failing to do so), or make
refreshing index optional.

As refreshing index is not the main task, it makes more sense to make
it optional.

Reported-by: Patrick Palka <patrick@parcs.ath.cx>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/reset.c  |  7 ++++---
 t/t7102-reset.sh | 11 +++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 6004803..a991344 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -320,7 +320,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (reset_type == NONE)
 		reset_type = MIXED; /* by default */
 
-	if (reset_type != SOFT && reset_type != MIXED)
+	if (reset_type != SOFT && (reset_type != MIXED || get_git_work_tree()))
 		setup_work_tree();
 
 	if (reset_type == MIXED && is_bare_repository())
@@ -340,8 +340,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 			int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;
 			if (read_from_tree(&pathspec, sha1))
 				return 1;
-			refresh_index(&the_index, flags, NULL, NULL,
-				      _("Unstaged changes after reset:"));
+			if (get_git_work_tree())
+				refresh_index(&the_index, flags, NULL, NULL,
+					      _("Unstaged changes after reset:"));
 		} else {
 			int err = reset_index(sha1, reset_type, quiet);
 			if (reset_type == KEEP && !err)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 8d4b50d..ee117e2 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -535,4 +535,15 @@ test_expect_success 'reset with paths accepts tree' '
 	git diff HEAD --exit-code
 '
 
+test_expect_success 'reset --mixed sets up work tree' '
+	git init mixed_worktree &&
+	(
+		cd mixed_worktree &&
+		test_commit dummy
+	) &&
+	: >expect &&
+	git --git-dir=mixed_worktree/.git --work-tree=mixed_worktree reset >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.8.5.2.240.g8478abd

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] reset: optionally setup worktree and refresh index on --mixed
  2014-02-16  2:28   ` [PATCH v2] reset: optionally setup worktree and refresh index " Nguyễn Thái Ngọc Duy
@ 2014-02-16  2:29     ` Eric Sunshine
  2014-02-16 18:19     ` Patrick Palka
  1 sibling, 0 replies; 6+ messages in thread
From: Eric Sunshine @ 2014-02-16  2:29 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: Git List, Junio C Hamano, patrick

On Sat, Feb 15, 2014 at 9:28 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Refreshing index requires work tree. So we have to options: always set

s/to/two/

> up work tree (and refuse to reset if failing to do so), or make
> refreshing index optional.
>
> As refreshing index is not the main task, it makes more sense to make
> it optional.
>
> Reported-by: Patrick Palka <patrick@parcs.ath.cx>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/reset.c  |  7 ++++---
>  t/t7102-reset.sh | 11 +++++++++++
>  2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 6004803..a991344 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -320,7 +320,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>         if (reset_type == NONE)
>                 reset_type = MIXED; /* by default */
>
> -       if (reset_type != SOFT && reset_type != MIXED)
> +       if (reset_type != SOFT && (reset_type != MIXED || get_git_work_tree()))
>                 setup_work_tree();
>
>         if (reset_type == MIXED && is_bare_repository())
> @@ -340,8 +340,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>                         int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;
>                         if (read_from_tree(&pathspec, sha1))
>                                 return 1;
> -                       refresh_index(&the_index, flags, NULL, NULL,
> -                                     _("Unstaged changes after reset:"));
> +                       if (get_git_work_tree())
> +                               refresh_index(&the_index, flags, NULL, NULL,
> +                                             _("Unstaged changes after reset:"));
>                 } else {
>                         int err = reset_index(sha1, reset_type, quiet);
>                         if (reset_type == KEEP && !err)
> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
> index 8d4b50d..ee117e2 100755
> --- a/t/t7102-reset.sh
> +++ b/t/t7102-reset.sh
> @@ -535,4 +535,15 @@ test_expect_success 'reset with paths accepts tree' '
>         git diff HEAD --exit-code
>  '
>
> +test_expect_success 'reset --mixed sets up work tree' '
> +       git init mixed_worktree &&
> +       (
> +               cd mixed_worktree &&
> +               test_commit dummy
> +       ) &&
> +       : >expect &&
> +       git --git-dir=mixed_worktree/.git --work-tree=mixed_worktree reset >actual &&
> +       test_cmp expect actual
> +'
> +
>  test_done
> --
> 1.8.5.2.240.g8478abd
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] reset: optionally setup worktree and refresh index on --mixed
  2014-02-16  2:28   ` [PATCH v2] reset: optionally setup worktree and refresh index " Nguyễn Thái Ngọc Duy
  2014-02-16  2:29     ` Eric Sunshine
@ 2014-02-16 18:19     ` Patrick Palka
  1 sibling, 0 replies; 6+ messages in thread
From: Patrick Palka @ 2014-02-16 18:19 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano

On Sat, Feb 15, 2014 at 9:28 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Refreshing index requires work tree. So we have to options: always set
> up work tree (and refuse to reset if failing to do so), or make
> refreshing index optional.
>
> As refreshing index is not the main task, it makes more sense to make
> it optional.
>
> Reported-by: Patrick Palka <patrick@parcs.ath.cx>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Thanks!  I can confirm that this change fixes my use case.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-16 18:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-14 18:57 git-reset does not seem to respect GIT_WORK_TREE Patrick Palka
2014-02-15  9:14 ` [PATCH] reset: setup worktree on --mixed Nguyễn Thái Ngọc Duy
2014-02-15  9:38   ` Duy Nguyen
2014-02-16  2:28   ` [PATCH v2] reset: optionally setup worktree and refresh index " Nguyễn Thái Ngọc Duy
2014-02-16  2:29     ` Eric Sunshine
2014-02-16 18:19     ` Patrick Palka

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.