git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Operations on unborn branch
@ 2012-11-27 17:25 Martin von Zweigbergk
  2012-11-27 20:25 ` Junio C Hamano
  2012-11-29 18:32 ` [RFC/PATCH 0/2] Fix "git reset" " Martin von Zweigbergk
  0 siblings, 2 replies; 18+ messages in thread
From: Martin von Zweigbergk @ 2012-11-27 17:25 UTC (permalink / raw)
  To: git

While looking at how to handle "git rebase --root", I noticed that
"git cherry-pick" fails with the following when run on an unborn
branch:

error: You do not have a valid HEAD
fatal: cherry-pick failed

I can not see any reason that it shouldn't work. "git cherry-pick -n"
does work. (For rebase, "git cherry-pick --ff" would be used, and I
think that should also work on an unborn branch.)

Also, "git reset" doesn't work on an unborn branch and I can not see
any reason that it shouldn't work. This was also asked on stack
overflow [1], and of course the solution is to use "git rm --cached",
but doesn't mean that "git reset" shouldn't work.

I have very limited time to work on git these days, so if anyone else
would like to work on any of this, I would be very happy. I _might_
take some time to fix the cherry-pick issue.

Btw, every time I run into problems like these with the treatment of
root commits, I can't help but wonder how things would look if git had
always had a single root commit (naturally with some dummy user,
timestamp etc to ensure sameness across repos). With my limited
knowledge, it seems like that would complicate a few things, but
simplify a lot of things (maybe I'm biased because of the things I
have happened to work on?). Has anyone spent some time seriously
thinking about this? I suppose it would be hard to introduce
backward-compatibly, and maybe this is very unrealistic even for git
2.0, but I would be curious to hear what others think.

Martin

[1] http://stackoverflow.com/questions/3894808/new-git-repository-and-already-git-reset-does-not-work

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

* Re: Operations on unborn branch
  2012-11-27 17:25 Operations on unborn branch Martin von Zweigbergk
@ 2012-11-27 20:25 ` Junio C Hamano
  2012-11-27 20:39   ` Martin von Zweigbergk
  2012-11-29 18:32 ` [RFC/PATCH 0/2] Fix "git reset" " Martin von Zweigbergk
  1 sibling, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2012-11-27 20:25 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> simplify a lot of things (maybe I'm biased because of the things I
> have happened to work on?)

Yes.  Do not waste time on it.

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

* Re: Operations on unborn branch
  2012-11-27 20:25 ` Junio C Hamano
@ 2012-11-27 20:39   ` Martin von Zweigbergk
  2012-11-28  7:12     ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Martin von Zweigbergk @ 2012-11-27 20:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Tue, Nov 27, 2012 at 12:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> simplify a lot of things (maybe I'm biased because of the things I
>> have happened to work on?)
>
> Yes.  Do not waste time on it.

Yes, no way I would waste time on that; I was mostly just curious.

What I might spend time on is to fix cherry-pick.

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

* Re: Operations on unborn branch
  2012-11-27 20:39   ` Martin von Zweigbergk
@ 2012-11-28  7:12     ` Junio C Hamano
  2012-11-30 16:39       ` Martin von Zweigbergk
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2012-11-28  7:12 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> On Tue, Nov 27, 2012 at 12:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>>
>>> simplify a lot of things (maybe I'm biased because of the things I
>>> have happened to work on?)
>>
>> Yes.  Do not waste time on it.
>
> Yes, no way I would waste time on that; I was mostly just curious.

You have to special case the edges whichever way you go.  You can
always add such a fixed parent commit whenever you create a new root
commit, but then the codepath that currently relies on the real root
commit not having any parent start needing to notice if the parent
is the fixed fake commit and exclude it from thee history.  Or you
can create a new root commit as parent-less like we currently do,
and any history examination do not have to special case "ah, I
thought there is a parent commit, but that turns out to be the fake
one, so I need to ignore it."  Creation of a root commit is a one-time
operation in any sane history; if we have to have special cases
somewhere anyway, it is better to have them in these one-time
operation codepaths.

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

