Re: [PATCH 04/19] reset: don't allow git reset -- $pathspec in bare repo

2013-01-10 Thread Martin von Zweigbergk
On Wed, Jan 9, 2013 at 11:32 AM, Junio C Hamano gits...@pobox.com wrote:
 Martin von Zweigbergk martinv...@gmail.com writes:

 ---
  builtin/reset.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

 With the patch that does not have any explicit check for bareness
 nor new error message to scold user with, it is rather hard to tell
 what is going on, without any description on what (if anything) is
 broken at the end user level and what remedy is done about that
 breakage...

Will include the following in a re-roll.

reset: don't allow git reset -- $pathspec in bare repo

Running e.g. git reset . in a bare repo results in an index file
being created from the HEAD commit. The differences compared to the
index are then printed as usual, but since there is no worktree, it
will appear as if all files are deleted. For example, in a bare clone
of git.git:

  Unstaged changes after reset:
  D   .gitattributes
  D   .gitignore
  D   .mailmap
  ...

This happens because the check for is_bare_repository() happens after
we branch off into read_from_tree() to reset with paths. Fix by moving
the branching point after the check.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 04/19] reset: don't allow git reset -- $pathspec in bare repo

2013-01-10 Thread Junio C Hamano
Martin von Zweigbergk martinv...@gmail.com writes:

 ... Fix by moving
 the branching point after the check.

OK, that is what I missed.  We have an existing check for mixed
reset, which was originally meant to handle case without any
pathspec but can use the same error condition (i.e. type is mixed
and repository is bare) and error message (i.e. no mixed reset in
a bare repository).  reset with pathspec was done before that
check kicked in.

Thanks for clarification (and sorry for the noise).

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 04/19] reset: don't allow git reset -- $pathspec in bare repo

2013-01-09 Thread Martin von Zweigbergk
---
 builtin/reset.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 045c960..664fad9 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -295,8 +295,6 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
else if (reset_type != NONE)
die(_(Cannot do %s reset with paths.),
_(reset_type_names[reset_type]));
-   return read_from_tree(pathspec, sha1,
-   quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
}
if (reset_type == NONE)
reset_type = MIXED; /* by default */
@@ -308,6 +306,10 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
die(_(%s reset is not allowed in a bare repository),
_(reset_type_names[reset_type]));
 
+   if (pathspec)
+   return read_from_tree(pathspec, sha1,
+   quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+
/* Soft reset does not touch the index file nor the working tree
 * at all, but requires them in a good order.  Other resets reset
 * the index file to the tree object we are switching to. */
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 04/19] reset: don't allow git reset -- $pathspec in bare repo

2013-01-09 Thread Junio C Hamano
Martin von Zweigbergk martinv...@gmail.com writes:

 ---
  builtin/reset.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

With the patch that does not have any explicit check for bareness
nor new error message to scold user with, it is rather hard to tell
what is going on, without any description on what (if anything) is
broken at the end user level and what remedy is done about that
breakage...


 diff --git a/builtin/reset.c b/builtin/reset.c
 index 045c960..664fad9 100644
 --- a/builtin/reset.c
 +++ b/builtin/reset.c
 @@ -295,8 +295,6 @@ int cmd_reset(int argc, const char **argv, const char 
 *prefix)
   else if (reset_type != NONE)
   die(_(Cannot do %s reset with paths.),
   _(reset_type_names[reset_type]));
 - return read_from_tree(pathspec, sha1,
 - quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
   }
   if (reset_type == NONE)
   reset_type = MIXED; /* by default */
 @@ -308,6 +306,10 @@ int cmd_reset(int argc, const char **argv, const char 
 *prefix)
   die(_(%s reset is not allowed in a bare repository),
   _(reset_type_names[reset_type]));
  
 + if (pathspec)
 + return read_from_tree(pathspec, sha1,
 + quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
 +
   /* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order.  Other resets reset
* the index file to the tree object we are switching to. */
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html