Re: [RFC/PATCH 1/2] reset: learn to reset to tree
On Tue, Dec 4, 2012 at 9:46 PM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@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. -- 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: [RFC/PATCH 1/2] reset: learn to reset to tree
On Sat, Dec 1, 2012 at 1:24 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: On Thu, Nov 29, 2012 at 2:00 PM, Martin von Zweigbergk martinv...@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 -- 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: [RFC/PATCH 1/2] reset: learn to reset to tree
Martin von Zweigbergk martinv...@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. -- 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: [RFC/PATCH 1/2] reset: learn to reset to tree
Martin von Zweigbergk martinv...@gmail.com writes: On Thu, Nov 29, 2012 at 2:00 PM, Martin von Zweigbergk martinv...@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/). -- 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: [RFC/PATCH 1/2] reset: learn to reset to tree
On Thu, Nov 29, 2012 at 2:00 PM, Martin von Zweigbergk martinv...@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? -- 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
[RFC/PATCH 1/2] reset: learn to reset to tree
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,
Re: [RFC/PATCH 1/2] reset: learn to reset to tree
Martin von Zweigbergk martinv...@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.
--
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: [RFC/PATCH 1/2] reset: learn to reset to tree
On Thu, Nov 29, 2012 at 10:47 AM, Junio C Hamano gits...@pobox.com wrote:
Martin von Zweigbergk martinv...@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.
--
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: [RFC/PATCH 1/2] reset: learn to reset to tree
Junio C Hamano gits...@pobox.com writes:
Martin von Zweigbergk martinv...@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?
--
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: [RFC/PATCH 1/2] reset: learn to reset to tree
Martin von Zweigbergk martinv...@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? -- 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: [RFC/PATCH 1/2] reset: learn to reset to tree
On Thu, Nov 29, 2012 at 11:13 AM, Junio C Hamano gits...@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. -- 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