* [RFC/PATCH 0/2] Fix "git reset" on unborn branch
  2012-11-27 17:25 Operations on unborn branch Martin von Zweigbergk
  2012-11-27 20:25 ` Junio C Hamano
@ 2012-11-29 18:32 ` Martin von Zweigbergk
  2012-11-29 18:32   ` [RFC/PATCH 1/2] reset: learn to reset to tree Martin von Zweigbergk
  2012-11-29 18:32   ` [RFC/PATCH 2/2] reset: learn to reset on unborn branch Martin von Zweigbergk
  1 sibling, 2 replies; 18+ messages in thread
From: Martin von Zweigbergk @ 2012-11-29 18:32 UTC (permalink / raw)
  To: git; +Cc: Martin von Zweigbergk

I decided to address this before "cherry-pick on unborn branch". RFC
mostly because I'm not sure about the user interface. When we have
agreed on that, I will add documentation.

Martin von Zweigbergk (2):
  reset: learn to reset to tree
  reset: learn to reset on unborn branch

 builtin/reset.c                     | 73 ++++++++++++++++++++++---------------
 t/t1512-rev-parse-disambiguation.sh |  4 --
 t/t7102-reset.sh                    | 26 +++++++++++++
 t/t7106-reset-unborn-branch.sh      | 52 ++++++++++++++++++++++++++
 4 files changed, 122 insertions(+), 33 deletions(-)
 create mode 100755 t/t7106-reset-unborn-branch.sh

-- 
1.8.0.1.240.ge8a1f5a

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

* [RFC/PATCH 1/2] reset: learn to reset to tree
  2012-11-29 18:32 ` [RFC/PATCH 0/2] Fix "git reset" " Martin von Zweigbergk
@ 2012-11-29 18:32   ` Martin von Zweigbergk
  2012-11-29 18:47     ` Junio C Hamano
  2012-11-29 18:32   ` [RFC/PATCH 2/2] reset: learn to reset on unborn branch Martin von Zweigbergk
  1 sibling, 1 reply; 18+ messages in thread
From: Martin von Zweigbergk @ 2012-11-29 18:32 UTC (permalink / raw)
  To: git; +Cc: Martin von Zweigbergk

In cases where HEAD is not supposed to be updated, there is no reason
that "git reset" should require a commit, a tree should be enough. So
make "git reset $rev^{tree}" work just like "git reset $rev", except
that the former will not update HEAD (since there is no commit to
point it to).

Disallow --soft with trees, since that is about updating only HEAD.

By not updating HEAD, "git reset $rev^{tree}" behaves quite like "git
reset $rev .". One might therefore consider requiring a path when
using reset with a tree to make that similarity more obvious. However,
a future commit will make "git reset" work on an unborn branch by
interpreting it as "git reset $empty_tree" and it would seem
unintuitive to the user to say "git reset ." on an unborn
branch. Requiring a path would also make "git reset --hard $tree"
disallowed.

This commit effectively undoes some of commit 13243c2 (reset: the
command takes committish, 2012-07-03). The command line argument is
now required to be an unambiguous treeish.

---

My implementation of lookup_commit_or_tree looks a little clunky. I'm
not very familiar with the API. Suggestions welcome.

Why is the "HEAD is now at ..." message printed only for --hard reset?
After all, HEAD is updated for all types of reset not involving paths.

 builtin/reset.c                     | 67 +++++++++++++++++++++----------------
 t/t1512-rev-parse-disambiguation.sh |  4 ---
 t/t7102-reset.sh                    | 26 ++++++++++++++
 3 files changed, 65 insertions(+), 32 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 915cc9f..cec9874 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -225,6 +225,21 @@ static void die_if_unmerged_cache(int reset_type)
 
 }
 
+static struct object *lookup_commit_or_tree(const char *rev) {
+	unsigned char sha1[20];
+	struct commit *commit;
+	struct tree *tree;
+	if (get_sha1_treeish(rev, sha1))
+		die(_("Failed to resolve '%s' as a valid ref."), rev);
+	commit = lookup_commit_reference_gently(sha1, 1);
+	if (commit)
+		return (struct object *) commit;
+	tree = parse_tree_indirect(sha1);
+	if (tree)
+		return (struct object *) tree;
+	die(_("Could not parse object '%s'."), rev);
+}
+
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
@@ -232,7 +247,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	const char *rev = "HEAD";
 	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
 				*old_orig = NULL, sha1_old_orig[20];
