Re: Proposal: create meaningful aliases for git reset's hard/soft/mixed
On Wed, Nov 23, 2011 at 10:51 AM, Junio C Hamano gits...@pobox.com wrote:
I am guilty of introducing git reset --soft HEAD^ before I invented
commit --amend during v1.3.0 timeframe to solve the issue soft reset
originally wanted to.
I do use commit --amend a lot, but I still appreciate having reset
--soft. For example, to squash the last few commits:
git reset --soft HEAD^^^ git commit --amend
or undo commit --amend:
git reset --soft HEAD@{1} git commit --amend
Maybe there's a better way of doing that?
--
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
Fwd: [RFC/FR] Should git checkout (-B|-b) branch master...branch work?
Oops, meant for all of you. -- Forwarded message -- From: Martin von Zweigbergk martinv...@gmail.com Date: Fri, Dec 21, 2012 at 8:45 AM Subject: Re: [RFC/FR] Should git checkout (-B|-b) branch master...branch work? To: Junio C Hamano gits...@pobox.com On Fri, Dec 21, 2012 at 7:58 AM, Junio C Hamano gits...@pobox.com wrote: $ git checkout -B branch old fork point Unfortunately, master...branch syntax does not seem to work for specifying the old fork point for this purpose I have personally always found it confusing to use the same syntax for specifying ranges/sets and single revisions. I keep forgetting what git diff A..B does. I know it doesn't do what I expect (i.e. git diff $(git merge-base A B) B), but I don't know what it does (maybe same as git diff A B (?), but that's besides the point). Having worked a bit on rebase, I know that $onto can also take the A...B form. So there is clearly some precedence for the ... syntax to refer to a revision in some contexts. I would have much preferred if it was possible to make the revision parser generally interpret e.g. A.^.B as the merge base of A and B (failing if not exactly one). It seems like something that must have come up before. Is there a particular reason this would not be a good idea? -- 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 1/2] tests: move test_cmp_rev to test-lib-functions
A function for checking that two given parameters refer to the same
revision was defined in several places, so move the definition to
test-lib-functions.sh instead.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
t/t1505-rev-parse-last.sh | 18 +-
t/t3404-rebase-interactive.sh | 6 --
t/t3507-cherry-pick-conflict.sh | 6 --
t/t3508-cherry-pick-many-commits.sh | 8 ++--
t/t3510-cherry-pick-sequence.sh | 6 --
t/t6030-bisect-porcelain.sh | 4 +---
t/test-lib-functions.sh | 7 +++
7 files changed, 15 insertions(+), 40 deletions(-)
diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
index d709ecf..4969edb 100755
--- a/t/t1505-rev-parse-last.sh
+++ b/t/t1505-rev-parse-last.sh
@@ -32,32 +32,24 @@ test_expect_success 'setup' '
#
# and 'side' should be the last branch
-test_rev_equivalent () {
-
- git rev-parse $1 expect
- git rev-parse $2 output
- test_cmp expect output
-
-}
-
test_expect_success '@{-1} works' '
- test_rev_equivalent side @{-1}
+ test_cmp_rev side @{-1}
'
test_expect_success '@{-1}~2 works' '
- test_rev_equivalent side~2 @{-1}~2
+ test_cmp_rev side~2 @{-1}~2
'
test_expect_success '@{-1}^2 works' '
- test_rev_equivalent side^2 @{-1}^2
+ test_cmp_rev side^2 @{-1}^2
'
test_expect_success '@{-1}@{1} works' '
- test_rev_equivalent side@{1} @{-1}@{1}
+ test_cmp_rev side@{1} @{-1}@{1}
'
test_expect_success '@{-2} works' '
- test_rev_equivalent master @{-2}
+ test_cmp_rev master @{-2}
'
test_expect_success '@{-3} fails' '
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 32fdc99..8462be1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -29,12 +29,6 @@ Initial setup:
. $TEST_DIRECTORY/lib-rebase.sh
-test_cmp_rev () {
- git rev-parse --verify $1 expect.rev
- git rev-parse --verify $2 actual.rev
- test_cmp expect.rev actual.rev
-}
-
set_fake_editor
# WARNING: Modifications to the initial repository can change the SHA ID used
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index c82f721..223b984 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -11,12 +11,6 @@ test_description='test cherry-pick and revert with conflicts
. ./test-lib.sh
-test_cmp_rev () {
- git rev-parse --verify $1 expect.rev
- git rev-parse --verify $2 actual.rev
- test_cmp expect.rev actual.rev
-}
-
pristine_detach () {
git checkout -f $1^0
git read-tree -u --reset HEAD
diff --git a/t/t3508-cherry-pick-many-commits.sh
b/t/t3508-cherry-pick-many-commits.sh
index 340afc7..4e7136b 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -5,15 +5,11 @@ test_description='test cherry-picking many commits'
. ./test-lib.sh
check_head_differs_from() {
- head=$(git rev-parse --verify HEAD)
- arg=$(git rev-parse --verify $1)
- test $head != $arg
+ ! test_cmp_rev HEAD $1
}
check_head_equals() {
- head=$(git rev-parse --verify HEAD)
- arg=$(git rev-parse --verify $1)
- test $head = $arg
+ test_cmp_rev HEAD $1
}
test_expect_success setup '
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index b5fb527..7b7a89d 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -24,12 +24,6 @@ pristine_detach () {
git clean -d -f -f -q -x
}
-test_cmp_rev () {
- git rev-parse --verify $1 expect.rev
- git rev-parse --verify $2 actual.rev
- test_cmp expect.rev actual.rev
-}
-
test_expect_success setup '
git config advice.detachedhead false
echo unrelated unrelated
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 72e28ee..3e0e15f 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -676,9 +676,7 @@ test_expect_success 'bisect fails if tree is broken on
trial commit' '
check_same()
{
echo Checking $1 is the same as $2
- git rev-parse $1 expected.same
- git rev-parse $2 expected.actual
- test_cmp expected.same expected.actual
+ test_cmp_rev $1 $2
}
test_expect_success 'bisect: --no-checkout - start commit bad' '
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 22a4f8f..fa62d01 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -602,6 +602,13 @@ test_cmp() {
$GIT_TEST_CMP $@
}
+# Tests that its two parameters refer to the same revision
+test_cmp_rev () {
+ git rev-parse --verify $1 expect.rev
+ git rev-parse --verify $2 actual.rev
+ test_cmp expect.rev actual.rev
+}
+
# Print a sequence of numbers or letters in increasing order. This is
# similar to GNU seq(1), but the latter
[PATCH 2/2] learn to pick/revert into unborn branch
From the user's point of view, it seems natural to think that
cherry-picking into an unborn branch should work, so make it work,
with or without --ff.
Cherry-picking anything other than a commit that only adds files, will
naturally result in conflicts. Similarly, revert also works, but will
result in conflicts unless the specified revision only deletes files.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
The plan is to use this for fixing git rebase --root as discussed in
http://thread.gmane.org/gmane.comp.version-control.git/205796
Is there a better way of creating an unborn branch than what I do in
the test cases?
sequencer.c | 19 +++
t/t3501-revert-cherry-pick.sh | 9 +
t/t3506-cherry-pick-ff.sh | 8
3 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 2260490..1ac1ceb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -186,14 +186,15 @@ static int error_dirty_index(struct replay_opts *opts)
return -1;
}
-static int fast_forward_to(const unsigned char *to, const unsigned char *from)
+static int fast_forward_to(const unsigned char *to, const unsigned char *from,
+ int unborn)
{
struct ref_lock *ref_lock;
read_cache();
if (checkout_fast_forward(from, to, 1))
exit(1); /* the callee should have complained already */
- ref_lock = lock_any_ref_for_update(HEAD, from, 0);
+ ref_lock = lock_any_ref_for_update(HEAD, unborn ? null_sha1 : from,
0);
return write_ref_sha1(ref_lock, to, cherry-pick);
}
@@ -390,7 +391,7 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
char *defmsg = NULL;
struct strbuf msgbuf = STRBUF_INIT;
- int res;
+ int res, unborn = 0;
if (opts-no_commit) {
/*
@@ -402,9 +403,10 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
if (write_cache_as_tree(head, 0, NULL))
die (_(Your index file is unmerged.));
} else {
- if (get_sha1(HEAD, head))
- return error(_(You do not have a valid HEAD));
- if (index_differs_from(HEAD, 0))
+ unborn = get_sha1(HEAD, head);
+ if (unborn)
+ hashcpy(head, EMPTY_TREE_SHA1_BIN);
+ if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : HEAD,
0))
return error_dirty_index(opts);
}
discard_cache();
@@ -435,8 +437,9 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
else
parent = commit-parents-item;
- if (opts-allow_ff parent !hashcmp(parent-object.sha1, head))
- return fast_forward_to(commit-object.sha1, head);
+ if (opts-allow_ff
+ (parent !hashcmp(parent-object.sha1, head) || !parent
unborn))
+return fast_forward_to(commit-object.sha1, head, unborn);
if (parent parse_commit(parent) 0)
/* TRANSLATORS: The first %s will be revert or
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 34c86e5..6f489e2 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -100,4 +100,13 @@ test_expect_success 'revert forbidden on dirty working
tree' '
'
+test_expect_success 'chery-pick on unborn branch' '
+ git checkout --orphan unborn
+ git rm --cached -r .
+ rm -rf *
+ git cherry-pick initial
+ git diff --quiet initial
+ ! test_cmp_rev initial HEAD
+'
+
test_done
diff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh
index 51ca391..373aad6 100755
--- a/t/t3506-cherry-pick-ff.sh
+++ b/t/t3506-cherry-pick-ff.sh
@@ -105,4 +105,12 @@ test_expect_success 'cherry pick a root commit with --ff' '
test $(git rev-parse --verify HEAD) =
1df192cd8bc58a2b275d842cede4d221ad9000d1
'
+test_expect_success 'chery-pick --ff on unborn branch' '
+ git checkout --orphan unborn
+ git rm --cached -r .
+ rm -rf *
+ git cherry-pick --ff first
+ test_cmp_rev first HEAD
+'
+
test_done
--
1.8.0.1.240.ge8a1f5a
--
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 2/2] learn to pick/revert into unborn branch
On Sat, Dec 22, 2012 at 7:24 PM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: From the user's point of view, it seems natural to think that cherry-picking into an unborn branch should work, so make it work, with or without --ff. I actually am having a hard time imagining how that could ever be natural. Fair enough. What's natural is of course very subjective. In my opinion, whenever possible, operations on an unborn branch should behave exactly as they would on an arbitrary commit whose tree just happens to be empty. Of course, pretty much any operation that needs more than the tree (indirectly) pointed to by HEAD would fail the whenever possible clause. I realize that cherry-pick _does_ need the current commit to record the parent of the resulting commit, but that almost seems like an implementation detail, i.e whether we're adding a parent or adding no parent (when on unborn branch) to the list of parents. In the same way, I think git reset should work on an unborn branch, even though there is no commit that we can be modifying index and working tree to match (from man page). I think many users, like me, think of unborn branches as having an empty tree, rather than being a special state before history is created. Sure, such thinking is not technically correct, but it still seems to be some people's intuition (including mine). Cherry-picking anything other than a commit that only adds files, will naturally result in conflicts. Similarly, revert also works, but will result in conflicts unless the specified revision only deletes files. You may be able to make it work for some definition of work, but I am not sure how useful it is. As for use cases, I didn't consider that much more than that it might be useful for implementing git rebase --root. I haven't implemented that yet, so I can't say for sure that it will work out. One use case might be to rewrite history by creating an new unborn branch and picking the initial commit and a subset of other commits. Anyway, I didn't implement it because I thought it would be very useful, but mostly because I just thought it should work (for completeness). I could resend as part of my rebase series (called mz/rebase-range at some point) once that's done. Then we can discuss another solution in the scope of that series if we don't agree on allowing on cherry-pick on an unborn branch. Btw, I have another series, which I'll send after 1.8.1, that teaches git reset to work on an unborn branch (among other things). We might want to decide to support both or neither of the commands on an unborn branch. On Sat, Dec 22, 2012 at 7:24 PM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: From the user's point of view, it seems natural to think that cherry-picking into an unborn branch should work, so make it work, with or without --ff. I actually am having a hard time imagining how that could ever be natural. When you are on an unborn branch, you may have some files in your working tree, and some of them may even be registered to the index, but the index is merely for your convenience to create your first commit, and as far as the history is concered, it does not matter. By definition you do not have any history in such a state. What does it even mean to cherry-pick another commit, especially without the --no-commit option? The resulting commit will carry the message taken from the original commit, but does what it says match what you have done? I can understand that it may sometimes make sense to do $ git show --diff-filter=A $that_commit | git apply as a way to further update the uncommitted state you have in the working tree, so I can sort of buy that --no-commit case might make some sense (but if you make a commit after cherry-pick --no-commit, you still get the log message from that commit, which does not explain the other things you have in your working tree) in a limited situation. It seems to me that the only case that may make sense is to grab the contents from an existing tree, which might be better served with $ git checkout $that_commit -- $these_paths_I_am_interested_in Cherry-picking anything other than a commit that only adds files, will naturally result in conflicts. Similarly, revert also works, but will result in conflicts unless the specified revision only deletes files. You may be able to make it work for some definition of work, but I am not sure how useful it is. Puzzled... -- 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 2/2] learn to pick/revert into unborn branch
On Sun, Dec 23, 2012 at 11:18 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: On Sat, Dec 22, 2012 at 7:24 PM, Junio C Hamano gits...@pobox.com wrote: I am not opposed to an internal use of the cherry-pick machinery to implement a corner case of rebase -i: 3. You run rebase -i --root to get this insn sheet: pick Add Makefile and hello.c for Hello world pick Add goodbye.c for Goodbye world and swap them: pick Add goodbye.c for Goodbye world pick Add Makefile and hello.c for Hello world Right, and as you point out in your later message, editing the initial commit is another (and more useful) use case. It could of course be special cased in git rebase, but I think doing it git cherry-pick is the right thing to do. Hopefully git-rebase.sh can then just reset to the void state and git-rebase--interactive.sh can just continue without knowing or caring whether it got started on an unborn branch. Hmm... I just realized the branch in unborn branch really means we don't have unborn detached HEAD, do we? So some more tricks are probably necessary after all. :-( [...] It transplants something that used to depend on the entire history behind it to be the beginning of the history so its log needs to be adjusted, but rebase -i can choose to always make it conflict and force the user to write a correct log message, so it won't expose the fundamental flaw you would add if you allowed the end-user facing cherry-pick to pick something to create a new root commit without interaction. If I understand you correctly, you are suggesting that git rebase should set the action from pick to edit for the first commit in the insn sheet if it is not a root commit. git rebase -i --root doesn't currently do that, but it certainly could. I agree that git reset without any commit parameter to reset the index and optionally the working tree (with --hard) should reset from an empty tree when you do not yet have any commit. Good to hear. But I do not think it has anything to do with cherry-pick to empty, so I do not agree with In the same way at all. See later comment. One use case might be to rewrite history by creating an new unborn branch and picking the initial commit and a subset of other commits. If you mean, in the above sample history, to git cherry-pick the commit that starts the Hello world and then do something else on top of the resulting history, how would that be different from forking from that existing root commit? True, the result would be the same. The user's thought process might be a little different (let me start from scratch vs let me start almost from scratch), but that's a very minor difference that I'm sure any user would quickly overcome. Anyway, I didn't implement it because I thought it would be very useful, but mostly because I just thought it should work (for completeness). I would not exactly call X complete if X works in one way in most cases and it works in quite a different way in one other case, only because it would have to barf if it wanted to work in the same way as in most cases, and the different behaviour is chosen only because X that does something is better than X that stops in an impossible situation and barfs. I agree, of course, but I don't see the behavior as different. When thinking about behavior around the root of the history, I imagine that all root commits actually have a parent, and that they all have the same parent. I also imagine that on an unborn branch, instead of being invalid, HEAD points to that same single root commit with an empty tree. Despite this model not matching git's, I find that this helps me reason about what the behavior of various commands should be. With this reasoning, cherry-picking into an unborn branch is no different from cherry-picking into any commit with an empty tree (which of course would be rare, but not forbidden). -- 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: Find the starting point of a local branch
On Sun, Dec 23, 2012 at 11:31 PM, Woody Wu narkewo...@gmail.com wrote: On Sun, Dec 23, 2012 at 11:09:58PM -0500, Seth Robertson wrote: In message 20121224035825.GA17203@zuhnb712, Woody Wu writes: How can I find out what's the staring reference point (a commit number or tag name) of a locally created branch? I can use gitk to find out it but this method is slow, I think there might be a command line to do it quickly. The answer is more complex than you probably suspected. Technically, `git log --oneline mybranch | tail -n 1` will tell you the starting point of any branch. But...I'm sure that isn't what you want to know. You want to know what commit was I at when I typed `git branch mybranch`? Yes, this is exactly I want to know. The problem is git doesn't record this information and doesn't have the slightest clue. But, you say, I can use `gitk` and see it. See? Right there. That isn't (necessarily) the starting point of the branch, it is the place where your branch diverged from some other branch. Git is actually quite able to tell you when the last time your branch diverged from some other branch. `git merge-base mybranch master` will tell you this, and is probably the answer you were looking for. This is not working to me since I have more than one local branch that diverged from the master, and in fact, the branch I have in question was diverged from another local branch. As Jeff mentions in a later message, git pull --rebase would probably do what you want. It works with local branches too. I once tried to add the same cleverness that git pull --rebase directly in git rebase [1], but there were several issues with those patches, one of was regarding the performance (git pull --rebase can be equally slow, but since it often involves network, users probably rarely notice). I think it would be nice to at least add it as an option to git rebase some day. Until then, git pull --rebase works fine. [1] http://thread.gmane.org/gmane.comp.version-control.git/166710 -- 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: Find the starting point of a local branch
On Thu, Dec 27, 2012 at 9:15 PM, Woody Wu narkewo...@gmail.com wrote: On Mon, Dec 24, 2012 at 09:24:39AM -0800, Martin von Zweigbergk wrote: On Sun, Dec 23, 2012 at 11:31 PM, Woody Wu narkewo...@gmail.com wrote: This is not working to me since I have more than one local branch that diverged from the master, and in fact, the branch I have in question was diverged from another local branch. As Jeff mentions in a later message, git pull --rebase would probably do what you want. It works with local branches too. I think what 'git pull --rebase' would do is to fetch from the origin and do a 'git rebase'. Not if the configured upstream is a local branch (see the branch.name.* configuration variables). In that case it will just rebase the local branch onto the new position of its upstream. If the upstream is not configured, I believe you can still do git pull --rebase . upstream branch. On one hand, I don't understand 'git rebase' so much from the manual, ont the other hand, I did not get the point why 'git rebase' has something to do with the thing I want to do (what I want is just query some kind of history information). I may have misunderstood or assumed things incorrectly that you wanted to rebase the commits on your branch. So why do you want to know? (Please ignore me if this was answered elsewhere in the thread that I might have missed.) Anyway, to answer your question, you could use a method similar to what git pull --rebase uses internally to figure out the branch point: git merge-base $(git rev-parse branch) $(git rev-list -g upstream branch) Hope that helps -- 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
Makefile dependency from 'configure' to 'GIT-VERSION-FILE'
Hi,
I use autoconf with git.git. I have noticed lately, especially when
doing things like git rebase -i --exec make, that ./configure is run
every time. If I understand correctly, this is because of 8242ff4
(build: reconfigure automatically if configure.ac changes,
2012-07-19). Just a few days before that commit, on 2012-07-15, the
branch jn/makefile-cleanup including 520a6cd (Makefile: move
GIT-VERSION-FILE dependencies closer to use, 2012-06-20) was merged
(to next?). I wonder if these two subjects were aware of each other.
The reason 'configure' depends on GIT-VERSION-FILE is because it
inserts the version into the call to AC_INIT. I have close to no
experience with autoconf or even make and it's not at all clear to me
why we need to pass the verison to AC_INIT. It seems like it's just
for messages printed by ./configure. If that's the case, we shouldn't
need to generate a new 'configure' file ever time. At the very least,
we shouldn't need to run it.
Do you think we should simply remove the dependency from 'configure'
to 'GIT-VERSION-FILE' and leave a comment there instead? Or should we
instead somehow make 'reconfigure' depend only on 'configure.ac'? Both
of these feel a little wrong to me, because they would remove real
dependencies. Maybe the (probably mangled) patch at the end of this
message is better?
Martin
diff --git a/Makefile b/Makefile
index 736ecd4..ec5d7ca 100644
--- a/Makefile
+++ b/Makefile
@@ -2267,12 +2267,9 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh
mv $@+ $@
endif # NO_PYTHON
-configure: configure.ac GIT-VERSION-FILE
- $(QUIET_GEN)$(RM) $@ $+ \
- sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
- $ $+ \
- autoconf -o $@ $+ \
- $(RM) $+
+configure: configure.ac
+ $(QUIET_GEN)$(RM) $@ \
+ autoconf -o $@ $
ifdef AUTOCONFIGURED
config.status: configure
diff --git a/configure.ac b/configure.ac
index ad215cc..00c3e38 100644
--- a/configure.ac
+++ b/configure.ac
@@ -142,7 +142,10 @@ fi
## Configure body starts here.
AC_PREREQ(2.59)
-AC_INIT([git], [@@GIT_VERSION@@], [git@vger.kernel.org])
+AC_INIT([git],
+ m4_esyscmd([ ./GIT-VERSION-GEN
+{ sed -ne 's/GIT_VERSION = //p' GIT-VERSION-FILE
| xargs echo -n; } ]),
+ [git@vger.kernel.org])
AC_CONFIG_SRCDIR([git.c])
--
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: Makefile dependency from 'configure' to 'GIT-VERSION-FILE'
On Tue, Jan 1, 2013 at 11:21 PM, Jonathan Nieder jrnie...@gmail.com wrote:
How about this patch (untested)?
Looks good. Thanks!
--- a/Makefile
+++ b/Makefile
@@ -2267,12 +2267,9 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % :
unimplemented.sh
mv $@+ $@
endif # NO_PYTHON
-configure: configure.ac GIT-VERSION-FILE
+configure: configure.ac
[...]
--- a/configure.ac
+++ b/configure.ac
@@ -142,7 +142,10 @@ fi
## Configure body starts here.
AC_PREREQ(2.59)
-AC_INIT([git], [@@GIT_VERSION@@], [git@vger.kernel.org])
+AC_INIT([git],
+ m4_esyscmd([ ./GIT-VERSION-GEN
+{ sed -ne 's/GIT_VERSION = //p' GIT-VERSION-FILE |
xargs echo -n; } ]),
+ [git@vger.kernel.org])
I don't think that would warrant dropping the GIT-VERSION-FILE
dependency, since the resulting configure script still hard-codes the
version number.
Yeah, you're right. I was merely sweeping the dependency under the rug :-(
diff --git a/Makefile b/Makefile
index 736ecd45..2a22041f 100644
--- a/Makefile
+++ b/Makefile
@@ -2275,7 +2275,7 @@ configure: configure.ac GIT-VERSION-FILE
$(RM) $+
ifdef AUTOCONFIGURED
-config.status: configure
+config.status: configure.ac
$(QUIET_GEN)if test -f config.status; then \
./config.status --recheck; \
else \
The next line just outside the context here does depend on
'configure', which is why I thought this would not be right. But it
seems impossible to get away from that, and AUTOCONFIGURED should only
be set when ./configure has been run (IIUC), so it's not even
realistic to have git reconfigure fail to find ./configure. So,
again, looks good.
--
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 v2] build: do not automatically reconfigure unless configure.ac changed
diff --git a/Makefile b/Makefile index 26b697d..2f5e2ab 100644 --- a/Makefile +++ b/Makefile @@ -2167,8 +2167,14 @@ configure: configure.ac GIT-VERSION-FILE $(RM) $+ ifdef AUTOCONFIGURED -config.status: configure - $(QUIET_GEN)if test -f config.status; then \ +# We avoid depending on 'configure' here, because it gets rebuilt +# every time GIT-VERSION-FILE is modified, only to update the embedded +# version number string, which config.status does not care about. We +# do want to recheck when the platform/environment detection logic +# changes, hence this depends on configure.ac. +config.status: configure.ac + $(QUIET_GEN)$(MAKE) configure \ + if test -f config.status; then \ ./config.status --recheck; \ else \ ./configure; \ Looks great (at least from my 'make'-incompetent point of view :-)). I do appreciate the comment. Thanks, everyone. -- 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
--- 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
[PATCH 02/19] reset $pathspec: exit with code 0 if successful
git reset $pathspec currently exits with a non-zero exit code if the
worktree is dirty after resetting, which is inconsistent with reset
without pathspec, and it makes it harder to know whether the command
really failed. Change it to exit with code 0 regardless of whether the
worktree is dirty so that non-zero indicates an error.
This makes the 4 disambiguation test cases in t7102 clearer since
they all used to fail, 3 of which failed due to changes in the
work tree. Now only the ambiguous one fails.
---
I suppose this makes documenting the exit code unnecessary, since
return zero iff successful is probably understood to be the default.
The variable in refresh_index() containing the value to be returned is
called has_errors. I'm guessing I shouldn't take the name too
seriously.
builtin/reset.c | 8 +++-
t/t2013-checkout-submodule.sh | 2 +-
t/t7102-reset.sh | 18 --
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 8cc7c72..65413d0 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -119,19 +119,17 @@ static void print_new_head_line(struct commit *commit)
static int update_index_refresh(int fd, struct lock_file *index_lock, int
flags)
{
- int result;
-
if (!index_lock) {
index_lock = xcalloc(1, sizeof(struct lock_file));
fd = hold_locked_index(index_lock, 1);
}
- result = refresh_index(the_index, (flags), NULL, NULL,
- _(Unstaged changes after reset:)) ? 1 : 0;
+ refresh_index(the_index, (flags), NULL, NULL,
+ _(Unstaged changes after reset:));
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(index_lock))
return error (Could not refresh index);
- return result;
+ return 0;
}
static void update_index_from_diff(struct diff_queue_struct *q,
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 70edbb3..06b18f8 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -23,7 +23,7 @@ test_expect_success 'reset submodule updates the index' '
git update-index --refresh
git diff-files --quiet
git diff-index --quiet --cached HEAD
- test_must_fail git reset HEAD^ submodule
+ git reset HEAD^ submodule
test_must_fail git diff-files --quiet
git reset submodule
git diff-files --quiet
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index b096dc8..81b2570 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -388,7 +388,8 @@ test_expect_success 'test --mixed paths' '
echo 4 file4
echo 5 file1
git add file1 file3 file4
- test_must_fail git reset HEAD -- file1 file2 file3
+ git reset HEAD -- file1 file2 file3
+ test_must_fail git diff --quiet
git diff output
test_cmp output expect
git diff --cached output
@@ -402,7 +403,8 @@ test_expect_success 'test resetting the index at give
paths' '
sub/file2
git update-index --add sub/file1 sub/file2
T=$(git write-tree)
- test_must_fail git reset HEAD sub/file2
+ git reset HEAD sub/file2
+ test_must_fail git diff --quiet
U=$(git write-tree)
echo $T
echo $U
@@ -440,7 +442,8 @@ test_expect_success 'resetting specific path that is
unmerged' '
echo 100644 $F3 3 file2
} | git update-index --index-info
git ls-files -u
- test_must_fail git reset HEAD file2
+ git reset HEAD file2
+ test_must_fail git diff --quiet
git diff-index --exit-code --cached HEAD
'
@@ -449,7 +452,8 @@ test_expect_success 'disambiguation (1)' '
git reset --hard
secondfile
git add secondfile
- test_must_fail git reset secondfile
+ git reset secondfile
+ test_must_fail git diff --quiet -- secondfile
test -z $(git diff --cached --name-only)
test -f secondfile
test ! -s secondfile
@@ -474,7 +478,8 @@ test_expect_success 'disambiguation (3)' '
secondfile
git add secondfile
rm -f secondfile
- test_must_fail git reset HEAD secondfile
+ git reset HEAD secondfile
+ test_must_fail git diff --quiet
test -z $(git diff --cached --name-only)
test ! -f secondfile
@@ -486,7 +491,8 @@ test_expect_success 'disambiguation (4)' '
secondfile
git add secondfile
rm -f secondfile
- test_must_fail git reset -- secondfile
+ git reset -- secondfile
+ test_must_fail git diff --quiet
test -z $(git diff --cached --name-only)
test ! -f secondfile
'
--
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
[PATCH 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given
By not returning from inside the if (pathspec) block, we can let the
pathspec-aware and pathspec-less code share a bit more, making it
easier to make future changes that should affect both cases. This also
highlights the similarity between read_from_tree() and reset_index().
---
Should error reporting be aligned too? Speaking of which,
do_diff_cache() never returns anything by 0. Is the return value for
future-proofing?
builtin/reset.c | 42 ++
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 254afa9..9bcad29 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -308,19 +308,6 @@ 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) {
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd = hold_locked_index(lock, 1);
- if (read_from_tree(pathspec, sha1))
- return 1;
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- if (write_cache(index_fd, active_cache, active_nr) ||
- commit_locked_index(lock))
- return error(Could not write new index file.);
- return 0;
- }
-
/* 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. */
@@ -330,11 +317,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type != SOFT) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
- int err = reset_index(sha1, reset_type, quiet);
- if (reset_type == KEEP !err)
- err = reset_index(sha1, MIXED, quiet);
- if (err)
- die(_(Could not reset index file to revision '%s'.),
rev);
+ if (pathspec) {
+ if (read_from_tree(pathspec, sha1))
+ return 1;
+ } else {
+ int err = reset_index(sha1, reset_type, quiet);
+ if (reset_type == KEEP !err)
+ err = reset_index(sha1, MIXED, quiet);
+ if (err)
+ die(_(Could not reset index file to revision
'%s'.), rev);
+ }
if (reset_type == MIXED) /* Report what has not been updated. */
update_index_refresh(
@@ -345,14 +337,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_(Could not write new index file.));
}
- /* Any resets update HEAD to the head being switched to,
-* saving the previous head in ORIG_HEAD before. */
- update_ref_status = update_refs(rev, sha1);
+ if (!pathspec) {
+ /* Any resets without paths update HEAD to the head being
+* switched to, saving the previous head in ORIG_HEAD before. */
+ update_ref_status = update_refs(rev, sha1);
- if (reset_type == HARD !update_ref_status !quiet)
- print_new_head_line(commit);
+ if (reset_type == HARD !update_ref_status !quiet)
+ print_new_head_line(commit);
- remove_branch_state();
+ remove_branch_state();
+ }
return update_ref_status;
}
--
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
[PATCH 05/19] reset.c: extract function for parsing arguments
Declutter cmd_reset() a bit by moving out the argument parsing to its
own function.
---
builtin/reset.c | 71 ++---
1 file changed, 38 insertions(+), 33 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 664fad9..9473725 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -198,36 +198,10 @@ static void die_if_unmerged_cache(int reset_type)
}
-int cmd_reset(int argc, const char **argv, const char *prefix)
-{
- int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
- int patch_mode = 0;
+const char **parse_args(int argc, const char **argv, const char *prefix, const
char **rev_ret) {
+ int i = 0;
const char *rev = HEAD;
- unsigned char sha1[20], *orig = NULL, sha1_orig[20],
- *old_orig = NULL, sha1_old_orig[20];
- const char **pathspec = NULL;
- struct commit *commit;
- struct strbuf msg = STRBUF_INIT;
- const struct option options[] = {
- OPT__QUIET(quiet, N_(be quiet, only report errors)),
- OPT_SET_INT(0, mixed, reset_type,
- N_(reset HEAD and index),
MIXED),
- OPT_SET_INT(0, soft, reset_type, N_(reset only HEAD),
SOFT),
- OPT_SET_INT(0, hard, reset_type,
- N_(reset HEAD, index and working tree), HARD),
- OPT_SET_INT(0, merge, reset_type,
- N_(reset HEAD, index and working tree),
MERGE),
- OPT_SET_INT(0, keep, reset_type,
- N_(reset HEAD but keep local changes), KEEP),
- OPT_BOOLEAN('p', patch, patch_mode, N_(select hunks
interactively)),
- OPT_END()
- };
-
- git_config(git_default_config, NULL);
-
- argc = parse_options(argc, argv, prefix, options, git_reset_usage,
- PARSE_OPT_KEEP_DASHDASH);
-
+ unsigned char unused[20];
/*
* Possible arguments are:
*
@@ -250,7 +224,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_committish(argv[i], unused)) {
/*
* Ok, argv[i] looks like a rev; it should not
* be a filename.
@@ -262,6 +236,40 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
verify_filename(prefix, argv[i], 1);
}
}
+ *rev_ret = rev;
+ return i argc ? get_pathspec(prefix, argv + i) : NULL;
+}
+
+int cmd_reset(int argc, const char **argv, const char *prefix)
+{
+ int reset_type = NONE, update_ref_status = 0, quiet = 0;
+ int patch_mode = 0;
+ const char *rev;
+ unsigned char sha1[20], *orig = NULL, sha1_orig[20],
+ *old_orig = NULL, sha1_old_orig[20];
+ const char **pathspec = NULL;
+ struct commit *commit;
+ struct strbuf msg = STRBUF_INIT;
+ const struct option options[] = {
+ OPT__QUIET(quiet, N_(be quiet, only report errors)),
+ OPT_SET_INT(0, mixed, reset_type,
+ N_(reset HEAD and index),
MIXED),
+ OPT_SET_INT(0, soft, reset_type, N_(reset only HEAD),
SOFT),
+ OPT_SET_INT(0, hard, reset_type,
+ N_(reset HEAD, index and working tree), HARD),
+ OPT_SET_INT(0, merge, reset_type,
+ N_(reset HEAD, index and working tree),
MERGE),
+ OPT_SET_INT(0, keep, reset_type,
+ N_(reset HEAD but keep local changes), KEEP),
+ OPT_BOOLEAN('p', patch, patch_mode, N_(select hunks
interactively)),
+ OPT_END()
+ };
+
+ git_config(git_default_config, NULL);
+
+ argc = parse_options(argc, argv, prefix, options, git_reset_usage,
+ PARSE_OPT_KEEP_DASHDASH);
+ pathspec = parse_args(argc, argv, prefix, rev);
if (get_sha1_committish(rev, sha1))
die(_(Failed to resolve '%s' as a valid ref.), rev);
@@ -277,9 +285,6 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_(Could not parse object '%s'.), rev);
hashcpy(sha1, commit-object.sha1);
- if (i argc)
- pathspec = get_pathspec(prefix, argv + i);
-
if (patch_mode) {
if (reset_type != NONE)
die(_(--patch is incompatible with
--{hard,mixed,soft}));
--
1.8.1.rc3.331.g1ef2165
--
To unsubscribe from this list: send the line unsubscribe git in
the
[PATCH 19/19] reset [--mixed]: use diff-based reset whether or not pathspec was given
Thanks to b65982b (Optimize diff-index --cached using cache-tree,
2009-05-20), resetting with paths is much faster than resetting
without paths. Some timings for the linux-2.6 repo to illustrate this
(best of five, warm cache):
reset reset .
real0m0.219s0m0.080s
user0m0.140s0m0.040s
sys 0m0.070s0m0.030s
These two commands should do the same thing, so instead of having the
user type the trailing . to get the faster do_diff_cache()-based
implementation, always use it when doing a mixed reset, with or
without paths (so git reset $rev would also be faster).
Comparing before and after (best of five):
Before After
reset(warm cache): 0.21 0.07
reset -q (warm cache)0.17 0.03
reset(cold cache): 10.31 2.72
reset -q (cold cache)7.64 0.38
---
Are unmerged entries handled the same? read_from_tree() calls
read_cache(), while reset_index() calls read_cache_unmerged(). I
haven't figured out if/why they should be different.
Are there other differences, or could unpack_trees() learn the same
optimization as do_diff_cache()? Actually, the commit mentioned above
does say
Tweak unpack_trees() logic that is used to read in the tree object
to catch the case where the tree entry we are looking at matches the
index as a whole by looking at the cache-tree.
If there are differences, we are clearly missing tests for them. And
it seems like any difference between them should be fixed, so git
reset and git reset . (from root of tree) do the same thing even
before this patch.
builtin/reset.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 5cd48ac..6db0a10 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 != SOFT) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
- if (pathspec) {
+ if (reset_type == MIXED) {
if (read_from_tree(pathspec, sha1))
return 1;
} else {
--
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
[PATCH 10/19] reset --keep: only write index file once
git reset --keep calls reset_index_file() twice, first doing a
two-way merge to the target revision, updating the index and worktree,
and then resetting the index. After each call, we write the index
file.
In the unlikely event that the second call to reset_index_file()
fails, the index will have been merged to the target revision, but
HEAD will not be updated, leaving the user with a dirty index.
By moving the locking, writing and committing out of
reset_index_file() and into the caller, we can avoid writing the index
twice, thereby making the sure we don't end up in the half-way reset
state. As a bonus, we speed up git reset --keep a little on the
linux-2.6 repo (best of five, warm cache):
Before After
real0m0.315s0m0.296s
user0m0.290s0m0.280s
sys 0m0.020s0m0.010s
---
builtin/reset.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 05ccfd4..8e5d097 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -38,14 +38,12 @@ static inline int is_merge(void)
return !access(git_path(MERGE_HEAD), F_OK);
}
-static int reset_index_file(const unsigned char *sha1, int reset_type, int
quiet)
+static int reset_index(const unsigned char *sha1, int reset_type, int quiet)
{
int nr = 1;
- int newfd;
struct tree_desc desc[2];
struct tree *tree;
struct unpack_trees_options opts;
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
memset(opts, 0, sizeof(opts));
opts.head_idx = 1;
@@ -67,8 +65,6 @@ static int reset_index_file(const unsigned char *sha1, int
reset_type, int quiet
opts.reset = 1;
}
- newfd = hold_locked_index(lock, 1);
-
read_cache_unmerged();
if (reset_type == KEEP) {
@@ -91,10 +87,6 @@ static int reset_index_file(const unsigned char *sha1, int
reset_type, int quiet
prime_cache_tree(active_cache_tree, tree);
}
- if (write_cache(newfd, active_cache, active_nr) ||
- commit_locked_index(lock))
- return error(_(Could not write new index file.));
-
return 0;
}
@@ -340,9 +332,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die_if_unmerged_cache(reset_type);
if (reset_type != SOFT) {
- int err = reset_index_file(sha1, reset_type, quiet);
+ struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+ int newfd = hold_locked_index(lock, 1);
+ int err = reset_index(sha1, reset_type, quiet);
if (reset_type == KEEP !err)
- err = reset_index_file(sha1, MIXED, quiet);
+ err = reset_index(sha1, MIXED, quiet);
+ if (!err
+ (write_cache(newfd, active_cache, active_nr) ||
+commit_locked_index(lock))) {
+ err = error(_(Could not write new index file.));
+ }
if (err)
die(_(Could not reset index file to revision '%s'.),
rev);
}
--
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
[PATCH 13/19] reset.c: move lock, write and commit out of update_index_refresh()
In preparation for the/a following patch, move the locking, writing
and committing of the index file out of update_index_refresh(). The
code duplication caused will soon be taken care of. What remains of
update_index_refresh() is just one line, but it is still called from
two places, so let's leave it for now.
In the process, we expose and fix the minor UI bug that makes us print
Could not refresh index when we fail to write the index file when
invoked with a pathspec. Copy the error message from the pathspec-less
codepath (Could not write new index file.).
---
builtin/reset.c | 34 ++
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index a21ba31..2243b95 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,19 +109,10 @@ static void print_new_head_line(struct commit *commit)
printf(\n);
}
-static int update_index_refresh(int fd, struct lock_file *index_lock, int
flags)
+static void update_index_refresh(int flags)
{
- if (!index_lock) {
- index_lock = xcalloc(1, sizeof(struct lock_file));
- fd = hold_locked_index(index_lock, 1);
- }
-
refresh_index(the_index, (flags), NULL, NULL,
_(Unstaged changes after reset:));
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(index_lock))
- return error (Could not refresh index);
- return 0;
}
static void update_index_from_diff(struct diff_queue_struct *q,
@@ -320,9 +311,14 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (pathspec) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd = hold_locked_index(lock, 1);
- return read_from_tree(pathspec, sha1) ||
- update_index_refresh(index_fd, lock,
- quiet ? REFRESH_QUIET :
REFRESH_IN_PORCELAIN);
+ if (read_from_tree(pathspec, sha1))
+ return 1;
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (write_cache(index_fd, active_cache, active_nr) ||
+ commit_locked_index(lock))
+ return error(Could not write new index file.);
+ return 0;
}
/* Soft reset does not touch the index file nor the working tree
@@ -350,9 +346,15 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type == HARD !update_ref_status !quiet)
print_new_head_line(commit);
- else if (reset_type == MIXED) /* Report what has not been updated. */
- update_index_refresh(0, NULL,
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ else if (reset_type == MIXED) { /* Report what has not been updated. */
+ struct lock_file *index_lock = xcalloc(1, sizeof(struct
lock_file));
+ int fd = hold_locked_index(index_lock, 1);
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (write_cache(fd, active_cache, active_nr) ||
+ commit_locked_index(index_lock))
+ error(Could not refresh index);
+ }
remove_branch_state();
--
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
[PATCH 09/19] reset.c: replace switch by if-else
---
builtin/reset.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 42d1563..05ccfd4 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -351,18 +351,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
* saving the previous head in ORIG_HEAD before. */
update_ref_status = update_refs(rev, sha1);
- switch (reset_type) {
- case HARD:
- if (!update_ref_status !quiet)
- print_new_head_line(commit);
- break;
- case SOFT: /* Nothing else to do. */
- break;
- case MIXED: /* Report what has not been updated. */
+ if (reset_type == HARD !update_ref_status !quiet)
+ print_new_head_line(commit);
+ else if (reset_type == MIXED) /* Report what has not been updated. */
update_index_refresh(0, NULL,
quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- break;
- }
remove_branch_state();
--
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
[PATCH 01/19] reset $pathspec: no need to discard index
Since 34110cd (Make 'unpack_trees()' have a separate source and
destination index, 2008-03-06), the index no longer gets clobbered by
do_diff_cache() and we can remove the code for discarding and
re-reading it.
There are two paths to update_index_refresh() from cmd_reset(), but on
both paths, either read_cache() or read_cache_unmerged() will have
been called, so the call to read_cache() in this method is redundant
(although practically free).
This speeds up git reset -- . a little on the linux-2.6 repo (best
of five, warm cache):
Before After
real0m0.093s0m0.080s
user0m0.040s0m0.020s
sys 0m0.050s0m0.050s
---
builtin/reset.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 915cc9f..8cc7c72 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -126,9 +126,6 @@ static int update_index_refresh(int fd, struct lock_file
*index_lock, int flags)
fd = hold_locked_index(index_lock, 1);
}
- if (read_cache() 0)
- return error(_(Could not read index));
-
result = refresh_index(the_index, (flags), NULL, NULL,
_(Unstaged changes after reset:)) ? 1 : 0;
if (write_cache(fd, active_cache, active_nr) ||
@@ -141,12 +138,6 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
struct diff_options *opt, void *data)
{
int i;
- int *discard_flag = data;
-
- /* do_diff_cache() mangled the index */
- discard_cache();
- *discard_flag = 1;
- read_cache();
for (i = 0; i q-nr; i++) {
struct diff_filespec *one = q-queue[i]-one;
@@ -179,17 +170,15 @@ static int read_from_tree(const char *prefix, const char
**argv,
unsigned char *tree_sha1, int refresh_flags)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd, index_was_discarded = 0;
+ int index_fd;
struct diff_options opt;
memset(opt, 0, sizeof(opt));
diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- opt.format_callback_data = index_was_discarded;
index_fd = hold_locked_index(lock, 1);
- index_was_discarded = 0;
read_cache();
if (do_diff_cache(tree_sha1, opt))
return 1;
@@ -197,9 +186,6 @@ static int read_from_tree(const char *prefix, const char
**argv,
diff_flush(opt);
diff_tree_release_paths(opt);
- if (!index_was_discarded)
- /* The index is still clobbered from do_diff_cache() */
- discard_cache();
return update_index_refresh(index_fd, lock, refresh_flags);
}
--
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
[PATCH 00/19] reset improvements
This is kind of a re-roll of [1] (wow, apparently it took me almost
two months to get done). The goal was, then and now, to teach git
reset to work on an unborn branch and to not require a commit when a
tree would do. This time, I also made some tangential improvements
along the way, mostly related to readability and performance.
As usual, the risker patches are towards the end. In particular, I
find it hard to evaluate how risky the last patch is. That last patch
is responsible for much of the improvements in the timing table below,
so it would be nice if it doesn't break things too badly (test pass,
of course). The timings are best-of-five, wall time.
Command Before After
reset (warm) 0.230.07
reset -q (warm) 0.230.03
reset . (warm) 0.090.07
reset -q . (warm)0.090.03
reset --keep (warm) 0.310.29
reset --keep -q (warm) 0.310.29
reset (cold) 9.742.60
reset -q (cold) 9.850.37
reset . (cold) 2.662.51
reset -q . (cold)2.590.33
reset --keep (cold) 7.587.52
reset --keep -q (cold) 7.377.21
[1] http://thread.gmane.org/gmane.comp.version-control.git/210568/focus=210855
Martin von Zweigbergk (19):
reset $pathspec: no need to discard index
reset $pathspec: exit with code 0 if successful
reset.c: pass pathspec around instead of (prefix, argv) pair
reset: don't allow git reset -- $pathspec in bare repo
reset.c: extract function for parsing arguments
reset.c: remove unnecessary variable 'i'
reset.c: extract function for updating {ORIG,}HEAD
reset.c: share call to die_if_unmerged_cache()
reset.c: replace switch by if-else
reset --keep: only write index file once
reset: avoid redundant error message
reset.c: move update_index_refresh() call out of read_from_tree()
reset.c: move lock, write and commit out of update_index_refresh()
reset [--mixed]: don't write index file twice
reset.c: finish entire cmd_reset() whether or not pathspec is given
reset [--mixed] --quiet: don't refresh index
reset $sha1 $pathspec: require $sha1 only to be treeish
reset: allow reset on unborn branch
reset [--mixed]: use diff-based reset whether or not pathspec was
given
builtin/reset.c| 281 +++--
t/t2013-checkout-submodule.sh | 2 +-
t/t7102-reset.sh | 26 +++-
t/t7106-reset-unborn-branch.sh | 52
4 files changed, 200 insertions(+), 161 deletions(-)
create mode 100755 t/t7106-reset-unborn-branch.sh
--
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
[PATCH 17/19] reset $sha1 $pathspec: require $sha1 only to be treeish
Resetting with paths does not update HEAD and there is nothing else
that a commit should be needed for. Relax the argument parsing so only
a tree is required.
The sha1 is only passed to read_from_tree(), which already only
requires a tree.
The rev variable we pass to run_add_interactive() will resolve to a
tree. 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.
Can the two blocks of code that look up commit or tree be made to
share more? I'm not very familiar with what functions are available. I
think I tried keeping a separate struct object *object to be able to
put the last three lines outside the blocks, but didn't like the
result.
builtin/reset.c | 46 ++
t/t7102-reset.sh | 8
2 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index a2e69eb..4c223bd 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -177,9 +177,10 @@ const char **parse_args(int argc, const char **argv, const
char *prefix, const c
/*
* Possible arguments are:
*
-* git reset [-opts] rev paths...
-* git reset [-opts] rev -- paths...
-* git reset [-opts] -- paths...
+* git reset [-opts] [rev]
+* git reset [-opts] tree [paths...]
+* git reset [-opts] tree -- [paths...]
+* git reset [-opts] -- [paths...]
* git reset [-opts] paths...
*
* At this point, argv points immediately after [-opts].
@@ -194,11 +195,13 @@ const char **parse_args(int argc, const char **argv,
const char *prefix, const c
}
/*
* Otherwise, argv[0] could be either rev or paths and
-* has to be unambiguous.
+* has to be unambiguous. If there is a single argument, it
+* can not be a tree
*/
- else if (!get_sha1_committish(argv[0], unused)) {
+ else if ((argc == 1 !get_sha1_committish(argv[0], unused)) ||
+(argc 1 !get_sha1_treeish(argv[0], unused))) {
/*
-* Ok, argv[0] looks like a rev; it should not
+* Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
verify_non_filename(prefix, argv[0]);
@@ -240,7 +243,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
- struct commit *commit;
+ struct commit *commit = NULL;
const struct option options[] = {
OPT__QUIET(quiet, N_(be quiet, only report errors)),
OPT_SET_INT(0, mixed, reset_type,
@@ -262,19 +265,22 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argc, argv, prefix, rev);
- 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);
+ if (!pathspec) {
+ if (get_sha1_committish(rev, sha1))
+ die(_(Failed to resolve '%s' as a valid revision.),
rev);
+ commit = lookup_commit_reference(sha1);
+ if (!commit)
+ die(_(Could not parse object '%s'.), rev);
+ hashcpy(sha1, commit-object.sha1);
+ } else {
+ struct tree *tree;
+ if (get_sha1_treeish(rev, sha1))
+ die(_(Failed to resolve '%s' as a valid tree.), rev);
+ tree = parse_tree_indirect(sha1);
+ if (!tree)
+ die(_(Could not parse object '%s'.), rev);
+ hashcpy(sha1, tree-object.sha1);
+ }
if (patch_mode) {
if (reset_type != NONE)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 81b2570..1fa2a5f 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -497,4 +497,12 @@ test_expect_success 'disambiguation (4)' '
test ! -f secondfile
'
+test_expect_success 'reset with paths accepts tree' '
+ # for simpler tests, drop
[PATCH 08/19] reset.c: share call to die_if_unmerged_cache()
Use a single condition to guard the call to die_if_unmerged_cache for
both --soft and --keep. This avoids the small distraction of the
precondition check from the logic following it.
Also change an instance of
if (e)
err = err || f();
to the almost as short, but clearer
if (e !err)
err = f();
(which is equivalent since we only care whether exit code is 0)
---
builtin/reset.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4d556e7..42d1563 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -336,15 +336,13 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
/* 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. */
- if (reset_type == SOFT)
+ if (reset_type == SOFT || reset_type == KEEP)
die_if_unmerged_cache(reset_type);
- else {
- int err;
- if (reset_type == KEEP)
- die_if_unmerged_cache(reset_type);
- err = reset_index_file(sha1, reset_type, quiet);
- if (reset_type == KEEP)
- err = err || reset_index_file(sha1, MIXED, quiet);
+
+ if (reset_type != SOFT) {
+ int err = reset_index_file(sha1, reset_type, quiet);
+ if (reset_type == KEEP !err)
+ err = reset_index_file(sha1, MIXED, quiet);
if (err)
die(_(Could not reset index file to revision '%s'.),
rev);
}
--
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
[PATCH 16/19] reset [--mixed] --quiet: don't refresh index
git reset [--mixed] without --quiet refreshes the index in order to
display the Unstaged changes after reset. When --quiet is given,
that output is suppressed, removing the need to refresh the index.
Other porcelain commands that care about a refreshed index should
already be refreshing it, so running e.g. git reset -q git diff
is still safe.
This commit together with 686b2de (oneway_merge(): only lstat() when
told to update worktree, 2012-12-20) removes all calls to lstat() the
worktree from the command.
This speeds up git reset -q a little on the linux-2.6 repo (best
of five, warm cache):
Before After
real0m0.215s0m0.176s
user0m0.150s0m0.130s
sys 0m0.060s0m0.040s
And with cold cache (best of five):
Before After
real0m11.351s 0m8.420s
user0m0.230s0m0.220s
sys 0m0.270s0m0.060s
---
There is a test case in t7102 called '--mixed refreshes the index',
but it only checks that right output it printed. Is the test case not
testing right or not named right? As you can see, I suspect it's the
name/description that should change.
builtin/reset.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 9bcad29..a2e69eb 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,12 +109,6 @@ static void print_new_head_line(struct commit *commit)
printf(\n);
}
-static void update_index_refresh(int flags)
-{
- refresh_index(the_index, (flags), NULL, NULL,
- _(Unstaged changes after reset:));
-}
-
static void update_index_from_diff(struct diff_queue_struct *q,
struct diff_options *opt, void *data)
{
@@ -328,9 +322,9 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_(Could not reset index file to revision
'%s'.), rev);
}
- if (reset_type == MIXED) /* Report what has not been updated. */
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (reset_type == MIXED !quiet) /* Report what has not been
updated. */
+ refresh_index(the_index, REFRESH_IN_PORCELAIN, NULL,
NULL,
+ _(Unstaged changes after reset:));
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
--
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
[PATCH 07/19] reset.c: extract function for updating {ORIG,}HEAD
By extracting the code for updating the HEAD and ORIG_HEAD symbolic
references to a separate function, we declutter cmd_reset() a bit and
we make it clear that e.g. the four variables {,sha1_}{,old_}orig are
only used by this code.
---
builtin/reset.c | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 68be05c..4d556e7 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -239,16 +239,35 @@ const char **parse_args(int argc, const char **argv,
const char *prefix, const c
return *argv ? get_pathspec(prefix, argv) : NULL;
}
+static int update_refs(const char *rev, const unsigned char *sha1) {
+ int update_ref_status;
+ struct strbuf msg = STRBUF_INIT;
+ unsigned char *orig = NULL, sha1_orig[20],
+ *old_orig = NULL, sha1_old_orig[20];
+
+ 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);
+ strbuf_release(msg);
+ return update_ref_status;
+}
+
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
int patch_mode = 0;
const char *rev;
- unsigned char sha1[20], *orig = NULL, sha1_orig[20],
- *old_orig = NULL, sha1_old_orig[20];
+ unsigned char sha1[20];
const char **pathspec = NULL;
struct commit *commit;
- struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
OPT__QUIET(quiet, N_(be quiet, only report errors)),
OPT_SET_INT(0, mixed, reset_type,
@@ -332,17 +351,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
/* 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);
+ update_ref_status = update_refs(rev, sha1);
switch (reset_type) {
case HARD:
@@ -359,7 +368,5 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
remove_branch_state();
- strbuf_release(msg);
-
return update_ref_status;
}
--
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
[PATCH 06/19] reset.c: remove unnecessary variable 'i'
Throughout most of parse_args(), the variable 'i' remains at 0. In the
remaining few cases, we can do pointer arithmentic on argv itself
instead.
---
This is clearly mostly a matter of taste. The remainder of the series
does not depend on it in any way.
builtin/reset.c | 29 ++---
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 9473725..68be05c 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -199,7 +199,6 @@ static void die_if_unmerged_cache(int reset_type)
}
const char **parse_args(int argc, const char **argv, const char *prefix, const
char **rev_ret) {
- int i = 0;
const char *rev = HEAD;
unsigned char unused[20];
/*
@@ -210,34 +209,34 @@ const char **parse_args(int argc, const char **argv,
const char *prefix, const c
* git reset [-opts] -- paths...
* git reset [-opts] paths...
*
-* At this point, argv[i] points immediately after [-opts].
+* At this point, argv points immediately after [-opts].
*/
- if (i argc) {
- if (!strcmp(argv[i], --)) {
- i++; /* reset to HEAD, possibly with paths */
- } else if (i + 1 argc !strcmp(argv[i+1], --)) {
- rev = argv[i];
- i += 2;
+ if (argc) {
+ if (!strcmp(argv[0], --)) {
+ argv++; /* reset to HEAD, possibly with paths */
+ } else if (argc 1 !strcmp(argv[1], --)) {
+ rev = argv[0];
+ argv += 2;
}
/*
-* Otherwise, argv[i] could be either rev or paths and
+* Otherwise, argv[0] could be either rev or paths and
* has to be unambiguous.
*/
- else if (!get_sha1_committish(argv[i], unused)) {
+ else if (!get_sha1_committish(argv[0], unused)) {
/*
-* Ok, argv[i] looks like a rev; it should not
+* Ok, argv[0] looks like a rev; it should not
* be a filename.
*/
- verify_non_filename(prefix, argv[i]);
- rev = argv[i++];
+ verify_non_filename(prefix, argv[0]);
+ rev = *argv++;
} else {
/* Otherwise we treat this as a filename */
- verify_filename(prefix, argv[i], 1);
+ verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
- return i argc ? get_pathspec(prefix, argv + i) : NULL;
+ return *argv ? get_pathspec(prefix, argv) : NULL;
}
int cmd_reset(int argc, const char **argv, const char *prefix)
--
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
[PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair
We use the path arguments in two places in reset.c: in
interactive_reset() and read_from_tree(). Both of these call
get_pathspec(), so we pass the (prefix, arv) pair to both
functions. Move the call to get_pathspec() out of these methods, for
two reasons: 1) One argument is simpler than two. 2) It lets us use
the (arguably clearer) if (pathspec) in place of if (i argc).
---
If I understand correctly, this should be rebased on top of
nd/parse-pathspec. Please let me know.
builtin/reset.c | 27 ++-
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 65413d0..045c960 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -153,26 +153,15 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
}
}
-static int interactive_reset(const char *revision, const char **argv,
-const char *prefix)
-{
- const char **pathspec = NULL;
-
- if (*argv)
- pathspec = get_pathspec(prefix, argv);
-
- return run_add_interactive(revision, --patch=reset, pathspec);
-}
-
-static int read_from_tree(const char *prefix, const char **argv,
- unsigned char *tree_sha1, int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
+ int refresh_flags)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd;
struct diff_options opt;
memset(opt, 0, sizeof(opt));
- diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), opt);
+ diff_tree_setup_paths(pathspec, opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
@@ -216,6 +205,7 @@ 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];
+ const char **pathspec = NULL;
struct commit *commit;
struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
@@ -287,22 +277,25 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_(Could not parse object '%s'.), rev);
hashcpy(sha1, commit-object.sha1);
+ if (i argc)
+ pathspec = get_pathspec(prefix, argv + i);
+
if (patch_mode) {
if (reset_type != NONE)
die(_(--patch is incompatible with
--{hard,mixed,soft}));
- return interactive_reset(rev, argv + i, prefix);
+ return run_add_interactive(rev, --patch=reset, pathspec);
}
/* git reset tree [--] paths... can be used to
* load chosen paths from the tree into the index without
* affecting the working tree nor HEAD. */
- if (i argc) {
+ if (pathspec) {
if (reset_type == MIXED)
warning(_(--mixed with paths is deprecated; use 'git
reset -- paths' instead.));
else if (reset_type != NONE)
die(_(Cannot do %s reset with paths.),
_(reset_type_names[reset_type]));
- return read_from_tree(prefix, argv + i, sha1,
+ return read_from_tree(pathspec, sha1,
quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
}
if (reset_type == NONE)
--
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
[PATCH 11/19] reset: avoid redundant error message
If writing or committing the new index file fails, we print Could not
write new index file. followed by Could not reset index file to
revision $rev.. The first message seems to imply the second, so print
only the first message.
---
builtin/reset.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 8e5d097..54e3c5b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -337,13 +337,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
int err = reset_index(sha1, reset_type, quiet);
if (reset_type == KEEP !err)
err = reset_index(sha1, MIXED, quiet);
- if (!err
- (write_cache(newfd, active_cache, active_nr) ||
-commit_locked_index(lock))) {
- err = error(_(Could not write new index file.));
- }
if (err)
die(_(Could not reset index file to revision '%s'.),
rev);
+ if (write_cache(newfd, active_cache, active_nr) ||
+ commit_locked_index(lock))
+ die(_(Could not write new index file.));
}
/* Any resets update HEAD to the head being switched 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
[PATCH 18/19] reset: allow reset on unborn branch
Some users seem to think, knowingly or not, that being on an unborn
branch is like having a commit with an empty tree checked out, but
when run on an unborn branch, git reset currently fails with:
fatal: Failed to resolve 'HEAD' as a valid ref.
Instead of making users figure out that they should run
git rm --cached -r .
, let's teach git reset without a revision argument, when on an
unborn branch, to behave as if the user asked to reset to an empty
tree. Don't take the analogy with an empty commit too far, though, but
still disallow explictly referring to HEAD in git reset HEAD.
---
builtin/reset.c| 17 --
t/t7106-reset-unborn-branch.sh | 52 ++
2 files changed, 62 insertions(+), 7 deletions(-)
create mode 100755 t/t7106-reset-unborn-branch.sh
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c223bd..5cd48ac 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -239,7 +239,7 @@ static int update_refs(const char *rev, const unsigned char
*sha1) {
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
- int patch_mode = 0;
+ int patch_mode = 0, unborn;
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
@@ -264,8 +264,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, git_reset_usage,
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argc, argv, prefix, rev);
-
- if (!pathspec) {
+ unborn = !strcmp(rev, HEAD) get_sha1(HEAD, sha1);
+ if (unborn) {
+ /* reset on unborn branch: treat as reset to empty tree */
+ hashcpy(sha1, EMPTY_TREE_SHA1_BIN);
+ } else if (!pathspec) {
if (get_sha1_committish(rev, sha1))
die(_(Failed to resolve '%s' as a valid revision.),
rev);
commit = lookup_commit_reference(sha1);
@@ -285,7 +288,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 run_add_interactive(rev, --patch=reset, pathspec);
+ return run_add_interactive(sha1_to_hex(sha1), --patch=reset,
pathspec);
}
/* git reset tree [--] paths... can be used to
@@ -337,16 +340,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_(Could not write new index file.));
}
- if (!pathspec) {
+ if (!pathspec !unborn) {
/* Any resets without paths update HEAD to the head being
* switched to, saving the previous head in ORIG_HEAD before. */
update_ref_status = update_refs(rev, sha1);
if (reset_type == HARD !update_ref_status !quiet)
print_new_head_line(commit);
-
- remove_branch_state();
}
+ if (!pathspec)
+ remove_branch_state();
return update_ref_status;
}
diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
new file mode 100755
index 000..4ff6af4
--- /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
+ test_must_fail git reset HEAD
+'
+
+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 is a no-op' '
+ rm .git/index
+ git add a
+ git reset --soft
+ test $(git ls-files) == a
+'
+
+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.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
[PATCH 12/19] reset.c: move update_index_refresh() call out of read_from_tree()
The final part of cmd_reset() essentially looks like:
if (pathspec) {
...
read_from_tree(...);
} else {
...
reset_index(...);
update_index_refresh(...);
...
}
where read_from_tree() internally also calls
update_index_refresh(). Move the call to update_index_refresh() out of
read_from_tree for symmetry with the 'else' block, making
read_from_tree() and reset_index() closer in functionality.
---
builtin/reset.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 54e3c5b..a21ba31 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -145,11 +145,8 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
}
}
-static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
- int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1)
{
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd;
struct diff_options opt;
memset(opt, 0, sizeof(opt));
@@ -157,7 +154,6 @@ static int read_from_tree(const char **pathspec, unsigned
char *tree_sha1,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- index_fd = hold_locked_index(lock, 1);
read_cache();
if (do_diff_cache(tree_sha1, opt))
return 1;
@@ -165,7 +161,7 @@ static int read_from_tree(const char **pathspec, unsigned
char *tree_sha1,
diff_flush(opt);
diff_tree_release_paths(opt);
- return update_index_refresh(index_fd, lock, refresh_flags);
+ return 0;
}
static void set_reflog_message(struct strbuf *sb, const char *action,
@@ -321,9 +317,13 @@ 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);
+ if (pathspec) {
+ struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+ int index_fd = hold_locked_index(lock, 1);
+ return read_from_tree(pathspec, sha1) ||
+ update_index_refresh(index_fd, lock,
+ 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
--
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 16/19] reset [--mixed] --quiet: don't refresh index
On Wed, Jan 9, 2013 at 9:01 AM, Jeff King p...@peff.net wrote: On Wed, Jan 09, 2013 at 12:16:13AM -0800, Martin von Zweigbergk wrote: git reset [--mixed] without --quiet refreshes the index in order to display the Unstaged changes after reset. When --quiet is given, that output is suppressed, removing the need to refresh the index. Other porcelain commands that care about a refreshed index should already be refreshing it, so running e.g. git reset -q git diff is still safe. Hmm. But git reset -q git diff-files would not be? Right. Actually, git reset -q git diff was perhaps not a good example, because its analogous plumbing command would be git reset -q git diff-files -p, which is also safe. But, as you say, git reset -q git diff-files (without -p) might list files for which only the stat information has changed. We have never been very clear about which commands refresh the index. Yes, git-reset's documentation doesn't mention it. Since reset is about manipulating the index, I'd expect it to be refreshed afterwards. On the other hand, since we have never guaranteed anything, perhaps a careful script should always use git update-index --refresh. Since git diff-files is a plumbing command, users of it to a hopefully a bit more careful than regular users, but you never know. I would not be too surprised if some of our own scripts are not that careful, though. I didn't find any, but I might have missed something. Regardless, this patch was tangential. The goal of this series can be achieved independently of this patch, so if it's too risky, we can drop easily drop it. Also, even though it does make git reset -q faster, I'm not sure how important that is in practice. Most use cases would probably refresh the index afterwards anyway. In such cases, the improvement on warm cache would still be there, but the relative improvement in the cold cache case would be pretty much gone (since the entire tree would be stat'ed by the following refresh anyway). Martin -- 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 16/19] reset [--mixed] --quiet: don't refresh index
On Wed, Jan 9, 2013 at 11:12 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: And as a Porcelain, I would rather expect it to leave the resulting index refreshed. Yeah, I guess you're right. Regular users (those using only porcelain) shouldn't notice, but it does make sense to think that the index would be refreshed after running a porcelain. And the risk of breaking people's scripts seems real too. I'll drop patch this from the re-roll (which I'll also make sure I'll sign off) (FYI, the reason I wrote this patch was because I was surprised that git reset did anything with the worktree at all.) -- 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
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 08/19] reset.c: share call to die_if_unmerged_cache()
On Wed, Jan 9, 2013 at 11:48 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: Use a single condition to guard the call to die_if_unmerged_cache for both --soft and --keep. This avoids the small distraction of the precondition check from the logic following it. Also change an instance of if (e) err = err || f(); to the almost as short, but clearer if (e !err) err = f(); (which is equivalent since we only care whether exit code is 0) It is not just equivalent, but should give us identical result, even if we cared the actual value. If err is initially 0, and f() evaluates to 2, err would be 1 in the first case, but 2 in the second case, right? I think the two might be identical in e.g. JavaScript and Python, but I don't use either much. -- 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 02/21] Add parse_pathspec() that converts cmdline args to struct pathspec
On Sat, Jan 5, 2013 at 10:20 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: + + /* No arguments, no prefix - no pathspec */ + if (!entry !prefix) + return; + /* No arguments with prefix - prefix pathspec */ When working with the old get_pathspec(), I remember wondering if a flag switching between no argument - prefix pathspec and no argument - no pathspec would be worthwhile. I think e.g. 'add' and 'clean' would use the former , while 'reset' and 'commit' would use the latter. Since you're now changing all the callers of get_pathspec(), it seems like the perfect time to ask this question. What do you think? -- 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 09/19] reset.c: replace switch by if-else
On Wed, Jan 9, 2013 at 11:53 AM, Junio C Hamano gits...@pobox.com wrote:
Martin von Zweigbergk martinv...@gmail.com writes:
---
builtin/reset.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 42d1563..05ccfd4 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -351,18 +351,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
* saving the previous head in ORIG_HEAD before. */
update_ref_status = update_refs(rev, sha1);
- switch (reset_type) {
- case HARD:
- if (!update_ref_status !quiet)
- print_new_head_line(commit);
- break;
- case SOFT: /* Nothing else to do. */
- break;
- case MIXED: /* Report what has not been updated. */
+ if (reset_type == HARD !update_ref_status !quiet)
+ print_new_head_line(commit);
+ else if (reset_type == MIXED) /* Report what has not been updated. */
update_index_refresh(0, NULL,
quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- break;
- }
Justification?
Clairvoyance -- the HARD case will soon be the only non-empty case.
It's also missing KEEP and MERGE (but the empty SOFT block is there).
I'll update the message. I will also move the patch a little later in
the series, closer to where it will be useful.
--
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 v2 05/21] commit: convert to use parse_pathspec
On Fri, Jan 11, 2013 at 3:20 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
diff --git a/cache.h b/cache.h
index e52365d..a3c316f 100644
--- a/cache.h
+++ b/cache.h
@@ -476,6 +476,9 @@ extern int ie_modified(const struct index_state *, struct
cache_entry *, struct
/* Pathspec magic */
#define PATHSPEC_FROMTOP(10)
+/* Pathspec flags */
+#define PATHSPEC_EMPTY_MATCH_ALL (10) /* No args means match everything */
+
struct pathspec {
const char **raw; /* get_pathspec() result, not freed by
free_pathspec() */
int nr;
diff --git a/setup.c b/setup.c
index 6e960b9..a26b6c0 100644
--- a/setup.c
+++ b/setup.c
@@ -280,6 +280,9 @@ void parse_pathspec(struct pathspec *pathspec,
if (!entry !prefix)
return;
+ if (!*argv (flags PATHSPEC_EMPTY_MATCH_ALL))
+ return;
+
/* No arguments with prefix - prefix pathspec */
if (!entry) {
static const char *raw[2];
I was surprised not to find these two hunks in 02/21. If they were
there, you wouldn't have to explain in the log message of that patch
that flags is for future-proofing. Also, *argv is written entry
in the surrounding conditions.
--
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 v3 03/31] Add parse_pathspec() that converts cmdline args to struct pathspec
On Sun, Jan 13, 2013 at 4:35 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
+static void parse_pathspec(struct pathspec *pathspec,
+ unsigned magic_mask, unsigned flags,
+ const char *prefix, const char **argv)
+{
+ struct pathspec_item *item;
+ const char *entry = *argv;
...
+ for (i = 0; i n; i++) {
+ const char *arg = argv[i];
Nit: *argv was assigned to entry above. Reassign that variable instead?
--
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] builtin/reset.c: Fix a sparse warning
On Mon, Jan 14, 2013 at 11:28 AM, Ramsay Jones ram...@ramsay1.demon.co.uk wrote: In particular, sparse issues an symbol not declared. Should it be static? warning for the 'parse_args' function. Since this function does not require greater than file visibility, we suppress this warning by simply adding the static modifier to it's decalaration. Oops, how did I miss that? Will fix in re-roll. Thanks. Martin -- 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 v2 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair
We use the path arguments in two places in reset.c: in
interactive_reset() and read_from_tree(). Both of these call
get_pathspec(), so we pass the (prefix, argv) pair to both
functions. Move the call to get_pathspec() out of these methods, for
two reasons: 1) One argument is simpler than two. 2) It lets us use
the (arguably clearer) if (pathspec) in place of if (i argc).
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 27 ++-
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 65413d0..045c960 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -153,26 +153,15 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
}
}
-static int interactive_reset(const char *revision, const char **argv,
-const char *prefix)
-{
- const char **pathspec = NULL;
-
- if (*argv)
- pathspec = get_pathspec(prefix, argv);
-
- return run_add_interactive(revision, --patch=reset, pathspec);
-}
-
-static int read_from_tree(const char *prefix, const char **argv,
- unsigned char *tree_sha1, int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
+ int refresh_flags)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd;
struct diff_options opt;
memset(opt, 0, sizeof(opt));
- diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), opt);
+ diff_tree_setup_paths(pathspec, opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
@@ -216,6 +205,7 @@ 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];
+ const char **pathspec = NULL;
struct commit *commit;
struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
@@ -287,22 +277,25 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_(Could not parse object '%s'.), rev);
hashcpy(sha1, commit-object.sha1);
+ if (i argc)
+ pathspec = get_pathspec(prefix, argv + i);
+
if (patch_mode) {
if (reset_type != NONE)
die(_(--patch is incompatible with
--{hard,mixed,soft}));
- return interactive_reset(rev, argv + i, prefix);
+ return run_add_interactive(rev, --patch=reset, pathspec);
}
/* git reset tree [--] paths... can be used to
* load chosen paths from the tree into the index without
* affecting the working tree nor HEAD. */
- if (i argc) {
+ if (pathspec) {
if (reset_type == MIXED)
warning(_(--mixed with paths is deprecated; use 'git
reset -- paths' instead.));
else if (reset_type != NONE)
die(_(Cannot do %s reset with paths.),
_(reset_type_names[reset_type]));
- return read_from_tree(prefix, argv + i, sha1,
+ return read_from_tree(pathspec, sha1,
quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
}
if (reset_type == NONE)
--
1.8.1.1.454.gce43f05
--
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 v2 04/19] 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. Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- 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.1.454.gce43f05 -- 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 v2 01/19] reset $pathspec: no need to discard index
Since 34110cd (Make 'unpack_trees()' have a separate source and
destination index, 2008-03-06), the index no longer gets clobbered by
do_diff_cache() and we can remove the code for discarding and
re-reading it.
There are two paths to update_index_refresh() from cmd_reset(), but on
both paths, either read_cache() or read_cache_unmerged() will have
been called, so the call to read_cache() in this method is redundant
(although practically free).
This speeds up git reset -- . a little on the linux-2.6 repo (best
of five, warm cache):
Before After
real0m0.093s0m0.080s
user0m0.040s0m0.020s
sys 0m0.050s0m0.050s
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 915cc9f..8cc7c72 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -126,9 +126,6 @@ static int update_index_refresh(int fd, struct lock_file
*index_lock, int flags)
fd = hold_locked_index(index_lock, 1);
}
- if (read_cache() 0)
- return error(_(Could not read index));
-
result = refresh_index(the_index, (flags), NULL, NULL,
_(Unstaged changes after reset:)) ? 1 : 0;
if (write_cache(fd, active_cache, active_nr) ||
@@ -141,12 +138,6 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
struct diff_options *opt, void *data)
{
int i;
- int *discard_flag = data;
-
- /* do_diff_cache() mangled the index */
- discard_cache();
- *discard_flag = 1;
- read_cache();
for (i = 0; i q-nr; i++) {
struct diff_filespec *one = q-queue[i]-one;
@@ -179,17 +170,15 @@ static int read_from_tree(const char *prefix, const char
**argv,
unsigned char *tree_sha1, int refresh_flags)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd, index_was_discarded = 0;
+ int index_fd;
struct diff_options opt;
memset(opt, 0, sizeof(opt));
diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- opt.format_callback_data = index_was_discarded;
index_fd = hold_locked_index(lock, 1);
- index_was_discarded = 0;
read_cache();
if (do_diff_cache(tree_sha1, opt))
return 1;
@@ -197,9 +186,6 @@ static int read_from_tree(const char *prefix, const char
**argv,
diff_flush(opt);
diff_tree_release_paths(opt);
- if (!index_was_discarded)
- /* The index is still clobbered from do_diff_cache() */
- discard_cache();
return update_index_refresh(index_fd, lock, refresh_flags);
}
--
1.8.1.1.454.gce43f05
--
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 v2 02/19] reset $pathspec: exit with code 0 if successful
git reset $pathspec currently exits with a non-zero exit code if the
worktree is dirty after resetting, which is inconsistent with reset
without pathspec, and it makes it harder to know whether the command
really failed. Change it to exit with code 0 regardless of whether the
worktree is dirty so that non-zero indicates an error.
This makes the 4 disambiguation test cases in t7102 clearer since
they all used to fail, 3 of which failed due to changes in the
work tree. Now only the ambiguous one fails.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 8 +++-
t/t2013-checkout-submodule.sh | 2 +-
t/t7102-reset.sh | 18 --
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 8cc7c72..65413d0 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -119,19 +119,17 @@ static void print_new_head_line(struct commit *commit)
static int update_index_refresh(int fd, struct lock_file *index_lock, int
flags)
{
- int result;
-
if (!index_lock) {
index_lock = xcalloc(1, sizeof(struct lock_file));
fd = hold_locked_index(index_lock, 1);
}
- result = refresh_index(the_index, (flags), NULL, NULL,
- _(Unstaged changes after reset:)) ? 1 : 0;
+ refresh_index(the_index, (flags), NULL, NULL,
+ _(Unstaged changes after reset:));
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(index_lock))
return error (Could not refresh index);
- return result;
+ return 0;
}
static void update_index_from_diff(struct diff_queue_struct *q,
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 70edbb3..06b18f8 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -23,7 +23,7 @@ test_expect_success 'reset submodule updates the index' '
git update-index --refresh
git diff-files --quiet
git diff-index --quiet --cached HEAD
- test_must_fail git reset HEAD^ submodule
+ git reset HEAD^ submodule
test_must_fail git diff-files --quiet
git reset submodule
git diff-files --quiet
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index b096dc8..81b2570 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -388,7 +388,8 @@ test_expect_success 'test --mixed paths' '
echo 4 file4
echo 5 file1
git add file1 file3 file4
- test_must_fail git reset HEAD -- file1 file2 file3
+ git reset HEAD -- file1 file2 file3
+ test_must_fail git diff --quiet
git diff output
test_cmp output expect
git diff --cached output
@@ -402,7 +403,8 @@ test_expect_success 'test resetting the index at give
paths' '
sub/file2
git update-index --add sub/file1 sub/file2
T=$(git write-tree)
- test_must_fail git reset HEAD sub/file2
+ git reset HEAD sub/file2
+ test_must_fail git diff --quiet
U=$(git write-tree)
echo $T
echo $U
@@ -440,7 +442,8 @@ test_expect_success 'resetting specific path that is
unmerged' '
echo 100644 $F3 3 file2
} | git update-index --index-info
git ls-files -u
- test_must_fail git reset HEAD file2
+ git reset HEAD file2
+ test_must_fail git diff --quiet
git diff-index --exit-code --cached HEAD
'
@@ -449,7 +452,8 @@ test_expect_success 'disambiguation (1)' '
git reset --hard
secondfile
git add secondfile
- test_must_fail git reset secondfile
+ git reset secondfile
+ test_must_fail git diff --quiet -- secondfile
test -z $(git diff --cached --name-only)
test -f secondfile
test ! -s secondfile
@@ -474,7 +478,8 @@ test_expect_success 'disambiguation (3)' '
secondfile
git add secondfile
rm -f secondfile
- test_must_fail git reset HEAD secondfile
+ git reset HEAD secondfile
+ test_must_fail git diff --quiet
test -z $(git diff --cached --name-only)
test ! -f secondfile
@@ -486,7 +491,8 @@ test_expect_success 'disambiguation (4)' '
secondfile
git add secondfile
rm -f secondfile
- test_must_fail git reset -- secondfile
+ git reset -- secondfile
+ test_must_fail git diff --quiet
test -z $(git diff --cached --name-only)
test ! -f secondfile
'
--
1.8.1.1.454.gce43f05
--
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 v2 17/19] reset $sha1 $pathspec: require $sha1 only to be treeish
Resetting with paths does not update HEAD and there is nothing else
that a commit should be needed for. Relax the argument parsing so only
a tree is required.
The sha1 is only passed to read_from_tree(), which already only
requires a tree.
The rev variable we pass to run_add_interactive() will resolve to a
tree. This is fine since interactive_reset only needs the parameter to
be a treeish and doesn't use it for display purposes.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 48 +++-
t/t7102-reset.sh | 8
2 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 520c1a5..b776867 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -178,9 +178,10 @@ static const char **parse_args(const char **argv, const
char *prefix, const char
/*
* Possible arguments are:
*
-* git reset [-opts] rev paths...
-* git reset [-opts] rev -- paths...
-* git reset [-opts] -- paths...
+* git reset [-opts] [rev]
+* git reset [-opts] tree [paths...]
+* git reset [-opts] tree -- [paths...]
+* git reset [-opts] -- [paths...]
* git reset [-opts] paths...
*
* At this point, argv points immediately after [-opts].
@@ -195,11 +196,13 @@ static const char **parse_args(const char **argv, const
char *prefix, const char
}
/*
* Otherwise, argv[0] could be either rev or paths and
-* has to be unambiguous.
+* has to be unambiguous. If there is a single argument, it
+* can not be a tree
*/
- else if (!get_sha1_committish(argv[0], unused)) {
+ else if ((!argv[1] !get_sha1_committish(argv[0], unused)) ||
+(argv[1] !get_sha1_treeish(argv[0], unused))) {
/*
-* Ok, argv[0] looks like a rev; it should not
+* Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
verify_non_filename(prefix, argv[0]);
@@ -241,7 +244,6 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
- struct commit *commit;
const struct option options[] = {
OPT__QUIET(quiet, N_(be quiet, only report errors)),
OPT_SET_INT(0, mixed, reset_type,
@@ -263,19 +265,23 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argv, prefix, rev);
- 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);
+ if (!pathspec) {
+ struct commit *commit;
+ if (get_sha1_committish(rev, sha1))
+ die(_(Failed to resolve '%s' as a valid revision.),
rev);
+ commit = lookup_commit_reference(sha1);
+ if (!commit)
+ die(_(Could not parse object '%s'.), rev);
+ hashcpy(sha1, commit-object.sha1);
+ } else {
+ struct tree *tree;
+ if (get_sha1_treeish(rev, sha1))
+ die(_(Failed to resolve '%s' as a valid tree.), rev);
+ tree = parse_tree_indirect(sha1);
+ if (!tree)
+ die(_(Could not parse object '%s'.), rev);
+ hashcpy(sha1, tree-object.sha1);
+ }
if (patch_mode) {
if (reset_type != NONE)
@@ -340,7 +346,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
update_ref_status = update_refs(rev, sha1);
if (reset_type == HARD !update_ref_status !quiet)
- print_new_head_line(commit);
+ print_new_head_line(lookup_commit_reference(sha1));
remove_branch_state();
}
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 81b2570..1fa2a5f 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -497,4 +497,12 @@ test_expect_success 'disambiguation (4)' '
test ! -f secondfile
'
+test_expect_success 'reset with paths accepts tree' '
+ # for simpler tests, drop last commit containing
[PATCH v2 19/19] reset [--mixed]: use diff-based reset whether or not pathspec was given
Thanks to b65982b (Optimize diff-index --cached using cache-tree,
2009-05-20), resetting with paths is much faster than resetting
without paths. Some timings for the linux-2.6 repo to illustrate this
(best of five, warm cache):
reset reset .
real0m0.219s0m0.080s
user0m0.140s0m0.040s
sys 0m0.070s0m0.030s
These two commands should do the same thing, so instead of having the
user type the trailing . to get the faster do_diff_cache()-based
implementation, always use it when doing a mixed reset, with or
without paths (so git reset $rev would also be faster).
Timing git reset shows that it indeed becomes as fast as
git reset . after this patch.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
It seems like a better solution would be for unpack_trees() learn the
same tricks as do_diff_cache(). I'm leaving that a challange for the
reader :-). I did have a look a unpack_trees(), but it looked rather
overwhelming.
builtin/reset.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 45b01eb..921afbe 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -322,7 +322,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type != SOFT) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
- if (pathspec) {
+ if (reset_type == MIXED) {
if (read_from_tree(pathspec, sha1))
return 1;
} else {
--
1.8.1.1.454.gce43f05
--
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 v2 18/19] reset: allow reset on unborn branch
Some users seem to think, knowingly or not, that being on an unborn
branch is like having a commit with an empty tree checked out, but
when run on an unborn branch, git reset currently fails with:
fatal: Failed to resolve 'HEAD' as a valid ref.
Instead of making users figure out that they should run
git rm --cached -r .
, let's teach git reset without a revision argument, when on an
unborn branch, to behave as if the user asked to reset to an empty
tree. Don't take the analogy with an empty commit too far, though, but
still disallow explictly referring to HEAD in git reset HEAD.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c| 16 -
t/t7106-reset-unborn-branch.sh | 52 ++
2 files changed, 62 insertions(+), 6 deletions(-)
create mode 100755 t/t7106-reset-unborn-branch.sh
diff --git a/builtin/reset.c b/builtin/reset.c
index b776867..45b01eb 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -240,7 +240,7 @@ static int update_refs(const char *rev, const unsigned char
*sha1)
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
- int patch_mode = 0;
+ int patch_mode = 0, unborn;
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
@@ -265,7 +265,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argv, prefix, rev);
- if (!pathspec) {
+ unborn = !strcmp(rev, HEAD) get_sha1(HEAD, sha1);
+ if (unborn) {
+ /* reset on unborn branch: treat as reset to empty tree */
+ hashcpy(sha1, EMPTY_TREE_SHA1_BIN);
+ } else if (!pathspec) {
struct commit *commit;
if (get_sha1_committish(rev, sha1))
die(_(Failed to resolve '%s' as a valid revision.),
rev);
@@ -286,7 +290,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 run_add_interactive(rev, --patch=reset, pathspec);
+ return run_add_interactive(sha1_to_hex(sha1), --patch=reset,
pathspec);
}
/* git reset tree [--] paths... can be used to
@@ -340,16 +344,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_(Could not write new index file.));
}
- if (!pathspec) {
+ if (!pathspec !unborn) {
/* Any resets without paths update HEAD to the head being
* switched to, saving the previous head in ORIG_HEAD before. */
update_ref_status = update_refs(rev, sha1);
if (reset_type == HARD !update_ref_status !quiet)
print_new_head_line(lookup_commit_reference(sha1));
-
- remove_branch_state();
}
+ if (!pathspec)
+ remove_branch_state();
return update_ref_status;
}
diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
new file mode 100755
index 000..8062cf5
--- /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
+ test_must_fail git reset HEAD
+'
+
+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 is a no-op' '
+ rm .git/index
+ git add a
+ git reset --soft
+ test $(git ls-files) = a
+'
+
+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.1.1.454.gce43f05
--
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 v2 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given
By not returning from inside the if (pathspec) block, we can let the
pathspec-aware and pathspec-less code share a bit more, making it
easier to make future changes that should affect both cases. This also
highlights the similarity between read_from_tree() and reset_index().
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 42 ++
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index e8a3e41..c316d9b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -309,19 +309,6 @@ 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) {
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd = hold_locked_index(lock, 1);
- if (read_from_tree(pathspec, sha1))
- return 1;
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- if (write_cache(index_fd, active_cache, active_nr) ||
- commit_locked_index(lock))
- return error(Could not write new index file.);
- return 0;
- }
-
/* 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. */
@@ -331,11 +318,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type != SOFT) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
- int err = reset_index(sha1, reset_type, quiet);
- if (reset_type == KEEP !err)
- err = reset_index(sha1, MIXED, quiet);
- if (err)
- die(_(Could not reset index file to revision '%s'.),
rev);
+ if (pathspec) {
+ if (read_from_tree(pathspec, sha1))
+ return 1;
+ } else {
+ int err = reset_index(sha1, reset_type, quiet);
+ if (reset_type == KEEP !err)
+ err = reset_index(sha1, MIXED, quiet);
+ if (err)
+ die(_(Could not reset index file to revision
'%s'.), rev);
+ }
if (reset_type == MIXED) /* Report what has not been updated. */
update_index_refresh(
@@ -346,14 +338,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_(Could not write new index file.));
}
- /* Any resets update HEAD to the head being switched to,
-* saving the previous head in ORIG_HEAD before. */
- update_ref_status = update_refs(rev, sha1);
+ if (!pathspec) {
+ /* Any resets without paths update HEAD to the head being
+* switched to, saving the previous head in ORIG_HEAD before. */
+ update_ref_status = update_refs(rev, sha1);
- if (reset_type == HARD !update_ref_status !quiet)
- print_new_head_line(commit);
+ if (reset_type == HARD !update_ref_status !quiet)
+ print_new_head_line(commit);
- remove_branch_state();
+ remove_branch_state();
+ }
return update_ref_status;
}
--
1.8.1.1.454.gce43f05
--
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 v2 13/19] reset.c: move lock, write and commit out of update_index_refresh()
In preparation for the/a following patch, move the locking, writing
and committing of the index file out of update_index_refresh(). The
code duplication caused will soon be taken care of. What remains of
update_index_refresh() is just one line, but it is still called from
two places, so let's leave it for now.
In the process, we expose and fix the minor UI bug that makes us print
Could not refresh index when we fail to write the index file when
invoked with a pathspec. Copy the error message from the pathspec-less
codepath (Could not write new index file.).
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 34 ++
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 70733c2..c1d6ef2 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,19 +109,10 @@ static void print_new_head_line(struct commit *commit)
printf(\n);
}
-static int update_index_refresh(int fd, struct lock_file *index_lock, int
flags)
+static void update_index_refresh(int flags)
{
- if (!index_lock) {
- index_lock = xcalloc(1, sizeof(struct lock_file));
- fd = hold_locked_index(index_lock, 1);
- }
-
refresh_index(the_index, (flags), NULL, NULL,
_(Unstaged changes after reset:));
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(index_lock))
- return error (Could not refresh index);
- return 0;
}
static void update_index_from_diff(struct diff_queue_struct *q,
@@ -321,9 +312,14 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (pathspec) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd = hold_locked_index(lock, 1);
- return read_from_tree(pathspec, sha1) ||
- update_index_refresh(index_fd, lock,
- quiet ? REFRESH_QUIET :
REFRESH_IN_PORCELAIN);
+ if (read_from_tree(pathspec, sha1))
+ return 1;
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (write_cache(index_fd, active_cache, active_nr) ||
+ commit_locked_index(lock))
+ return error(Could not write new index file.);
+ return 0;
}
/* Soft reset does not touch the index file nor the working tree
@@ -351,9 +347,15 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type == HARD !update_ref_status !quiet)
print_new_head_line(commit);
- else if (reset_type == MIXED) /* Report what has not been updated. */
- update_index_refresh(0, NULL,
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ else if (reset_type == MIXED) { /* Report what has not been updated. */
+ struct lock_file *index_lock = xcalloc(1, sizeof(struct
lock_file));
+ int fd = hold_locked_index(index_lock, 1);
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (write_cache(fd, active_cache, active_nr) ||
+ commit_locked_index(index_lock))
+ error(Could not refresh index);
+ }
remove_branch_state();
--
1.8.1.1.454.gce43f05
--
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 v2 10/19] reset: avoid redundant error message
If writing or committing the new index file fails, we print Could not
write new index file. followed by Could not reset index file to
revision $rev.. The first message seems to imply the second, so print
only the first message.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 7c440ad..97fa9f7 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -338,13 +338,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
int err = reset_index(sha1, reset_type, quiet);
if (reset_type == KEEP !err)
err = reset_index(sha1, MIXED, quiet);
- if (!err
- (write_cache(newfd, active_cache, active_nr) ||
-commit_locked_index(lock))) {
- err = error(_(Could not write new index file.));
- }
if (err)
die(_(Could not reset index file to revision '%s'.),
rev);
+ if (write_cache(newfd, active_cache, active_nr) ||
+ commit_locked_index(lock))
+ die(_(Could not write new index file.));
}
/* Any resets update HEAD to the head being switched to,
--
1.8.1.1.454.gce43f05
--
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 v2 14/19] reset [--mixed]: only write index file once
When doing a mixed reset without paths, the index is locked, read,
reset, and written back as part of the actual reset operation (in
reset_index()). Then, when showing the list of worktree modifications,
we lock the index again, refresh it, and write it.
Change this so we only write the index once, making git reset a
little faster. It does mean that the index lock will be held a little
longer, but the difference is small compared to the time spent
refreshing the index.
There is one minor functional difference: We used to say Could not
write new index file. if the first write failed, and Could not
refresh index if the second write failed. Now, we will only use the
first message.
This speeds up git reset a little on the linux-2.6 repo (best of
five, warm cache):
Before After
real0m0.239s0m0.214s
user0m0.160s0m0.130s
sys 0m0.070s0m0.080s
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index c1d6ef2..e8a3e41 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -336,6 +336,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
err = reset_index(sha1, MIXED, quiet);
if (err)
die(_(Could not reset index file to revision '%s'.),
rev);
+
+ if (reset_type == MIXED) /* Report what has not been updated. */
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
die(_(Could not write new index file.));
@@ -347,15 +352,6 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type == HARD !update_ref_status !quiet)
print_new_head_line(commit);
- else if (reset_type == MIXED) { /* Report what has not been updated. */
- struct lock_file *index_lock = xcalloc(1, sizeof(struct
lock_file));
- int fd = hold_locked_index(index_lock, 1);
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(index_lock))
- error(Could not refresh index);
- }
remove_branch_state();
--
1.8.1.1.454.gce43f05
--
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 v2 05/19] reset.c: extract function for parsing arguments
Declutter cmd_reset() a bit by moving out the argument parsing to its
own function.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 70 +++--
1 file changed, 38 insertions(+), 32 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 664fad9..58f0f61 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -198,36 +198,11 @@ static void die_if_unmerged_cache(int reset_type)
}
-int cmd_reset(int argc, const char **argv, const char *prefix)
+static const char **parse_args(int argc, const char **argv, const char
*prefix, const char **rev_ret)
{
- int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
- int patch_mode = 0;
+ int i = 0;
const char *rev = HEAD;
- unsigned char sha1[20], *orig = NULL, sha1_orig[20],
- *old_orig = NULL, sha1_old_orig[20];
- const char **pathspec = NULL;
- struct commit *commit;
- struct strbuf msg = STRBUF_INIT;
- const struct option options[] = {
- OPT__QUIET(quiet, N_(be quiet, only report errors)),
- OPT_SET_INT(0, mixed, reset_type,
- N_(reset HEAD and index),
MIXED),
- OPT_SET_INT(0, soft, reset_type, N_(reset only HEAD),
SOFT),
- OPT_SET_INT(0, hard, reset_type,
- N_(reset HEAD, index and working tree), HARD),
- OPT_SET_INT(0, merge, reset_type,
- N_(reset HEAD, index and working tree),
MERGE),
- OPT_SET_INT(0, keep, reset_type,
- N_(reset HEAD but keep local changes), KEEP),
- OPT_BOOLEAN('p', patch, patch_mode, N_(select hunks
interactively)),
- OPT_END()
- };
-
- git_config(git_default_config, NULL);
-
- argc = parse_options(argc, argv, prefix, options, git_reset_usage,
- PARSE_OPT_KEEP_DASHDASH);
-
+ unsigned char unused[20];
/*
* Possible arguments are:
*
@@ -250,7 +225,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_committish(argv[i], unused)) {
/*
* Ok, argv[i] looks like a rev; it should not
* be a filename.
@@ -262,6 +237,40 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
verify_filename(prefix, argv[i], 1);
}
}
+ *rev_ret = rev;
+ return i argc ? get_pathspec(prefix, argv + i) : NULL;
+}
+
+int cmd_reset(int argc, const char **argv, const char *prefix)
+{
+ int reset_type = NONE, update_ref_status = 0, quiet = 0;
+ int patch_mode = 0;
+ const char *rev;
+ unsigned char sha1[20], *orig = NULL, sha1_orig[20],
+ *old_orig = NULL, sha1_old_orig[20];
+ const char **pathspec = NULL;
+ struct commit *commit;
+ struct strbuf msg = STRBUF_INIT;
+ const struct option options[] = {
+ OPT__QUIET(quiet, N_(be quiet, only report errors)),
+ OPT_SET_INT(0, mixed, reset_type,
+ N_(reset HEAD and index),
MIXED),
+ OPT_SET_INT(0, soft, reset_type, N_(reset only HEAD),
SOFT),
+ OPT_SET_INT(0, hard, reset_type,
+ N_(reset HEAD, index and working tree), HARD),
+ OPT_SET_INT(0, merge, reset_type,
+ N_(reset HEAD, index and working tree),
MERGE),
+ OPT_SET_INT(0, keep, reset_type,
+ N_(reset HEAD but keep local changes), KEEP),
+ OPT_BOOLEAN('p', patch, patch_mode, N_(select hunks
interactively)),
+ OPT_END()
+ };
+
+ git_config(git_default_config, NULL);
+
+ argc = parse_options(argc, argv, prefix, options, git_reset_usage,
+ PARSE_OPT_KEEP_DASHDASH);
+ pathspec = parse_args(argc, argv, prefix, rev);
if (get_sha1_committish(rev, sha1))
die(_(Failed to resolve '%s' as a valid ref.), rev);
@@ -277,9 +286,6 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_(Could not parse object '%s'.), rev);
hashcpy(sha1, commit-object.sha1);
- if (i argc)
- pathspec = get_pathspec(prefix, argv + i);
-
if (patch_mode) {
if (reset_type != NONE)
die(_(--patch is incompatible with
--{hard,mixed,soft}));
--
1.8.1.1.454.gce43f05
[PATCH v2 11/19] reset.c: replace switch by if-else
The switch statement towards the end of reset.c is missing case arms
for KEEP and MERGE for no obvious reason, and soon the only non-empty
case arm will be the one for HARD. So let's proactively replace it by
if-else, which will let us move one if statement out without leaving
funny-looking left-overs.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 97fa9f7..c3eb2eb 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -349,18 +349,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
* saving the previous head in ORIG_HEAD before. */
update_ref_status = update_refs(rev, sha1);
- switch (reset_type) {
- case HARD:
- if (!update_ref_status !quiet)
- print_new_head_line(commit);
- break;
- case SOFT: /* Nothing else to do. */
- break;
- case MIXED: /* Report what has not been updated. */
+ if (reset_type == HARD !update_ref_status !quiet)
+ print_new_head_line(commit);
+ else if (reset_type == MIXED) /* Report what has not been updated. */
update_index_refresh(0, NULL,
quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- break;
- }
remove_branch_state();
--
1.8.1.1.454.gce43f05
--
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 v2 16/19] reset.c: inline update_index_refresh()
Now that there is only one caller left to the single-line method
update_index_refresh(), inline it.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index c316d9b..520c1a5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,12 +109,6 @@ static void print_new_head_line(struct commit *commit)
printf(\n);
}
-static void update_index_refresh(int flags)
-{
- refresh_index(the_index, (flags), NULL, NULL,
- _(Unstaged changes after reset:));
-}
-
static void update_index_from_diff(struct diff_queue_struct *q,
struct diff_options *opt, void *data)
{
@@ -329,9 +323,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_(Could not reset index file to revision
'%s'.), rev);
}
- if (reset_type == MIXED) /* Report what has not been updated. */
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (reset_type == MIXED) { /* Report what has not been updated.
*/
+ int flags = quiet ? REFRESH_QUIET :
REFRESH_IN_PORCELAIN;
+ refresh_index(the_index, flags, NULL, NULL,
+ _(Unstaged changes after reset:));
+ }
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
--
1.8.1.1.454.gce43f05
--
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 v2 07/19] reset.c: extract function for updating {ORIG_,}HEAD
By extracting the code for updating the HEAD and ORIG_HEAD symbolic
references to a separate function, we declutter cmd_reset() a bit and
we make it clear that e.g. the four variables {,sha1_}{,old_}orig are
only used by this code.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index d89cf4d..2187d64 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -240,16 +240,35 @@ static const char **parse_args(const char **argv, const
char *prefix, const char
return argv[0] ? get_pathspec(prefix, argv) : NULL;
}
+static int update_refs(const char *rev, const unsigned char *sha1)
+{
+ int update_ref_status;
+ struct strbuf msg = STRBUF_INIT;
+ unsigned char *orig = NULL, sha1_orig[20],
+ *old_orig = NULL, sha1_old_orig[20];
+
+ 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);
+ strbuf_release(msg);
+ return update_ref_status;
+}
+
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
int patch_mode = 0;
const char *rev;
- unsigned char sha1[20], *orig = NULL, sha1_orig[20],
- *old_orig = NULL, sha1_old_orig[20];
+ unsigned char sha1[20];
const char **pathspec = NULL;
struct commit *commit;
- struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
OPT__QUIET(quiet, N_(be quiet, only report errors)),
OPT_SET_INT(0, mixed, reset_type,
@@ -333,17 +352,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
/* 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);
+ update_ref_status = update_refs(rev, sha1);
switch (reset_type) {
case HARD:
@@ -360,7 +369,5 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
remove_branch_state();
- strbuf_release(msg);
-
return update_ref_status;
}
--
1.8.1.1.454.gce43f05
--
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 v2 09/19] reset --keep: only write index file once
git reset --keep calls reset_index_file() twice, first doing a
two-way merge to the target revision, updating the index and worktree,
and then resetting the index. After each call, we write the index
file.
In the unlikely event that the second call to reset_index_file()
fails, the index will have been merged to the target revision, but
HEAD will not be updated, leaving the user with a dirty index.
By moving the locking, writing and committing out of
reset_index_file() and into the caller, we can avoid writing the index
twice, thereby making the sure we don't end up in the half-way reset
state. As a bonus, we speed up git reset --keep a little on the
linux-2.6 repo (best of five, warm cache):
Before After
real0m0.315s0m0.296s
user0m0.290s0m0.280s
sys 0m0.020s0m0.010s
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4e34195..7c440ad 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -38,14 +38,12 @@ static inline int is_merge(void)
return !access(git_path(MERGE_HEAD), F_OK);
}
-static int reset_index_file(const unsigned char *sha1, int reset_type, int
quiet)
+static int reset_index(const unsigned char *sha1, int reset_type, int quiet)
{
int nr = 1;
- int newfd;
struct tree_desc desc[2];
struct tree *tree;
struct unpack_trees_options opts;
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
memset(opts, 0, sizeof(opts));
opts.head_idx = 1;
@@ -67,8 +65,6 @@ static int reset_index_file(const unsigned char *sha1, int
reset_type, int quiet
opts.reset = 1;
}
- newfd = hold_locked_index(lock, 1);
-
read_cache_unmerged();
if (reset_type == KEEP) {
@@ -91,10 +87,6 @@ static int reset_index_file(const unsigned char *sha1, int
reset_type, int quiet
prime_cache_tree(active_cache_tree, tree);
}
- if (write_cache(newfd, active_cache, active_nr) ||
- commit_locked_index(lock))
- return error(_(Could not write new index file.));
-
return 0;
}
@@ -341,9 +333,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die_if_unmerged_cache(reset_type);
if (reset_type != SOFT) {
- int err = reset_index_file(sha1, reset_type, quiet);
+ struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+ int newfd = hold_locked_index(lock, 1);
+ int err = reset_index(sha1, reset_type, quiet);
if (reset_type == KEEP !err)
- err = reset_index_file(sha1, MIXED, quiet);
+ err = reset_index(sha1, MIXED, quiet);
+ if (!err
+ (write_cache(newfd, active_cache, active_nr) ||
+commit_locked_index(lock))) {
+ err = error(_(Could not write new index file.));
+ }
if (err)
die(_(Could not reset index file to revision '%s'.),
rev);
}
--
1.8.1.1.454.gce43f05
--
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 v2 00/19] reset improvements
Changes since v1:
- Spelling fixes.
- Explained how git reset -- $pathspec in bare repo is broken.
- Provided motivation for replacement of switch by if-else
- Fixed argv/argc handling by removing use of argc.
- Replaced don't refresh index on --quiet patch by one that just
inlines update_index_refresh()
- Incorporated fixes from Junio's repo
- Provided some motivation for replace switch by if-else amd moved
the patch later in the series.
Thanks for reviewing!
Martin von Zweigbergk (19):
reset $pathspec: no need to discard index
reset $pathspec: exit with code 0 if successful
reset.c: pass pathspec around instead of (prefix, argv) pair
reset: don't allow git reset -- $pathspec in bare repo
reset.c: extract function for parsing arguments
reset.c: remove unnecessary variable 'i'
reset.c: extract function for updating {ORIG_,}HEAD
reset.c: share call to die_if_unmerged_cache()
reset --keep: only write index file once
reset: avoid redundant error message
reset.c: replace switch by if-else
reset.c: move update_index_refresh() call out of read_from_tree()
reset.c: move lock, write and commit out of update_index_refresh()
reset [--mixed]: only write index file once
reset.c: finish entire cmd_reset() whether or not pathspec is given
reset.c: inline update_index_refresh()
reset $sha1 $pathspec: require $sha1 only to be treeish
reset: allow reset on unborn branch
reset [--mixed]: use diff-based reset whether or not pathspec was
given
builtin/reset.c| 283 +++--
t/t2013-checkout-submodule.sh | 2 +-
t/t7102-reset.sh | 26 +++-
t/t7106-reset-unborn-branch.sh | 52
4 files changed, 203 insertions(+), 160 deletions(-)
create mode 100755 t/t7106-reset-unborn-branch.sh
--
1.8.1.1.454.gce43f05
--
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 v2 12/19] reset.c: move update_index_refresh() call out of read_from_tree()
The final part of cmd_reset() essentially looks like:
if (pathspec) {
...
read_from_tree(...);
} else {
...
reset_index(...);
update_index_refresh(...);
...
}
where read_from_tree() internally also calls
update_index_refresh(). Move the call to update_index_refresh() out of
read_from_tree for symmetry with the 'else' block, making
read_from_tree() and reset_index() closer in functionality.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index c3eb2eb..70733c2 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -145,11 +145,8 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
}
}
-static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
- int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1)
{
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd;
struct diff_options opt;
memset(opt, 0, sizeof(opt));
@@ -157,7 +154,6 @@ static int read_from_tree(const char **pathspec, unsigned
char *tree_sha1,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- index_fd = hold_locked_index(lock, 1);
read_cache();
if (do_diff_cache(tree_sha1, opt))
return 1;
@@ -165,7 +161,7 @@ static int read_from_tree(const char **pathspec, unsigned
char *tree_sha1,
diff_flush(opt);
diff_tree_release_paths(opt);
- return update_index_refresh(index_fd, lock, refresh_flags);
+ return 0;
}
static void set_reflog_message(struct strbuf *sb, const char *action,
@@ -322,9 +318,13 @@ 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);
+ if (pathspec) {
+ struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+ int index_fd = hold_locked_index(lock, 1);
+ return read_from_tree(pathspec, sha1) ||
+ update_index_refresh(index_fd, lock,
+ 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
--
1.8.1.1.454.gce43f05
--
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 v2 08/19] reset.c: share call to die_if_unmerged_cache()
Use a single condition to guard the call to die_if_unmerged_cache for
both --soft and --keep. This avoids the small distraction of the
precondition check from the logic following it.
Also change an instance of
if (e)
err = err || f();
to the almost as short, but clearer
if (e !err)
err = f();
(which is equivalent since we only care whether exit code is 0)
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
builtin/reset.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 2187d64..4e34195 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -337,15 +337,13 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
/* 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. */
- if (reset_type == SOFT)
+ if (reset_type == SOFT || reset_type == KEEP)
die_if_unmerged_cache(reset_type);
- else {
- int err;
- if (reset_type == KEEP)
- die_if_unmerged_cache(reset_type);
- err = reset_index_file(sha1, reset_type, quiet);
- if (reset_type == KEEP)
- err = err || reset_index_file(sha1, MIXED, quiet);
+
+ if (reset_type != SOFT) {
+ int err = reset_index_file(sha1, reset_type, quiet);
+ if (reset_type == KEEP !err)
+ err = reset_index_file(sha1, MIXED, quiet);
if (err)
die(_(Could not reset index file to revision '%s'.),
rev);
}
--
1.8.1.1.454.gce43f05
--
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 v2 06/19] reset.c: remove unnecessary variable 'i'
Throughout most of parse_args(), the variable 'i' remains at 0. Many
references are still made to the variable even when it could only have
the value 0. This made at least me, who has relatively little
experience with C programming styles, think that parts of the function
was meant to be part of a loop. To avoid such confusion, remove the
variable and also the 'argc' parameter and check for NULL trailing
argv instead.
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com
---
I explained a bit more why I was confused by the current style, but
I'm also perfectly happy if you just drop the patch (there would be
some minor conflicts in a later patch, though).
builtin/reset.c | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 58f0f61..d89cf4d 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -198,9 +198,8 @@ static void die_if_unmerged_cache(int reset_type)
}
-static const char **parse_args(int argc, const char **argv, const char
*prefix, const char **rev_ret)
+static const char **parse_args(const char **argv, const char *prefix, const
char **rev_ret)
{
- int i = 0;
const char *rev = HEAD;
unsigned char unused[20];
/*
@@ -211,34 +210,34 @@ static const char **parse_args(int argc, const char
**argv, const char *prefix,
* git reset [-opts] -- paths...
* git reset [-opts] paths...
*
-* At this point, argv[i] points immediately after [-opts].
+* At this point, argv points immediately after [-opts].
*/
- if (i argc) {
- if (!strcmp(argv[i], --)) {
- i++; /* reset to HEAD, possibly with paths */
- } else if (i + 1 argc !strcmp(argv[i+1], --)) {
- rev = argv[i];
- i += 2;
+ if (argv[0]) {
+ if (!strcmp(argv[0], --)) {
+ argv++; /* reset to HEAD, possibly with paths */
+ } else if (argv[1] !strcmp(argv[1], --)) {
+ rev = argv[0];
+ argv += 2;
}
/*
-* Otherwise, argv[i] could be either rev or paths and
+* Otherwise, argv[0] could be either rev or paths and
* has to be unambiguous.
*/
- else if (!get_sha1_committish(argv[i], unused)) {
+ else if (!get_sha1_committish(argv[0], unused)) {
/*
-* Ok, argv[i] looks like a rev; it should not
+* Ok, argv[0] looks like a rev; it should not
* be a filename.
*/
- verify_non_filename(prefix, argv[i]);
- rev = argv[i++];
+ verify_non_filename(prefix, argv[0]);
+ rev = *argv++;
} else {
/* Otherwise we treat this as a filename */
- verify_filename(prefix, argv[i], 1);
+ verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
- return i argc ? get_pathspec(prefix, argv + i) : NULL;
+ return argv[0] ? get_pathspec(prefix, argv) : NULL;
}
int cmd_reset(int argc, const char **argv, const char *prefix)
@@ -270,7 +269,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, git_reset_usage,
PARSE_OPT_KEEP_DASHDASH);
- pathspec = parse_args(argc, argv, prefix, rev);
+ pathspec = parse_args(argv, prefix, rev);
if (get_sha1_committish(rev, sha1))
die(_(Failed to resolve '%s' as a valid ref.), rev);
--
1.8.1.1.454.gce43f05
--
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 v2 06/19] reset.c: remove unnecessary variable 'i'
I suppose this was meant for everyone. Adding back the others. On Tue, Jan 15, 2013 at 10:27 AM, Holding, Lawrence (NZ) lawrence.hold...@cubic.com wrote: Maybe use *argv instead of argv[0]? Sure. Everywhere? Also in the lines added in patch 17/19 that refer to both argv[0] and argv[1], such as argv[1] !get_sha1_treeish(argv[0], unused)? Or is this just a sign that I'm making the code _more_ confusing to those who are more familiar with C? -- 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 v2 17/19] fixup! reset $sha1 $pathspec: require $sha1 only to be treeish
---
Sorry, I forgot the documentation updates. I hope this looks ok. Can
you squash this in, Junio? Thanks.
I don't think any documentation update is necessary for the reset on
unborn branch patch. Let me know if you think differently.
Documentation/git-reset.txt | 18 +-
builtin/reset.c | 4 ++--
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 978d8da..a404b47 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -8,20 +8,20 @@ git-reset - Reset current HEAD to the specified state
SYNOPSIS
[verse]
-'git reset' [-q] [commit] [--] paths...
-'git reset' (--patch | -p) [commit] [--] [paths...]
+'git reset' [-q] [tree-ish] [--] paths...
+'git reset' (--patch | -p) [tree-sh] [--] [paths...]
'git reset' [--soft | --mixed | --hard | --merge | --keep] [-q] [commit]
DESCRIPTION
---
-In the first and second form, copy entries from commit to the index.
+In the first and second form, copy entries from tree-ish to the index.
In the third form, set the current branch head (HEAD) to commit, optionally
-modifying index and working tree to match. The commit defaults to HEAD
-in all forms.
+modifying index and working tree to match. The tree-ish/commit defaults
+to HEAD in all forms.
-'git reset' [-q] [commit] [--] paths...::
+'git reset' [-q] [tree-ish] [--] paths...::
This form resets the index entries for all paths to their
- state at commit. (It does not affect the working tree, nor
+ state at tree-ish. (It does not affect the working tree, nor
the current branch.)
+
This means that `git reset paths` is the opposite of `git add
@@ -34,9 +34,9 @@ Alternatively, using linkgit:git-checkout[1] and specifying a
commit, you
can copy the contents of a path out of a commit to the index and to the
working tree in one go.
-'git reset' (--patch | -p) [commit] [--] [paths...]::
+'git reset' (--patch | -p) [tree-ish] [--] [paths...]::
Interactively select hunks in the difference between the index
- and commit (defaults to HEAD). The chosen hunks are applied
+ and tree-ish (defaults to HEAD). The chosen hunks are applied
in reverse to the index.
+
This means that `git reset -p` is the opposite of `git add -p`, i.e.
diff --git a/builtin/reset.c b/builtin/reset.c
index b776867..cb84f1b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -23,8 +23,8 @@
static const char * const git_reset_usage[] = {
N_(git reset [--mixed | --soft | --hard | --merge | --keep] [-q]
[commit]),
- N_(git reset [-q] commit [--] paths...),
- N_(git reset --patch [commit] [--] [paths...]),
+ N_(git reset [-q] tree-ish [--] paths...),
+ N_(git reset --patch [tree-ish] [--] [paths...]),
NULL
};
--
1.8.1.1.454.gce43f05
--
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 v2 17/19] fixup! reset $sha1 $pathspec: require $sha1 only to be treeish
On Wed, Jan 16, 2013 at 10:00 AM, Martin von Zweigbergk martinv...@gmail.com wrote: --- Sorry, I forgot the documentation updates. I hope this looks ok. Can you squash this in, Junio? Thanks. I see the series just entered 'next', so I guess it would have to go on top then. Perhaps with a commit message like as simple as the following. Let me know if you prefer it to be resent as a proper patch. Sorry about the noise. reset: update documentation to require only tree-ish with paths When resetting with paths, we no longer require a commit argument, but only a tree-ish. Update the documentation and synopsis accordingly. -- 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] git rm -u
On Sun, Jan 20, 2013 at 1:27 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: git add -u is one of the only exceptions (with git grep). I consider this as a bug, and think this should be changed. This has been discussed several times here, but no one took the time to actually do the change - As we have the from the root magic pathspec these days, requiring git add -u :/ when the user really means to add everything is no longer too much of a burden, but if we suddenly changed git add -u to mean git add -u ., that is too much of a change in the semantics. And I think someone (Jeff?) pointed out that that last part is even more true for git clean, which also currently works on the current directory if not told otherwise. -- 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 v3 08/31] parse_pathspec: add PATHSPEC_EMPTY_MATCH_ALL
Hi, I was tempted to ask this before, and the recent thread regarding add -u/A [1] convinced me to. On Sun, Jan 13, 2013 at 4:35 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: We have two ways of dealing with empty pathspec: 1. limit it to current prefix 2. match the entire working directory Some commands go with #1, some with #2. get_pathspec() and parse_pathspec() only supports #1. Make it support #2 too via PATHSPEC_EMPTY_MATCH_ALL flag. If #2 is indeed the direction we want to go, then maybe we should make that the default behavior from parse_pathspec()? I.e. rename the flag PATHSPEC_EMPTY_MATCH_PREFIX (or something). Makes sense? Btw, Matthieu was asking where we use #1. If you do invert the name and meaning of the flag, then the answer to that question should be (mostly?) obvious from a re-roll of your series (i.e. all the places where PATHSPEC_EMPTY_MATCH_PREFIX is used). Martin [1] http://thread.gmane.org/gmane.comp.version-control.git/213988/focus=214113 -- 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 1/3] git-am: record full index line in the patch used while rebasing
On Thu, Jan 31, 2013 at 8:32 PM, Junio C Hamano gits...@pobox.com wrote: Earlier, a230949 (am --rebasing: get patch body from commit, not from mailbox, 2012-06-26) learned to regenerate patch body from the commit object while rebasing, instead of reading from the rebase-am front-end. While doing so, it used git diff-tree but without giving it the --full-index option. This does not matter for in-repository objects; during rebasing, any abbreviated object name should uniquely identify them. But we may be rebasing a commit that contains a change to a gitlink, in which case we usually should not have the object (it names a commit in the submodule). A full object name is necessary to later reconstruct a fake ancestor index for them. From what I can understand, this all makes sense. I didn't notice the emails about the breakage until now, but I wouldn't have known where to even start looking anyway, so thanks a lot for taking care of it. -- 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] rebase --preserve-merges keeps empty merge commits
I'm working on a re-roll of http://thread.gmane.org/gmane.comp.version-control.git/205796 and finally got around to including test cases for what you fixed in this patch. I want to make sure I'm testing what you fixed here. See questions below. On Sat, Jan 12, 2013 at 12:46 PM, Phil Hord ho...@cisco.com wrote: Since 90e1818f9a (git-rebase: add keep_empty flag, 2012-04-20) 'git rebase --preserve-merges' fails to preserve empty merge commits unless --keep-empty is also specified. Merge commits should be preserved in order to preserve the structure of the rebased graph, even if the merge commit does not introduce changes to the parent. Teach rebase not to drop merge commits only because they are empty. Consider a history like # a---b---c # \ \ # d---l #\ # e # \ # C where 'l' is tree-same with 'd' and 'C' introduces the same change as 'c'. My test case runs 'git rebase -p e l' and expects the result to look like # a---b---c # \ \ # d \ #\ \ # e---l A special case which is not handled by this change is for a merge commit whose parents are now the same commit because all the previous different parents have been dropped as a result of this rebase or some previous operation. And for this case, the test case runs 'git rebase -p C l'. Is that what you meant here? Before your patch, git would just say Nothing to do and after your patch, we get # a---b---c # \ \ # d \ #\ \ # e \ # \ \ # C---l As you say, your patch doesn't try to handle this case, but at least the new behavior seems better. I think we would ideally want the recreated 'l' to have only 'C' as parent in this case. Does that make sense? Martin -- 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] rebase --preserve-merges keeps empty merge commits
On Fri, Feb 1, 2013 at 1:05 PM, Phil Hord ho...@cisco.com wrote: This is probably right, but it is not exactly the case that caused my itch. I think my branch looked like [...] That also makes sense. I'll add tests for both cases. Your patch makes both of them pass. # a---b---c # \ \ # d \ #\ \ # e \ # \ \ # C---l As you say, your patch doesn't try to handle this case, but at least the new behavior seems better. I think we would ideally want the recreated 'l' to have only 'C' as parent in this case. Does that make sense? This is not what I meant, but it is a very interesting corner case. I am not sure I have a solid opinion on what the result should be here. Neither do I, so I'll just drop the test case. Thanks. Here is the corner case I was thinking of. I did not test this to see if this will happen, but I conceived that it might. Suppose you have this tree where # a---b---c # \ # d---g---l #\ / # C where 'C' introduced the same changes as 'c'. When I execute 'git rebase -p l c', I expect that I will end up with this: # a---b---c---d--- # \ \ # ---g---l That is, 'C' gets skipped because it introduces the same changes already seen in 'c'. So 'g' now has two parents: 'd' and 'C^'. But 'C^' is 'd', so 'g' now has two parents, both of whom are 'd'. I think it should collapse to this instead: # a---b---c---d---g---l I think this is actually what you will get. But I think it will only be linearized if the branch that should be dropped is the second parent. I have two tests for this, but I need to simplify them a little to see that that (parent number) is the only difference. I hope this is clear, but please let me know if I made it too confusing. Very clear. Thanks. -- 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: `git checkout --orpan` leaves a dirty worktree
I'm curious what your use case is. The behavior has been inconvenient for me too, but I have only used it in test cases; I have no real use case where I wanted to create an unborn/orphan branch. On Fri, Feb 8, 2013 at 11:50 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Hi, Why should I have to `git rm -rf .` after a `git checkout --orphan`? What sort of misfeature/ incomplete feature is this? Ram -- 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 -- 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 v8 4/4] git-rebase: add keep_empty flag
On Fri, Apr 20, 2012 at 7:36 AM, Neil Horman nhor...@tuxdriver.com wrote:
pick_one () {
ff=--ff
+
case $1 in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac
case $force_rebase in '') ;; ?*) ff= ;; esac
output git rev-parse --verify $sha1 || die Invalid commit name:
$sha1
+
+ if is_empty_commit $sha1
+ then
+ empty_args=--allow-empty
+ fi
+
test -d $rewritten
pick_one_preserving_merges $@ return
- output git cherry-pick $ff $@
+ output git cherry-pick $empty_args $ff $@
The is_empty_commit check seems to mean that if $sha1 is an empty
commit, we pass the --allow-empty option to cherry-pick. If it's not
empty, we don't. The word allow in allow-empty suggests that even
if the commit is not empty, cherry-pick would not mind. So, can we
always pass allow-empty to cherry-pick (i.e. even if the commit to
pick is not empty)?
Sorry I'm commenting so late; I didn't have time to look at your
patches when you sent them, but I'm currently working on the code
touched by this patch.
--
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 v8 4/4] git-rebase: add keep_empty flag
On Wed, Jul 18, 2012 at 12:10 AM, Johannes Sixt j.s...@viscovery.net wrote:
Am 7/18/2012 8:20, schrieb Martin von Zweigbergk:
On Fri, Apr 20, 2012 at 7:36 AM, Neil Horman nhor...@tuxdriver.com wrote:
pick_one () {
ff=--ff
+
case $1 in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac
case $force_rebase in '') ;; ?*) ff= ;; esac
output git rev-parse --verify $sha1 || die Invalid commit name:
$sha1
+
+ if is_empty_commit $sha1
+ then
+ empty_args=--allow-empty
+ fi
+
test -d $rewritten
pick_one_preserving_merges $@ return
- output git cherry-pick $ff $@
+ output git cherry-pick $empty_args $ff $@
The is_empty_commit check seems to mean that if $sha1 is an empty
commit, we pass the --allow-empty option to cherry-pick. If it's not
empty, we don't. The word allow in allow-empty suggests that even
if the commit is not empty, cherry-pick would not mind. So, can we
always pass allow-empty to cherry-pick (i.e. even if the commit to
pick is not empty)?
I don't think so. If the commit is not empty, but all its changes are
already in HEAD, then it will become empty when cherry-picked to HEAD.
In such a case, we usually do not want to record an empty commit, but stop
rebase to give to user a chance to deal with the situation.
Ah, makes sense. Thanks!
--
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 3/7] git-rebase--interactive: group all $preserve_merges code
The code in git-rebase--interactive that creates the todo file
contains if-blocks that depend on whether $preserve_merges is
active. There is only a very small amount of code in between that is
shared with non-merge-preserving code path, so remove the repeated
conditions and duplicate the small amount of shared code instead.
---
git-rebase--interactive.sh | 69 ++
1 file changed, 33 insertions(+), 36 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index fa722b6..4bb8e3f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -793,28 +793,6 @@ mkdir $state_dir || die Could not create temporary
$state_dir
: $state_dir/interactive || die Could not mark as interactive
write_basic_state
-if test t = $preserve_merges
-then
- if test -z $rebase_root
- then
- mkdir $rewritten
- for c in $(git merge-base --all $orig_head $upstream)
- do
- echo $onto $rewritten/$c ||
- die Could not init rewritten commits
- done
- else
- mkdir $rewritten
- echo $onto $rewritten/root ||
- die Could not init rewritten commits
- fi
- # No cherry-pick because our first pass is to determine
- # parents to rewrite and skipping dropped commits would
- # prematurely end our probe
- merges_option=
-else
- merges_option=--no-merges --cherry-pick
-fi
shorthead=$(git rev-parse --short $orig_head)
shortonto=$(git rev-parse --short $onto)
@@ -839,16 +817,30 @@ add_pick_line () {
printf '%s\n' ${comment_out}pick $1 $2 $todo
}
-git rev-list $merges_option --pretty=oneline --abbrev-commit \
- --abbrev=7 --reverse --left-right --topo-order \
- $revisions | \
- sed -n s/^//p |
-while read -r shortsha1 rest
-do
- if test t != $preserve_merges
+if test t = $preserve_merges
+then
+ if test -z $rebase_root
then
- add_pick_line $shortsha1 $rest
+ mkdir $rewritten
+ for c in $(git merge-base --all $orig_head $upstream)
+ do
+ echo $onto $rewritten/$c ||
+ die Could not init rewritten commits
+ done
else
+ mkdir $rewritten
+ echo $onto $rewritten/root ||
+ die Could not init rewritten commits
+ fi
+ # No cherry-pick because our first pass is to determine
+ # parents to rewrite and skipping dropped commits would
+ # prematurely end our probe
+ git rev-list --pretty=oneline --abbrev-commit \
+ --abbrev=7 --reverse --left-right --topo-order \
+ $revisions |
+ sed -n s/^//p |
+ while read -r shortsha1 rest
+ do
sha1=$(git rev-parse $shortsha1)
if test -z $rebase_root
then
@@ -868,12 +860,8 @@ do
touch $rewritten/$sha1
add_pick_line $shortsha1 $rest
fi
- fi
-done
-
-# Watch for commits that been dropped by --cherry-pick
-if test t = $preserve_merges
-then
+ done
+ # Watch for commits that been dropped by --cherry-pick
mkdir $dropped
# Save all non-cherry-picked changes
git rev-list $revisions --left-right --cherry-pick | \
@@ -895,6 +883,15 @@ then
rm $rewritten/$rev
fi
done
+else
+ git rev-list --no-merges --cherry-pick --pretty=oneline --abbrev-commit
\
+ --abbrev=7 --reverse --left-right --topo-order \
+ $revisions |
+ sed -n s/^//p |
+ while read -r shortsha1 rest
+ do
+ add_pick_line $shortsha1 $rest
+ done
fi
test -s $todo || echo noop $todo
--
1.7.11.1.104.ge7b44f1
--
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 2/7] git-rebase--interactive.sh: extract function for adding pick line
Extract the code that adds a possibly commented-out pick line to the
todo file. This lets us reuse it more easily later.
---
git-rebase--interactive.sh | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bef7bc0..fa722b6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -828,23 +828,26 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
fi
-git rev-list $merges_option --pretty=oneline --abbrev-commit \
- --abbrev=7 --reverse --left-right --topo-order \
- $revisions | \
- sed -n s/^//p |
-while read -r shortsha1 rest
-do
- if test -z $keep_empty is_empty_commit $shortsha1
+add_pick_line () {
+ if test -z $keep_empty is_empty_commit $1
then
comment_out=#
else
comment_out=
fi
+ printf '%s\n' ${comment_out}pick $1 $2 $todo
+}
+git rev-list $merges_option --pretty=oneline --abbrev-commit \
+ --abbrev=7 --reverse --left-right --topo-order \
+ $revisions | \
+ sed -n s/^//p |
+while read -r shortsha1 rest
+do
if test t != $preserve_merges
then
- printf '%s\n' ${comment_out}pick $shortsha1 $rest $todo
+ add_pick_line $shortsha1 $rest
else
sha1=$(git rev-parse $shortsha1)
if test -z $rebase_root
@@ -863,7 +866,7 @@ do
if test f = $preserve
then
touch $rewritten/$sha1
- printf '%s\n' ${comment_out}pick $shortsha1 $rest
$todo
+ add_pick_line $shortsha1 $rest
fi
fi
done
--
1.7.11.1.104.ge7b44f1
--
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 5/7] rebase -p: use --cherry-mark for todo file
While building the todo file, 'rebase -p' needs to find the cherry-picked commits in the branch that is about to be rebased. For this, it calculates the set difference between the full set of commits and the non-cherry-picked ones (as reported by 'git rev-list --left-right --cherry-pick'). Now that have the 'git rev-list --cherry-mark' option (since adbbb31 (revision.c: introduce --cherry-mark, 2011-03-07)), we can instead use that option to get the set of cherry-picked commits. --- git-rebase--interactive.sh | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 9715830..47beb58 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -859,17 +859,12 @@ then add_pick_line $sha1 fi done - # Watch for commits that been dropped by --cherry-pick + # Now drop cherry-picked commits mkdir $dropped - # Save all non-cherry-picked changes - git rev-list $revisions --left-right --cherry-pick | \ - sed -n s/^//p $state_dir/not-cherry-picks - # Now all commits and note which ones are missing in - # not-cherry-picks and hence being dropped - git rev-list $revisions | + git rev-list $revisions --cherry-mark --right-only | sed -ne s/^=//p | while read rev do - if test -f $rewritten/$rev -a $(sane_grep $rev $state_dir/not-cherry-picks) = + if test -f $rewritten/$rev then # Use -f2 because if rev-list is telling us this commit is # not worthwhile, we don't want to track its multiple heads, -- 1.7.11.1.104.ge7b44f1 -- 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 6/7] rebase -p: don't request --left-right only to ignore left side
While generating the todo file, rebase -p calls 'git rev-list --left-right a...b' (with 'a' equal to $upstream or $onto and 'b' equal to $orig_head) and its output is piped through 'sed -n s/^//p', making it equivalent to 'git rev-list --right-only a...b'. Change the invocation to exactly that. (One could alternatively change it to 'git rev-list a..b', which would be even simpler, if it wasn't for the fact that we already have the revision range expression in a variable.) --- git-rebase--interactive.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 47beb58..cd5a2cc 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -836,8 +836,7 @@ then # No cherry-pick because our first pass is to determine # parents to rewrite and skipping dropped commits would # prematurely end our probe - git rev-list $revisions --reverse --left-right --topo-order | - sed -n s/^//p | + git rev-list $revisions --reverse --right-only --topo-order | while read -r sha1 do if test -z $rebase_root -- 1.7.11.1.104.ge7b44f1 -- 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 4/7] git-rebase--interactive.sh: look up subject in add_pick_line
The todo file is generated using (more-or-less) 'git rev-list
$revisions --pretty=oneline --abbrev-commit --abbrev=7', i.e. by
letting 'git rev-list' output both the abbreviated sha1 and the
subject line. To allow us to more easily generate the list of commits
to rebase by using commands that don't support outputting the subject
line, move this logic into add_pick_line.
---
git-rebase--interactive.sh | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 4bb8e3f..9715830 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -814,7 +814,8 @@ add_pick_line () {
else
comment_out=
fi
- printf '%s\n' ${comment_out}pick $1 $2 $todo
+ line=$(git rev-list -1 --pretty=oneline --abbrev-commit --abbrev=7 $1)
+ printf '%s\n' ${comment_out}pick $line $todo
}
if test t = $preserve_merges
@@ -835,13 +836,10 @@ then
# No cherry-pick because our first pass is to determine
# parents to rewrite and skipping dropped commits would
# prematurely end our probe
- git rev-list --pretty=oneline --abbrev-commit \
- --abbrev=7 --reverse --left-right --topo-order \
- $revisions |
+ git rev-list $revisions --reverse --left-right --topo-order |
sed -n s/^//p |
- while read -r shortsha1 rest
+ while read -r sha1
do
- sha1=$(git rev-parse $shortsha1)
if test -z $rebase_root
then
preserve=t
@@ -858,7 +856,7 @@ then
if test f = $preserve
then
touch $rewritten/$sha1
- add_pick_line $shortsha1 $rest
+ add_pick_line $sha1
fi
done
# Watch for commits that been dropped by --cherry-pick
@@ -884,13 +882,12 @@ then
fi
done
else
- git rev-list --no-merges --cherry-pick --pretty=oneline --abbrev-commit
\
- --abbrev=7 --reverse --left-right --topo-order \
- $revisions |
+ git rev-list $revisions --reverse --left-right --topo-order \
+ --no-merges --cherry-pick |
sed -n s/^//p |
- while read -r shortsha1 rest
+ while read -r sha1
do
- add_pick_line $shortsha1 $rest
+ add_pick_line $sha1
done
fi
--
1.7.11.1.104.ge7b44f1
--
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 0/7] correctly calculate patches to rebase
These seven patches replace the broken one I sent in http://thread.gmane.org/gmane.comp.version-control.git/200644/focus=200648. I hope I got the handling of empty commits right this time. Martin von Zweigbergk (7): git-rebase--am.sh: avoid special-casing --keep-empty git-rebase--interactive.sh: extract function for adding pick line git-rebase--interactive: group all $preserve_merges code git-rebase--interactive.sh: look up subject in add_pick_line rebase -p: use --cherry-mark for todo file rebase -p: don't request --left-right only to ignore left side rebase (without -p): correctly calculate patches to rebase git-am.sh | 10 +- git-rebase--am.sh | 20 +++ git-rebase--interactive.sh | 87 -- git-rebase--merge.sh | 2 +- git-rebase.sh | 11 +++--- t/t3401-rebase-partial.sh | 17 + t/t3406-rebase-message.sh | 14 7 files changed, 81 insertions(+), 80 deletions(-) -- 1.7.11.1.104.ge7b44f1 -- 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 7/7] rebase (without -p): correctly calculate patches to rebase
The different types of rebase use different ways of calculating the
patches to rebase.
'git rebase' (without -m/-i/-p) uses
git format-patch --ignore-if-in-upstream $upstream..$orig_head
'git rebase -m' uses
git rev-list $upstream..$orig_head
'git rebase -i' (without -p) uses
git rev-list $upstream...$orig_head --left-right --no-merges \
--cherry-pick | sed -n s/^//p
, which could also have been written
git rev-list $upstream...$orig_head --right-only --no-merges \
--cherry-pick
'git rebase -p' uses
git rev-list $upstream...$orig_head --right-only
followed by cherry-picked commits found by
git rev-list $upstream...$orig_head --cherry-mark --right-only |
| sed -ne s/^=//p
As Knut Franke reported in [1], the fact that there is no
--ignore-if-in-upstream or equivalent when using merge-based rebase
means that unnecessary conflicts can arise due to commits
cherry-picked between $orig_head and $upstream.
With all the other types, there is a different problem with the method
of calculating the commits to rebase. Copying the example history from
[1]:
.-c
/
a---b---d---e---f
\
.-g---E
Commit E is here a cherry-pick of e. If we now run 'git rebase [-i|-p]
--onto c f E', the commits to rebase will be those on E that are not
equivalent to any of those in f, which in this case would be only
'g'. Commit 'E' would thus be lost.
To solve both of the above problems, we want to find the commits in
$upstream..$orig_head that are not cherry-picked in
$orig_head..$onto. There is unfortunately no direct way of finding
these commits using 'git rev-list', so we will have to resort to using
'git cherry' and filter for lines starting with '+'. This works for
all but 'rebase -p', since 'git cherry' ignores merges.
As a side-effect, we also avoid the cost of formatting patches.
Test case updates for 'rebase -m' by Knut, the rest by Martin.
[1] http://thread.gmane.org/gmane.comp.version-control.git/161917
Helped-by: Knut Franke knut.fra...@gmx.de
Signed-off-by: Martin von Zweigbergk martin.von.zweigbe...@gmail.com
---
git-rebase--am.sh | 6 ++
git-rebase--interactive.sh | 8 +++-
git-rebase--merge.sh | 2 +-
git-rebase.sh | 11 ---
t/t3401-rebase-partial.sh | 17 +
t/t3406-rebase-message.sh | 14 +++---
6 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 37c1b23..fe3fdd1 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -16,11 +16,9 @@ skip)
;;
esac
-test -n $rebase_root root_flag=--root
test -n $keep_empty git_am_opt=$git_am_opt --keep-empty
-git format-patch -k --stdout --full-index --ignore-if-in-upstream \
- --src-prefix=a/ --dst-prefix=b/ \
- --no-renames $root_flag $revisions |
+generate_revisions |
+sed -e 's/\([0-9a-f]\{40\}\)/From \1 Mon Sep 17 00:00:00 2001/' |
git am $git_am_opt --rebasing --resolvemsg=$resolvemsg
move_to_original_branch
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index cd5a2cc..da32ca7 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -800,10 +800,8 @@ if test -z $rebase_root
# this is now equivalent to ! -z $upstream
then
shortupstream=$(git rev-parse --short $upstream)
- revisions=$upstream...$orig_head
shortrevisions=$shortupstream..$shorthead
else
- revisions=$onto...$orig_head
shortrevisions=$shorthead
fi
@@ -822,6 +820,7 @@ if test t = $preserve_merges
then
if test -z $rebase_root
then
+ revisions=$upstream...$orig_head
mkdir $rewritten
for c in $(git merge-base --all $orig_head $upstream)
do
@@ -829,6 +828,7 @@ then
die Could not init rewritten commits
done
else
+ revisions=$onto...$orig_head
mkdir $rewritten
echo $onto $rewritten/root ||
die Could not init rewritten commits
@@ -876,9 +876,7 @@ then
fi
done
else
- git rev-list $revisions --reverse --left-right --topo-order \
- --no-merges --cherry-pick |
- sed -n s/^//p |
+ generate_revisions |
while read -r sha1
do
add_pick_line $sha1
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index b10f2cf..bf4ec4b 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -131,7 +131,7 @@ echo $onto_name $state_dir/onto_name
write_basic_state
msgnum=0
-for cmt in `git rev-list --reverse --no-merges $revisions`
+for cmt in $(generate_revisions)
do
msgnum=$(($msgnum + 1))
echo $cmt $state_dir/cmt.$msgnum
diff --git a/git-rebase.sh b/git-rebase.sh
index 1cd0633..0fdff87 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -530,6 +530,10 @@ then
GIT_PAGER='' git diff --stat
Re: [PATCH 0/7] correctly calculate patches to rebase
Argh! Sorry about the sendemail.chainreplyto=true. I must have read that warning message incorrectly :-(. -- 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 1/7] git-rebase--am.sh: avoid special-casing --keep-empty
Since 0fbb95d (am: don't call mailinfo if $rebasing, 2012-06-26), the
patch body to apply when running 'git am --rebasing' is not taken from
the mbox, but directly from the commit. If such a commit is empty,
'git am --rebasing' still happily applies it and commits. However,
since the input to 'git am --rebasing' only ever comes from 'git
format-patch', which completely leaves the commit out from its output
if it's empty, no empty commits are ever created by 'git am
--rebasing'. By teaching 'git am --rebasing' a --keep-empty option and
letting the caller decide whether or not to keep empty commits, we can
unify the two different mechanisms that git-rebase--am.sh uses for
rebasing.
---
git-am.sh | 10 +-
git-rebase--am.sh | 20 ++--
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index b6a5300..37641b7 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -37,7 +37,8 @@ abort restore the original branch and abort the
patching operation.
committer-date-is-author-datelie about committer date
ignore-date use current timestamp for author date
rerere-autoupdate update the index with reused conflict resolution if possible
-rebasing* (internal use for git-rebase)
+rebasing* (internal use for git-rebase)
+keep-empty* (internal use for git-rebase)
. git-sh-setup
. git-sh-i18n
@@ -375,6 +376,7 @@ git_apply_opt=
committer_date_is_author_date=
ignore_date=
allow_rerere_autoupdate=
+keep_empty=
if test $(git config --bool --get am.keepcr) = true
then
@@ -414,6 +416,8 @@ do
abort=t ;;
--rebasing)
rebasing=t threeway=t ;;
+ --keep-empty)
+ keep_empty=t ;;
-d|--dotest)
die $(gettext -d option is no longer supported. Do not
use.)
;;
@@ -669,6 +673,10 @@ do
echo $commit $dotest/original-commit
get_author_ident_from_commit $commit
$dotest/author-script
git diff-tree --root --binary $commit $dotest/patch
+ test -s $dotest/patch || test -n $keep_empty || {
+ go_next
+ continue
+ }
else
git mailinfo $keep $no_inbody_headers $scissors $utf8
$dotest/msg $dotest/patch \
$dotest/$msgnum $dotest/info ||
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 392ebc9..37c1b23 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -17,20 +17,12 @@ skip)
esac
test -n $rebase_root root_flag=--root
-
-if test -n $keep_empty
-then
- # we have to do this the hard way. git format-patch completely squashes
- # empty commits and even if it didn't the format doesn't really lend
- # itself well to recording empty patches. fortunately, cherry-pick
- # makes this easy
- git cherry-pick --allow-empty $revisions
-else
- git format-patch -k --stdout --full-index --ignore-if-in-upstream \
- --src-prefix=a/ --dst-prefix=b/ \
- --no-renames $root_flag $revisions |
- git am $git_am_opt --rebasing --resolvemsg=$resolvemsg
-fi move_to_original_branch
+test -n $keep_empty git_am_opt=$git_am_opt --keep-empty
+git format-patch -k --stdout --full-index --ignore-if-in-upstream \
+ --src-prefix=a/ --dst-prefix=b/ \
+ --no-renames $root_flag $revisions |
+git am $git_am_opt --rebasing --resolvemsg=$resolvemsg
+move_to_original_branch
ret=$?
test 0 != $ret -a -d $state_dir write_basic_state
--
1.7.11.1.104.ge7b44f1
--
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 4/7] git-rebase--interactive.sh: look up subject in add_pick_line
Thanks for reviewing.
On Fri, Jul 20, 2012 at 1:14 AM, Johannes Sixt j.s...@viscovery.net wrote:
Am 7/18/2012 9:27, schrieb Martin von Zweigbergk:
@@ -814,7 +814,8 @@ add_pick_line () {
else
comment_out=
fi
- printf '%s\n' ${comment_out}pick $1 $2 $todo
+ line=$(git rev-list -1 --pretty=oneline --abbrev-commit --abbrev=7 $1)
+ printf '%s\n' ${comment_out}pick $line $todo
I don't like this. On Windows, rebase -i is already slow, and these extra
processes will make it even slower.
I don't like it either :-(.
Anything that can be done about this? Perhaps the rev-list call can
generate all of the full SHA1, the short SHA1, and the subject with a
--pretty format?
After patch 7/7, cherry is used instead of rev-list. Ideally, I would
have liked to teach git rev-list --cherry-pick to somehow use a
limit just like cherry does, but I couldn't think of a generic way
of doing that (in this case, we want to say something like range
a..b, but drop commits that are equivalent to any in b..c). I
actually don't remember if I gave up because I couldn't think of a
sensible way of specifying ranges like that, or if I just ran out of
time (not familiar with the revision-walking code). Now it seems to me
that something like git rev-list a..b --not-cherry-picks b..c makes
sense, but maybe it's just too specific and we should just support the
limited (no pun intended) case we need to emulate git cherry, i.e.
something like git rev-list --cherry-with-limit=a c...b. Feedback
appreciated.
Martin
--
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 7/7] rebase (without -p): correctly calculate patches to rebase
On Fri, Jul 20, 2012 at 1:18 AM, Johannes Sixt j.s...@viscovery.net wrote:
Am 7/18/2012 9:27, schrieb Martin von Zweigbergk:
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 37c1b23..fe3fdd1 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -16,11 +16,9 @@ skip)
;;
esac
-test -n $rebase_root root_flag=--root
test -n $keep_empty git_am_opt=$git_am_opt --keep-empty
-git format-patch -k --stdout --full-index --ignore-if-in-upstream \
- --src-prefix=a/ --dst-prefix=b/ \
- --no-renames $root_flag $revisions |
+generate_revisions |
+sed -e 's/\([0-9a-f]\{40\}\)/From \1 Mon Sep 17 00:00:00 2001/' |
git am $git_am_opt --rebasing --resolvemsg=$resolvemsg
move_to_original_branch
Just curious (as all tests pass): What does this do? It looks like
format-patch is not called anymore and git-am sees only SHA1s. Does it
force git-am to cherry-pick the patches?
That probably deserves to be mentioned in the commit message. Or maybe
in as a comment in the code. Either way, since 0fbb95d (am: don't call
mailinfo if $rebasing, 2012-06-26), 'git am --rebasing' never looks at
anything but the sha1, so most of the output from 'git format-patch'
is currently ignored. It doesn't do cherry-pick, though, but runs 'git
diff-tree' and other commands and then feeds the result to 'git
apply', just like a regular 'git am' invocation would.
Martin
--
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 v3 6/7] Remove unused and bad gettext block from git-am
On Tue, Jul 24, 2012 at 11:27 AM, Jonathan Nieder jrnie...@gmail.com wrote: Hi, Jiang Xin wrote: Gettext message should not start with '-' nor '--'. Since the '-d' and '--dotest' options do not exist in OPTIONS_SPEC variable, it's safe to remove the block. The above justification is not a sufficient reason to stop giving helpful advice when someone uses an option that was historically supported: I think Jiang is saying that git am --dotest=... already errors out because dotest is not in the OPTIONS_SPEC. See 98ef23b (git-am: minor cleanups, 2009-01-28). Or am I missing something? -- 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: What's cooking in git.git (Jul 2012, #08; Thu, 26)
On Thu, Jul 26, 2012 at 11:09 PM, Junio C Hamano gits...@pobox.com wrote: * mz/rebase-range (2012-07-18) 7 commits - rebase (without -p): correctly calculate patches to rebase - rebase -p: don't request --left-right only to ignore left side - rebase -p: use --cherry-mark for todo file - git-rebase--interactive.sh: look up subject in add_pick_line - git-rebase--interactive: group all $preserve_merges code - git-rebase--interactive.sh: extract function for adding pick line - git-rebase--am.sh: avoid special-casing --keep-empty Expecting a reroll. Yep, will try to get some time for this soon. Will probably try using patch-id as you suggested. -- 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 1/2] remove unnecessary parameter from get_patch_ids()
get_patch_ids() takes an already initialized rev_info and a
prefix. The prefix is used when initalizing a second rev_info. Since
the initialized rev_info already has a prefix and it seems the prefix
doesn't change, we can used the prefix from the initialized rev_info
to initialize the second rev_info.
Signed-off-by: Martin von Zweigbergk martin.von.zweigbe...@gmail.com
---
builtin/log.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index ecc2793..7a92e3f 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -696,7 +696,7 @@ static int reopen_stdout(struct commit *commit, const char
*subject,
return 0;
}
-static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const
char *prefix)
+static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
{
struct rev_info check_rev;
struct commit *commit;
@@ -717,7 +717,7 @@ static void get_patch_ids(struct rev_info *rev, struct
patch_ids *ids, const cha
init_patch_ids(ids);
/* given a range a..b get all patch ids for b..a */
- init_revisions(check_rev, prefix);
+ init_revisions(check_rev, rev-prefix);
o1-flags ^= UNINTERESTING;
o2-flags ^= UNINTERESTING;
add_pending_object(check_rev, o1, o1);
@@ -1306,7 +1306,7 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
if (hashcmp(o[0].item-sha1, o[1].item-sha1) == 0)
return 0;
}
- get_patch_ids(rev, ids, prefix);
+ get_patch_ids(rev, ids);
}
if (!use_stdout)
@@ -1525,7 +1525,7 @@ int cmd_cherry(int argc, const char **argv, const char
*prefix)
return 0;
}
- get_patch_ids(revs, ids, prefix);
+ get_patch_ids(revs, ids);
if (limit add_pending_commit(limit, revs, UNINTERESTING))
die(_(Unknown commit %s), limit);
--
1.7.11.1.104.ge7b44f1
--
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 2/2] log: remove redundant check for merge commit
On Fri, Jul 27, 2012 at 3:30 PM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: Also re-initializing rev_info fields to the same values already set in init_revisions(). Oops, that should have been _avoid_ re-initializing. I suspect that explicit initialization to revs.ignore_merges outself can be called belt-and-braces defensive programming to avoid getting surprised by future changes to what init_revisions() would do, so I do not have strong preference either way. I suspected the same, but OTOH, if it was ever changed, one would have to go through all call sites anyway. I checked the other calls to init_revisions (48 of them, I think) and I found only 3 other places where a field was re-initialized to the same value. In this case it was done inconsistently even within the file. Seeing init_revisions() followed by revs.ignore_merges = 1; in one place but not the other can easily lead one to believe that the other place does not ignore merges. Do you want a reroll with updated commit messages (the missing avoid above, the dropped seems like about the prefix in 1/2)? Since you don't have a strong preference about the explicit initialization, I assume I can leave those hunks in. I would clarify my reasoning, though. Martin -- 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 2/2] log: remove redundant check for merge commit
Sorry, I meant to CC the list. See below. On Sat, Jul 28, 2012 at 9:56 PM, Martin von Zweigbergk martin.von.zweigbe...@gmail.com wrote: On Fri, Jul 27, 2012 at 11:52 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: It seems to have some interaction with your other topic, though. These two patches alone will pass the existing tests, but merging it with mz/rebase-range breaks t3412. I didn't look into it, but perhaps this breaks git cherry in some way? Yes, it breaks git cherry quite badly, by not ignoring merges at all. I incorrectly assumed that ignore_merges was about revision traversal, but now I think it's only diff output from 'git log' (and possibly others). What I think tricked me was seeing that ignore_merges = 1 closely followed by a comment saying ignore merges. But now I think the explicit code to ignore merges is necessary (as show by the failing test case), but can be replaced by rev_info.max_parents = 1. Setting ignore_merges = 1, OTOH, now seems doubly redundant: not only does it set the same value as was set in init_revisions, but it's also irrelevant. Since cherry doesn't generate any diff output, I think ignore_merges is never used. Flipping the values of all of ignore_merges, combine_merges and diff does not have any effect on test cases at least. I hope my explanation makes some sense at least... I'll send a reroll when I get time. -- 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 v2 2/3] cherry: don't set ignored rev_info options
Ever since cherry was built-in in e827633 (Built-in cherry, 2006-10-24), it has set a bunch of options on the the rev_info that are only used while outputting a patch. But since the built-in cherry command never needs to output any patch (it uses add_commit_patch_id and has_commit_patch_id instead), these options are just distractions, so remove them. Signed-off-by: Martin von Zweigbergk martin.von.zweigbe...@gmail.com --- builtin/log.c | 4 1 file changed, 4 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 7a92e3f..8cea1e5 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1508,10 +1508,6 @@ int cmd_cherry(int argc, const char **argv, const char *prefix) } init_revisions(revs, prefix); - revs.diff = 1; - revs.combine_merges = 0; - revs.ignore_merges = 1; - DIFF_OPT_SET(revs.diffopt, RECURSIVE); if (add_pending_commit(head, revs, 0)) die(_(Unknown commit %s), head); -- 1.7.11.1.104.ge7b44f1 -- 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 v2 1/3] remove unnecessary parameter from get_patch_ids()
get_patch_ids() takes an already initialized rev_info and a
prefix. The prefix is used when initalizing a second rev_info. Since
the initialized rev_info already has a prefix and the prefix never
changes, we can used the prefix from the initialized rev_info to
initialize the second rev_info.
Signed-off-by: Martin von Zweigbergk martin.von.zweigbe...@gmail.com
---
builtin/log.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index ecc2793..7a92e3f 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -696,7 +696,7 @@ static int reopen_stdout(struct commit *commit, const char
*subject,
return 0;
}
-static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const
char *prefix)
+static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
{
struct rev_info check_rev;
struct commit *commit;
@@ -717,7 +717,7 @@ static void get_patch_ids(struct rev_info *rev, struct
patch_ids *ids, const cha
init_patch_ids(ids);
/* given a range a..b get all patch ids for b..a */
- init_revisions(check_rev, prefix);
+ init_revisions(check_rev, rev-prefix);
o1-flags ^= UNINTERESTING;
o2-flags ^= UNINTERESTING;
add_pending_object(check_rev, o1, o1);
@@ -1306,7 +1306,7 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
if (hashcmp(o[0].item-sha1, o[1].item-sha1) == 0)
return 0;
}
- get_patch_ids(rev, ids, prefix);
+ get_patch_ids(rev, ids);
}
if (!use_stdout)
@@ -1525,7 +1525,7 @@ int cmd_cherry(int argc, const char **argv, const char
*prefix)
return 0;
}
- get_patch_ids(revs, ids, prefix);
+ get_patch_ids(revs, ids);
if (limit add_pending_commit(limit, revs, UNINTERESTING))
die(_(Unknown commit %s), limit);
--
1.7.11.1.104.ge7b44f1
--
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 v2 0/3] Small log simplifications
Separated out the removal of the unused diff options into patch 2/3 and added the necessary max_parents=1 in patch 3/3. Martin von Zweigbergk (3): remove unnecessary parameter from get_patch_ids() cherry: don't set ignored rev_info options log: remove redundant check for merge commit builtin/log.c | 22 ++ 1 file changed, 6 insertions(+), 16 deletions(-) -- 1.7.11.1.104.ge7b44f1 -- 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 v2 3/3] log: remove redundant check for merge commit
While walking the revision list in get_patch_ids and cmd_cherry, we
check for each commit if there is more than one parent and ignore the
commit if that is the case. Instead, set rev_info.max_parents to 1 and
let the revision traversal code handle it for us.
Signed-off-by: Martin von Zweigbergk martin.von.zweigbe...@gmail.com
---
builtin/log.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 8cea1e5..3423d11 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -718,6 +718,7 @@ static void get_patch_ids(struct rev_info *rev, struct
patch_ids *ids)
/* given a range a..b get all patch ids for b..a */
init_revisions(check_rev, rev-prefix);
+ check_rev.max_parents = 1;
o1-flags ^= UNINTERESTING;
o2-flags ^= UNINTERESTING;
add_pending_object(check_rev, o1, o1);
@@ -726,10 +727,6 @@ static void get_patch_ids(struct rev_info *rev, struct
patch_ids *ids)
die(_(revision walk setup failed));
while ((commit = get_revision(check_rev)) != NULL) {
- /* ignore merges */
- if (commit-parents commit-parents-next)
- continue;
-
add_commit_patch_id(commit, ids);
}
@@ -1508,6 +1505,7 @@ int cmd_cherry(int argc, const char **argv, const char
*prefix)
}
init_revisions(revs, prefix);
+ revs.max_parents = 1;
if (add_pending_commit(head, revs, 0))
die(_(Unknown commit %s), head);
@@ -1530,10 +1528,6 @@ int cmd_cherry(int argc, const char **argv, const char
*prefix)
if (prepare_revision_walk(revs))
die(_(revision walk setup failed));
while ((commit = get_revision(revs)) != NULL) {
- /* ignore merges */
- if (commit-parents commit-parents-next)
- continue;
-
commit_list_insert(commit, list);
}
--
1.7.11.1.104.ge7b44f1
--
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 1/7] am: suppress error output from a conditional
On Tue, Apr 23, 2013 at 7:01 AM, Ramkumar Ramachandra artag...@gmail.com wrote: When testing if the $dotest directory exists, and if $next is greater than $last When can that happen? If one edits the todo? -- 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 2/7] rebase -i: don't error out if $state_dir already exists
On Tue, Apr 23, 2013 at 7:01 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Practically speaking, the only reason why a `mkdir $state_dir` would fail is because $state_dir already exists. Would we ever get to this point in the code if it already exists? Also, I had the feeling that the check it might fail if the user does not have permissions to create the directory. Is there always something else that will fail first? -- 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 5/7] rebase -i: return control to the caller, for housekeeping
On Tue, Apr 23, 2013 at 7:02 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index cc3a9a7..9514e31 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -597,7 +597,7 @@ do_next () {
fi
;;
esac
- test -s $todo return
+ test -s $todo return 1
Unlike the other cases below, this seems to be replacing success by
failure. What does it mean in practice that $todo is empty?
comment_for_reflog finish
newhead=$(git rev-parse HEAD)
@@ -623,17 +623,15 @@ do_next () {
$GIT_DIR/hooks/post-rewrite rebase $rewritten_list
true # we don't care if this hook failed
fi
- rm -rf $state_dir
- git gc --auto
warn Successfully rebased and updated $head_name.
- exit
+ return 0
So after this patch, the warning will coming before gc is run. It's
a change, but it seems fine. gc usually only prints a few line, right?
}
do_rest () {
while :
do
- do_next
+ do_next break
done
}
Normally one would break if unsuccessful. What would fail if this was
replaced by do_next || break and the above .. return 1 was ..
return. I assume that was your first attempt, but why did it not
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: [PATCH] rebase: use reflog to find common base with upstream
On Wed, Oct 16, 2013 at 11:53 AM, John Keeping j...@keeping.me.uk wrote:
Commit 15a147e (rebase: use @{upstream} if no upstream specified,
2011-02-09) says:
Make it default to 'git rebase @{upstream}'. That is also what
'git pull [--rebase]' defaults to, so it only makes sense that
'git rebase' defaults to the same thing.
but that isn't actually the case. Since commit d44e712 (pull: support
rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually
chosen the most recent reflog entry which is an ancestor of the current
branch if it can find one.
It is exactly this inconsistency between git rebase and git pull
--rebase that confused me enough to make me send my first email to
this list almost 4 years ago [1], so thanks for working on this! I
finished that thread with:
Would it make sense to teach git rebase the same tricks as git
pull --rebase?
Then it took me a year before I sent a patch not unlike this one [2].
To summarize, the patch did not get accepted then because it makes
rebase a little slower (or a lot slower in some cases). git pull
--rebase is of course at least as slow in the same cases, but because
it often involves connecting to a remote host, people would probably
blame the connection rather than git itself even in those rare (?)
cases.
I think
git merge-base HEAD $(git rev-list -g $upstream_name)
is roughly correct and hopefully fast enough. That can lead to too
long a command line, so I was planning on teaching merge-base a
--stdin option, but never got around to it.
Martin
[1] http://thread.gmane.org/gmane.comp.version-control.git/136339
[2] http://thread.gmane.org/gmane.comp.version-control.git/166710
--
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] rebase: use reflog to find common base with upstream
On Wed, Oct 16, 2013 at 11:53 AM, John Keeping j...@keeping.me.uk wrote:
Commit 15a147e (rebase: use @{upstream} if no upstream specified,
2011-02-09) says:
Make it default to 'git rebase @{upstream}'. That is also what
'git pull [--rebase]' defaults to, so it only makes sense that
'git rebase' defaults to the same thing.
but that isn't actually the case. Since commit d44e712 (pull: support
rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually
chosen the most recent reflog entry which is an ancestor of the current
branch if it can find one.
Change rebase so that it uses the same logic.
Signed-off-by: John Keeping j...@keeping.me.uk
---
git-rebase.sh | 8
t/t3400-rebase.sh | 6 --
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index 226752f..fd36cf7 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -437,6 +437,14 @@ then
error_on_missing_default_upstream rebase rebase \
against git rebase branch
fi
+ for reflog in $(git rev-list -g $upstream_name 2/dev/null)
+ do
+ if test $reflog = $(git merge-base $reflog HEAD)
+ then
+ upstream_name=$reflog
+ break
+ fi
+ done
;;
*) upstream_name=$1
shift
A little later, onto_name gets assigned like so:
onto_name=${onto-$upstream_name}
So if upstream_name was set above, then onto would get the same value,
which is not what we want, right? It seems like this block of code
should come a bit later.
I also think it not be run only when rebase was run without a given
upstream. If the configured upstream is origin/master, it seems like
it would be surprising to get different behavior from git rebase and
git rebase origin/master.
--
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] rebase: use reflog to find common base with upstream
On Mon, Oct 21, 2013 at 4:24 AM, John Keeping j...@keeping.me.uk wrote:
On Sun, Oct 20, 2013 at 10:03:29PM -0700, Martin von Zweigbergk wrote:
On Wed, Oct 16, 2013 at 11:53 AM, John Keeping j...@keeping.me.uk wrote:
Commit 15a147e (rebase: use @{upstream} if no upstream specified,
2011-02-09) says:
Make it default to 'git rebase @{upstream}'. That is also what
'git pull [--rebase]' defaults to, so it only makes sense that
'git rebase' defaults to the same thing.
but that isn't actually the case. Since commit d44e712 (pull: support
rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually
chosen the most recent reflog entry which is an ancestor of the current
branch if it can find one.
It is exactly this inconsistency between git rebase and git pull
--rebase that confused me enough to make me send my first email to
this list almost 4 years ago [1], so thanks for working on this! I
finished that thread with:
Would it make sense to teach git rebase the same tricks as git
pull --rebase?
Then it took me a year before I sent a patch not unlike this one [2].
To summarize, the patch did not get accepted then because it makes
rebase a little slower (or a lot slower in some cases). git pull
--rebase is of course at least as slow in the same cases, but because
it often involves connecting to a remote host, people would probably
blame the connection rather than git itself even in those rare (?)
cases.
I think
git merge-base HEAD $(git rev-list -g $upstream_name)
is roughly correct and hopefully fast enough. That can lead to too
long a command line, so I was planning on teaching merge-base a
--stdin option, but never got around to it.
I'm not sure we should worry about the additional overhead here. In the
common case, we should hit a common ancestor within the first couple of
reflog entries; and in the case that will be slow, it's likely that
there are a lot of differences between the branches so the cherry
comparison phase will take a while anyway.
Perhaps true. I created a simple commit based on my origin/master@{1}
in git.git, which happened to be 136 commits behind origin/master.
Before (a modified version of) your patch, it took 0.756s to rebase it
(best of 5) and afterwards it took 0.720s.
And in a worse case: The same test with one commit off my
origin/master@{13}, 2910 behind origin/master, shows an increase from
2.75s to 4.04s.
And a degenerate case: I created a test branch (called u) with
1000-entry reflog from the output of git rev-list --first-parent
origin/master | head -1000 | tac and created the same simple commit
as before off of the end of this reflog (u@{999}). This ended up 3769
commits behind u@{0} (aka origin/master). In this case it went from
3.43s to 3m32s. Obviously, this was a degenerate case designed to be
slow, but I think it's still worth noting that one can get such O(n^2)
behavior e.g. if one lets a branch get out of sync with an upstream
that's very frequently fetches (I've heard of people running
short-interval cron jobs that fetch from a remote).
I do like the feature, but I'm still concerned about this last case.
--
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: Re* Bug report: reset -p HEAD
Sorry about the regression and thanks for report and fixes.
On Thu, Oct 24, 2013 at 9:24 PM, Jeff King p...@peff.net wrote:
On Thu, Oct 24, 2013 at 08:40:13PM -0700, Junio C Hamano wrote:
Maarten de Vries maar...@de-vri.es writes:
Some more info: It used to work as intended. Using a bisect shows it
has been broken by commit 166ec2e9.
Thanks.
A knee-jerk change without thinking what side-effect it has for you
to try out.
builtin/reset.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index f2f9d55..a3088d9 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -304,7 +304,10 @@ 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 run_add_interactive(sha1_to_hex(sha1), --patch=reset,
pathspec);
+ return run_add_interactive(
+ (unborn || strcmp(rev, HEAD))
+ ? sha1_to_hex(sha1)
+ : HEAD, --patch=reset, pathspec);
}
I think that's the correct fix for the regression. You are restoring
the original, pre-166ec2e9 behavior for just the HEAD case. I do not
think add--interactive does any other magic between a symbolic rev and
its sha1, except for recognizing HEAD specially. However, if you wanted
to minimize the potential impact of 166ec2e9, you could pass the sha1
_only_ in the unborn case, like this:
Plus, the end result is more readable, IMHO.
diff --git a/builtin/reset.c b/builtin/reset.c
index f2f9d55..bfdd8d3 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -283,6 +283,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (unborn) {
/* reset on unborn branch: treat as reset to empty tree */
hashcpy(sha1, EMPTY_TREE_SHA1_BIN);
+ rev = EMPTY_TREE_SHA1_HEX;
} else if (!pathspec.nr) {
struct commit *commit;
if (get_sha1_committish(rev, sha1))
@@ -304,7 +305,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 run_add_interactive(sha1_to_hex(sha1),
--patch=reset, pathspec);
+ return run_add_interactive(rev, --patch=reset, pathspec);
}
/* git reset tree [--] paths... can be used to
That fixes any possible regression from add--interactive treating the
two cases differently. On an unborn branch, we will still say apply
this hunk rather than unstage this hunk. That's not a regression,
because it simply didn't work before, but it's not ideal. To fix that,
we need to somehow tell add--interactive this is HEAD, but use the
empty tree because it's unborn. I can think of a few simple-ish ways:
1. Pass the head/not-head flag as a separate option.
2. Pass HEAD even in the unborn case; teach add--interactive to
convert an unborn HEAD to the empty tree.
3. Teach add--interactive to recognize the empty tree sha1 as an
unstage path.
I kind of like (3). At first glance, it is wrong; we will also treat
git reset -p $(git hash-object -t tree /dev/null) as if HEAD had
been passed. But if you are explicitly passing the empty tree like that,
I think saying unstage makes a lot of sense.
Makes sense to me. I'm sure others can implement that much faster than
I can, but I feel a little guilty, so I'm happy to do it if no one
else wants to, as long as we agree this is the way we want to go.
--
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 v3 2/2] merge-base: teach --fork-point mode
Thanks for taking care of this! Maybe John or I can finally get the changes to rebase done after this. A few comments below. Sorry I didn't find time to review the earlier revisions. On Fri, Oct 25, 2013 at 2:38 PM, Junio C Hamano gits...@pobox.com wrote: + +where `origin/master` used to point at commits B3, B2, B1 and now it +points at B, and your `topic` branch was stated on top of it back s/stated/started/ + bases = get_merge_bases_many(derived, revs.nr, revs.commit, 0); + + /* +* There should be one and only one merge base, when we found +* a common ancestor among reflog entries. +*/ + if (!bases || bases-next) + return 1; + + /* And the found one must be one of the reflog entries */ + for (i = 0; i revs.nr; i++) + if (bases-item-object == revs.commit[i]-object) + break; /* found */ + if (revs.nr = i) + return 1; /* not found */ + + printf(%s\n, sha1_to_hex(bases-item-object.sha1)); + free_commit_list(bases); + return 0; Should free_commit_list also be called in the two return 1 cases above? I suppose the process will exit soon after this, but that seems to be true for all three cases. diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh index f80bba8..4f09db0 100755 --- a/t/t6010-merge-base.sh +++ b/t/t6010-merge-base.sh @@ -230,4 +230,31 @@ test_expect_success 'criss-cross merge-base for octopus-step' ' test_cmp expected.sorted actual.sorted ' +test_expect_success 'using reflog to find the fork point' ' + git reset --hard + git checkout -b base $E + + ( + for count in 1 2 3 4 5 + do + git commit --allow-empty -m Base commit #$count + git rev-parse HEAD expect$count + git checkout -B derived + git commit --allow-empty -m Derived #$count + git rev-parse HEAD derived$count + git checkout base || exit 1 I think this creates a history like ---E---B1--B2--B3--B4--B5 (base) \ \ \ \ \ D1 D2 D3 D4 D5 (derived) So I think the following test would pass even if you drop the --fork-point. Did you mean to create a fan-shaped history by resetting base to $E on every iteration above? + git merge-base --fork-point base $(cat derived$count) actual + test_cmp expect$count actual || exit 1 -- 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 16/16] add: avoid yoda conditions
I was recently confused by the yoda condition in this block of code from [1] + for (i = 0; i revs.nr; i++) + if (bases-item-object == revs.commit[i]-object) + break; /* found */ + if (revs.nr = i) I think I was particularly surprised because it came so soon after the i revs.nr. I didn't bother commenting because it seemed too subjective and the code base has tons of these. Something as simple as git grep '[0-9] []' *.c finds a bunch (probably with lots of false positives and negatives). I guess what I'm trying to say is that either we accept them and get used to reading them without being surprised, or we can change a bit more than one at a time perhaps? I understand that this was an occurrence you just happened to run into, and I'm not saying that a patch has to deal with _all_ occurrences. I'm more just wondering if we want mention our position, whatever it is, in CodingGuidelines. Martin [1] http://thread.gmane.org/gmane.comp.version-control.git/236252/focus=236716 On Thu, Oct 31, 2013 at 2:25 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- builtin/add.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/add.c b/builtin/add.c index 226f758..9b30356 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -429,7 +429,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) argc--; argv++; - if (0 = addremove_explicit) + if (addremove_explicit = 0) addremove = addremove_explicit; else if (take_worktree_changes ADDREMOVE_DEFAULT) addremove = 0; /* -u was given but not -A */ -- 1.8.4.2+fc1 -- 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 -- 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] rebase: use reflog to find common base with upstream
On Sun, Dec 8, 2013 at 12:06 PM, John Keeping j...@keeping.me.uk wrote:
Commit 15a147e (rebase: use @{upstream} if no upstream specified,
2011-02-09) says:
Make it default to 'git rebase @{upstream}'. That is also what
'git pull [--rebase]' defaults to, so it only makes sense that
'git rebase' defaults to the same thing.
but that isn't actually the case. Since commit d44e712 (pull: support
rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually
chosen the most recent reflog entry which is an ancestor of the current
branch if it can find one.
In my mind, 'git pull --rebase' does default to @{u}, it just started
interpreting it differently in d44e712, but maybe I'm just being
defensive :-).
In a similar way, I think your patch is about interpreting the
upstream argument differently, not about changing the default upstream
argument. This is why I think git rebase and git rebase
origin/master should be the same (when origin/master is the
configured upstream).
--
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