-	struct commit *commit;
+	struct object *object;
+	struct commit *commit = NULL;
 	struct strbuf msg = STRBUF_INIT;
 	const struct option options[] = {
 		OPT__QUIET(&quiet, N_("be quiet, only report errors")),
@@ -276,7 +292,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		 * Otherwise, argv[i] could be either <rev> or <paths> and
 		 * has to be unambiguous.
 		 */
-		else if (!get_sha1_committish(argv[i], sha1)) {
+		else if (!get_sha1_treeish(argv[i], sha1)) {
 			/*
 			 * Ok, argv[i] looks like a rev; it should not
 			 * be a filename.
@@ -289,19 +305,12 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	if (get_sha1_committish(rev, sha1))
-		die(_("Failed to resolve '%s' as a valid ref."), rev);
-
-	/*
-	 * NOTE: As "git reset $treeish -- $path" should be usable on
-	 * any tree-ish, this is not strictly correct. We are not
-	 * moving the HEAD to any commit; we are merely resetting the
-	 * entries in the index to that of a treeish.
-	 */
-	commit = lookup_commit_reference(sha1);
-	if (!commit)
-		die(_("Could not parse object '%s'."), rev);
-	hashcpy(sha1, commit->object.sha1);
+	object = lookup_commit_or_tree(rev);
+	if (object->type == OBJ_COMMIT)
+		commit = (struct commit*) object;
+	else if (reset_type == SOFT)
+		die(_("--soft requires a commit, which '%s' is not"), rev);
+	hashcpy(sha1, object->sha1);
 
 	if (patch_mode) {
 		if (reset_type != NONE)
@@ -347,23 +356,25 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 			die(_("Could not reset index file to revision '%s'."), rev);
 	}
 
-	/* Any resets update HEAD to the head being switched to,
-	 * saving the previous head in ORIG_HEAD before. */
-	if (!get_sha1("ORIG_HEAD", sha1_old_orig))
-		old_orig = sha1_old_orig;
-	if (!get_sha1("HEAD", sha1_orig)) {
-		orig = sha1_orig;
-		set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
-		update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+	if (commit) {
+		/* Any resets update HEAD to the head being switched to,
+		 * saving the previous head in ORIG_HEAD before. */
+		if (!get_sha1("ORIG_HEAD", sha1_old_orig))
+			old_orig = sha1_old_orig;
+		if (!get_sha1("HEAD", sha1_orig)) {
+			orig = sha1_orig;
+			set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
+			update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+		}
+		else if (old_orig)
+			delete_ref("ORIG_HEAD", old_orig, 0);
+		set_reflog_message(&msg, "updating HEAD", rev);
+		update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR);
 	}
-	else if (old_orig)
-		delete_ref("ORIG_HEAD", old_orig, 0);
-	set_reflog_message(&msg, "updating HEAD", rev);
-	update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR);
 
 	switch (reset_type) {
 	case HARD:
-		if (!update_ref_status && !quiet)
+		if (commit && !update_ref_status && !quiet)
 			print_new_head_line(commit);
 		break;
 	case SOFT: /* Nothing else to do. */
diff --git a/t/t1512-rev-parse-disambiguation.sh b/t/t1512-rev-parse-disambiguation.sh
index 6b3d797..bc1e40c 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -121,10 +121,6 @@ test_expect_success 'git log takes only commit-ish' '
 	git log 000000000
 '
 
-test_expect_success 'git reset takes only commit-ish' '
-	git reset 000000000
-'
-
 test_expect_success 'first tag' '
 	# create one tag 0000000000f8f
 	git tag -a -m j7cp83um v1.0.0
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index b096dc8..d723ef5 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -491,4 +491,30 @@ test_expect_success 'disambiguation (4)' '
 	test ! -f secondfile
 '
 
+test_expect_success 'reset to tree does not update HEAD' '
+	git reset --hard HEAD &&
+	rev_before=$(git rev-parse HEAD) &&
+	git reset HEAD^^{tree} &&
+	test $(git rev-parse HEAD) == $rev_before
+'
+
+test_expect_success 'reset to tree' '
+	# for simpler tests, drop last commit containing added files
+	git reset --hard HEAD^ &&
+	git reset HEAD^^{tree} &&
+	git diff --cached HEAD^ --exit-code &&
+	git diff HEAD --exit-code
+'
+
+test_expect_success 'reset --hard to tree' '
+	git reset --hard &&
+	git reset --hard HEAD^^{tree} &&
+	git diff --cached HEAD^ --exit-code &&
+	git diff HEAD^ --exit-code
+'
+
+test_expect_success 'reset to tree not allowed with --soft}' '
+	test_must_fail git reset --soft HEAD^^{tree}
+'
+
 test_done
-- 
1.8.0.1.240.ge8a1f5a

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

* [RFC/PATCH 2/2] reset: learn to reset on unborn branch
  2012-11-29 18:32 ` [RFC/PATCH 0/2] Fix "git reset" " Martin von Zweigbergk
  2012-11-29 18:32   ` [RFC/PATCH 1/2] reset: learn to reset to tree Martin von Zweigbergk
@ 2012-11-29 18:32   ` Martin von Zweigbergk
  1 sibling, 0 replies; 18+ messages in thread
From: Martin von Zweigbergk @ 2012-11-29 18:32 UTC (permalink / raw)
  To: git; +Cc: Martin von Zweigbergk

When run on an unborn branch, "git reset" currently fails with:

  fatal: Failed to resolve 'HEAD' as a valid ref.

Fix this by interpreting it as a reset to the empty tree.

If --patch is given, we currently pass the revision specifier, as
given on the command line, to interactive_reset(). On an unborn
branch, HEAD can of course not be resolved, so we need to pass the
sha1 of the empty tree to interactive_reset() as well. This is fine
since interactive_reset only needs the parameter to be a treeish and
doesn't use it for display purposes.

---

Is it correct that interactive_reset does not use the revision
specifier for display purposes? Or, worse, that it requires it to be a
commit in some cases? I tried it and didn't see any problem.

 builtin/reset.c                | 10 +++++---
 t/t7106-reset-unborn-branch.sh | 52 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 3 deletions(-)
 create mode 100755 t/t7106-reset-unborn-branch.sh

diff --git a/builtin/reset.c b/builtin/reset.c
index cec9874..3845225 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -229,7 +229,10 @@ static struct object *lookup_commit_or_tree(const char *rev) {
 	unsigned char sha1[20];
 	struct commit *commit;
 	struct tree *tree;
-	if (get_sha1_treeish(rev, sha1))
+	if (!strcmp(rev, "HEAD") && get_sha1("HEAD", sha1)) {
+		// unborn branch: reset to empty tree
+		hashcpy(sha1, EMPTY_TREE_SHA1_BIN);
+	} else if (get_sha1_treeish(rev, sha1))
 		die(_("Failed to resolve '%s' as a valid ref."), rev);
 	commit = lookup_commit_reference_gently(sha1, 1);
 	if (commit)
@@ -292,7 +295,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		 * Otherwise, argv[i] could be either <rev> or <paths> and
 		 * has to be unambiguous.
 		 */
-		else if (!get_sha1_treeish(argv[i], sha1)) {
+		else if (!strcmp(argv[i], "HEAD") ||
+			 !get_sha1_treeish(argv[i], sha1)) {
 			/*
 			 * Ok, argv[i] looks like a rev; it should not
 			 * be a filename.
@@ -315,7 +319,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (patch_mode) {
 		if (reset_type != NONE)
 			die(_("--patch is incompatible with --{hard,mixed,soft}"));
-		return interactive_reset(rev, argv + i, prefix);
+		return interactive_reset(sha1_to_hex(sha1), argv + i, prefix);
 	}
 
 	/* git reset tree [--] paths... can be used to
diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
new file mode 100755
index 0000000..67d45be
--- /dev/null
+++ b/t/t7106-reset-unborn-branch.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='git reset should work on unborn branch'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo a >a &&
+	echo b >b
+'
+
+test_expect_success 'reset' '
+	git add a b &&
+	git reset &&
+	test "$(git ls-files)" == ""
+'
+
+test_expect_success 'reset HEAD' '
+	rm .git/index &&
+	git add a b &&
+	git reset HEAD &&
+	test "$(git ls-files)" == ""
+'
+
+test_expect_success 'reset $file' '
+	rm .git/index &&
+	git add a b &&
+	git reset a &&
+	test "$(git ls-files)" == "b"
+'
+
+test_expect_success 'reset -p' '
+	rm .git/index &&
+	git add a &&
+	echo y | git reset -p &&
+	test "$(git ls-files)" == ""
+'
+
+test_expect_success 'reset --soft not allowed' '
+	rm .git/index &&
+	git add a &&
+	test_must_fail git reset --soft
+'
+
+test_expect_success 'reset --hard' '
+	rm .git/index &&
+	git add a &&
+	git reset --hard &&
+	test "$(git ls-files)" == "" &&
+	test_path_is_missing a
+'
+
+test_done
-- 
1.8.0.1.240.ge8a1f5a

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

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
  2012-11-29 18:32   ` [RFC/PATCH 1/2] reset: learn to reset to tree Martin von Zweigbergk
@ 2012-11-29 18:47     ` Junio C Hamano
  2012-11-29 19:04       ` Martin von Zweigbergk
  2012-11-29 19:13       ` Junio C Hamano
  0 siblings, 2 replies; 18+ messages in thread
From: Junio C Hamano @ 2012-11-29 18:47 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> In cases where HEAD is not supposed to be updated, there is no reason
> that "git reset" should require a commit, a tree should be enough. So
> make "git reset $rev^{tree}" work just like "git reset $rev", except
> that the former will not update HEAD (since there is no commit to
> point it to).

That is a horrible design I have to nack, unless you require
pathspec.  You cannot tell what "git reset $sha1" would do without
checking the type of the object $sha1 refers to.  If you do this
only when pathspec is present, then the design is very reasonable.

> Disallow --soft with trees, since that is about updating only HEAD.

Likewise.

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

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
  2012-11-29 18:47     ` Junio C Hamano
@ 2012-11-29 19:04       ` Martin von Zweigbergk
  2012-11-29 19:36         ` Junio C Hamano
  2012-11-29 19:13       ` Junio C Hamano
  1 sibling, 1 reply; 18+ messages in thread
From: Martin von Zweigbergk @ 2012-11-29 19:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Nov 29, 2012 at 10:47 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> In cases where HEAD is not supposed to be updated, there is no reason
>> that "git reset" should require a commit, a tree should be enough. So
>> make "git reset $rev^{tree}" work just like "git reset $rev", except
>> that the former will not update HEAD (since there is no commit to
>> point it to).
>
> That is a horrible design I have to nack, unless you require
> pathspec.  You cannot tell what "git reset $sha1" would do without
> checking the type of the object $sha1 refers to.  If you do this
> only when pathspec is present, then the design is very reasonable.

Very good point. Thanks! I now see that "git checkout" also requires a
path when given a tree.

So then "git reset" on an unborn branch would imply "git reset
$empty_tree -- ." instead. And "git reset --hard $tree" would not be
allowed. And the intersection of these -- "git reset --hard" on and
unborn branch -- would also not work. Would the correct fix be to
first make "git reset --hard -- $path" work (*sigh*)? I have never
understood why that doesn't (shouldn't) work.

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

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
  2012-11-29 18:47     ` Junio C Hamano
  2012-11-29 19:04       ` Martin von Zweigbergk
@ 2012-11-29 19:13       ` Junio C Hamano
  2012-11-29 22:00         ` Martin von Zweigbergk
  1 sibling, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2012-11-29 19:13 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git

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

> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> In cases where HEAD is not supposed to be updated, there is no reason
>> that "git reset" should require a commit, a tree should be enough. So
>> make "git reset $rev^{tree}" work just like "git reset $rev", except
>> that the former will not update HEAD (since there is no commit to
>> point it to).
>
> That is a horrible design I have to nack, unless you require
> pathspec.  You cannot tell what "git reset $sha1" would do without
> checking the type of the object $sha1 refers to.  If you do this
> only when pathspec is present, then the design is very reasonable.

The above applies to an _arbitrary_ $sha1.

Allowing "reset $tree -- $pathspec" is a very good addition in the
same sense that "git checkout $tree -- $pathspec" is useful.  These
two commands, "reset" and "checkout", share that the source we grab
the blobs out of only need to be a tree and does not have to be a
commit, and the only difference between them is where the blobs we
grabbed out of that tree go, either only to the index or to both the
index and the working tree.

But I do not think it is connected, at least at the level the end
users perceive, to the issue of "reset" issued while on an unborn
branch.

If you limit the scope of the behaviour change exposed to the end
users so that you would make

	$ git reset [HEAD]

act as a short-hand for

	$ rm -f $GIT_DIR/index

when HEAD points at an unborn branch, and similarly make

	$ git reset --hard [HEAD]

act as a short-hand for

	$ rm -f $GIT_DIR/index
        $ git clean -f -d

in such a case, I do not think it is unreasonable at all.

In such a case,

	$ git reset --soft [HEAD]

would become just a no-op.  Earlier you were on an unborn branch,
and after "reset --soft", nothing changes.

Hmm?

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

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
  2012-11-29 19:04       ` Martin von Zweigbergk
@ 2012-11-29 19:36         ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2012-11-29 19:36 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> Would the correct fix be to
> first make "git reset --hard -- $path" work (*sigh*)? I have never
> understood why that doesn't (shouldn't) work.

What does it even mean, even when you are on an existing commit, to
hard reset partially?

Perhaps you looking for "git checkout $tree -- $path"?

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

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
  2012-11-29 19:13       ` Junio C Hamano
@ 2012-11-29 22:00         ` Martin von Zweigbergk
  2012-11-30 16:45           ` Martin von Zweigbergk
  0 siblings, 1 reply; 18+ messages in thread
From: Martin von Zweigbergk @ 2012-11-29 22:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Nov 29, 2012 at 11:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
> [...]These
> two commands, "reset" and "checkout", share that the source we grab
> the blobs out of only need to be a tree and does not have to be a
> commit, and the only difference between them is where the blobs we
> grabbed out of that tree go, either only to the index or to both the
> index and the working tree.

Slightly off topic, but another difference (or somehow another aspect
of the same difference?) that has tripped me up a few times is that
"git checkout $rev ." only affects added and modified files (in $rev
compared to HEAD), but "git reset $rev ." would also delete deleted
files from the index. I suppose this is also a partial answer to your
question in another message:

> What does it even mean, even when you are on an existing commit, to
> hard reset partially?
>
> Perhaps you looking for "git checkout $tree -- $path"?

A more direct answer would be that I would expect "git reset --hard
$rev -- ." to behave like "git reset --hard $rev", except that it
wouldn't update HEAD. It seems to me that that would be similar to how
"git reset $rev -- ." behaves like "git reset $rev", except that it
doesn't update HEAD. But reset and checkout with and without paths
still confuse me after years of using git, so I wouldn't be surprised
if I'm not making any sense.

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

* Re: Operations on unborn branch
  2012-11-28  7:12     ` Junio C Hamano
@ 2012-11-30 16:39       ` Martin von Zweigbergk
  0 siblings, 0 replies; 18+ messages in thread
From: Martin von Zweigbergk @ 2012-11-30 16:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Tue, Nov 27, 2012 at 11:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> You have to special case the edges whichever way you go.  [...]

If I understand you correctly, you're saying that revision walking
would need a different special case. This is the most obvious
difference, it seems. "git show" would also need different
special-casing. But rebase wouldn't need --root, diff-tree wouldn't
need --root, any operations on an unborn branch would just work (incl
reset, cherry-pick).

Again, this is hypothetical, so I'll stop the complaining now :-)

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

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
  2012-11-29 22:00         ` Martin von Zweigbergk
@ 2012-11-30 16:45           ` Martin von Zweigbergk
  2012-12-01  9:24             ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Martin von Zweigbergk @ 2012-11-30 16:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Nov 29, 2012 at 2:00 PM, Martin von Zweigbergk
<martinvonz@gmail.com> wrote:
> Slightly off topic, but another difference (or somehow another aspect
> of the same difference?) that has tripped me up a few times is that
> "git checkout $rev ." only affects added and modified files (in $rev
> compared to HEAD), but "git reset $rev ." would also delete deleted
> files from the index.

Actually, what is the reasoning behind this difference? It almost
seems like a bug. I think I have just thought it was too obvious to be
a bug before, but thinking more about it, I can't see any reason why
"git checkout $rev" should delete files, but "git checkout $rev ."
shouldn't. I hope I'm just hallucinating or missing something. Can
someone shed some light on this?

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

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
  2012-11-30 16:45           ` Martin von Zweigbergk
@ 2012-12-01  9:24             ` Junio C Hamano
  2012-12-05  3:45               ` Martin von Zweigbergk
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2012-12-01  9:24 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> On Thu, Nov 29, 2012 at 2:00 PM, Martin von Zweigbergk
> <martinvonz@gmail.com> wrote:
>> Slightly off topic, but another difference (or somehow another aspect
>> of the same difference?) that has tripped me up a few times is that
>> "git checkout $rev ." only affects added and modified files...

"checkout $commit pathspec" has always been about checking out the
contents stored in the paths that match the pathspec from the named
commit to the index and also o the working tree.  "checkout
pathspec" is similar, but the stuff comes out of the index.

When pathspec is "dir/", it does not match the directory whose name
is "dir".  The pathspec matches the paths that store blobs under
that directory.

In other words, "checkout dir/" (or "checkout HEAD~4 dir/) has never
been about "please remove everything in dir/, and then check out
everything in dir/ from the index (or from HEAD~4)".  The "please
remove everything in dir/" part is not the job of "checkout"; of
course, you can do it as a separate step (e.g. "rm -fr dir/").

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

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
  2012-12-01  9:24             ` Junio C Hamano
@ 2012-12-05  3:45               ` Martin von Zweigbergk
  2012-12-05  5:46                 ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Martin von Zweigbergk @ 2012-12-05  3:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Sat, Dec 1, 2012 at 1:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> On Thu, Nov 29, 2012 at 2:00 PM, Martin von Zweigbergk
>> <martinvonz@gmail.com> wrote:
>>> Slightly off topic, but another difference (or somehow another aspect
>>> of the same difference?) that has tripped me up a few times is that
>>> "git checkout $rev ." only affects added and modified files...
>
> "checkout $commit pathspec" has always been about ...

I suppose the "has always been" is meant to say that it's hard to
change at this point, not that it's more intuitive the way it works..?

> ...checking out the
> contents stored in the paths that match the pathspec from the named
> commit to the index and also o the working tree.

I think I have always thought that "git checkout $commit $pathspec"
would replace the section(s) of the tree defined by $pathspec. (I'm
using "tree" in the more general sense here, as I'm understood the
index is not stored as a tree.)

> When pathspec is "dir/", it does not match the directory whose name
> is "dir".  The pathspec matches the paths that store blobs under
> that directory.

Ah, right. Unlike "git reset dir/", IIUC.

More importantly, when is it desirable not to delete deleted entries?
I find it much easier to imagine uses a "git checkout $commit
$pathspec" that does delete deleted entries. It seems like this must
have been discussed in depth before, so feel free to point me to an
old thread.

If it doesn't seem too strange to you and others if I make "git reset
--hard [$commit] $pathspec" work just like had expected "git checkout
$commit $pathspec", I might look into that when I get some time.

> ...The "please
> remove everything in dir/" part is not the job of "checkout"; of
> course, you can do it as a separate step (e.g. "rm -fr dir/").

"rm -rf dir/" would of course delete everything in there, including
e.g. build artifacts....

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

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
  2012-12-05  3:45               ` Martin von Zweigbergk
@ 2012-12-05  5:46                 ` Junio C Hamano
  2012-12-05 12:58                   ` Martin von Zweigbergk
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2012-12-05  5:46 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> More importantly, when is it desirable not to delete deleted entries?

When I am trying to check out contents of Documentation/ directory
as of an older edition because we made mistakes updating the files
in recent versions, with "git checkout v1.9.0 Documentation/", for
example.  Perhaps somebody had this bright idea of reformatting our
docs with "= Newer Style =" section headers, replacing the underline
style, and we found our toolchain depend on the underline style, or
something.  The new files in the same directory added since v1.9.0
may share the same mistake as the files whose recent such changes I
am nuking with this operation, but that does not mean I want to
retype the contents of them from scratch; I'd rather keep them
around so that I can fix them up by hand.

I would have to say that it is more common; I do not recall a time I
wanted to replace everything in a directory (and only there without
touching other parts of the tree) with an old version, removing new
ones.  "git checkout [$commit] $paths" is still an operation to help
me build new history forward starting from HEAD, and is not about
start building on top of the old $commit.  Losing the work I've done
to the files that did not exist in $commit:$paths is almost always
*not* what I would expect to happen with the command.

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

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
  2012-12-05  5:46                 ` Junio C Hamano
@ 2012-12-05 12:58                   ` Martin von Zweigbergk
  0 siblings, 0 replies; 18+ messages in thread
From: Martin von Zweigbergk @ 2012-12-05 12:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Tue, Dec 4, 2012 at 9:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> More importantly, when is it desirable not to delete deleted entries?
>
> When I am trying to check out contents of Documentation/ directory
> as of an older edition because we made mistakes updating the files
> in recent versions, with "git checkout v1.9.0 Documentation/", for
> example.  Perhaps somebody had this bright idea of reformatting our
> docs with "= Newer Style =" section headers, replacing the underline
> style, and we found our toolchain depend on the underline style, or
> something.  The new files in the same directory added since v1.9.0
> may share the same mistake as the files whose recent such changes I
> am nuking with this operation, but that does not mean I want to
> retype the contents of them from scratch; I'd rather keep them
> around so that I can fix them up by hand.

I think I follow, but why, then, would you not have the save problem
with hunks *within* files that have been added in the new version? Or
is the only change to Documentation/ since the "broken" commit that a
new file has been added? That seems like a rather narrow use case in
that case? "git checkout -p" seems more generally useful (whether that
command deleted deleted files or not). It feels like I'm missing
something...

> I would have to say that it is more common; I do not recall a time I
> wanted to replace everything in a directory (and only there without
> touching other parts of the tree) with an old version, removing new
> ones.

It has happened a few times for me. I think this usually happens when
I realize that I had a better solution for some subsystem (under some
path) in some other branch (perhaps from a previous attempt at the
same problem) or an in older commit. Knowing that "git checkout $rev
$path" doesn't do what I expect and that "git reset --hard $rev $path"
is not allowed, I think I would usually do "git reset $rev $path &&
git checkout $path".

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

end of thread, other threads:[~2012-12-05 12:58 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-27 17:25 Operations on unborn branch Martin von Zweigbergk
2012-11-27 20:25 ` Junio C Hamano
2012-11-27 20:39   ` Martin von Zweigbergk
2012-11-28  7:12     ` Junio C Hamano
2012-11-30 16:39       ` Martin von Zweigbergk
2012-11-29 18:32 ` [RFC/PATCH 0/2] Fix "git reset" " Martin von Zweigbergk
2012-11-29 18:32   ` [RFC/PATCH 1/2] reset: learn to reset to tree Martin von Zweigbergk
2012-11-29 18:47     ` Junio C Hamano
2012-11-29 19:04       ` Martin von Zweigbergk
2012-11-29 19:36         ` Junio C Hamano
2012-11-29 19:13       ` Junio C Hamano
2012-11-29 22:00         ` Martin von Zweigbergk
2012-11-30 16:45           ` Martin von Zweigbergk
2012-12-01  9:24             ` Junio C Hamano
2012-12-05  3:45               ` Martin von Zweigbergk
2012-12-05  5:46                 ` Junio C Hamano
2012-12-05 12:58                   ` Martin von Zweigbergk
2012-11-29 18:32   ` [RFC/PATCH 2/2] reset: learn to reset on unborn branch Martin von Zweigbergk

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 NNTP newsgroup(s).