Re: [RFC] git checkout $tree -- $path always rewrites files
Trying again from plain old gmail which I think does not send a multipart content. On Fri, Nov 7, 2014 at 11:06 PM, Martin von Zweigbergk martinv...@gmail.com wrote: Is this also related to git checkout $rev . not removing removed files? What you say about the difference in implementation between checkout and reset sounds vaguely familiar from when I looked at it some years ago. Curiously, I just talked to Jonathan about this over lunch yesterday. I think we would both be happy to get that oddity of checkout fixed. If what you mention here is indeed related to fixing that, it does complicate things with regards to backwards compatibility. On Fri Nov 07 2014 at 11:24:09 AM Jeff King p...@peff.net wrote: On Fri, Nov 07, 2014 at 09:14:42AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: Is there a reason that we don't use this diff technique for checkout? I suspect that the reasons are probably mixture of these: (1) the code path may descend from checkout-index and predates the in-core diff machinery; (2) in the context of checkout-index, it was more desirable to be able to say I want the contents on the filesystem, even if you think I already have it there, as you as a new software are likely to be wrong and I know better; or (3) it was easier to code that way ;-) I do not see there is any reason not to do what you suggest. OK. It's not very much code (and can mostly be lifted from git-reset), so I may take a stab at it. -Peff -- 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] git checkout $tree -- $path always rewrites files
I'm not sure I had seen that particular thread, but it seems like we're all in agreement on that topic. I'm keeping my fingers crossed that Jeff will have time to tackle that this time :-) On Fri, Nov 7, 2014 at 11:35 PM, Junio C Hamano gits...@pobox.com wrote: I think that has direct linkage; what you have in mind I think is http://thread.gmane.org/gmane.comp.version-control.git/234903/focus=234935 What is on thread here is an implementation glitch, not semantic one. Checking out a directory as opposed to set of paths that match the pathspec was a deliberate design choice, not an implementation glitch. Pardon HTML, misspellings and grammos, typed on a tablet. On Nov 7, 2014 11:10 PM, Martin von Zweigbergk martinv...@gmail.com wrote: Trying again from plain old gmail which I think does not send a multipart content. On Fri, Nov 7, 2014 at 11:06 PM, Martin von Zweigbergk martinv...@gmail.com wrote: Is this also related to git checkout $rev . not removing removed files? What you say about the difference in implementation between checkout and reset sounds vaguely familiar from when I looked at it some years ago. Curiously, I just talked to Jonathan about this over lunch yesterday. I think we would both be happy to get that oddity of checkout fixed. If what you mention here is indeed related to fixing that, it does complicate things with regards to backwards compatibility. On Fri Nov 07 2014 at 11:24:09 AM Jeff King p...@peff.net wrote: On Fri, Nov 07, 2014 at 09:14:42AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: Is there a reason that we don't use this diff technique for checkout? I suspect that the reasons are probably mixture of these: (1) the code path may descend from checkout-index and predates the in-core diff machinery; (2) in the context of checkout-index, it was more desirable to be able to say I want the contents on the filesystem, even if you think I already have it there, as you as a new software are likely to be wrong and I know better; or (3) it was easier to code that way ;-) I do not see there is any reason not to do what you suggest. OK. It's not very much code (and can mostly be lifted from git-reset), so I may take a stab at it. -Peff -- 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] git checkout $tree -- $path always rewrites files
First of all, thanks again for spending time on this. On Sat, Nov 8, 2014 at 12:30 AM, Jeff King p...@peff.net wrote: On Fri, Nov 07, 2014 at 11:35:59PM -0800, Junio C Hamano wrote: So just to be clear, the behavior we want is that: echo foo some-new-path git add some-new-path git checkout HEAD -- . will delete some-new-path (whereas the current code turns it into an untracked file). Yes, I think that's what I would expect. What should: git checkout HEAD -- some-new-path do in that case? With the current code, it actually barfs, complaining that nothing matched some-new-path (because it is not part of HEAD, and therefore we don't consider it at all), and aborts the whole operation. I think we would want to delete some-new-path in that case, too. I don't think we'd want it to be deleted. I would view 'git reset --hard' as the role model here, and that command (without paths) would not remove the file. And applying it to a path should not change the behavior, just restrict it to the paths, right? -- 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/13] Use git merge instead of git pull .
On Sat, Aug 24, 2013 at 9:19 PM, Jonathan Nieder jrnie...@gmail.com wrote: Thomas Ackermann wrote: --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -1784,17 +1784,6 @@ repository that you pulled from. fast-forwards,fast-forward; instead, your branch will just be updated to point to the latest commit from the upstream branch.) -The `git pull` command can also be given `.` as the remote repository, -in which case it just merges in a branch from the current repository; so -the commands - -- -$ git pull . branch -$ git merge branch -- - -are roughly equivalent. The former is actually very commonly used. - I wonder if it would make sense to say they simply *are* equivalent. I.e., what differences are there between those two commands, and could git pull be tweaked to eliminate them? One difference is that git pull can be configured to rebase. [...] @@ -2259,7 +2248,7 @@ When you are happy with the state of this change, you can pull it into the test branch in preparation to make it public: I realize that pull here is not necessarily about the command, but perhaps it would still make sense to change 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
[PATCH] add test for 'git rebase --keep-empty'
Signed-off-by: Martin von Zweigbergk martin.von.zweigbe...@gmail.com --- While trying to use patch-id instead of --ignore-if-in-upstream/--cherry-pick/cherry/etc, I noticed that patch-id ignores empty patches and I was surprised that tests still pass. This test case would be useful to protect --keep-empty. t/t3401-rebase-partial.sh | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh index 7f8693b..b89b512 100755 --- a/t/t3401-rebase-partial.sh +++ b/t/t3401-rebase-partial.sh @@ -47,7 +47,14 @@ test_expect_success 'rebase ignores empty commit' ' git commit --allow-empty -m empty test_commit D git rebase C - test $(git log --format=%s C..) = D + test $(git log --format=%s C..) = D +' + +test_expect_success 'rebase --keep-empty' ' + git reset --hard D + git rebase --keep-empty C + test $(git log --format=%s C..) = D +empty ' test_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 v2] add tests for 'git rebase --keep-empty'
Add test cases for 'git rebase --keep-empty' with and without an empty commit already in upstream. The empty commit that is about to be rebased should be kept in both cases. Signed-off-by: Martin von Zweigbergk martin.von.zweigbe...@gmail.com --- Added another test for when the upstream already has an empty commit. The test case protects the current behavior; I just assume the current behavior is what we want. While writing the test case, I also noticed that an interrupted 'git rebase --keep-empty' can not be continued 'git rebase --continue', but instead needs 'git cherry-pick --continue'. I guess this shouldn't really be surprising given that it's implemented in terms of cherry-pick. This should be fixed once all the different kinds of rebase use the same way of finding the commits to rebase, so I wouldn't worry about fixing this specific problem right now. t/t3401-rebase-partial.sh | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh index 7f8693b..58f4823 100755 --- a/t/t3401-rebase-partial.sh +++ b/t/t3401-rebase-partial.sh @@ -47,7 +47,23 @@ test_expect_success 'rebase ignores empty commit' ' git commit --allow-empty -m empty test_commit D git rebase C - test $(git log --format=%s C..) = D + test $(git log --format=%s C..) = D +' + +test_expect_success 'rebase --keep-empty' ' + git reset --hard D + git rebase --keep-empty C + test $(git log --format=%s C..) = D +empty +' + +test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' ' + git reset --hard A + git commit --allow-empty -m also-empty + git rebase --keep-empty D + test $(git log --format=%s A..) = also-empty +D +empty ' test_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
cherry-pick and 'log --no-walk' and ordering
A while ago when I was looking at revision.c, I was surprised to see that commits are sorted even when --no-walk is passed, but as 8e64006 (Teach revision machinery about --no-walk, 2007-07-24) points out, this can be useful for doing $ git log --abbrev-commit --pretty=oneline --decorate --all --no-walk and get the result sorted by date. However, it can also be useful _not_ to get a result sorted by date, e.g. when doing something like generate an ordered list of revisions | git rev-list --oneline --no-walk --stdin. Would a --no-sort option to rev-list be appreciated or are there better solutions? There is also cherry-pick/revert, which I _think_ does not really want the revisions sorted. cherry-pick implicitly reverses the order of the walk, so 'git cherry-pick branch~2..branch' applies them in the right order (at least in the absence of clock skew). The documentation for cherry-pick suggests git rev-list --reverse master -- README | git cherry-pick -n --stdin, which I think makes no sense -- this would reverse the output from rev-list only to have it reversed again in cherry-pick, if it wasn't for the sorting by date. I think the --reverse passed to rev-list might even break the cherry-pick if there are commits in non-increasing date order. This is also supported by the fact that test still pass after applying the patch below. I think the test cases make more sense after the patch. Do others agree with the analysis? I suppose it's too late to change cherry-pick to start differentiating between git cherry-pick commit1 commit2 and git cherry-pick commit2 commit1, but I think we should at least update the documentation as in the patch below (or maybe even with a --topo-order passed to cherry-pick?). We could possibly change cherry-pick's ordering from the default ordering to topological ordering. Martin Sorry about the mangled whitespace below; just for reference, not intended to be applied. diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index 0e170a5..454e205 100644 --- a/Documentation/git-cherry-pick.txt +++ b/Documentation/git-cherry-pick.txt @@ -181,7 +181,7 @@ EXAMPLES are in next but not HEAD to the current branch, creating a new commit for each new change. -`git rev-list --reverse master -- README | git cherry-pick -n --stdin`:: +`git rev-list master -- README | git cherry-pick -n --stdin`:: Apply the changes introduced by all commits on the master branch that touched README to the working tree and index, diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh index 75f7ff4..020baaf 100755 --- a/t/t3508-cherry-pick-many-commits.sh +++ b/t/t3508-cherry-pick-many-commits.sh @@ -164,7 +164,7 @@ test_expect_success 'cherry-pick --stdin works' ' git checkout -f master git reset --hard first test_tick - git rev-list --reverse first..fourth | git cherry-pick --stdin + git rev-list first..fourth | git cherry-pick --stdin git diff --quiet other git diff --quiet HEAD other check_head_differs_from fourth diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index f4e6450..9e28910 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -400,7 +400,7 @@ test_expect_success '--continue of single-pick respects -x' ' test_expect_success '--continue respects -x in first commit in multi-pick' ' pristine_detach initial - test_must_fail git cherry-pick -x picked anotherpick + test_must_fail git cherry-pick -x anotherpick picked echo c foo git add foo git cherry-pick --continue @@ -430,7 +430,7 @@ test_expect_success '--signoff is not automatically propagated to resolved confl test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' ' pristine_detach initial - test_must_fail git cherry-pick -s picked anotherpick + test_must_fail git cherry-pick -s anotherpick picked echo c foo git add foo git cherry-pick --continue -- 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: cherry-pick and 'log --no-walk' and ordering
On Fri, Aug 10, 2012 at 2:38 PM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: There is also cherry-pick/revert, which I _think_ does not really want the revisions sorted. Yes, I think sequencer.c::prepare_revs() is wrong to unconditoinally call prepare_revision_walk(). It instead should first check the revs-pending.objects list to see if what was given by the caller is a mere collection of individual objects or a range expression (i.e. check if any of them is marked with UNINTERESTING), and refrain from going into the body of the preparation steps, which has to involve sorting. Do you mean has to involve sorting as in has to involve sorting in order not to break current users of e.g. 'git log --no-walk --branches' or revision walking inherently involves sorting? My current working assumption is that it is the former. I will make rev_info.no_walk a tri-state {walk, no-walk-sorted, no-walk-unsorted}. The third state would be used from cherry-pick/revert (and maybe git-show, although it should make no difference). I would also expose the third state to rev-list's command line, maybe as --no-walk=unsorted. Actually, all but command-line parsing is done now and test seem fine, with quite a small patch: $ git diff --stat builtin/log.c| 2 +- builtin/revert.c | 2 +- revision.c | 5 +++-- revision.h | 6 +- 4 files changed, 10 insertions(+), 5 deletions(-) Did you see a problem with this approach, since you said that sequencer shouldn't unconditionally call prepare_revision_walk()? I can see that git-show needs to go through revs-pending.objects because it handles tags and stuff, but cherry-pick/revert only seem to need the revisions. 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 0/4] Re: cherry-pick and 'log --no-walk' and ordering
On Mon, Aug 13, 2012 at 12:17 AM, Junio C Hamano gits...@pobox.com wrote: y...@google.com writes: [Administrivia: I somehow doubt y...@google.com would reach you, and futzed with the To: line above] :-( Sorry, sendemail.from now set. (I apparently answered y instead of just enter to accept the default.) I actually think --no-walk, especially when given any negative revision, that sorts is fundamentally a flawed concept (it led to the inconsistency that made git show A..B C vs git show C A..B behave differently, which we had to fix recently). I completely agree. Would anything break if we take your patch, but without two possibilities to revs-no_walk option (i.e. we never sort under no_walk)? That is, the core of your change would become something like this: I also thought the sorting was just a bug. From what I understand by looking how the code has evolved, the sorting in the no-walk case was not intentional, but more of a consequence of the implementation. That patch you suggested was my first attempt and led me to find the broken cherry-pick test cases that I then fixed in patch 2/4. But, it clearly would break the test case in t4202 called 'git log --no-walk commits sorts by commit time'. So I started digging from there and found e.g. http://thread.gmane.org/gmane.comp.version-control.git/123205/focus=123216 For convenience, I have pasted the commit message of the commit mentioned in that thread at the end of this email. So we would be breaking at least Johannes's use case if we changed it. I would think almost everyone who doesn't already know would expect git rev-list A B to list them in that order, so is a migration desired? Or just change the default for --no-walk from sorted to unsorted in git 2.0? By the way, git-log's documentation says By default, the commits are shown in reverse chronological order., which to some degree is in support of the current behavior. commit 8e64006eee9c82eba513b98306c179c9e2385e4e Author: Johannes Schindelin johannes.schinde...@gmx.de Date: Tue Jul 24 00:38:40 2007 +0100 Teach revision machinery about --no-walk The flag no_walk is present in struct rev_info since a long time, but so far has been in use exclusively by git show. With this flag, you can see all your refs, ordered by date of the last commit: $ git log --abbrev-commit --pretty=oneline --decorate --all --no-walk which is extremely helpful if you have to juggle with a lot topic branches, and do not remember in which one you introduced that uber debug option, or simply want to get an overview what is cooking. (Note that the git log invocation above does not output the same as $ git show --abbrev-commit --pretty=oneline --decorate --all --quiet since git show keeps the alphabetic order that --all returns the refs in, even if the option --date-order was passed.) For good measure, this also adds the --do-walk option which overrides --no-walk. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de Signed-off-by: Junio C Hamano gits...@pobox.com -- 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 0/4] Re: cherry-pick and 'log --no-walk' and ordering
On Mon, Aug 13, 2012 at 10:05 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: ... so is a migration desired? Or just change the default for --no-walk from sorted to unsorted in git 2.0? I think the proper support for Johannes's case should give users more control on what to sort on, and that switch should not be tied to --no-walk. After all, being able to sort commits in the result of limit_list() with various criteria would equally useful as being able to sort commits listed on the command line with --no-walk. Think about what git shortlog A..B does, for example. It is like first enumerating commits within the given range, and sorting the result using author as the primary and then timestamp as the secondary sort column. So let's not even think about migration, and go in the direction of giving --no-walk two flavours, for now. Either it keeps the order commits were given from the command line, or it does the default sort using the timestamp. We can later add the --sort-on option that would work with or without --no-walk for people who want output that is differently sorted, but that is outside the scope of your series. Makes sense. The shortlog example is a good example of sorting that completely reorders the commit graph sometimes even making sense for ranges. 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: [PATCH 2/4] revisions passed to cherry-pick should be in default order
On Mon, Aug 13, 2012 at 1:05 PM, Junio C Hamano gits...@pobox.com wrote: y...@google.com writes: From: Martin von Zweigbergk martin.von.zweigbe...@gmail.com 'git cherry-pick' internally sets the --reverse option while walking revisions, so that 'git cherry-pick branch@{u}..branch' will apply the revisions starting at the oldest one. If no uninteresing revisions are given, --no-walk is implied. Still, the documentation for 'git cherry-pick --stdin' uses the following example: git rev-list --reverse master -- README | git cherry-pick -n --stdin The above would seem to reverse the revisions in the output (which it does), and then pipe them to 'git cherry-pick', which would reverse them again and apply them in the wrong order. I think we have cleared this confusion up in the previous discussion. It it sequencer's bug that reorders the commits when the caller (rev-list --reverse in this case) gives list of individual commits to replay. So I think we are all OK with chucking this patch. Am I mistaken? I can't really say. I suppose the current patch is smaller (it can't really get smaller than one line), but iterating over the arguments the sequencer level might be more correct. Would the result be different in some cases? I would be happy to add a test case at least, although I'm not sure when I would have time to implement it in sequencer. To connect to the other mail I sent on this thread (in parallel with yours), do you think git cherrry-pick HEAD HEAD~1 should apply the commits in the same order as git cherry-pick HEAD~2..HEAD (which would give the same result if passed to 'rev-list --no-walk' for a linear history) or in the order specified on the command line? I couldn't find any conclusive evidence of what was intended in either log messages or test cases. -- 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] rev-list docs: clarify --topo-order description
On Mon, Aug 13, 2012 at 3:21 PM, Junio C Hamano gits...@pobox.com wrote: * Let's do this before I forget...; came up in discussion $gmane/203370 Thanks! That definitely confused me (and I suppose I stupidly didn't test with a proper range). Documentation/rev-list-options.txt | 29 +++-- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 6a4b635..dc501ee 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -579,15 +579,32 @@ Commit Ordering By default, the commits are shown in reverse chronological order. It seems likely that those reading the above sentence will continue on to read about --topo-order, but still, do you think the descendant commits are shown before parents part belong here instead? --topo-order:: - - This option makes them appear in topological order (i.e. - descendant commits are shown before their parents). + This option makes them appear in topological order. Even + without this option, descendant commits are shown before + their parents, but this tries to avoid showing commits on + multiple lines of history intermixed. --date-order:: - This option is similar to '--topo-order' in the sense that no - parent comes before all of its children, but otherwise things - are still ordered in the commit timestamp order. + Show no parents before all of its children, but otherwise + show commits in the commit timestamp order. ++ +For example, in a commit history like this: ++ + + +---1247 + \ \ +3568--- + + ++ +where the numbers denote the order of commit timestamps, `git +rev-list` and friends with `--date-order` show the commits in the +timestamp order: 8 7 6 5 4 3 2 1. ++ +With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5 +3 1), to avoid commits from two branches mixed together. It would help at least me to also know what the output would be without either of --date-order or --topo-order. (Does the default order have a name, by the way?) -- 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] rev-list docs: clarify --topo-order description
On Mon, Aug 13, 2012 at 4:05 PM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 6a4b635..dc501ee 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -579,15 +579,32 @@ Commit Ordering By default, the commits are shown in reverse chronological order. It seems likely that those reading the above sentence will continue on to read about --topo-order, but still, do you think the descendant commits are shown before parents part belong here instead? I do not think so. When you are not limited (i.e. limit_list() is not called), you could do something like git rev-list 4 5 in a history like this: --1---5---2---3---4 and get end up getting 5 4 3 2 1, and 2 certainly doesn't get shown before 5 does. Oh, interesting. I had no idea, although that does make sense. Thanks. Still, the Even without this option strongly suggests to me that what follows (descendant commits are shown before parents) applies to the By default case. Would it be correct to say something like By default, the commits are shown in reverse chronological order. When commit limiting is in effect, descendant commits are shown before parents.? I'm not sure the commit limiting section in the man page involves the same options as limit_list (I rather think they don't), but I don't know if there's a better term to use in the documentation either. -- 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: send-email and in-reply-to = n
On Mon, Aug 13, 2012 at 4:53 PM, Junio C Hamano gits...@pobox.com wrote: Stephen Boyd bebar...@gmail.com writes: Can we throw up a big warning or just outright fail if someone types 'n' or 'y' and hits enter for the in-reply-to question in git-send-email? I saw a git-send-email sent patch with an In-Reply-To header containing n on lkml today and it makes threading in my mail client get confused. Yeah, I think it is a good idea to minimally sanity check the answer to in-reply-to (and possibly other fields); perhaps does it have @ and dot would be a good enough heuristics. Please make it so ;-) And if you do, please include the check for the value for the From: header in the and possibly other fields. I made the same mistake when asked about that value just a few days ago. -- 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/4] revisions passed to cherry-pick should be in default order
On Mon, Aug 13, 2012 at 2:05 PM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: To connect to the other mail I sent on this thread (in parallel with yours), do you think git cherrry-pick HEAD HEAD~1 should apply the commits in the same order as git cherry-pick HEAD~2..HEAD (which would give the same result if passed to 'rev-list --no-walk' for a linear history) or in the order specified on the command line? Definitely the latter; I do not think of any semi-reasonable excuse to do otherwise. Indeed. My patches tried to fix the wrong problem. Sorry I'm slow, but I think I'm finally starting to understand what you've been saying all along about the bug being in sequencer. I'll try to recapitulate a bit for my own and maybe others' understanding. For simplicity, let's assume a linear history with unique timestamps, but not necessarily increasing with each commit. Currently: 1) 'git cherry-pick A..C' picks the commits order in reverse default order 2) 'git cherry-pick B C' picks the commits in chronological order 3) 'git rev-list --reverse A..C | git cherry-pick --stdin' behaves just like 'git cherry-pick B C' and therefore picks the commits in chronological order In cases 2) and 3), even though cherry-pick tells the revision walker not to walk, it still sorts the commits in reverse chronological order. But cherry-pick also tells the revision walker explicitly to reverse the list, so in the end, the order is chronological. In case 2), however, the first ordering make no difference in this limited case (IIUC). So the default ordering (which would be C, then B in this case, regardless of timestamps), gets reversed and B gets applied first, followed by C. So all of the above case give the right result in the end as long as the timestamps are chronological, and case 1) gives the right result regardless. The other two cases only works in most cases because the unexpcted sorting when no-walk is in effect counteracts the final reversal. When I noticed that the order of inputs to cases 2) and 3) above was ignored, and thinking that 'git rev-list A..C | git cherry-pick --stdin' should mimic 'git cherry-pick A..C', I incorrectly thought that the error was the use of --reverse to 'git rev-list' as well as the sorting done in the no-walk case. I think completely ignored case 2) at this point. I now think I understand that the sorting done in the no-walk case is indeed incorrect, but that the --reverse passed to rev-list is correct. Instead, the final reversal, which is currently unconditional, should not be done in the no-walk case. IIUC, this could be implemented by making cherry-pick iterate over rev_info.pending.objects just like 'git show' does when not walking. Junio, I think it makes sense to just drop this whole series for now. I'll probably include patch 1/4 in my stalled rebase-range series instead. If I understood you correctly, you didn't have any objections to that 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 2/4] revisions passed to cherry-pick should be in default order
On Wed, Aug 15, 2012 at 10:16 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: So all of the above case give the right result in the end as long as the timestamps are chronological, and case 1) gives the right result regardless. The other two cases only works in most cases because the unexpcted sorting when no-walk is in effect counteracts the final reversal. In short, if you have three commits in a row, A--B--C, with timestamps that are not skewed, and want to replay changes of B and then C in that order, all three you listed ends up doing the right thing. But if you want to apply the change C and then B: - git cherry-pick A..C is obviously not a way to do so, so we won't discuss it further. - git cherry-pick C B is the most natural way the user would want to express this request, but because of the sorting (i.e. commit_list_sort_by_date() in prepare_revision_walk(), combined with -reverse in sequencer.c::prepare_revs()), it applies B and then C. That is the real bug. Feeding the revs to git cherry-pick --stdin in the order the user wishes them to be applied has the same issue. Exactly. I actually think your approach to place the do not sort when we are not walking logic in prepare_revision_walk() makes more sense. show has to look at pending.objects[] because it needs to show objects other than commits (e.g. git show :foo), so there won't be any change in its implementation with your change. It will have to look at pending.objects[] itself. Yes, I noticed that's why show has to do it that way. But cherry-pick and sequencer-derived commands only deal with commits. It would be far less error prone to let them call get_revision() repeatedly like all other revision enumerating commands do, than to have them go over the pending.objects[] list, dereferencing tags and using only commits. The resulting callers would be more readable, too, I would think. Makes sense, I'll try to implement it that way. I was afraid that we would need to call prepare_revision_walk() once first and then if we afterwards find out that we should not walk, we would need to call it again without the reverse option. But after looking at how rev_info.reverse is used, it seem like it's only used in get_revision(), so we can leave it either on or off during the prepare_revision_walk() and the and set appropriately before calling get_revision(), like so: init_revisions(revs); revs.no_walk = REVISION_WALK_NO_WALK_UNSORTED; setup_revisions(...); prepare_revision_walk(revs); revs.reverse = !revs.no_walk; // iterate over revisions -- 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/4] revisions passed to cherry-pick should be in default order
On Wed, Aug 15, 2012 at 11:39 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: Makes sense, I'll try to implement it that way. I was afraid that we would need to call prepare_revision_walk() once first and then if we afterwards find out that we should not walk, we would need to call it again without the reverse option. But after looking at how rev_info.reverse is used, it seem like it's only used in get_revision(), so we can leave it either on or off during the prepare_revision_walk() and the and set appropriately before calling get_revision(), like so: init_revisions(revs); revs.no_walk = REVISION_WALK_NO_WALK_UNSORTED; setup_revisions(...); prepare_revision_walk(revs); revs.reverse = !revs.no_walk; Sorry, but I do not understand why you frutz with reverse after prepare, and not before. I think you can just set no_walk and let setup_revisions() turn it off upon seeing a range (this happens in add_pending_object()). Ah, of course. For some reason I thought that was called from prepare_revision_walk() After setup_revisions() returns, if no_walk is still set, you only got individual refs without ranges, so no reversing required. Yes, it's in the other case (e.g. 'git cherry-pick A..C', when no_walk is not set), that we need to set reverse before walking. You also need to be careful about revert that shares the code; when reverting range A..C in your example, you want to undo C and then B, and you do not want to reverse them. Yep. It looks like this, so should be safe. But thanks for the reminder. if (opts-action != REPLAY_REVERT) opts-revs-reverse ^= 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 v2] rev-list docs: clarify --topo-order description
On Wed, Aug 15, 2012 at 1:02 PM, Junio C Hamano gits...@pobox.com wrote: diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 6a4b635..9404d08 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -578,16 +578,33 @@ Commit Ordering By default, the commits are shown in reverse chronological order. As I said before, this led me to believe that the commits are shown in reverse chronological order when neither of --topo-order or --date-order is passed. I agree that we should avoid specifying more than necessary about the default case. But in this case, what is specified is not exactly true (in the face of clock skew). Or am I misunderstanding or misinterpreting something? I still don't understand the revision walker well enough to be able to provide a better wording, but I think even the rather crude By default, the order is unspecified. would at least not be as easy to misinterpret (if that's what I did) and is definitely not over-specified. Of course, if someone can think of something better, I'm all for it. Regardless of the above comment about the patch context, what your patch actually changes looks good. Thanks again. -- 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] revision (no-)walking in order
I'm still working on a re-roll of my rebase-range series, but I think these three are quite unrelated and shouldn't be held up by that other series. Junio, thanks for all the help with explaining revision walking. It was a little blurry for a long time, but at least I feel more comfortable with these few patches now. Btw, the rebase-range series seems to need (or be greatly simplified), although I'm not 100% sure yet, by teaching patch-id --keep-empty, which would be its first command line option. Let me know if you (plural) sees a problem with that. Btw2, I'm migrating my email to martinv...@gmail.com (not y...@google.com ;-) which saves a few keystrokes and matches some of my other accounts, so these patches will be the first ones from the new address. Martin von Zweigbergk (3): teach log --no-walk=unsorted, which avoids sorting demonstrate broken 'git cherry-pick three one two' cherry-pick/revert: respect order of revisions to pick Documentation/rev-list-options.txt | 12 builtin/log.c | 2 +- builtin/revert.c| 2 +- revision.c | 18 +++--- revision.h | 6 +- sequencer.c | 4 +++- t/t3508-cherry-pick-many-commits.sh | 15 +++ t/t4202-log.sh | 10 ++ 8 files changed, 58 insertions(+), 11 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 1/3] teach log --no-walk=unsorted, which avoids sorting
When 'git log' is passed the --no-walk option, no revision walk takes place, naturally. Perhaps somewhat surprisingly, however, the provided revisions still get sorted by commit date. So e.g 'git log --no-walk HEAD HEAD~1' and 'git log --no-walk HEAD~1 HEAD' give the same result (unless the two revisions share the commit date, in which case they will retain the order given on the command line). As the commit that introduced --no-walk (8e64006 (Teach revision machinery about --no-walk, 2007-07-24)) points out, the sorting is intentional, to allow things like git log --abbrev-commit --pretty=oneline --decorate --all --no-walk to show all refs in order by commit date. But there are also other cases where the sorting is not wanted, such as command producing revisions in order | git log --oneline --no-walk --stdin To accomodate both cases, leave the decision of whether or not to sort up to the caller, by allowing --no-walk={sorted,unsorted}, defaulting to 'sorted' for backward-compatibility reasons. Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- Documentation/rev-list-options.txt | 12 builtin/log.c | 2 +- builtin/revert.c | 2 +- revision.c | 18 +++--- revision.h | 6 +- t/t4202-log.sh | 10 ++ 6 files changed, 40 insertions(+), 10 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index def1340..5436eba 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -636,10 +636,14 @@ These options are mostly targeted for packing of git repositories. Only useful with '--objects'; print the object IDs that are not in packs. ---no-walk:: - - Only show the given revs, but do not traverse their ancestors. - This has no effect if a range is specified. +--no-walk[=(sorted|unsorted)]:: + + Only show the given commits, but do not traverse their ancestors. + This has no effect if a range is specified. If the argument + unsorted is given, the commits are show in the order they were + given on the command line. Otherwise (if sorted or no argument + was given), the commits are show in reverse chronological order + by commit time. --do-walk:: diff --git a/builtin/log.c b/builtin/log.c index ecc2793..20838b1 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -456,7 +456,7 @@ int cmd_show(int argc, const char **argv, const char *prefix) init_revisions(rev, prefix); rev.diff = 1; rev.always_show_header = 1; - rev.no_walk = 1; + rev.no_walk = REVISION_WALK_NO_WALK_SORTED; rev.diffopt.stat_width = -1;/* Scale to real terminal size */ memset(opt, 0, sizeof(opt)); diff --git a/builtin/revert.c b/builtin/revert.c index 82d1bf8..42ce399 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -193,7 +193,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) struct setup_revision_opt s_r_opt; opts-revs = xmalloc(sizeof(*opts-revs)); init_revisions(opts-revs, NULL); - opts-revs-no_walk = 1; + opts-revs-no_walk = REVISION_WALK_NO_WALK_SORTED; if (argc 2) usage_with_options(usage_str, options); memset(s_r_opt, 0, sizeof(s_r_opt)); diff --git a/revision.c b/revision.c index 442a945..66ba2e6 100644 --- a/revision.c +++ b/revision.c @@ -1300,7 +1300,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg !strcmp(arg, --no-walk) || !strcmp(arg, --do-walk) || !strcmp(arg, --bisect) || !prefixcmp(arg, --glob=) || !prefixcmp(arg, --branches=) || !prefixcmp(arg, --tags=) || - !prefixcmp(arg, --remotes=)) + !prefixcmp(arg, --remotes=) || !prefixcmp(arg, --no-walk=)) { unkv[(*unkc)++] = arg; return 1; @@ -1695,7 +1695,18 @@ static int handle_revision_pseudo_opt(const char *submodule, } else if (!strcmp(arg, --not)) { *flags ^= UNINTERESTING; } else if (!strcmp(arg, --no-walk)) { - revs-no_walk = 1; + revs-no_walk = REVISION_WALK_NO_WALK_SORTED; + } else if (!prefixcmp(arg, --no-walk=)) { + /* +* Detached form (--no-walk X as opposed to --no-walk=X) +* not allowed, since the argument is optional. +*/ + if (!strcmp(arg + 10, sorted)) + revs-no_walk = REVISION_WALK_NO_WALK_SORTED; + else if (!strcmp(arg + 10, unsorted)) + revs-no_walk = REVISION_WALK_NO_WALK_UNSORTED; + else + return error(invalid argument to --no-walk); } else if (!strcmp(arg
[PATCH v2 2/3] demonstrate broken 'git cherry-pick three one two'
Cherry-picking commits out of order (w.r.t. commit time stamp) doesn't currently work. Add a test case to demonstrate it. Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- t/t3508-cherry-pick-many-commits.sh | 15 +++ 1 file changed, 15 insertions(+) diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh index 75f7ff4..fff20c3 100755 --- a/t/t3508-cherry-pick-many-commits.sh +++ b/t/t3508-cherry-pick-many-commits.sh @@ -44,6 +44,21 @@ test_expect_success 'cherry-pick first..fourth works' ' check_head_differs_from fourth ' +test_expect_failure 'cherry-pick three one two works' ' + git checkout -f first + test_commit one + test_commit two + test_commit three + git checkout -f master + git reset --hard first + git cherry-pick three one two + git diff --quiet three + git diff --quiet HEAD three + test $(git log --reverse --format=%s first..) == three +one +two +' + test_expect_success 'output to keep user entertained during multi-pick' ' cat -\EOF expected [master OBJID] second -- 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] cherry-pick/revert: respect order of revisions to pick
When giving multiple individual revisions to cherry-pick or revert, as in 'git cherry-pick A B' or 'git revert B A', one would expect them to be picked/reverted in the order given on the command line. They are instead ordered by their commit timestamp -- in chronological order for cherry-pick and in reverse chronological order for revert. This matches the order in which one would usually give them on the command line, making this bug somewhat hard to notice. Still, it has been reported at least once before [1]. It seems like the chronological sorting happened by accident because the revision walker has traditionally always sorted commits in reverse chronological order when rev_info.no_walk was enabled. In the case of 'git revert B A' where B is newer than A, this sorting is a no-op. For 'git cherry-pick A B', the sorting would reverse the arguments, but because the sequencer also flips the rev_info.reverse flag when picking (as opposed to reverting), the end result is a chronological order. The rev_info.reverse flag was probably flipped so that the revision walker emits B before C in 'git cherry-pick A..C'; that it happened to effectively undo the unexpected sorting done when not walking, was probably a coincidence that allowed this bug to happen at all. Fix the bug by telling the revision walker not to sort the commits when not walking. The only case we want to reverse the order is now when cherry-picking and walking revisions (rev_info.no_walk = 0). [1] http://thread.gmane.org/gmane.comp.version-control.git/164794 Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- builtin/revert.c| 2 +- sequencer.c | 4 +++- t/t3508-cherry-pick-many-commits.sh | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index 42ce399..98ad641 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -193,7 +193,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) struct setup_revision_opt s_r_opt; opts-revs = xmalloc(sizeof(*opts-revs)); init_revisions(opts-revs, NULL); - opts-revs-no_walk = REVISION_WALK_NO_WALK_SORTED; + opts-revs-no_walk = REVISION_WALK_NO_WALK_UNSORTED; if (argc 2) usage_with_options(usage_str, options); memset(s_r_opt, 0, sizeof(s_r_opt)); diff --git a/sequencer.c b/sequencer.c index bf078f2..9f32104 100644 --- a/sequencer.c +++ b/sequencer.c @@ -543,7 +543,9 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) static void prepare_revs(struct replay_opts *opts) { - if (opts-action != REPLAY_REVERT) + // picking (but not reverting) ranges (but not individual revisions) + // should be done in reverse + if (opts-action == REPLAY_PICK !opts-revs-no_walk) opts-revs-reverse ^= 1; if (prepare_revision_walk(opts-revs)) diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh index fff20c3..04b5ad4 100755 --- a/t/t3508-cherry-pick-many-commits.sh +++ b/t/t3508-cherry-pick-many-commits.sh @@ -44,7 +44,7 @@ test_expect_success 'cherry-pick first..fourth works' ' check_head_differs_from fourth ' -test_expect_failure 'cherry-pick three one two works' ' +test_expect_success 'cherry-pick three one two works' ' git checkout -f first test_commit one test_commit two -- 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/4] merge-base: --is-ancestor A B
(Resending as plain text, sorry about duplicate, Junio.) On Thu, Aug 30, 2012 at 4:13 PM, Junio C Hamano gits...@pobox.com wrote: In many scripted Porcelain commands, we find this idiom: if test $(git rev-parse --verify A) = $(git merge-base A B) then ... A is an ancestor of B ... fi But you do not have to compute exact merge-base only to see if A is an ancestor of B. Give them a more direct way to use the underlying machinery. Thanks! It has bugged me ever since I first saw that idiom that there was no way that better shows the intent. + if (argc != 2) + die(--is-ancestor takes exactly two commits); I think git merge-base shows the usage message regardless if argc 2, so this only happens when more than two arguments are given. Maybe include --is-ancestor in the usage message? + if (is_ancestor (show_all | octopus | reduce)) + die(--is-reachable cannot be used with other options); I suppose --is-reachable should be --is-ancestor. -- 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: Aw: Re: git blame shows wrong Not commited yet entries
On Fri, Aug 31, 2012 at 10:58 AM, Junio C Hamano gits...@pobox.com wrote: And git blame $path probably should expect $path is something that appear in the tree of HEAD; apparently it does not. That probably makes sense. For anyone deciding to implement that, note that git blame -C [-C [-C]] $path should probably not expect the same, so the following still works. cp COPYING COPY git blame -C -C -C COPY Btw, why isn't -C -C -C the same as -CCC? Should 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 v2 2/3] rebase -i: Teach --edit-todo action
On Sun, Sep 16, 2012 at 8:17 AM, Andrew Wong andrew.k...@gmail.com wrote: diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index fd535b0..da067ec 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -12,7 +12,7 @@ SYNOPSIS [upstream] [branch] 'git rebase' [-i | --interactive] [options] [--exec cmd] [--onto newbase] --root [branch] -'git rebase' --continue | --skip | --abort +'git rebase' --continue | --skip | --abort | --edit-todo I guess you should add --edit-todo to OPTIONS_SPEC in git-rebase.sh as well. The OPTIONS_SPEC needs another little update too. I have included a patch at the end of this email that you include in a re-roll. + git_sequence_editor $todo || + die_abort Could not execute editor die_abort seems a little harsh -- it will discard the rebase state. Plain die would be better, I think. Also, if you even need to break the line after the || operator, you might want to indent the remainder by one tab. This file is quite consistent in using that style, although I don't know what the preferred style is in general in git. git var GIT_COMMITTER_IDENT /dev/null || diff --git a/git-rebase.sh b/git-rebase.sh index 15da926..e5a289c 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -194,6 +195,10 @@ do test $total_argc -eq 2 || usage action=${1##--} ;; + --edit-todo) + test $total_argc -eq 2 || usage + action=${1##--} + ;; It looks like this could be trivially combined with the previous case arm, making the match --continue|--skip|--abort|--edit-todo). --8-- Author: Martin von Zweigbergk martinv...@gmail.com rebase usage: subcommands can not be combined with -i Since 95135b0 (rebase: stricter check of standalone sub command, 2011-02-06), git-rebase has not allowed to use -i together with e.g. --continue. Yet, when rebase started using OPTIONS_SPEC in 45e2acf (rebase: define options in OPTIONS_SPEC, 2011-02-28), the usage message included git-rebase [-i] --continue | --abort | --skip Remove the [-i] from this line. Signed-off-by: Martin von Zweigbergk martinv...@gmail.com diff --git a/git-rebase.sh b/git-rebase.sh index 15da926..e6b43a2 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -8,7 +8,7 @@ OPTIONS_KEEPDASHDASH= OPTIONS_SPEC=\ git rebase [-i] [options] [--exec cmd] [--onto newbase] [upstream] [branch] git rebase [-i] [options] [--exec cmd] [--onto newbase] --root [branch] -git-rebase [-i] --continue | --abort | --skip +git-rebase --continue | --abort | --skip -- Available options are v,verbose! display a diffstat of what changed 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
Re: [PATCH 4/4] rebase -i: Add tests for --edit-todo
On Mon, Sep 17, 2012 at 10:23 PM, Andrew Wong andrew.k...@gmail.com wrote: On 09/18/12 00:58, Martin von Zweigbergk wrote: On Mon, Sep 17, 2012 at 6:28 PM, Andrew Wong andrew.k...@gmail.com wrote: + test M = $(git cat-file commit HEAD^ | sed -ne \$p) + test L = $(git cat-file commit HEAD | sed -ne \$p) I couldn't find $ (match last line) in the POSIX man page for sed. Besides, I think $(git show -s --format=%s HEAD) reads better. It's under Addresses in sed: ... a '$' character that addresses the last line of input ... Ah, I just didn't look hard enough; sorry. Good to know. FWIW, Acked-by: Martin von Zweigbergk martinv...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH] add t3420-rebase-topology
Add more test cases to check that the topology after a rebase is as expected. Conflicts are not considered, but patch-equivalence is. --- Tests pass and fail as indicated by the suffix (_success/_failure). Your input especially appreciated on whether you agree with the intent of the test cases. For example, do you agree that 'rebase --onto does not re-apply patches in onto' is desirable? And if you do, then do you also agree that 'rebase --root --onto ignores patch in onto' is desirable? How about 'rebase --root is not a no-op'? One might think that --force would be necessary, but on the other hand, if that was the case, the only point (AFAICT) of git rebase --root branch without --force would be to linearize history, so I instead made the test case confirm that --root without --onto effectively behaves as if --force was also passed. Feedback on the structure/setup and style is of course also appreciated. t/t3420-rebase-topology.sh | 348 + 1 file changed, 348 insertions(+) create mode 100755 t/t3420-rebase-topology.sh diff --git a/t/t3420-rebase-topology.sh b/t/t3420-rebase-topology.sh new file mode 100755 index 000..024a2b4 --- /dev/null +++ b/t/t3420-rebase-topology.sh @@ -0,0 +1,348 @@ +#!/bin/sh + +test_description='effect of rebase on topology' +. ./test-lib.sh + + +# q---C---r +# / +# a---b---c---d!--e---p +# \ +# f---g!--h +#\ +# j---E---k +# \ \ +# n---H---w +# +# x---y---B +# +# +# ! = empty +# uppercase = cherry-picked +# p = reverted e +# +# TODO: +# prune graph to what's needed + +empty () { + git commit --allow-empty -m $1 + git tag $1 +} + +cherry_pick () { + git cherry-pick -n $1 + git commit -m $2 + git tag $2 +} + +revert () { + git revert -n $1 + git commit -m $2 + git tag $2 +} + + +test_expect_success 'setup' ' + test_commit a + test_commit b + test_commit c + empty d + test_commit e + revert e p + git checkout b + test_commit f + empty g + test_commit h + git checkout f + test_commit j + cherry_pick e E + test_commit k + git checkout j + test_commit n + cherry_pick h H + git merge -m w E + git tag w + git checkout b + test_commit q + cherry_pick c C + test_commit r + git checkout --orphan disjoint + git rm -rf . + test_commit x + test_commit y + cherry_pick b B +' + +reset () { + git rebase --abort + git reset --hard +} + +test_range () { + test $(git log --reverse --topo-order --format=%s $1 | xargs) = $2 +} + +test_revisions () { + expected=$1 + shift + test $(git log --format=%s --no-walk=unsorted $@ | xargs) = $expected +} + +same_revision () { + test $(git rev-parse $1) = $(git rev-parse $2) +} + +# the following 5 (?) tests copy t3400 tests, but check the history rather than status code and/or stdout +run () { +echo ' + reset + git rebase '$@' c j + same_revision HEAD~2 c + test_range c.. f j +' +} +test_expect_success 'simple rebase' $(run) +test_expect_success 'simple rebase -m' $(run -m) +test_expect_success 'simple rebase -i' $(run -i) +test_expect_success 'simple rebase -p' $(run -p) + +run () { +echo ' + reset + git rebase '$@' b j + same_revision HEAD j +' +} +test_expect_success 'rebase is no-op if upstream is an ancestor' $(run) +test_expect_success 'rebase -m is no-op if upstream is an ancestor' $(run -m) +test_expect_success 'rebase -i is no-op if upstream is an ancestor' $(run -i) +test_expect_success 'rebase -p is no-op if upstream is an ancestor' $(run -p) + +run () { +echo ' + reset + git rebase '$@' --force b j + ! same_revision HEAD j + test_range b.. f j +' +} +test_expect_success 'rebase --force' $(run) +test_expect_success 'rebase -m --force' $(run -m) +test_expect_success 'rebase -i --force' $(run -i) +test_expect_failure 'rebase -p --force' $(run -p) + +run () { +echo ' + reset + git rebase '$@' j b + same_revision HEAD j +' +} +test_expect_success 'rebase fast-forwards if an ancestor of upstream' $(run) +test_expect_success 'rebase -m fast-forwards if an ancestor of upstream' $(run -m) +test_expect_success 'rebase -i fast-forwards if an ancestor of upstream' $(run -i) +test_expect_success 'rebase -p fast-forwards if an ancestor of upstream' $(run -p) + +run () { +echo ' + reset + git rebase '$@' p k + test_range p.. f j k +' +} +test_expect_success 'rebase ignores patch in upstream' $(run) +test_expect_failure 'rebase -m ignores patch in upstream' $(run -m) +test_expect_success 'rebase -i ignores patch in upstream' $(run -i) +test_expect_success 'rebase -p ignores patch in upstream' $(run -p) + +run () { +echo ' + reset +
Re: [RFC PATCH] add t3420-rebase-topology
On Tue, Sep 18, 2012 at 12:51 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: do you agree that 'rebase --onto does not re-apply patches in onto' is desirable? This depends on how you look at --onto. Recall the most typical and the original use case of rebase: A'--C' your rebased work / ---o---o---o---F---B'--o---T master ^\ v1.7.12 A---B---C your work You could view git rebase master as a short-hand for $ git rebase --onto $(git merge-base master HEAD) master Exactly. I frequently consider it a short-hand for that. It might be worth pointing out that 'git pull --rebase', which might be one of the most frequent uses of rebase, internally often does git rebase --onto upstream upstream@{...} branch where upstream@{...} is the most recent upstream that is an ancestor of branch. For example, if your work is based on origin/pu and you send the bottom-most patch (B in the figure below) to the maintainer and and it gets applied to pu. Running git pull --rebase would then lead to git rebase --onto T A. You would want this to drop B. C' your rebased work / ---o---o---o---F---B'--o---T origin/pu \ A origin/pu@{1} \ B---C your work The intended use case for --onto, however, is primarily to replay a history to older base, i.e. [...] a moral equivalent of $ git checkout v1.7.12 $ git cherry-pick A B C ;# or git cherry-pick master..HEAD Yes, this is the alternative way of looking it at and exactly why I, too, was not sure how it should behave. You could argue that you can compute the patch equivalence between the commits in onto..master and commits in master..HEAD and filter out the equivalent commits I'm not sure if you meant master..onto rather than onto..master. Rebase (well, all flavors of rebase but -m) currently drops patches from master..HEAD that are also in HEAD..master. This is what the rebase --onto does not lose patches in upstream test is about. It is also one of the main problems that I try to fix in my long-stalled rebase-range series. I think we should drop patches in master..HEAD that are also in HEAD..onto (which is almost the same as master..onto). The replay to an updated base case (i.e. without --onto) Or _with_ --onto as in the above example from git pull --rebase. On the other hand, when the user replays to an older base, she has some idea what constitutes a series that she is replaying (i.e. $(git merge-base master HEAD)..HEAD). It smells to go against the user's world model if the command silently filtered commits by patch equivalence. If it's truly about rebasing onto an older base, there can't possibly be any patches in HEAD..onto, so assuming you agree with my reasoning above that those are the patches we should drop, rebasing onto older history would be safe. Besides, the whole point of a separate onto is to allow the user to specify a commit that does not have a straightforward ancestry relationship with the bottom of the series (i.e. either master or F), and computation of patch equivalence is expected to be much higher. Given that it is unlikely to find any match, it feels doubly wrong to always run git cherry equivalent in that case. Yes, this was my only concern (apart from it possibly being conceptually wrong to do, depending on what the user meant by issuing the command). How about 'rebase --root is not a no-op'? ---o---o---o---F---B'--o---T master ^\ v1.7.12 A---B---C your work If git rebase F when you are at C in the above illustration (reproduced only the relevant parts) is a no-op (and I think it should be), git rebase --root in the illustration below ought to be as well, no? F---B'--o---T master \ A---B---C your work Yeah, that's what I thought as well at first. I think my test case even started out as rebase --root _is_ a no-op. When thinking about how to handle roots in general, I often imagine a single virtual root commit (parent of all initial commits), and that reasoning also implies that git rebase --root should be a no-op. Then I saw that the test case failed (or perhaps I remembered how it is implemented with the clever fake root/initial commit) and started thinking about why anyone would use git rebase --root if it was a no-op. I could only think of using it to linearize history, but that doesn't seem like a very likely use case. So it seems like weighing purity/correctness against usefulness to me. I'm not sure which way to go. Thanks for quick and detailed feedback on an RFC patch. Martin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More
Re: [RFC PATCH] add t3420-rebase-topology
[+Chris Webb regarding git rebase --root] First of all, thanks for a meticulous review! On Tue, Sep 18, 2012 at 12:53 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 9/18/2012 8:31, schrieb Martin von Zweigbergk: Since here and in the following tests the test cases and test descriptions vary in the same way, wouldn't it make sense to factor the description out as well? Definitely. I just couldn't think of a good way of doing it, so thanks for great and concrete suggestions! (Watch your quoting, though.) Switched to putting the test body in double quotes as you suggested in your examples and used single quotes for strings within the test body. +run () { +echo ' + reset + git rebase '$@' --keep-empty p h + test_range p.. f g h +' +} +test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' $(run) +test_expect_failure 'rebase -m --keep-empty keeps empty even if already in upstream' $(run -m) +test_expect_failure 'rebase -i --keep-empty keeps empty even if already in upstream' $(run -i) +test_expect_failure 'rebase -p --keep-empty keeps empty even if already in upstream' $(run -p) is in upstream is decided by the patch text. If an empty commit is already in upstream, this adds another one with the same or a different commit message and authorship information. Dubious, but since it is opt-in, it should be OK. Yes, it is a little dubious. See http://thread.gmane.org/gmane.comp.version-control.git/203097/focus=203159 and Junio's answer, which I think makes sense. +run () { +echo ' + reset + git rebase '$@' j w + test_range j.. E n H || test_range j.. n H E +' Chaining tests with || is dangerous: you do not know whether the first failed because the condition is not satisfied or because of some other failure. Good point. Thanks. Why is this needed in the first place? Shouldn't the history be deterministic, provided that the commit timestamps are all distinct? It may be deterministic, but it's not specified, I think, so I didn't want to depend on the order. Thinking more about it, though, I think it's good to protect the current behavior from patches that change the order of the parents. Although it may not be incorrect to change the order, it would at least protect against accidental changes. It turns out that rebase -i goes through the commits in --topo-order, while the others use default order, I think. Which flavor should pass the test case and which should fail (and be fixed)? I would personally prefer to say that rebase -i is correct in using --topo-order and that the others should be fixed. Again, it's not specified, but I would hate to have them behave differently. +run () { +echo ' + reset + git rebase '$@' --root c + ! same_revision HEAD c + test_range c a b c +' +} +test_expect_success 'rebase --root is not a no-op' $(run) +test_expect_success 'rebase -m --root is not a no-op' $(run -m) +test_expect_success 'rebase -i --root is not a no-op' $(run -i) +test_expect_success 'rebase -p --root is not a no-op' $(run -p) Why? Is it more like --root implies --force? It doesn't currently exactly imply --force, but the effect is the same. Also see my reply to Junio's email in this thread. Maybe Chris has some thoughts on this? +run () { +echo ' + reset + git rebase '$@' --root --onto e y + test_range e.. x y +' +} +test_expect_success 'rebase --root --onto' $(run) +test_expect_failure 'rebase -m --root --onto' $(run -m) +test_expect_success 'rebase -i --root --onto' $(run -i) +test_expect_success 'rebase -p --root --onto' $(run -p) Where does this rebase start? Ah, --root stands in for the upstream argument, hence, y is the tip to rebase. Right? Then it makes sense. Thanks for pointing that out. I changed the order to git rebase --onto e --root y. I hope that makes it clearer. +test_expect_success 'rebase -p re-creates merge from upstream' ' + reset + git rebase -p k w + same_revision HEAD^ H + same_revision HEAD^2 k +' IMO, this tests the wrong thing. You have this history: ---j---E---k \ \ n---H---w where E is the second parent of w. What does it mean to rebase w onto k? IMO, it is a meaningless operation, and the outcome is irrelevant. It would make sense to test that this history results after the upstream at H moved forward: ---j---E---k \ \ n---H \ \ \ z---w' That is, w began a topic by mergeing the sidebranch E; then upstream advanced to z, and now you rebase the topic to the new upstream. Fair enough. Changed accordingly. +test_expect_success 'rebase -p re-creates internal merge' ' + reset + git rebase -p c w + test_revisions f j n E H w HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD You must also test for c; otherwise the test would succeed if rebase did nothing at all. This comment applies to all
Re: [RFC PATCH] add t3420-rebase-topology
On Thu, Sep 27, 2012 at 5:20 AM, Chris Webb ch...@arachsys.com wrote: You're right that rebase --root without --onto always creates a brand new root as a result of the implementation using a sentinel commit. Clearly this is what's wanted with --interactive That's not as clear as one might first think. git rebase -i actually honors --force; if one marks a commit for edit, but then --continue without making any changes, the next commit(s) will be fast-forwarded. See the first few lines of pick_one (or pick_one_preserving_merges). but rebase --root with neither --onto nor --interactive is a slightly odd combination for which I struggle to imagine a natural use. Perhaps you're right that for consistency it should be a no-op unless --force-rebase is given? If we did this, this combination would be a no-op unconditionally as by definition we're always descended from the root of our current commit. For consistency, it seems like git rebase -p --root should always be a no-op, while git rebase [-i/-m] --root should be no-op if the history has no merges. Also, since git rebase -i tries to fast-forward through existing commits, it seems like git rebase -i --root should ideally not create a sentinel commit, but instead stop at the first commit marked for editing. If, OTOH, --force-rebase is given, we should rewrite history from the first commit, which in the case of --root would mean creating a sentinel commit. So, in short, I have a feeling that the sentinel commit should be created if and only if both --root and --force-rebase (but not --onto) are given. However, given the not-very-useful behaviour, I suspect that rebase --root is much more likely to be a mistyped version of rebase -i --root than rebase --root --force-rebase. (Unless I'm missing a reasonable use for this? History linearisation perhaps?) Yes, the not-very-useful-ness of this is the clear argument against making them no-ops. But I have to say I was slightly surprised when I tried git rebase --root for the first time and it created completely new history for me. As you say, git rebase --root is probably often a mistyped git rebase -i --root, and if that is the case, it seems nicer (in addition to being more consistent) if we don't do anything rather than rewriting the history. The history rewriting might even go unnoticed and come as an unpleasant surprise later. When working on a new project, git rebase -i --root might even be a convenient replacement for git rebase -i initial commit even when one does not want to rewrite the initial commit itself, and in such a case, the user would clearly not want a sentinel commit either. So I'm getting more and more convinced that the sentinel commit should only be created if --force-rebase was given. Let me know if I'm missing something. 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: git rebase -p and patch equivalent commits
On Tue, Oct 16, 2012 at 12:58 PM, Damien Robert damien.olivier.robert+gm...@gmail.com wrote: Now feature is rebased against master. How would you rebase the branches patch1, patch2 and build so that they keep the same layout? I tried to rebase patch1 and patch2, hoping that rebase -p build would use the rebased commits for the merge but it creates new commits (that are patch equivalents to patch1 and patch2) and merge them. So I can think of two ways to proceed: 1) only rebase patch1 and patch2, and then remerge them again in build. If the build branch really is just a build branch, then I would probably choose this option. This start to get complicated if I have some commits in build after the merge What would such commits contain? Is it something related to your build system that you can automate? If not, should they perhaps rather have been included in one of the patch branches? Or are they related to interactions between the patch branches? If the latter, I would probably serialize the dependent branches (e.g. basing patch2 on patch1). 2) I can rebase -p the build branch first, and then reset --soft patch1 Did you mean --hard/--keep here? Or why would you use --soft? and patch2 so that they point to the right commits in the rebased branch. This way looks easier to do with more complicated layout, I just need to find a good way of finding where the rebased commits for patch1 and patch2 are, and I was thinking of using notes for that. I don't quite understand why you would want to do that if the build branch is just to make sure test pass on the merged result, but, yes, this method would probably be easier if you do need to keep both the build branch and the patchX branches up to date. Which branch do you actively work on at this point? Both the build branch and the patchX branches? Is it that you have sent patch1 and patch2 for review and you want to base your next topic on the merged result? I assume not, since you said it was a build branch. But if that was the case (i.e. somewhat active development on build, patch1 and patch2 (perhaps due to review comments)), I would probably still rebase one branch at a time, recreate the merge (possibly using rerere), and then rebase --onto new-merge old-merge build. -- 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 0/5] rebase: improve the keep-empty
Hi, I think I have some patches at home that instead teach 'git am' the --keep-empty flag. Does that make sense? It's been a while since I looked at it, but I'll try to take a look tonight (PST). Martin On Tue, May 28, 2013 at 6:29 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Hi, I've been analyzing 'git rebase' and found that the --keep-empty option triggers a very very different behavior. Here's a bunch of patches that make it behave like the 'am' does does for the most part. There's only a few minor changes, after which it might be possible to replace the whole 'am' mode to use cherr-pick instead. Felipe Contreras (5): rebase: split the cherry-pick stuff rebase: fix 'cherry' mode storage rebase: fix sequence continuation rebase: fix abort of cherry mode rebase: fix cherry-pick invocations .gitignore| 1 + Makefile | 1 + git-rebase--am.sh | 65 ++- git-rebase--cherry.sh | 55 +++ git-rebase.sh | 8 +++ 5 files changed, 93 insertions(+), 37 deletions(-) create mode 100644 git-rebase--cherry.sh -- 1.8.3.rc3.312.g47657de -- 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 v2 3/8] rebase: cherry-pick: fix sequence continuation
As Junio asked in the previous iteration, shouldn't this have been in the first patch? On Tue, May 28, 2013 at 9:16 PM, Felipe Contreras felipe.contre...@gmail.com wrote: We are not in am mode. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- git-rebase--cherrypick.sh | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh index 51354af..2fa4993 100644 --- a/git-rebase--cherrypick.sh +++ b/git-rebase--cherrypick.sh @@ -5,13 +5,15 @@ case $action in continue) - git am --resolved --resolvemsg=$resolvemsg - move_to_original_branch + git cherry-pick --continue + move_to_original_branch + rm -rf $state_dir exit ;; skip) - git am --skip --resolvemsg=$resolvemsg - move_to_original_branch + git cherry-pick --skip + move_to_original_branch + rm -rf $state_dir exit ;; esac -- 1.8.3.rc3.312.g47657de -- 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 v2 4/8] rebase: cherry-pick: fix abort of cherry mode
Same here: should this have been in the first patch? If not, do you know for how long it has been broken (since which commit)? On Tue, May 28, 2013 at 9:16 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- git-rebase.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/git-rebase.sh b/git-rebase.sh index 76900a0..9b5d78b 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -335,6 +335,7 @@ skip) run_specific_rebase ;; abort) + test $type == cherrypick git cherry-pick --abort git rerere clear read_basic_state case $head_name in -- 1.8.3.rc3.312.g47657de -- 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 v2 2/8] rebase: cherry-pick: fix mode storage
Actually, are all of 2/8 - 7/8 fixes for things that broke in patch 1/8? On Tue, May 28, 2013 at 9:16 PM, Felipe Contreras felipe.contre...@gmail.com wrote: We don't use the 'rebase-apply'. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- git-rebase--cherrypick.sh | 4 git-rebase.sh | 5 - 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/git-rebase--cherrypick.sh b/git-rebase--cherrypick.sh index cbf80f9..51354af 100644 --- a/git-rebase--cherrypick.sh +++ b/git-rebase--cherrypick.sh @@ -18,6 +18,9 @@ esac test -n $rebase_root root_flag=--root +mkdir $state_dir || die Could not create temporary $state_dir +: $state_dir/cherrypick || die Could not mark as cherrypick + # 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 @@ -32,3 +35,4 @@ then fi move_to_original_branch +rm -rf $state_dir diff --git a/git-rebase.sh b/git-rebase.sh index f929ca3..76900a0 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -174,6 +174,9 @@ then then type=interactive interactive_rebase=explicit + elif test -f $merge_dir/cherrypick + then + type=cherrypick else type=merge fi @@ -382,7 +385,7 @@ then elif test -n $keep_empty then type=cherrypick - state_dir=$apply_dir + state_dir=$merge_dir else type=am state_dir=$apply_dir -- 1.8.3.rc3.312.g47657de -- 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 v2 3/8] rebase: cherry-pick: fix sequence continuation
On Tue, May 28, 2013 at 10:41 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, May 29, 2013 at 12:33 AM, Martin von Zweigbergk martinv...@gmail.com wrote: As Junio asked in the previous iteration, shouldn't this have been in the first patch? No, the first patch is splitting the code without introducing any functional changes. This is fixing a bug that already exists, we could fix it before the split, or after, but mixing the split and the fix at the same time is a no-no. Oh, now I remember. I ran into that bug once. One change splits, the other change fixes, what's wrong with that? I didn't say there was anything wrong. I was asking if the bug was there before (and I didn't see an answer when Junio asked). 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: [RFC/PATCH v2 3/8] rebase: cherry-pick: fix sequence continuation
:-) On Tue, May 28, 2013 at 11:05 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, May 29, 2013 at 12:51 AM, Martin von Zweigbergk martinv...@gmail.com wrote: On Tue, May 28, 2013 at 10:41 PM, Felipe Contreras felipe.contre...@gmail.com wrote: One change splits, the other change fixes, what's wrong with that? I didn't say there was anything wrong. I was asking if the bug was there before (and I didn't see an answer when Junio asked). Why wouldn't it be before? Did I mention a commit that introduced a problem? No. Did any patch in this series introduce a problem? No. All we've done in this series is 1) reorganize the code without introducing *ANY* functional changes, and 2) fix a bug. If you see 1) introducing a problem, or 2) introducing a problem, then mention that in *those* patches. If there is no problem with 1) or 2) then it follows the problem already exists. -- Felipe Contreras -- 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/7] add tests for rebasing of empty commits
--- t/t3401-rebase-partial.sh | 24 t/t3420-rebase-topology-linear.sh | 58 +++ 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh index 58f4823..7ba1797 100755 --- a/t/t3401-rebase-partial.sh +++ b/t/t3401-rebase-partial.sh @@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr test_path_is_missing .git/rebase-merge ' -test_expect_success 'rebase ignores empty commit' ' - git reset --hard A - git commit --allow-empty -m empty - test_commit D - git rebase C - test $(git log --format=%s C..) = D -' - -test_expect_success 'rebase --keep-empty' ' - git reset --hard D - git rebase --keep-empty C - test $(git log --format=%s C..) = D -empty -' - -test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' ' - git reset --hard A - git commit --allow-empty -m also-empty - git rebase --keep-empty D - test $(git log --format=%s A..) = also-empty -D -empty -' - test_done diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh index 1152491..40fe264 100755 --- a/t/t3420-rebase-topology-linear.sh +++ b/t/t3420-rebase-topology-linear.sh @@ -160,4 +160,62 @@ test_run_rebase failure -m test_run_rebase failure -i test_run_rebase failure -p +# a---b---c---j! +# \ +# d---k!--l +# +# ! = empty +test_expect_success 'setup of linear history for empty commit tests' ' + git checkout c + make_empty j + git checkout d + make_empty k + test_commit l +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* drops empty commit + reset_rebase + git rebase $* c l + test_cmp_rev c HEAD~2 + test_linear_range 'd l' c.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --keep-empty + reset_rebase + git rebase $* --keep-empty c l + test_cmp_rev c HEAD~3 + test_linear_range 'd k l' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --keep-empty keeps empty even if already in upstream + reset_rebase + git rebase $* --keep-empty j l + test_cmp_rev j HEAD~3 + test_linear_range 'd k l' j.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + test_done -- 1.8.2.674.gd17d3d2 -- 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 6/7] t3406: modernize style
Update the following: - Quote 'setup' - Remove blank lines within test case body - Use test_commit instead of custom quick_one - Create branch topic from tag created by test_commit --- t/t3406-rebase-message.sh | 30 +- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh index e6a9a0d..fe8c27f 100755 --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -4,27 +4,17 @@ test_description='messages from rebase operation' . ./test-lib.sh -quick_one () { - echo $1 file$1 - git add file$1 - test_tick - git commit -m $1 -} +test_expect_success 'setup' ' + test_commit O fileO + test_commit X fileX + test_commit A fileA + test_commit B fileB + test_commit Y fileY -test_expect_success setup ' - quick_one O - git branch topic - quick_one X - quick_one A - quick_one B - quick_one Y - - git checkout topic - quick_one A - quick_one B - quick_one Z + git checkout -b topic O + git cherry-pick A B + test_commit Z fileZ git tag start - ' cat expect \EOF @@ -34,12 +24,10 @@ Committed: 0003 Z EOF test_expect_success 'rebase -m' ' - git rebase -m master report sed -n -e /^Already applied: /p \ -e /^Committed: /p report actual test_cmp expect actual - ' test_expect_success 'rebase --stat' ' -- 1.8.2.674.gd17d3d2 -- 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/7] add simple tests of consistency across rebase types
Helped-by: Johannes Sixt j...@kdbg.org --- t/lib-rebase.sh | 15 t/t3420-rebase-topology-linear.sh | 78 +++ 2 files changed, 93 insertions(+) create mode 100755 t/t3420-rebase-topology-linear.sh diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 6ccf797..62b3887 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -65,3 +65,18 @@ EOF test_set_editor $(pwd)/fake-editor.sh chmod a+x fake-editor.sh } + +# checks that the revisions in $2 represent a linear range with the +# subjects in $1 +test_linear_range () { + ! { git log --format=%p $2 | sane_grep ;} + expected=$1 + set -- $(git log --reverse --format=%s $2) + test $expected = $* +} + +reset_rebase () { + git rebase --abort # may fail; ignore exit code + git reset --hard + git clean -f +} diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh new file mode 100755 index 000..c4b32db --- /dev/null +++ b/t/t3420-rebase-topology-linear.sh @@ -0,0 +1,78 @@ +#!/bin/sh + +test_description='basic rebase topology tests' +. ./test-lib.sh +. $TEST_DIRECTORY/lib-rebase.sh + +# a---b---c +# \ +# d---e +test_expect_success 'setup' ' + test_commit a + test_commit b + test_commit c + git checkout b + test_commit d + test_commit e +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result simple rebase $* + reset_rebase + git rebase $* c e + test_cmp_rev c HEAD~2 + test_linear_range 'd e' c.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* is no-op if upstream is an ancestor + reset_rebase + git rebase $* b e + test_cmp_rev e HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* -f rewrites even if upstream is an ancestor + reset_rebase + git rebase $* -f b e + ! test_cmp_rev e HEAD + test_cmp_rev b HEAD~2 + test_linear_range 'd e' b.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* fast-forwards if an ancestor of upstream + reset_rebase + git rebase $* e b + test_cmp_rev e HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_done -- 1.8.2.674.gd17d3d2 -- 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 4/7] add tests for rebasing root
--- t/t3420-rebase-topology-linear.sh | 129 ++ 1 file changed, 129 insertions(+) diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh index 40fe264..2429aa8 100755 --- a/t/t3420-rebase-topology-linear.sh +++ b/t/t3420-rebase-topology-linear.sh @@ -218,4 +218,133 @@ test_run_rebase failure -m test_run_rebase failure -i test_run_rebase failure -p +# m +# / +# a---b---c---g +# +# x---y---B +# +# uppercase = cherry-picked +# m = reverted b +# +# Reverted patches are there for tests to be able to check if a commit +# that introduced the same change as another commit is +# dropped. Without reverted commits, we could get false positives +# because applying the patch succeeds, but simply results in no +# changes. +test_expect_success 'setup of linear history for test involving root' ' + git checkout b + revert m b + git checkout --orphan disjoint + git rm -rf . + test_commit x + test_commit y + cherry_pick B b +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root + reset_rebase + git rebase $* --onto c --root y + test_cmp_rev c HEAD~2 + test_linear_range 'x y' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* without --onto --root with disjoint history + reset_rebase + git rebase $* c y + test_cmp_rev c HEAD~2 + test_linear_range 'x y' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root drops patch in onto + reset_rebase + git rebase $* --onto m --root B + test_cmp_rev m HEAD~2 + test_linear_range 'x y' m.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root with merge-base does not go to root + reset_rebase + git rebase $* --onto m --root g + test_cmp_rev m HEAD~2 + test_linear_range 'c g' m.. + +} + +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* without --onto --root with disjoint history drops patch in onto + reset_rebase + git rebase $* m B + test_cmp_rev m HEAD~2 + test_linear_range 'x y' m.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --root on linear history is a no-op + reset_rebase + git rebase $* --root c + test_cmp_rev c HEAD + +} +test_run_rebase failure '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* -f --root on linear history causes re-write + reset_rebase + git rebase $* -f --root c + ! test_cmp_rev a HEAD~2 + test_linear_range 'a b c' HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + test_done -- 1.8.2.674.gd17d3d2 -- 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 7/7] tests: move test for rebase messages from t3400 to t3406
t3406 is supposed to test messages from rebase operation, so let's move tests in t3400 that fit that description into 3406. Most of the functionality they tested, except for the messages, has now been subsumed by t3420. --- t/t3400-rebase.sh | 22 -- t/t3406-rebase-message.sh | 22 ++ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b436ef4..45a55e9 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -59,28 +59,6 @@ test_expect_success 'rebase against master' ' git rebase master ' -test_expect_success 'rebase against master twice' ' - git rebase master out - test_i18ngrep Current branch my-topic-branch is up to date out -' - -test_expect_success 'rebase against master twice with --force' ' - git rebase --force-rebase master out - test_i18ngrep Current branch my-topic-branch is up to date, rebase forced out -' - -test_expect_success 'rebase against master twice from another branch' ' - git checkout my-topic-branch^ - git rebase master my-topic-branch out - test_i18ngrep Current branch my-topic-branch is up to date out -' - -test_expect_success 'rebase fast-forward to master' ' - git checkout my-topic-branch^ - git rebase my-topic-branch out - test_i18ngrep Fast-forwarded HEAD to my-topic-branch out -' - test_expect_success 'the rebase operation should not have destroyed author information' ' ! (git log | grep Author: | grep ) ' diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh index fe8c27f..0392e36 100755 --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -30,6 +30,28 @@ test_expect_success 'rebase -m' ' test_cmp expect actual ' +test_expect_success 'rebase against master twice' ' + git rebase master out + test_i18ngrep Current branch topic is up to date out +' + +test_expect_success 'rebase against master twice with --force' ' + git rebase --force-rebase master out + test_i18ngrep Current branch topic is up to date, rebase forced out +' + +test_expect_success 'rebase against master twice from another branch' ' + git checkout topic^ + git rebase master topic out + test_i18ngrep Current branch topic is up to date out +' + +test_expect_success 'rebase fast-forward to master' ' + git checkout topic^ + git rebase topic out + test_i18ngrep Fast-forwarded HEAD to topic out +' + test_expect_success 'rebase --stat' ' git reset --hard start git rebase --stat master diffstat.txt -- 1.8.2.674.gd17d3d2 -- 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 5/7] add tests for rebasing merged history
--- t/t3400-rebase.sh | 31 + t/t3401-rebase-partial.sh | 45 --- t/t3404-rebase-interactive.sh | 10 +- t/t3409-rebase-preserve-merges.sh | 53 t/t3425-rebase-topology-merges.sh | 250 ++ 5 files changed, 252 insertions(+), 137 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3425-rebase-topology-merges.sh diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b58fa1a..b436ef4 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' ' echo Side C git add C git commit -m Add C - git checkout -b nonlinear my-topic-branch - echo Edit B - git add B - git commit -m Modify B - git merge side - git checkout -b upstream-merged-nonlinear - git merge master git checkout -f my-topic-branch git tag topic ' @@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' ' git rebase master ' -test_expect_success 'rebase after merge master' ' - git checkout --detach refs/tags/topic - git branch -D topic - git reset --hard topic - git merge master - git rebase master - ! (git show | grep ^Merge:) -' - -test_expect_success 'rebase of history with merges is linearized' ' - git checkout nonlinear - test 4 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - -test_expect_success 'rebase of history with merges after upstream merge is linearized' ' - git checkout upstream-merged-nonlinear - test 5 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - test_expect_success 'rebase a single mode change' ' git checkout master + git branch -D topic echo 1 X git add X test_tick diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh deleted file mode 100755 index 7ba1797..000 --- a/t/t3401-rebase-partial.sh +++ /dev/null @@ -1,45 +0,0 @@ -#!/bin/sh -# -# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland -# - -test_description='git rebase should detect patches integrated upstream - -This test cherry-picks one local change of two into master branch, and -checks that git rebase succeeds with only the second patch in the -local branch. -' -. ./test-lib.sh - -test_expect_success 'prepare repository with topic branch' ' - test_commit A - git checkout -b my-topic-branch - test_commit B - test_commit C - git checkout -f master - test_commit A2 A.t -' - -test_expect_success 'pick top patch from topic branch into master' ' - git cherry-pick C - git checkout -f my-topic-branch -' - -test_debug ' - git cherry master - git format-patch -k --stdout --full-index master /dev/null - gitk --all sleep 1 -' - -test_expect_success 'rebase topic branch against new master and check git am did not get halted' ' - git rebase master - test_path_is_missing .git/rebase-apply -' - -test_expect_success 'rebase --merge topic branch that was partially merged upstream' ' - git reset --hard C - git rebase --merge master - test_path_is_missing .git/rebase-merge -' - -test_done diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index a58406d..ffcaf02 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' ' test $one = $(git rev-parse HEAD~2) ' -test_expect_success 'ignore patch if in upstream' ' - HEAD=$(git rev-parse HEAD) - git checkout -b has-cherry-picked HEAD^ +test_expect_success '--continue tries to commit, even for edit' ' echo unrelated file7 git add file7 test_tick git commit -m unrelated change - git cherry-pick $HEAD - EXPECT_COUNT=1 git rebase -i $HEAD - test $HEAD = $(git rev-parse HEAD^) -' - -test_expect_success '--continue tries to commit, even for edit' ' parent=$(git rev-parse HEAD^) test_tick FAKE_LINES=edit 1 git rebase -i HEAD^ diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh index 6de4e22..2e0c364 100755 --- a/t/t3409-rebase-preserve-merges.sh +++ b/t/t3409-rebase-preserve-merges.sh @@ -11,14 +11,6 @@ Run git rebase -p and check that merges are properly carried along GIT_AUTHOR_EMAIL=bogus_email_address export GIT_AUTHOR_EMAIL -# Clone 1 (trivial merge): -# -# A1--A2 -- origin/master -# \ \ -# B1--M -- topic -#\ -# B2 -- origin/topic -# # Clone 2 (conflicting merge): # # A1--A2--B3 -- origin/master @@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL # \--A3
[PATCH v2 2/7] add tests for rebasing with patch-equivalence present
--- t/lib-rebase.sh | 17 t/t3420-rebase-topology-linear.sh | 85 +++ 2 files changed, 102 insertions(+) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 62b3887..16eeb1c 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -80,3 +80,20 @@ reset_rebase () { git reset --hard git clean -f } + +cherry_pick () { + git cherry-pick -n $2 + git commit -m $1 + git tag $1 +} + +revert () { + git revert -n $2 + git commit -m $1 + git tag $1 +} + +make_empty () { + git commit --allow-empty -m $1 + git tag $1 +} diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh index c4b32db..1152491 100755 --- a/t/t3420-rebase-topology-linear.sh +++ b/t/t3420-rebase-topology-linear.sh @@ -75,4 +75,89 @@ test_run_rebase success -m test_run_rebase success -i test_run_rebase success -p +# f +# / +# a---b---c---g---h +# \ +# d---G---i +# +# uppercase = cherry-picked +# h = reverted g +# +# Reverted patches are there for tests to be able to check if a commit +# that introduced the same change as another commit is +# dropped. Without reverted commits, we could get false positives +# because applying the patch succeeds, but simply results in no +# changes. +test_expect_success 'setup of linear history for range selection tests' ' + git checkout c + test_commit g + revert h g + git checkout d + cherry_pick G g + test_commit i + git checkout b + test_commit f +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* drops patches in upstream + reset_rebase + git rebase $* h i + test_cmp_rev h HEAD~2 + test_linear_range 'd i' h.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* can drop last patch if in upstream + reset_rebase + git rebase $* h G + test_cmp_rev h HEAD^ + test_linear_range 'd' h.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto does not lose patches in upstream + reset_rebase + git rebase $* --onto f h i + test_cmp_rev f HEAD~3 + test_linear_range 'd G i' f.. + +} +test_run_rebase failure '' +test_run_rebase success -m +test_run_rebase failure -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto drops patches in onto + reset_rebase + git rebase $* --onto h f i + test_cmp_rev h HEAD~2 + test_linear_range 'd i' h.. + +} +test_run_rebase failure '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + test_done -- 1.8.2.674.gd17d3d2 -- 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/7] Rebase topology test
After way too long, here is finally a new version of the tests I sent at: http://thread.gmane.org/gmane.comp.version-control.git/205796. I have split the test up into two files. They stil take quite some time to run. Martin von Zweigbergk (7): add simple tests of consistency across rebase types add tests for rebasing with patch-equivalence present add tests for rebasing of empty commits add tests for rebasing root add tests for rebasing merged history t3406: modernize style tests: move test for rebase messages from t3400 to t3406 t/lib-rebase.sh | 32 t/t3400-rebase.sh | 53 +- t/t3401-rebase-partial.sh | 69 t/t3404-rebase-interactive.sh | 10 +- t/t3406-rebase-message.sh | 50 +++--- t/t3409-rebase-preserve-merges.sh | 53 -- t/t3420-rebase-topology-linear.sh | 350 ++ t/t3425-rebase-topology-merges.sh | 250 +++ 8 files changed, 664 insertions(+), 203 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3420-rebase-topology-linear.sh create mode 100755 t/t3425-rebase-topology-merges.sh -- 1.8.2.674.gd17d3d2 -- 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 2/7] add tests for rebasing with patch-equivalence present
On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +# f +# / +# a---b---c---g---h +# \ +# d---G---i ... +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto drops patches in onto + reset_rebase + git rebase $* --onto h f i + test_cmp_rev h HEAD~2 + test_linear_range 'd i' h.. Isn't this expectation wrong? The upstream of the rebased branch is f, and it does not contain G. Hence, G should be replayed. Since h is the reversal of g, the state at h is the same as at c, and applying G should succeed (it is the same change as g). Therefore, I think the correct expectation is: test_linear_range 'd G i' h.. Good question! It is really not obvious what the right answer is. Some arguments in favor of dropping 'G': 1. Let's say origin/master points to 'b' when you start the 'd G i' branch. You then send the 'G' patch to Junio who applies it as 'g' (cherry-picking direction is reversed compared to figure, but same effect). You then git pull --rebase when master on origin points to 'h'. Because of the cleverness in 'git pull --rebase', it issues 'git rebase --onto h b i'. In this case it's clearly useful to have the patch dropped. 2. In the test a little before the above one, we instead do 'git rebase --onto f h i' and make sure that the 'G' is _not_ lost. In that case it doesn't matter what's in $branch..$upstream. Do we agree that $branch..$upstream should never matter (instead, $upstream is only used to find merge base with $branch)? Do we also agree that 'git rebase a b' should be identical to 'git rebase --onto a a b'? Because 'git rebase h i' should clearly drop 'G', then so should 'git rebase --onto h h i'. Then, if we agreed that $branch..$upstream doesn't matter, 'git rebase --onto h f i' should behave the same, no? The set of commits to rebase that I was thinking of using was $upstream..$branch, unless equivalent with patch in $branch..$onto. But I'm not very confident about my conclusions above :-) 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 v2 4/7] add tests for rebasing root
On Wed, May 29, 2013 at 12:31 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root with merge-base does not go to root + reset_rebase + git rebase $* --onto m --root g + test_cmp_rev m HEAD~2 + test_linear_range 'c g' m.. Here you check the outcome. There is no explicit check whether the rebase attempted to replay a and b. But that check is implicit: If a or b were attempted to replay, the rebase would have been interrupted with no new changes. Right? Because 'm' is a reverted 'b', I think if it had gone to the root, we would have seen 'b m c g' (I _think_ 'a' would be silently skipped at least in am mode). +test_run_rebase failure -p Just curious: Does the last one fail because the result is not correct or because it does go to the root? Because the result is not correct; it first checks out 'm', but something goes wrong (maybe because 'm' gets written to /rewritten/root?) and it somehow fast-forwards to 'c' (from 'b'?). -- 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 2/7] add tests for rebasing with patch-equivalence present
On Wed, May 29, 2013 at 10:41 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk martinv...@gmail.com wrote: On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +# f +# / +# a---b---c---g---h +# \ +# d---G---i ... +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto drops patches in onto + reset_rebase + git rebase $* --onto h f i + test_cmp_rev h HEAD~2 + test_linear_range 'd i' h.. Isn't this expectation wrong? The upstream of the rebased branch is f, and it does not contain G. Hence, G should be replayed. Since h is the reversal of g, the state at h is the same as at c, and applying G should succeed (it is the same change as g). Therefore, I think the correct expectation is: test_linear_range 'd G i' h.. Good question! It is really not obvious what the right answer is. Some arguments in favor of dropping 'G': I think the answer is obvious; G should not be dropped. Maybe it made sense to drop g in upstream, but d fixes an issue, and it makes sense to apply G on upstream. Well, maybe I was wrong in thinking that dropping 'G' in 'git rebase --onto f h i' is bad. It seems to complicate things a lot, so maybe we should just decide that it's fine to do that (to drop 'G' in that case). Since that's mostly how it has worked forever and no one seems to have reported a problem with it, I'm probably just being paranoid. Thoughts? -- 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 2/7] add tests for rebasing with patch-equivalence present
On Wed, May 29, 2013 at 11:40 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, May 30, 2013 at 1:14 AM, Martin von Zweigbergk martinv...@gmail.com wrote: On Wed, May 29, 2013 at 10:41 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk martinv...@gmail.com wrote: On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +# f +# / +# a---b---c---g---h +# \ +# d---G---i ... +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto drops patches in onto + reset_rebase + git rebase $* --onto h f i + test_cmp_rev h HEAD~2 + test_linear_range 'd i' h.. Isn't this expectation wrong? The upstream of the rebased branch is f, and it does not contain G. Hence, G should be replayed. Since h is the reversal of g, the state at h is the same as at c, and applying G should succeed (it is the same change as g). Therefore, I think the correct expectation is: test_linear_range 'd G i' h.. Good question! It is really not obvious what the right answer is. Some arguments in favor of dropping 'G': I think the answer is obvious; G should not be dropped. Maybe it made sense to drop g in upstream, but d fixes an issue, and it makes sense to apply G on upstream. Well, maybe I was wrong in thinking that dropping 'G' in 'git rebase --onto f h i' is bad. It seems to complicate things a lot, so maybe we should just decide that it's fine to do that (to drop 'G' in that case). Since that's mostly how it has worked forever and no one seems to have reported a problem with it, I'm probably just being paranoid. Thoughts? Huh? I said the opposite; G should *not* be dropped. I suspect you missed that I said 'git rebase --onto f h i', not 'git rebase --onto h f i'. Sorry, I should have pointed that out. -- 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 2/7] add tests for rebasing with patch-equivalence present
On Thu, May 30, 2013 at 5:54 AM, Johannes Sixt j...@kdbg.org wrote: Am 30.05.2013 07:30, schrieb Martin von Zweigbergk: On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +# f +# / +# a---b---c---g---h +# \ +# d---G---i ... +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto drops patches in onto + reset_rebase + git rebase $* --onto h f i + test_cmp_rev h HEAD~2 + test_linear_range 'd i' h.. Isn't this expectation wrong? The upstream of the rebased branch is f, and it does not contain G. Hence, G should be replayed. Since h is the reversal of g, the state at h is the same as at c, and applying G should succeed (it is the same change as g). Therefore, I think the correct expectation is: test_linear_range 'd G i' h.. Good question! It is really not obvious what the right answer is. Some arguments in favor of dropping 'G': 1. Let's say origin/master points to 'b' when you start the 'd G i' branch. You then send the 'G' patch to Junio who applies it as 'g' (cherry-picking direction is reversed compared to figure, but same effect). You then git pull --rebase when master on origin points to 'h'. Because of the cleverness in 'git pull --rebase', it issues 'git rebase --onto h b i'. The reason for this git pull cleverness is to be prepared for rewritten history: b'--c'--g'--h' / a---b \ d---G---i to avoid that b is rebased. Right. It doesn't currently drop 'G' and maybe it shouldn't, so let's leave it as is for now, I would say. 2. In the test a little before the above one, we instead do 'git rebase --onto f h i' and make sure that the 'G' is _not_ lost. In that case it doesn't matter what's in $branch..$upstream. Do we agree that $branch..$upstream should never matter (instead, $upstream is only used to find merge base with $branch)? No, we do not agree. $branch..$upstream should be the set of patches that should be omitted. $branch..$onto should not matter. $onto is only used to specify the destination of the rebased commits. Ok. As I said to Felipe, I'm not sure why I was so convinced that it's bad to lose the patch in 'git rebase --onto f h i'. It can result in lost work, but it seems rare enough that no one has reported it, AFAIK. I'll change those tests in a re-roll, and perhaps I'll drop a few of them. Let me know if you (anyone) disagree. Martin PS. Thanks for a meticulous review! -- 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 5/7] add tests for rebasing merged history
On Wed, May 29, 2013 at 12:57 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +# a---b---c +# \ \ +# d---e \ +#\ \ \ +# n---o---w---v +# \ +# z +#TODO: make all flavors of rebase use --topo-order +test_run_rebase success 'e n o' '' +test_run_rebase success 'e n o' -m +test_run_rebase success 'n o e' -i As test_commit offers predictable timestamps, I think you can work around this discrepancy by generating commits n and o before e. (That is not a solution--just a workaround that depends on the current implementation--because the order in which parents of a merge are listed is unspecified.) I actually liked it as documentation of the current inconsistency and with an explicit TODO. I have addressed the remainder of your comments in this and the next message. Thanks again. -- 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 v3 0/7] Rebase topology test
Patches are now expected to be dropped iff they are on upstream. I've also followed all of Johannes's other suggestions except for the one about topo-order. Martin von Zweigbergk (7): add simple tests of consistency across rebase types add tests for rebasing with patch-equivalence present add tests for rebasing of empty commits add tests for rebasing root add tests for rebasing merged history t3406: modernize style tests: move test for rebase messages from t3400 to t3406 t/lib-rebase.sh | 32 t/t3400-rebase.sh | 53 +- t/t3401-rebase-partial.sh | 69 t/t3404-rebase-interactive.sh | 10 +- t/t3406-rebase-message.sh | 50 +++--- t/t3409-rebase-preserve-merges.sh | 53 -- t/t3420-rebase-topology-linear.sh | 350 ++ t/t3425-rebase-topology-merges.sh | 252 +++ 8 files changed, 666 insertions(+), 203 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3420-rebase-topology-linear.sh create mode 100755 t/t3425-rebase-topology-merges.sh -- 1.8.2.674.gd17d3d2 -- 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 v3 1/7] add simple tests of consistency across rebase types
Helped-by: Johannes Sixt j...@kdbg.org --- t/lib-rebase.sh | 15 t/t3420-rebase-topology-linear.sh | 78 +++ 2 files changed, 93 insertions(+) create mode 100755 t/t3420-rebase-topology-linear.sh diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 6ccf797..62b3887 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -65,3 +65,18 @@ EOF test_set_editor $(pwd)/fake-editor.sh chmod a+x fake-editor.sh } + +# checks that the revisions in $2 represent a linear range with the +# subjects in $1 +test_linear_range () { + ! { git log --format=%p $2 | sane_grep ;} + expected=$1 + set -- $(git log --reverse --format=%s $2) + test $expected = $* +} + +reset_rebase () { + git rebase --abort # may fail; ignore exit code + git reset --hard + git clean -f +} diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh new file mode 100755 index 000..c4b32db --- /dev/null +++ b/t/t3420-rebase-topology-linear.sh @@ -0,0 +1,78 @@ +#!/bin/sh + +test_description='basic rebase topology tests' +. ./test-lib.sh +. $TEST_DIRECTORY/lib-rebase.sh + +# a---b---c +# \ +# d---e +test_expect_success 'setup' ' + test_commit a + test_commit b + test_commit c + git checkout b + test_commit d + test_commit e +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result simple rebase $* + reset_rebase + git rebase $* c e + test_cmp_rev c HEAD~2 + test_linear_range 'd e' c.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* is no-op if upstream is an ancestor + reset_rebase + git rebase $* b e + test_cmp_rev e HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* -f rewrites even if upstream is an ancestor + reset_rebase + git rebase $* -f b e + ! test_cmp_rev e HEAD + test_cmp_rev b HEAD~2 + test_linear_range 'd e' b.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* fast-forwards if an ancestor of upstream + reset_rebase + git rebase $* e b + test_cmp_rev e HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_done -- 1.8.2.674.gd17d3d2 -- 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 v3 3/7] add tests for rebasing of empty commits
--- t/t3401-rebase-partial.sh | 24 t/t3420-rebase-topology-linear.sh | 58 +++ 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh index 58f4823..7ba1797 100755 --- a/t/t3401-rebase-partial.sh +++ b/t/t3401-rebase-partial.sh @@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr test_path_is_missing .git/rebase-merge ' -test_expect_success 'rebase ignores empty commit' ' - git reset --hard A - git commit --allow-empty -m empty - test_commit D - git rebase C - test $(git log --format=%s C..) = D -' - -test_expect_success 'rebase --keep-empty' ' - git reset --hard D - git rebase --keep-empty C - test $(git log --format=%s C..) = D -empty -' - -test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' ' - git reset --hard A - git commit --allow-empty -m also-empty - git rebase --keep-empty D - test $(git log --format=%s A..) = also-empty -D -empty -' - test_done diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh index 75cc476..81e3d59 100755 --- a/t/t3420-rebase-topology-linear.sh +++ b/t/t3420-rebase-topology-linear.sh @@ -160,4 +160,62 @@ test_run_rebase success -m test_run_rebase success -i test_run_rebase success -p +# a---b---c---j! +# \ +# d---k!--l +# +# ! = empty +test_expect_success 'setup of linear history for empty commit tests' ' + git checkout c + make_empty j + git checkout d + make_empty k + test_commit l +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* drops empty commit + reset_rebase + git rebase $* c l + test_cmp_rev c HEAD~2 + test_linear_range 'd l' c.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --keep-empty + reset_rebase + git rebase $* --keep-empty c l + test_cmp_rev c HEAD~3 + test_linear_range 'd k l' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --keep-empty keeps empty even if already in upstream + reset_rebase + git rebase $* --keep-empty j l + test_cmp_rev j HEAD~3 + test_linear_range 'd k l' j.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + test_done -- 1.8.2.674.gd17d3d2 -- 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 v3 2/7] add tests for rebasing with patch-equivalence present
--- t/lib-rebase.sh | 17 t/t3420-rebase-topology-linear.sh | 85 +++ 2 files changed, 102 insertions(+) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 62b3887..16eeb1c 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -80,3 +80,20 @@ reset_rebase () { git reset --hard git clean -f } + +cherry_pick () { + git cherry-pick -n $2 + git commit -m $1 + git tag $1 +} + +revert () { + git revert -n $2 + git commit -m $1 + git tag $1 +} + +make_empty () { + git commit --allow-empty -m $1 + git tag $1 +} diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh index c4b32db..75cc476 100755 --- a/t/t3420-rebase-topology-linear.sh +++ b/t/t3420-rebase-topology-linear.sh @@ -75,4 +75,89 @@ test_run_rebase success -m test_run_rebase success -i test_run_rebase success -p +# f +# / +# a---b---c---g---h +# \ +# d---G---i +# +# uppercase = cherry-picked +# h = reverted g +# +# Reverted patches are there for tests to be able to check if a commit +# that introduced the same change as another commit is +# dropped. Without reverted commits, we could get false positives +# because applying the patch succeeds, but simply results in no +# changes. +test_expect_success 'setup of linear history for range selection tests' ' + git checkout c + test_commit g + revert h g + git checkout d + cherry_pick G g + test_commit i + git checkout b + test_commit f +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* drops patches in upstream + reset_rebase + git rebase $* h i + test_cmp_rev h HEAD~2 + test_linear_range 'd i' h.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* can drop last patch if in upstream + reset_rebase + git rebase $* h G + test_cmp_rev h HEAD^ + test_linear_range 'd' h.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto drops patches in upstream + reset_rebase + git rebase $* --onto f h i + test_cmp_rev f HEAD~2 + test_linear_range 'd i' f.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto does not drop patches in onto + reset_rebase + git rebase $* --onto h f i + test_cmp_rev h HEAD~3 + test_linear_range 'd G i' h.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + test_done -- 1.8.2.674.gd17d3d2 -- 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 v3 7/7] tests: move test for rebase messages from t3400 to t3406
t3406 is supposed to test messages from rebase operation, so let's move tests in t3400 that fit that description into 3406. Most of the functionality they tested, except for the messages, has now been subsumed by t3420. --- t/t3400-rebase.sh | 22 -- t/t3406-rebase-message.sh | 22 ++ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b436ef4..45a55e9 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -59,28 +59,6 @@ test_expect_success 'rebase against master' ' git rebase master ' -test_expect_success 'rebase against master twice' ' - git rebase master out - test_i18ngrep Current branch my-topic-branch is up to date out -' - -test_expect_success 'rebase against master twice with --force' ' - git rebase --force-rebase master out - test_i18ngrep Current branch my-topic-branch is up to date, rebase forced out -' - -test_expect_success 'rebase against master twice from another branch' ' - git checkout my-topic-branch^ - git rebase master my-topic-branch out - test_i18ngrep Current branch my-topic-branch is up to date out -' - -test_expect_success 'rebase fast-forward to master' ' - git checkout my-topic-branch^ - git rebase my-topic-branch out - test_i18ngrep Fast-forwarded HEAD to my-topic-branch out -' - test_expect_success 'the rebase operation should not have destroyed author information' ' ! (git log | grep Author: | grep ) ' diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh index fe8c27f..0392e36 100755 --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -30,6 +30,28 @@ test_expect_success 'rebase -m' ' test_cmp expect actual ' +test_expect_success 'rebase against master twice' ' + git rebase master out + test_i18ngrep Current branch topic is up to date out +' + +test_expect_success 'rebase against master twice with --force' ' + git rebase --force-rebase master out + test_i18ngrep Current branch topic is up to date, rebase forced out +' + +test_expect_success 'rebase against master twice from another branch' ' + git checkout topic^ + git rebase master topic out + test_i18ngrep Current branch topic is up to date out +' + +test_expect_success 'rebase fast-forward to master' ' + git checkout topic^ + git rebase topic out + test_i18ngrep Fast-forwarded HEAD to topic out +' + test_expect_success 'rebase --stat' ' git reset --hard start git rebase --stat master diffstat.txt -- 1.8.2.674.gd17d3d2 -- 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 v3 5/7] add tests for rebasing merged history
--- t/t3400-rebase.sh | 31 + t/t3401-rebase-partial.sh | 45 --- t/t3404-rebase-interactive.sh | 10 +- t/t3409-rebase-preserve-merges.sh | 53 t/t3425-rebase-topology-merges.sh | 252 ++ 5 files changed, 254 insertions(+), 137 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3425-rebase-topology-merges.sh diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b58fa1a..b436ef4 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' ' echo Side C git add C git commit -m Add C - git checkout -b nonlinear my-topic-branch - echo Edit B - git add B - git commit -m Modify B - git merge side - git checkout -b upstream-merged-nonlinear - git merge master git checkout -f my-topic-branch git tag topic ' @@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' ' git rebase master ' -test_expect_success 'rebase after merge master' ' - git checkout --detach refs/tags/topic - git branch -D topic - git reset --hard topic - git merge master - git rebase master - ! (git show | grep ^Merge:) -' - -test_expect_success 'rebase of history with merges is linearized' ' - git checkout nonlinear - test 4 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - -test_expect_success 'rebase of history with merges after upstream merge is linearized' ' - git checkout upstream-merged-nonlinear - test 5 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - test_expect_success 'rebase a single mode change' ' git checkout master + git branch -D topic echo 1 X git add X test_tick diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh deleted file mode 100755 index 7ba1797..000 --- a/t/t3401-rebase-partial.sh +++ /dev/null @@ -1,45 +0,0 @@ -#!/bin/sh -# -# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland -# - -test_description='git rebase should detect patches integrated upstream - -This test cherry-picks one local change of two into master branch, and -checks that git rebase succeeds with only the second patch in the -local branch. -' -. ./test-lib.sh - -test_expect_success 'prepare repository with topic branch' ' - test_commit A - git checkout -b my-topic-branch - test_commit B - test_commit C - git checkout -f master - test_commit A2 A.t -' - -test_expect_success 'pick top patch from topic branch into master' ' - git cherry-pick C - git checkout -f my-topic-branch -' - -test_debug ' - git cherry master - git format-patch -k --stdout --full-index master /dev/null - gitk --all sleep 1 -' - -test_expect_success 'rebase topic branch against new master and check git am did not get halted' ' - git rebase master - test_path_is_missing .git/rebase-apply -' - -test_expect_success 'rebase --merge topic branch that was partially merged upstream' ' - git reset --hard C - git rebase --merge master - test_path_is_missing .git/rebase-merge -' - -test_done diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index a58406d..ffcaf02 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' ' test $one = $(git rev-parse HEAD~2) ' -test_expect_success 'ignore patch if in upstream' ' - HEAD=$(git rev-parse HEAD) - git checkout -b has-cherry-picked HEAD^ +test_expect_success '--continue tries to commit, even for edit' ' echo unrelated file7 git add file7 test_tick git commit -m unrelated change - git cherry-pick $HEAD - EXPECT_COUNT=1 git rebase -i $HEAD - test $HEAD = $(git rev-parse HEAD^) -' - -test_expect_success '--continue tries to commit, even for edit' ' parent=$(git rev-parse HEAD^) test_tick FAKE_LINES=edit 1 git rebase -i HEAD^ diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh index 6de4e22..2e0c364 100755 --- a/t/t3409-rebase-preserve-merges.sh +++ b/t/t3409-rebase-preserve-merges.sh @@ -11,14 +11,6 @@ Run git rebase -p and check that merges are properly carried along GIT_AUTHOR_EMAIL=bogus_email_address export GIT_AUTHOR_EMAIL -# Clone 1 (trivial merge): -# -# A1--A2 -- origin/master -# \ \ -# B1--M -- topic -#\ -# B2 -- origin/topic -# # Clone 2 (conflicting merge): # # A1--A2--B3 -- origin/master @@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL # \--A3
[PATCH v3 4/7] add tests for rebasing root
--- t/t3420-rebase-topology-linear.sh | 129 ++ 1 file changed, 129 insertions(+) diff --git a/t/t3420-rebase-topology-linear.sh b/t/t3420-rebase-topology-linear.sh index 81e3d59..659a7b3 100755 --- a/t/t3420-rebase-topology-linear.sh +++ b/t/t3420-rebase-topology-linear.sh @@ -218,4 +218,133 @@ test_run_rebase failure -m test_run_rebase failure -i test_run_rebase failure -p +# m +# / +# a---b---c---g +# +# x---y---B +# +# uppercase = cherry-picked +# m = reverted b +# +# Reverted patches are there for tests to be able to check if a commit +# that introduced the same change as another commit is +# dropped. Without reverted commits, we could get false positives +# because applying the patch succeeds, but simply results in no +# changes. +test_expect_success 'setup of linear history for test involving root' ' + git checkout b + revert m b + git checkout --orphan disjoint + git rm -rf . + test_commit x + test_commit y + cherry_pick B b +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root + reset_rebase + git rebase $* --onto c --root y + test_cmp_rev c HEAD~2 + test_linear_range 'x y' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* without --onto --root with disjoint history + reset_rebase + git rebase $* c y + test_cmp_rev c HEAD~2 + test_linear_range 'x y' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root drops patch in onto + reset_rebase + git rebase $* --onto m --root B + test_cmp_rev m HEAD~2 + test_linear_range 'x y' m.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root with merge-base does not go to root + reset_rebase + git rebase $* --onto m --root g + test_cmp_rev m HEAD~2 + test_linear_range 'c g' m.. + +} + +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* without --onto --root with disjoint history drops patch in onto + reset_rebase + git rebase $* m B + test_cmp_rev m HEAD~2 + test_linear_range 'x y' m.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --root on linear history is a no-op + reset_rebase + git rebase $* --root c + test_cmp_rev c HEAD + +} +test_run_rebase failure '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* -f --root on linear history causes re-write + reset_rebase + git rebase $* -f --root c + ! test_cmp_rev a HEAD~2 + test_linear_range 'a b c' HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + test_done -- 1.8.2.674.gd17d3d2 -- 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 v3 6/7] t3406: modernize style
Update the following: - Quote 'setup' - Remove blank lines within test case body - Use test_commit instead of custom quick_one - Create branch topic from tag created by test_commit --- t/t3406-rebase-message.sh | 30 +- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh index e6a9a0d..fe8c27f 100755 --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -4,27 +4,17 @@ test_description='messages from rebase operation' . ./test-lib.sh -quick_one () { - echo $1 file$1 - git add file$1 - test_tick - git commit -m $1 -} +test_expect_success 'setup' ' + test_commit O fileO + test_commit X fileX + test_commit A fileA + test_commit B fileB + test_commit Y fileY -test_expect_success setup ' - quick_one O - git branch topic - quick_one X - quick_one A - quick_one B - quick_one Y - - git checkout topic - quick_one A - quick_one B - quick_one Z + git checkout -b topic O + git cherry-pick A B + test_commit Z fileZ git tag start - ' cat expect \EOF @@ -34,12 +24,10 @@ Committed: 0003 Z EOF test_expect_success 'rebase -m' ' - git rebase -m master report sed -n -e /^Already applied: /p \ -e /^Committed: /p report actual test_cmp expect actual - ' test_expect_success 'rebase --stat' ' -- 1.8.2.674.gd17d3d2 -- 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 v4 5/7] add tests for rebasing merged history
--- The reason is that this check is incomplete: test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD Nice catch! This should fix it. I couldn't use the method you suggested because of how test_revision_subjects works (repeated revisions are ignored), but this makes the check stricter anyway. Junio, all the previous patches are unchanged since v3, so I'm not resending them. Thanks. t/t3400-rebase.sh | 31 + t/t3401-rebase-partial.sh | 45 --- t/t3404-rebase-interactive.sh | 10 +- t/t3409-rebase-preserve-merges.sh | 53 t/t3425-rebase-topology-merges.sh | 258 ++ 5 files changed, 260 insertions(+), 137 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3425-rebase-topology-merges.sh diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b58fa1a..b436ef4 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' ' echo Side C git add C git commit -m Add C - git checkout -b nonlinear my-topic-branch - echo Edit B - git add B - git commit -m Modify B - git merge side - git checkout -b upstream-merged-nonlinear - git merge master git checkout -f my-topic-branch git tag topic ' @@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' ' git rebase master ' -test_expect_success 'rebase after merge master' ' - git checkout --detach refs/tags/topic - git branch -D topic - git reset --hard topic - git merge master - git rebase master - ! (git show | grep ^Merge:) -' - -test_expect_success 'rebase of history with merges is linearized' ' - git checkout nonlinear - test 4 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - -test_expect_success 'rebase of history with merges after upstream merge is linearized' ' - git checkout upstream-merged-nonlinear - test 5 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - test_expect_success 'rebase a single mode change' ' git checkout master + git branch -D topic echo 1 X git add X test_tick diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh deleted file mode 100755 index 7ba1797..000 --- a/t/t3401-rebase-partial.sh +++ /dev/null @@ -1,45 +0,0 @@ -#!/bin/sh -# -# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland -# - -test_description='git rebase should detect patches integrated upstream - -This test cherry-picks one local change of two into master branch, and -checks that git rebase succeeds with only the second patch in the -local branch. -' -. ./test-lib.sh - -test_expect_success 'prepare repository with topic branch' ' - test_commit A - git checkout -b my-topic-branch - test_commit B - test_commit C - git checkout -f master - test_commit A2 A.t -' - -test_expect_success 'pick top patch from topic branch into master' ' - git cherry-pick C - git checkout -f my-topic-branch -' - -test_debug ' - git cherry master - git format-patch -k --stdout --full-index master /dev/null - gitk --all sleep 1 -' - -test_expect_success 'rebase topic branch against new master and check git am did not get halted' ' - git rebase master - test_path_is_missing .git/rebase-apply -' - -test_expect_success 'rebase --merge topic branch that was partially merged upstream' ' - git reset --hard C - git rebase --merge master - test_path_is_missing .git/rebase-merge -' - -test_done diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index a58406d..ffcaf02 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' ' test $one = $(git rev-parse HEAD~2) ' -test_expect_success 'ignore patch if in upstream' ' - HEAD=$(git rev-parse HEAD) - git checkout -b has-cherry-picked HEAD^ +test_expect_success '--continue tries to commit, even for edit' ' echo unrelated file7 git add file7 test_tick git commit -m unrelated change - git cherry-pick $HEAD - EXPECT_COUNT=1 git rebase -i $HEAD - test $HEAD = $(git rev-parse HEAD^) -' - -test_expect_success '--continue tries to commit, even for edit' ' parent=$(git rev-parse HEAD^) test_tick FAKE_LINES=edit 1 git rebase -i HEAD^ diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh index 6de4e22..2e0c364 100755 --- a/t/t3409-rebase-preserve-merges.sh +++ b/t/t3409-rebase-preserve-merges.sh @@ -11,14
Re: [PATCH v2 1/7] add simple tests of consistency across rebase types
On Tue, May 28, 2013 at 11:39 PM, Martin von Zweigbergk martinv...@gmail.com wrote: create mode 100755 t/t3420-rebase-topology-linear.sh Just FYI, there's another test case with the same number (t3420-rebase-autostash) in pu. I don't know how you normally handle such cases. -- 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 1/7] add simple tests of consistency across rebase types
On Mon, Jun 3, 2013 at 11:05 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: On Tue, May 28, 2013 at 11:39 PM, Martin von Zweigbergk martinv...@gmail.com wrote: create mode 100755 t/t3420-rebase-topology-linear.sh Just FYI, there's another test case with the same number (t3420-rebase-autostash) in pu. I don't know how you normally handle such cases. Thanks for a heads-up. Usually, the series that appears later on the list yields and finds a unique number. In this case, that's my series. Want a resend or do you want to do it yourself? I'm fine either way, whatever is easiest for you. -- 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 v5 7/7] tests: move test for rebase messages from t3400 to t3406
t3406 is supposed to test messages from rebase operation, so let's move tests in t3400 that fit that description into 3406. Most of the functionality they tested, except for the messages, has now been subsumed by t3420. --- t/t3400-rebase.sh | 22 -- t/t3406-rebase-message.sh | 22 ++ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b436ef4..45a55e9 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -59,28 +59,6 @@ test_expect_success 'rebase against master' ' git rebase master ' -test_expect_success 'rebase against master twice' ' - git rebase master out - test_i18ngrep Current branch my-topic-branch is up to date out -' - -test_expect_success 'rebase against master twice with --force' ' - git rebase --force-rebase master out - test_i18ngrep Current branch my-topic-branch is up to date, rebase forced out -' - -test_expect_success 'rebase against master twice from another branch' ' - git checkout my-topic-branch^ - git rebase master my-topic-branch out - test_i18ngrep Current branch my-topic-branch is up to date out -' - -test_expect_success 'rebase fast-forward to master' ' - git checkout my-topic-branch^ - git rebase my-topic-branch out - test_i18ngrep Fast-forwarded HEAD to my-topic-branch out -' - test_expect_success 'the rebase operation should not have destroyed author information' ' ! (git log | grep Author: | grep ) ' diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh index fe8c27f..0392e36 100755 --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -30,6 +30,28 @@ test_expect_success 'rebase -m' ' test_cmp expect actual ' +test_expect_success 'rebase against master twice' ' + git rebase master out + test_i18ngrep Current branch topic is up to date out +' + +test_expect_success 'rebase against master twice with --force' ' + git rebase --force-rebase master out + test_i18ngrep Current branch topic is up to date, rebase forced out +' + +test_expect_success 'rebase against master twice from another branch' ' + git checkout topic^ + git rebase master topic out + test_i18ngrep Current branch topic is up to date out +' + +test_expect_success 'rebase fast-forward to master' ' + git checkout topic^ + git rebase topic out + test_i18ngrep Fast-forwarded HEAD to topic out +' + test_expect_success 'rebase --stat' ' git reset --hard start git rebase --stat master diffstat.txt -- 1.8.3.497.g83fddbe -- 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 v5 0/7] Rebase topology test
The only change since v4 should be that t3420 was renamed t3421. Martin von Zweigbergk (7): add simple tests of consistency across rebase types add tests for rebasing with patch-equivalence present add tests for rebasing of empty commits add tests for rebasing root add tests for rebasing merged history t3406: modernize style tests: move test for rebase messages from t3400 to t3406 t/lib-rebase.sh | 32 t/t3400-rebase.sh | 53 +- t/t3401-rebase-partial.sh | 69 t/t3404-rebase-interactive.sh | 10 +- t/t3406-rebase-message.sh | 50 +++--- t/t3409-rebase-preserve-merges.sh | 53 -- t/t3421-rebase-topology-linear.sh | 350 ++ t/t3425-rebase-topology-merges.sh | 258 8 files changed, 672 insertions(+), 203 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3421-rebase-topology-linear.sh create mode 100755 t/t3425-rebase-topology-merges.sh -- 1.8.3.497.g83fddbe -- 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 v5 2/7] add tests for rebasing with patch-equivalence present
--- t/lib-rebase.sh | 17 t/t3421-rebase-topology-linear.sh | 85 +++ 2 files changed, 102 insertions(+) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 62b3887..16eeb1c 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -80,3 +80,20 @@ reset_rebase () { git reset --hard git clean -f } + +cherry_pick () { + git cherry-pick -n $2 + git commit -m $1 + git tag $1 +} + +revert () { + git revert -n $2 + git commit -m $1 + git tag $1 +} + +make_empty () { + git commit --allow-empty -m $1 + git tag $1 +} diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index c4b32db..75cc476 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -75,4 +75,89 @@ test_run_rebase success -m test_run_rebase success -i test_run_rebase success -p +# f +# / +# a---b---c---g---h +# \ +# d---G---i +# +# uppercase = cherry-picked +# h = reverted g +# +# Reverted patches are there for tests to be able to check if a commit +# that introduced the same change as another commit is +# dropped. Without reverted commits, we could get false positives +# because applying the patch succeeds, but simply results in no +# changes. +test_expect_success 'setup of linear history for range selection tests' ' + git checkout c + test_commit g + revert h g + git checkout d + cherry_pick G g + test_commit i + git checkout b + test_commit f +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* drops patches in upstream + reset_rebase + git rebase $* h i + test_cmp_rev h HEAD~2 + test_linear_range 'd i' h.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* can drop last patch if in upstream + reset_rebase + git rebase $* h G + test_cmp_rev h HEAD^ + test_linear_range 'd' h.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto drops patches in upstream + reset_rebase + git rebase $* --onto f h i + test_cmp_rev f HEAD~2 + test_linear_range 'd i' f.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto does not drop patches in onto + reset_rebase + git rebase $* --onto h f i + test_cmp_rev h HEAD~3 + test_linear_range 'd G i' h.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + test_done -- 1.8.3.497.g83fddbe -- 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 v5 3/7] add tests for rebasing of empty commits
--- t/t3401-rebase-partial.sh | 24 t/t3421-rebase-topology-linear.sh | 58 +++ 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh index 58f4823..7ba1797 100755 --- a/t/t3401-rebase-partial.sh +++ b/t/t3401-rebase-partial.sh @@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr test_path_is_missing .git/rebase-merge ' -test_expect_success 'rebase ignores empty commit' ' - git reset --hard A - git commit --allow-empty -m empty - test_commit D - git rebase C - test $(git log --format=%s C..) = D -' - -test_expect_success 'rebase --keep-empty' ' - git reset --hard D - git rebase --keep-empty C - test $(git log --format=%s C..) = D -empty -' - -test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' ' - git reset --hard A - git commit --allow-empty -m also-empty - git rebase --keep-empty D - test $(git log --format=%s A..) = also-empty -D -empty -' - test_done diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index 75cc476..81e3d59 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -160,4 +160,62 @@ test_run_rebase success -m test_run_rebase success -i test_run_rebase success -p +# a---b---c---j! +# \ +# d---k!--l +# +# ! = empty +test_expect_success 'setup of linear history for empty commit tests' ' + git checkout c + make_empty j + git checkout d + make_empty k + test_commit l +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* drops empty commit + reset_rebase + git rebase $* c l + test_cmp_rev c HEAD~2 + test_linear_range 'd l' c.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --keep-empty + reset_rebase + git rebase $* --keep-empty c l + test_cmp_rev c HEAD~3 + test_linear_range 'd k l' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --keep-empty keeps empty even if already in upstream + reset_rebase + git rebase $* --keep-empty j l + test_cmp_rev j HEAD~3 + test_linear_range 'd k l' j.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + test_done -- 1.8.3.497.g83fddbe -- 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 v5 6/7] t3406: modernize style
Update the following: - Quote 'setup' - Remove blank lines within test case body - Use test_commit instead of custom quick_one - Create branch topic from tag created by test_commit --- t/t3406-rebase-message.sh | 30 +- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh index e6a9a0d..fe8c27f 100755 --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -4,27 +4,17 @@ test_description='messages from rebase operation' . ./test-lib.sh -quick_one () { - echo $1 file$1 - git add file$1 - test_tick - git commit -m $1 -} +test_expect_success 'setup' ' + test_commit O fileO + test_commit X fileX + test_commit A fileA + test_commit B fileB + test_commit Y fileY -test_expect_success setup ' - quick_one O - git branch topic - quick_one X - quick_one A - quick_one B - quick_one Y - - git checkout topic - quick_one A - quick_one B - quick_one Z + git checkout -b topic O + git cherry-pick A B + test_commit Z fileZ git tag start - ' cat expect \EOF @@ -34,12 +24,10 @@ Committed: 0003 Z EOF test_expect_success 'rebase -m' ' - git rebase -m master report sed -n -e /^Already applied: /p \ -e /^Committed: /p report actual test_cmp expect actual - ' test_expect_success 'rebase --stat' ' -- 1.8.3.497.g83fddbe -- 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 v5 4/7] add tests for rebasing root
--- t/t3421-rebase-topology-linear.sh | 129 ++ 1 file changed, 129 insertions(+) diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index 81e3d59..659a7b3 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -218,4 +218,133 @@ test_run_rebase failure -m test_run_rebase failure -i test_run_rebase failure -p +# m +# / +# a---b---c---g +# +# x---y---B +# +# uppercase = cherry-picked +# m = reverted b +# +# Reverted patches are there for tests to be able to check if a commit +# that introduced the same change as another commit is +# dropped. Without reverted commits, we could get false positives +# because applying the patch succeeds, but simply results in no +# changes. +test_expect_success 'setup of linear history for test involving root' ' + git checkout b + revert m b + git checkout --orphan disjoint + git rm -rf . + test_commit x + test_commit y + cherry_pick B b +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root + reset_rebase + git rebase $* --onto c --root y + test_cmp_rev c HEAD~2 + test_linear_range 'x y' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* without --onto --root with disjoint history + reset_rebase + git rebase $* c y + test_cmp_rev c HEAD~2 + test_linear_range 'x y' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root drops patch in onto + reset_rebase + git rebase $* --onto m --root B + test_cmp_rev m HEAD~2 + test_linear_range 'x y' m.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root with merge-base does not go to root + reset_rebase + git rebase $* --onto m --root g + test_cmp_rev m HEAD~2 + test_linear_range 'c g' m.. + +} + +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* without --onto --root with disjoint history drops patch in onto + reset_rebase + git rebase $* m B + test_cmp_rev m HEAD~2 + test_linear_range 'x y' m.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --root on linear history is a no-op + reset_rebase + git rebase $* --root c + test_cmp_rev c HEAD + +} +test_run_rebase failure '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* -f --root on linear history causes re-write + reset_rebase + git rebase $* -f --root c + ! test_cmp_rev a HEAD~2 + test_linear_range 'a b c' HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + test_done -- 1.8.3.497.g83fddbe -- 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 v5 1/7] add simple tests of consistency across rebase types
Helped-by: Johannes Sixt j...@kdbg.org --- t/lib-rebase.sh | 15 t/t3421-rebase-topology-linear.sh | 78 +++ 2 files changed, 93 insertions(+) create mode 100755 t/t3421-rebase-topology-linear.sh diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 6ccf797..62b3887 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -65,3 +65,18 @@ EOF test_set_editor $(pwd)/fake-editor.sh chmod a+x fake-editor.sh } + +# checks that the revisions in $2 represent a linear range with the +# subjects in $1 +test_linear_range () { + ! { git log --format=%p $2 | sane_grep ;} + expected=$1 + set -- $(git log --reverse --format=%s $2) + test $expected = $* +} + +reset_rebase () { + git rebase --abort # may fail; ignore exit code + git reset --hard + git clean -f +} diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh new file mode 100755 index 000..c4b32db --- /dev/null +++ b/t/t3421-rebase-topology-linear.sh @@ -0,0 +1,78 @@ +#!/bin/sh + +test_description='basic rebase topology tests' +. ./test-lib.sh +. $TEST_DIRECTORY/lib-rebase.sh + +# a---b---c +# \ +# d---e +test_expect_success 'setup' ' + test_commit a + test_commit b + test_commit c + git checkout b + test_commit d + test_commit e +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result simple rebase $* + reset_rebase + git rebase $* c e + test_cmp_rev c HEAD~2 + test_linear_range 'd e' c.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* is no-op if upstream is an ancestor + reset_rebase + git rebase $* b e + test_cmp_rev e HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* -f rewrites even if upstream is an ancestor + reset_rebase + git rebase $* -f b e + ! test_cmp_rev e HEAD + test_cmp_rev b HEAD~2 + test_linear_range 'd e' b.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* fast-forwards if an ancestor of upstream + reset_rebase + git rebase $* e b + test_cmp_rev e HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_done -- 1.8.3.497.g83fddbe -- 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 v5 5/7] add tests for rebasing merged history
--- t/t3400-rebase.sh | 31 + t/t3401-rebase-partial.sh | 45 --- t/t3404-rebase-interactive.sh | 10 +- t/t3409-rebase-preserve-merges.sh | 53 t/t3425-rebase-topology-merges.sh | 258 ++ 5 files changed, 260 insertions(+), 137 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3425-rebase-topology-merges.sh diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b58fa1a..b436ef4 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' ' echo Side C git add C git commit -m Add C - git checkout -b nonlinear my-topic-branch - echo Edit B - git add B - git commit -m Modify B - git merge side - git checkout -b upstream-merged-nonlinear - git merge master git checkout -f my-topic-branch git tag topic ' @@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' ' git rebase master ' -test_expect_success 'rebase after merge master' ' - git checkout --detach refs/tags/topic - git branch -D topic - git reset --hard topic - git merge master - git rebase master - ! (git show | grep ^Merge:) -' - -test_expect_success 'rebase of history with merges is linearized' ' - git checkout nonlinear - test 4 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - -test_expect_success 'rebase of history with merges after upstream merge is linearized' ' - git checkout upstream-merged-nonlinear - test 5 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - test_expect_success 'rebase a single mode change' ' git checkout master + git branch -D topic echo 1 X git add X test_tick diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh deleted file mode 100755 index 7ba1797..000 --- a/t/t3401-rebase-partial.sh +++ /dev/null @@ -1,45 +0,0 @@ -#!/bin/sh -# -# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland -# - -test_description='git rebase should detect patches integrated upstream - -This test cherry-picks one local change of two into master branch, and -checks that git rebase succeeds with only the second patch in the -local branch. -' -. ./test-lib.sh - -test_expect_success 'prepare repository with topic branch' ' - test_commit A - git checkout -b my-topic-branch - test_commit B - test_commit C - git checkout -f master - test_commit A2 A.t -' - -test_expect_success 'pick top patch from topic branch into master' ' - git cherry-pick C - git checkout -f my-topic-branch -' - -test_debug ' - git cherry master - git format-patch -k --stdout --full-index master /dev/null - gitk --all sleep 1 -' - -test_expect_success 'rebase topic branch against new master and check git am did not get halted' ' - git rebase master - test_path_is_missing .git/rebase-apply -' - -test_expect_success 'rebase --merge topic branch that was partially merged upstream' ' - git reset --hard C - git rebase --merge master - test_path_is_missing .git/rebase-merge -' - -test_done diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index a58406d..ffcaf02 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' ' test $one = $(git rev-parse HEAD~2) ' -test_expect_success 'ignore patch if in upstream' ' - HEAD=$(git rev-parse HEAD) - git checkout -b has-cherry-picked HEAD^ +test_expect_success '--continue tries to commit, even for edit' ' echo unrelated file7 git add file7 test_tick git commit -m unrelated change - git cherry-pick $HEAD - EXPECT_COUNT=1 git rebase -i $HEAD - test $HEAD = $(git rev-parse HEAD^) -' - -test_expect_success '--continue tries to commit, even for edit' ' parent=$(git rev-parse HEAD^) test_tick FAKE_LINES=edit 1 git rebase -i HEAD^ diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh index 6de4e22..2e0c364 100755 --- a/t/t3409-rebase-preserve-merges.sh +++ b/t/t3409-rebase-preserve-merges.sh @@ -11,14 +11,6 @@ Run git rebase -p and check that merges are properly carried along GIT_AUTHOR_EMAIL=bogus_email_address export GIT_AUTHOR_EMAIL -# Clone 1 (trivial merge): -# -# A1--A2 -- origin/master -# \ \ -# B1--M -- topic -#\ -# B2 -- origin/topic -# # Clone 2 (conflicting merge): # # A1--A2--B3 -- origin/master @@ -36,16 +28,6 @@ export GIT_AUTHOR_EMAIL # \--A3
Re: [PATCH v5 1/7] add simple tests of consistency across rebase types
On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano gits...@pobox.com wrote: + +# checks that the revisions in $2 represent a linear range with the +# subjects in $1 +test_linear_range () { + ! { git log --format=%p $2 | sane_grep ;} An interesting way to spell: test $(git rev-list --merges $2 | wc -l) = 0 Heh, true. I'll change that. (My version was based on the one in git-rebase.sh, around line 495.) +reset_rebase () { + git rebase --abort # may fail; ignore exit code test_might_fail to catch unusual exit codes? Will change. +# a---b---c +# \ +# d---e +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* fast-forwards if an ancestor of upstream The description is a non-sentence, and while I can tell what it wants to say, I do not have a good suggestion for rephrasing this. Changing description to ... fast-forwards from an ancestor of upstream. This is asking to rebase the history leading to b on top of e, but e already includes everything in b, so it just turns into a no-op of not moving from e. So it is not even a fast-forward. + reset_rebase + git rebase $* e b + test_cmp_rev e HEAD Well, git rebase e b is of course a kind of short form of git checkout b git rebase e. While it's true that the implementation doesn't bother checking out b first, that's just an optimization, but let me know if you meant something else. Thanks. Will wait another day or two for further comments before I send another version. -- 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 v5 1/7] add simple tests of consistency across rebase types
On Mon, Jun 3, 2013 at 11:15 PM, Johannes Sixt j.s...@viscovery.net wrote: Am 6/4/2013 7:14, schrieb Martin von Zweigbergk: On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano gits...@pobox.com wrote: + +# checks that the revisions in $2 represent a linear range with the +# subjects in $1 +test_linear_range () { + ! { git log --format=%p $2 | sane_grep ;} An interesting way to spell: test $(git rev-list --merges $2 | wc -l) = 0 Heh, true. I'll change that. Then I think it would be even better written as revlist_merges=$(git rev-list --merges $2) test -z $revlist_merges so as not to ignore errors in the git invocation (and at least one less fork()). Done. I'll send it out in a day or two. -- 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 v5 5/7] add tests for rebasing merged history
On Tue, Jun 4, 2013 at 10:18 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: --- +#TODO: make all flavors of rebase use --topo-order +test_run_rebase success 'e n o' '' +test_run_rebase success 'e n o' -m +test_run_rebase success 'n o e' -i I do not quite follow this TODO. While I think it would be nice to update rebase so that all variants consider replaying the commits in the same order, in this history you have: +# a---b---c +# \ \ +# d---e \ +#\ \ \ +# n---o---w---v +# \ +# z as long as o comes after n and e is shown before n or after o, which all three expected results satisify, it is in --topo-order, isn't it? True, the TODO was too specific. I intended to get the list of commits to rebase for all kinds of rebase by using 'git rev-list --topo-order', but it doesn't really matter how the order is decided; my goal was just to make it consistent. I'll update the TODOs. +test_expect_success rebase -p re-creates merge from side branch + reset_rebase + git rebase -p z w + test_cmp_rev z HEAD^ + test_cmp_rev w^2 HEAD^2 + Hmm, turning the left one to the right one? +# d---e d---e +#\ \ \ \ +# n---o---w === n---o \ +# \ \ \ +# z z---W If w were a merge of o into e (i.e. w^1 were e not o), what should happen? Would we get the same topology? Yes, it seems like it does yield the same topology. That seems to be what I tested at first. Search for wrong in [1]. I think Johannes's point was that it was not realistic, not that he's against it working in the same way independent of parent order. I don't feel strongly on whether to include a test for each direction. Unless others do, I guess I'll leave it as is. (But I did add a test case just now to see, so it's very little work for me if someone does want it included.) In other words, when asked to replay w on top of z, how would we decide which parent to keep (in this case, e is kept)? Keep any parent that is not an ancestor of the new base? Or something like that. [1] http://thread.gmane.org/gmane.comp.version-control.git/205796/focus=205806 -- 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 v6 3/7] add tests for rebasing of empty commits
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- t/t3401-rebase-partial.sh | 24 t/t3421-rebase-topology-linear.sh | 58 +++ 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh index 58f4823..7ba1797 100755 --- a/t/t3401-rebase-partial.sh +++ b/t/t3401-rebase-partial.sh @@ -42,28 +42,4 @@ test_expect_success 'rebase --merge topic branch that was partially merged upstr test_path_is_missing .git/rebase-merge ' -test_expect_success 'rebase ignores empty commit' ' - git reset --hard A - git commit --allow-empty -m empty - test_commit D - git rebase C - test $(git log --format=%s C..) = D -' - -test_expect_success 'rebase --keep-empty' ' - git reset --hard D - git rebase --keep-empty C - test $(git log --format=%s C..) = D -empty -' - -test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' ' - git reset --hard A - git commit --allow-empty -m also-empty - git rebase --keep-empty D - test $(git log --format=%s A..) = also-empty -D -empty -' - test_done diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index ddcbfc6..f19f0d0 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -160,4 +160,62 @@ test_run_rebase success -m test_run_rebase success -i test_run_rebase success -p +# a---b---c---j! +# \ +# d---k!--l +# +# ! = empty +test_expect_success 'setup of linear history for empty commit tests' ' + git checkout c + make_empty j + git checkout d + make_empty k + test_commit l +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* drops empty commit + reset_rebase + git rebase $* c l + test_cmp_rev c HEAD~2 + test_linear_range 'd l' c.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --keep-empty + reset_rebase + git rebase $* --keep-empty c l + test_cmp_rev c HEAD~3 + test_linear_range 'd k l' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --keep-empty keeps empty even if already in upstream + reset_rebase + git rebase $* --keep-empty j l + test_cmp_rev j HEAD~3 + test_linear_range 'd k l' j.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + test_done -- 1.8.3.497.g83fddbe -- 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 v6 2/7] add tests for rebasing with patch-equivalence present
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- t/lib-rebase.sh | 17 t/t3421-rebase-topology-linear.sh | 85 +++ 2 files changed, 102 insertions(+) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 1e0ff28..4b74ae4 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -81,3 +81,20 @@ reset_rebase () { git reset --hard git clean -f } + +cherry_pick () { + git cherry-pick -n $2 + git commit -m $1 + git tag $1 +} + +revert () { + git revert -n $2 + git commit -m $1 + git tag $1 +} + +make_empty () { + git commit --allow-empty -m $1 + git tag $1 +} diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index 60365d1..ddcbfc6 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -75,4 +75,89 @@ test_run_rebase success -m test_run_rebase success -i test_run_rebase success -p +# f +# / +# a---b---c---g---h +# \ +# d---G---i +# +# uppercase = cherry-picked +# h = reverted g +# +# Reverted patches are there for tests to be able to check if a commit +# that introduced the same change as another commit is +# dropped. Without reverted commits, we could get false positives +# because applying the patch succeeds, but simply results in no +# changes. +test_expect_success 'setup of linear history for range selection tests' ' + git checkout c + test_commit g + revert h g + git checkout d + cherry_pick G g + test_commit i + git checkout b + test_commit f +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* drops patches in upstream + reset_rebase + git rebase $* h i + test_cmp_rev h HEAD~2 + test_linear_range 'd i' h.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* can drop last patch if in upstream + reset_rebase + git rebase $* h G + test_cmp_rev h HEAD^ + test_linear_range 'd' h.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto drops patches in upstream + reset_rebase + git rebase $* --onto f h i + test_cmp_rev f HEAD~2 + test_linear_range 'd i' f.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto does not drop patches in onto + reset_rebase + git rebase $* --onto h f i + test_cmp_rev h HEAD~3 + test_linear_range 'd G i' h.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + test_done -- 1.8.3.497.g83fddbe -- 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 v6 1/7] add simple tests of consistency across rebase types
Helped-by: Johannes Sixt j...@kdbg.org Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- t/lib-rebase.sh | 16 t/t3421-rebase-topology-linear.sh | 78 +++ 2 files changed, 94 insertions(+) create mode 100755 t/t3421-rebase-topology-linear.sh diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 6ccf797..1e0ff28 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -65,3 +65,19 @@ EOF test_set_editor $(pwd)/fake-editor.sh chmod a+x fake-editor.sh } + +# checks that the revisions in $2 represent a linear range with the +# subjects in $1 +test_linear_range () { + revlist_merges=$(git rev-list --merges $2) + test -z $revlist_merges + expected=$1 + set -- $(git log --reverse --format=%s $2) + test $expected = $* +} + +reset_rebase () { + test_might_fail git rebase --abort + git reset --hard + git clean -f +} diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh new file mode 100755 index 000..60365d1 --- /dev/null +++ b/t/t3421-rebase-topology-linear.sh @@ -0,0 +1,78 @@ +#!/bin/sh + +test_description='basic rebase topology tests' +. ./test-lib.sh +. $TEST_DIRECTORY/lib-rebase.sh + +# a---b---c +# \ +# d---e +test_expect_success 'setup' ' + test_commit a + test_commit b + test_commit c + git checkout b + test_commit d + test_commit e +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result simple rebase $* + reset_rebase + git rebase $* c e + test_cmp_rev c HEAD~2 + test_linear_range 'd e' c.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* is no-op if upstream is an ancestor + reset_rebase + git rebase $* b e + test_cmp_rev e HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* -f rewrites even if upstream is an ancestor + reset_rebase + git rebase $* -f b e + ! test_cmp_rev e HEAD + test_cmp_rev b HEAD~2 + test_linear_range 'd e' b.. + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* fast-forwards from ancestor of upstream + reset_rebase + git rebase $* e b + test_cmp_rev e HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + +test_done -- 1.8.3.497.g83fddbe -- 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 v6 7/7] tests: move test for rebase messages from t3400 to t3406
t3406 is supposed to test messages from rebase operation, so let's move tests in t3400 that fit that description into 3406. Most of the functionality they tested, except for the messages, has now been subsumed by t3420. Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- t/t3400-rebase.sh | 22 -- t/t3406-rebase-message.sh | 22 ++ 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b436ef4..45a55e9 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -59,28 +59,6 @@ test_expect_success 'rebase against master' ' git rebase master ' -test_expect_success 'rebase against master twice' ' - git rebase master out - test_i18ngrep Current branch my-topic-branch is up to date out -' - -test_expect_success 'rebase against master twice with --force' ' - git rebase --force-rebase master out - test_i18ngrep Current branch my-topic-branch is up to date, rebase forced out -' - -test_expect_success 'rebase against master twice from another branch' ' - git checkout my-topic-branch^ - git rebase master my-topic-branch out - test_i18ngrep Current branch my-topic-branch is up to date out -' - -test_expect_success 'rebase fast-forward to master' ' - git checkout my-topic-branch^ - git rebase my-topic-branch out - test_i18ngrep Fast-forwarded HEAD to my-topic-branch out -' - test_expect_success 'the rebase operation should not have destroyed author information' ' ! (git log | grep Author: | grep ) ' diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh index fe8c27f..0392e36 100755 --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -30,6 +30,28 @@ test_expect_success 'rebase -m' ' test_cmp expect actual ' +test_expect_success 'rebase against master twice' ' + git rebase master out + test_i18ngrep Current branch topic is up to date out +' + +test_expect_success 'rebase against master twice with --force' ' + git rebase --force-rebase master out + test_i18ngrep Current branch topic is up to date, rebase forced out +' + +test_expect_success 'rebase against master twice from another branch' ' + git checkout topic^ + git rebase master topic out + test_i18ngrep Current branch topic is up to date out +' + +test_expect_success 'rebase fast-forward to master' ' + git checkout topic^ + git rebase topic out + test_i18ngrep Fast-forwarded HEAD to topic out +' + test_expect_success 'rebase --stat' ' git reset --hard start git rebase --stat master diffstat.txt -- 1.8.3.497.g83fddbe -- 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 v6 4/7] add tests for rebasing root
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- t/t3421-rebase-topology-linear.sh | 129 ++ 1 file changed, 129 insertions(+) diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index f19f0d0..e67add6 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -218,4 +218,133 @@ test_run_rebase failure -m test_run_rebase failure -i test_run_rebase failure -p +# m +# / +# a---b---c---g +# +# x---y---B +# +# uppercase = cherry-picked +# m = reverted b +# +# Reverted patches are there for tests to be able to check if a commit +# that introduced the same change as another commit is +# dropped. Without reverted commits, we could get false positives +# because applying the patch succeeds, but simply results in no +# changes. +test_expect_success 'setup of linear history for test involving root' ' + git checkout b + revert m b + git checkout --orphan disjoint + git rm -rf . + test_commit x + test_commit y + cherry_pick B b +' + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root + reset_rebase + git rebase $* --onto c --root y + test_cmp_rev c HEAD~2 + test_linear_range 'x y' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* without --onto --root with disjoint history + reset_rebase + git rebase $* c y + test_cmp_rev c HEAD~2 + test_linear_range 'x y' c.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root drops patch in onto + reset_rebase + git rebase $* --onto m --root B + test_cmp_rev m HEAD~2 + test_linear_range 'x y' m.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase success -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root with merge-base does not go to root + reset_rebase + git rebase $* --onto m --root g + test_cmp_rev m HEAD~2 + test_linear_range 'c g' m.. + +} + +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* without --onto --root with disjoint history drops patch in onto + reset_rebase + git rebase $* m B + test_cmp_rev m HEAD~2 + test_linear_range 'x y' m.. + +} +test_run_rebase success '' +test_run_rebase failure -m +test_run_rebase success -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --root on linear history is a no-op + reset_rebase + git rebase $* --root c + test_cmp_rev c HEAD + +} +test_run_rebase failure '' +test_run_rebase failure -m +test_run_rebase failure -i +test_run_rebase failure -p + +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* -f --root on linear history causes re-write + reset_rebase + git rebase $* -f --root c + ! test_cmp_rev a HEAD~2 + test_linear_range 'a b c' HEAD + +} +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase success -p + test_done -- 1.8.3.497.g83fddbe -- 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 v6 6/7] t3406: modernize style
Update the following: - Quote 'setup' - Remove blank lines within test case body - Use test_commit instead of custom quick_one - Create branch topic from tag created by test_commit Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- t/t3406-rebase-message.sh | 30 +- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh index e6a9a0d..fe8c27f 100755 --- a/t/t3406-rebase-message.sh +++ b/t/t3406-rebase-message.sh @@ -4,27 +4,17 @@ test_description='messages from rebase operation' . ./test-lib.sh -quick_one () { - echo $1 file$1 - git add file$1 - test_tick - git commit -m $1 -} +test_expect_success 'setup' ' + test_commit O fileO + test_commit X fileX + test_commit A fileA + test_commit B fileB + test_commit Y fileY -test_expect_success setup ' - quick_one O - git branch topic - quick_one X - quick_one A - quick_one B - quick_one Y - - git checkout topic - quick_one A - quick_one B - quick_one Z + git checkout -b topic O + git cherry-pick A B + test_commit Z fileZ git tag start - ' cat expect \EOF @@ -34,12 +24,10 @@ Committed: 0003 Z EOF test_expect_success 'rebase -m' ' - git rebase -m master report sed -n -e /^Already applied: /p \ -e /^Committed: /p report actual test_cmp expect actual - ' test_expect_success 'rebase --stat' ' -- 1.8.3.497.g83fddbe -- 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 v6 0/8] Rebase topology test
Changes since v5: * Improved test_linear_range * Changed TODOs to be about consistency, not --topo-order Martin von Zweigbergk (7): add simple tests of consistency across rebase types add tests for rebasing with patch-equivalence present add tests for rebasing of empty commits add tests for rebasing root add tests for rebasing merged history t3406: modernize style tests: move test for rebase messages from t3400 to t3406 t/lib-rebase.sh | 33 t/t3400-rebase.sh | 53 +- t/t3401-rebase-partial.sh | 69 t/t3404-rebase-interactive.sh | 10 +- t/t3406-rebase-message.sh | 50 +++--- t/t3409-rebase-preserve-merges.sh | 53 -- t/t3421-rebase-topology-linear.sh | 350 ++ t/t3425-rebase-topology-merges.sh | 258 8 files changed, 673 insertions(+), 203 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3421-rebase-topology-linear.sh create mode 100755 t/t3425-rebase-topology-merges.sh -- 1.8.3.497.g83fddbe -- 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 v6 5/7] add tests for rebasing merged history
Signed-off-by: Martin von Zweigbergk martinv...@gmail.com --- t/t3400-rebase.sh | 31 + t/t3401-rebase-partial.sh | 45 --- t/t3404-rebase-interactive.sh | 10 +- t/t3409-rebase-preserve-merges.sh | 53 t/t3425-rebase-topology-merges.sh | 258 ++ 5 files changed, 260 insertions(+), 137 deletions(-) delete mode 100755 t/t3401-rebase-partial.sh create mode 100755 t/t3425-rebase-topology-merges.sh diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index b58fa1a..b436ef4 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -40,13 +40,6 @@ test_expect_success 'prepare repository with topic branches' ' echo Side C git add C git commit -m Add C - git checkout -b nonlinear my-topic-branch - echo Edit B - git add B - git commit -m Modify B - git merge side - git checkout -b upstream-merged-nonlinear - git merge master git checkout -f my-topic-branch git tag topic ' @@ -106,31 +99,9 @@ test_expect_success 'rebase from ambiguous branch name' ' git rebase master ' -test_expect_success 'rebase after merge master' ' - git checkout --detach refs/tags/topic - git branch -D topic - git reset --hard topic - git merge master - git rebase master - ! (git show | grep ^Merge:) -' - -test_expect_success 'rebase of history with merges is linearized' ' - git checkout nonlinear - test 4 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - -test_expect_success 'rebase of history with merges after upstream merge is linearized' ' - git checkout upstream-merged-nonlinear - test 5 = $(git rev-list master.. | wc -l) - git rebase master - test 3 = $(git rev-list master.. | wc -l) -' - test_expect_success 'rebase a single mode change' ' git checkout master + git branch -D topic echo 1 X git add X test_tick diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh deleted file mode 100755 index 7ba1797..000 --- a/t/t3401-rebase-partial.sh +++ /dev/null @@ -1,45 +0,0 @@ -#!/bin/sh -# -# Copyright (c) 2006 Yann Dirson, based on t3400 by Amos Waterland -# - -test_description='git rebase should detect patches integrated upstream - -This test cherry-picks one local change of two into master branch, and -checks that git rebase succeeds with only the second patch in the -local branch. -' -. ./test-lib.sh - -test_expect_success 'prepare repository with topic branch' ' - test_commit A - git checkout -b my-topic-branch - test_commit B - test_commit C - git checkout -f master - test_commit A2 A.t -' - -test_expect_success 'pick top patch from topic branch into master' ' - git cherry-pick C - git checkout -f my-topic-branch -' - -test_debug ' - git cherry master - git format-patch -k --stdout --full-index master /dev/null - gitk --all sleep 1 -' - -test_expect_success 'rebase topic branch against new master and check git am did not get halted' ' - git rebase master - test_path_is_missing .git/rebase-apply -' - -test_expect_success 'rebase --merge topic branch that was partially merged upstream' ' - git reset --hard C - git rebase --merge master - test_path_is_missing .git/rebase-merge -' - -test_done diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index a58406d..ffcaf02 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -477,19 +477,11 @@ test_expect_success 'interrupted squash works as expected (case 2)' ' test $one = $(git rev-parse HEAD~2) ' -test_expect_success 'ignore patch if in upstream' ' - HEAD=$(git rev-parse HEAD) - git checkout -b has-cherry-picked HEAD^ +test_expect_success '--continue tries to commit, even for edit' ' echo unrelated file7 git add file7 test_tick git commit -m unrelated change - git cherry-pick $HEAD - EXPECT_COUNT=1 git rebase -i $HEAD - test $HEAD = $(git rev-parse HEAD^) -' - -test_expect_success '--continue tries to commit, even for edit' ' parent=$(git rev-parse HEAD^) test_tick FAKE_LINES=edit 1 git rebase -i HEAD^ diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh index 6de4e22..2e0c364 100755 --- a/t/t3409-rebase-preserve-merges.sh +++ b/t/t3409-rebase-preserve-merges.sh @@ -11,14 +11,6 @@ Run git rebase -p and check that merges are properly carried along GIT_AUTHOR_EMAIL=bogus_email_address export GIT_AUTHOR_EMAIL -# Clone 1 (trivial merge): -# -# A1--A2 -- origin/master -# \ \ -# B1--M -- topic -#\ -# B2 -- origin/topic -# # Clone 2 (conflicting merge): # # A1--A2--B3 -- origin/master
Re: Bad attitudes and problems in the Git community (was: Re: [PATCH 2/2] Move sequencer to builtin)
On Mon, Jun 10, 2013 at 9:58 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Mon, Jun 10, 2013 at 4:05 AM, Stefano Lattarini stefano.lattar...@gmail.com wrote: You need two sides to have an argument. I disagree. Unless you mean than, whenever a part behaves in a hostile and aggressive way, the other part should just silently knuckle under. You are wrong. If a bum in the street starts talking about you about why you are going to hell, and you reply to him and argue. Who has the fault of starting an argument? I'm not sure I follow the analogy. Are you the bum or the passer-by? Sorry, couldn't help 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: Bad attitudes and problems in the Git community (was: Re: [PATCH 2/2] Move sequencer to builtin)
Yes, sorry. I find this whole story quite amusing (albeit distracting and unnecessary), but sorry for adding to the spam. I'll be quiet now. On Mon, Jun 10, 2013 at 11:33 AM, Martin Langhoff martin.langh...@gmail.com wrote: On Mon, Jun 10, 2013 at 2:11 PM, Martin von Zweigbergk martinv...@gmail.com wrote: On Mon, Jun 10, 2013 at 9:58 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Mon, Jun 10, 2013 at 4:05 AM, Stefano Lattarini stefano.lattar...@gmail.com wrote: You need two sides to have an argument. I disagree. Unless you mean than, whenever a part behaves in a hostile and aggressive way, the other part should just silently knuckle under. You are wrong. If a bum in the street starts talking about you about why you are going to hell, and you reply to him and argue. Who has the fault of starting an argument? I'm not sure I follow the analogy. Are you the bum or the passer-by? http://xkcd.com/386/ Someone is wrong on the Internet! Let it be. m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- 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 v4 45/45] tests: update topology tests
On Sun, Jun 9, 2013 at 9:40 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- t/t3425-rebase-topology-merges.sh | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh index 5400a05..96cc479 100755 --- a/t/t3425-rebase-topology-merges.sh +++ b/t/t3425-rebase-topology-merges.sh @@ -70,9 +70,8 @@ test_run_rebase () { test_linear_range \'$expected\' d.. } -#TODO: make order consistent across all flavors of rebase -test_run_rebase success 'e n o' '' -test_run_rebase success 'e n o' -m +test_run_rebase success 'n o e' '' +test_run_rebase success 'n o e' -m test_run_rebase success 'n o e' -i If you do end up re-sending the series on top of my series, I'd prefer to see the end result having the first argument inlined, so these few lines become simply: test_run_rebase success '' test_run_rebase success -m test_run_rebase success -i -- 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] reset: trivial refactoring
On Thu, Jun 13, 2013 at 11:15 AM, Felipe Contreras felipe.contre...@gmail.com wrote: @@ -82,7 +82,7 @@ static int reset_index(const unsigned char *sha1, int reset_type, int quiet) if (unpack_trees(nr, desc, opts)) return -1; - if (reset_type == MIXED || reset_type == HARD) { + if (reset_type == HARD) { Are you sure that this can not be reached given that... @@ -323,8 +323,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix) struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); int newfd = hold_locked_index(lock, 1); if (reset_type == MIXED) { + int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN; if (read_from_tree(pathspec, sha1)) return 1; + refresh_index(the_index, flags, NULL, NULL, + _(Unstaged changes after reset:)); } else { int err = reset_index(sha1, reset_type, quiet); if (reset_type == KEEP !err) ...the line after this one reads err = reset_index(sha1, MIXED, quiet); ? I don't know what the consequence of not calling prime_cache_tree() would be, though. The merging of the two if blocks looks good. 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: [PATCH 1/3] rebase: guard against missing files in read_basic_state()
On Thu, Jun 13, 2013 at 3:29 PM, Junio C Hamano gits...@pobox.com wrote: Ramkumar Ramachandra artag...@gmail.com writes: A more troublesome is that nobody seems to check the return value of this function. If head-name, onto or orig-head is missing, is that an error condition that should make the callers of read_basic_state stop and refuse to proceed? Since we unconditionally write those three (and 'quiet'), it seems reasonable to require all of them to be there when continuing, so I think you're right that we should fail fast. The way the cascade is used seems to indicate that, but up to the point where it sents $verbose. If and only if head-name, onto, orig-head and quiet can be read in state-dir, verbose in state-dir is checked and only then $verbose is set. Martin, this seems to be from your series around early Feburary 2011. Do you recall why these checks are cascaded this way? I do not offhand think of a good reason. Neither do I. I think the cascading after 'quiet' is just a mistake on my part. The consequences are probably close to none, since if one of earlier commands fail, the other files will probably not be there either. (Not defending it; I'm happy if it gets fixed, e.g. by making it fail fast.) -- 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 mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems
On Tue, Jun 18, 2013 at 12:28 AM, Johannes Sixt j.s...@viscovery.net wrote: The recently introduced tests used uppercase letters to denote cherry-picks of commits having the corresponding lowercase letter names. The helper functions also set up tags with the names of the commits. But this constellation fails on case-insensitive file systems because there cannot be distinct tags with names that differ only in case. Use a less subtle convention for the names of cherry-picked commits. Makes sense, and the patch looks good. Thanks. Knowing that the tests would take their time to complete on Windows, I'm curious just how slow it is. Are we talking minutes? -- 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: detached HEAD before root commit - possible?
On Sun, Jun 23, 2013 at 4:54 PM, Jonathan Nieder jrnie...@gmail.com wrote: In other words, HEAD always either points to an unborn or existing branch or an existing commit. It's not clear to me what it would mean to detach from an unborn branch. I think it should mean that the next commit would be a root commit (of course) and that HEAD would be detached and point to that commit. I find that it helps to think of unborn branches (and unborn detached HEAD) as pointing to some fixed root commit. I wanted an unborn detached HEAD once when working on rebase. Imagine what git rebase --root would do when run on a detached HEAD. It is currently slightly broken because it forces the rebase (i.e. creates a new root commit). This is inconsistent with e.g. git rebase HEAD~10, which won't do anything (assuming linear history). It would have been nice if git rebase --root could be implemented as: 1. create unborn detached HEAD 2. cherry-pick commits 3. set branch (or not, if started from detached HEAD) Instead, what I ended up doing at some point was to create a temporary unborn branch that I deleted after picking the first commit. Anyway, that was just to show another point of view; I'm not suggesting we should implement support for unborn detached HEAD. 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: Operations on unborn branch
On Tue, Nov 27, 2012 at 12:25 PM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: simplify a lot of things (maybe I'm biased because of the things I have happened to work on?) Yes. Do not waste time on it. Yes, no way I would waste time on that; I was mostly just curious. What I might spend time on is to fix cherry-pick. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 2/2] reset: learn to reset on unborn branch
When run on an unborn branch, git reset currently fails with: fatal: Failed to resolve 'HEAD' as a valid ref. Fix this by interpreting it as a reset to the empty tree. If --patch is given, we currently pass the revision specifier, as given on the command line, to interactive_reset(). On an unborn branch, HEAD can of course not be resolved, so we need to pass the sha1 of the empty tree to interactive_reset() as well. This is fine since interactive_reset only needs the parameter to be a treeish and doesn't use it for display purposes. --- Is it correct that interactive_reset does not use the revision specifier for display purposes? Or, worse, that it requires it to be a commit in some cases? I tried it and didn't see any problem. builtin/reset.c| 10 +--- t/t7106-reset-unborn-branch.sh | 52 ++ 2 files changed, 59 insertions(+), 3 deletions(-) create mode 100755 t/t7106-reset-unborn-branch.sh diff --git a/builtin/reset.c b/builtin/reset.c index cec9874..3845225 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -229,7 +229,10 @@ static struct object *lookup_commit_or_tree(const char *rev) { unsigned char sha1[20]; struct commit *commit; struct tree *tree; - if (get_sha1_treeish(rev, sha1)) + if (!strcmp(rev, HEAD) get_sha1(HEAD, sha1)) { + // unborn branch: reset to empty tree + hashcpy(sha1, EMPTY_TREE_SHA1_BIN); + } else if (get_sha1_treeish(rev, sha1)) die(_(Failed to resolve '%s' as a valid ref.), rev); commit = lookup_commit_reference_gently(sha1, 1); if (commit) @@ -292,7 +295,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix) * Otherwise, argv[i] could be either rev or paths and * has to be unambiguous. */ - else if (!get_sha1_treeish(argv[i], sha1)) { + else if (!strcmp(argv[i], HEAD) || +!get_sha1_treeish(argv[i], sha1)) { /* * Ok, argv[i] looks like a rev; it should not * be a filename. @@ -315,7 +319,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) if (patch_mode) { if (reset_type != NONE) die(_(--patch is incompatible with --{hard,mixed,soft})); - return interactive_reset(rev, argv + i, prefix); + return interactive_reset(sha1_to_hex(sha1), argv + i, prefix); } /* git reset tree [--] paths... can be used to diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh new file mode 100755 index 000..67d45be --- /dev/null +++ b/t/t7106-reset-unborn-branch.sh @@ -0,0 +1,52 @@ +#!/bin/sh + +test_description='git reset should work on unborn branch' +. ./test-lib.sh + +test_expect_success 'setup' ' + echo a a + echo b b +' + +test_expect_success 'reset' ' + git add a b + git reset + test $(git ls-files) == +' + +test_expect_success 'reset HEAD' ' + rm .git/index + git add a b + git reset HEAD + test $(git ls-files) == +' + +test_expect_success 'reset $file' ' + rm .git/index + git add a b + git reset a + test $(git ls-files) == b +' + +test_expect_success 'reset -p' ' + rm .git/index + git add a + echo y | git reset -p + test $(git ls-files) == +' + +test_expect_success 'reset --soft not allowed' ' + rm .git/index + git add a + test_must_fail git reset --soft +' + +test_expect_success 'reset --hard' ' + rm .git/index + git add a + git reset --hard + test $(git ls-files) == + test_path_is_missing a +' + +test_done -- 1.8.0.1.240.ge8a1f5a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 0/2] Fix git reset on unborn branch
I decided to address this before cherry-pick on unborn branch. RFC mostly because I'm not sure about the user interface. When we have agreed on that, I will add documentation. Martin von Zweigbergk (2): reset: learn to reset to tree reset: learn to reset on unborn branch builtin/reset.c | 73 ++--- t/t1512-rev-parse-disambiguation.sh | 4 -- t/t7102-reset.sh| 26 + t/t7106-reset-unborn-branch.sh | 52 ++ 4 files changed, 122 insertions(+), 33 deletions(-) create mode 100755 t/t7106-reset-unborn-branch.sh -- 1.8.0.1.240.ge8a1f5a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 1/2] reset: learn to reset to tree
In cases where HEAD is not supposed to be updated, there is no reason that git reset should require a commit, a tree should be enough. So make git reset $rev^{tree} work just like git reset $rev, except that the former will not update HEAD (since there is no commit to point it to). Disallow --soft with trees, since that is about updating only HEAD. By not updating HEAD, git reset $rev^{tree} behaves quite like git reset $rev .. One might therefore consider requiring a path when using reset with a tree to make that similarity more obvious. However, a future commit will make git reset work on an unborn branch by interpreting it as git reset $empty_tree and it would seem unintuitive to the user to say git reset . on an unborn branch. Requiring a path would also make git reset --hard $tree disallowed. This commit effectively undoes some of commit 13243c2 (reset: the command takes committish, 2012-07-03). The command line argument is now required to be an unambiguous treeish. --- My implementation of lookup_commit_or_tree looks a little clunky. I'm not very familiar with the API. Suggestions welcome. Why is the HEAD is now at ... message printed only for --hard reset? After all, HEAD is updated for all types of reset not involving paths. builtin/reset.c | 67 + t/t1512-rev-parse-disambiguation.sh | 4 --- t/t7102-reset.sh| 26 ++ 3 files changed, 65 insertions(+), 32 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index 915cc9f..cec9874 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -225,6 +225,21 @@ static void die_if_unmerged_cache(int reset_type) } +static struct object *lookup_commit_or_tree(const char *rev) { + unsigned char sha1[20]; + struct commit *commit; + struct tree *tree; + if (get_sha1_treeish(rev, sha1)) + die(_(Failed to resolve '%s' as a valid ref.), rev); + commit = lookup_commit_reference_gently(sha1, 1); + if (commit) + return (struct object *) commit; + tree = parse_tree_indirect(sha1); + if (tree) + return (struct object *) tree; + die(_(Could not parse object '%s'.), rev); +} + int cmd_reset(int argc, const char **argv, const char *prefix) { int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0; @@ -232,7 +247,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix) const char *rev = HEAD; unsigned char sha1[20], *orig = NULL, sha1_orig[20], *old_orig = NULL, sha1_old_orig[20]; - struct commit *commit; + struct object *object; + struct commit *commit = NULL; struct strbuf msg = STRBUF_INIT; const struct option options[] = { OPT__QUIET(quiet, N_(be quiet, only report errors)), @@ -276,7 +292,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) * Otherwise, argv[i] could be either rev or paths and * has to be unambiguous. */ - else if (!get_sha1_committish(argv[i], sha1)) { + else if (!get_sha1_treeish(argv[i], sha1)) { /* * Ok, argv[i] looks like a rev; it should not * be a filename. @@ -289,19 +305,12 @@ int cmd_reset(int argc, const char **argv, const char *prefix) } } - if (get_sha1_committish(rev, sha1)) - die(_(Failed to resolve '%s' as a valid ref.), rev); - - /* -* NOTE: As git reset $treeish -- $path should be usable on -* any tree-ish, this is not strictly correct. We are not -* moving the HEAD to any commit; we are merely resetting the -* entries in the index to that of a treeish. -*/ - commit = lookup_commit_reference(sha1); - if (!commit) - die(_(Could not parse object '%s'.), rev); - hashcpy(sha1, commit-object.sha1); + object = lookup_commit_or_tree(rev); + if (object-type == OBJ_COMMIT) + commit = (struct commit*) object; + else if (reset_type == SOFT) + die(_(--soft requires a commit, which '%s' is not), rev); + hashcpy(sha1, object-sha1); if (patch_mode) { if (reset_type != NONE) @@ -347,23 +356,25 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die(_(Could not reset index file to revision '%s'.), rev); } - /* Any resets update HEAD to the head being switched to, -* saving the previous head in ORIG_HEAD before. */ - if (!get_sha1(ORIG_HEAD, sha1_old_orig)) - old_orig = sha1_old_orig; - if (!get_sha1(HEAD, sha1_orig)) { - orig = sha1_orig; - set_reflog_message(msg, updating ORIG_HEAD, NULL); - update_ref(msg.buf, ORIG_HEAD, orig,
Re: [RFC/PATCH 1/2] reset: learn to reset to tree
On Thu, Nov 29, 2012 at 10:47 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: In cases where HEAD is not supposed to be updated, there is no reason that git reset should require a commit, a tree should be enough. So make git reset $rev^{tree} work just like git reset $rev, except that the former will not update HEAD (since there is no commit to point it to). That is a horrible design I have to nack, unless you require pathspec. You cannot tell what git reset $sha1 would do without checking the type of the object $sha1 refers to. If you do this only when pathspec is present, then the design is very reasonable. Very good point. Thanks! I now see that git checkout also requires a path when given a tree. So then git reset on an unborn branch would imply git reset $empty_tree -- . instead. And git reset --hard $tree would not be allowed. And the intersection of these -- git reset --hard on and unborn branch -- would also not work. Would the correct fix be to first make git reset --hard -- $path work (*sigh*)? I have never understood why that doesn't (shouldn't) work. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/2] reset: learn to reset to tree
On Thu, Nov 29, 2012 at 11:13 AM, Junio C Hamano gits...@pobox.com wrote: [...]These two commands, reset and checkout, share that the source we grab the blobs out of only need to be a tree and does not have to be a commit, and the only difference between them is where the blobs we grabbed out of that tree go, either only to the index or to both the index and the working tree. Slightly off topic, but another difference (or somehow another aspect of the same difference?) that has tripped me up a few times is that git checkout $rev . only affects added and modified files (in $rev compared to HEAD), but git reset $rev . would also delete deleted files from the index. I suppose this is also a partial answer to your question in another message: What does it even mean, even when you are on an existing commit, to hard reset partially? Perhaps you looking for git checkout $tree -- $path? A more direct answer would be that I would expect git reset --hard $rev -- . to behave like git reset --hard $rev, except that it wouldn't update HEAD. It seems to me that that would be similar to how git reset $rev -- . behaves like git reset $rev, except that it doesn't update HEAD. But reset and checkout with and without paths still confuse me after years of using git, so I wouldn't be surprised if I'm not making any sense. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Operations on unborn branch
On Tue, Nov 27, 2012 at 11:12 PM, Junio C Hamano gits...@pobox.com wrote: You have to special case the edges whichever way you go. [...] If I understand you correctly, you're saying that revision walking would need a different special case. This is the most obvious difference, it seems. git show would also need different special-casing. But rebase wouldn't need --root, diff-tree wouldn't need --root, any operations on an unborn branch would just work (incl reset, cherry-pick). Again, this is hypothetical, so I'll stop the complaining now :-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/2] reset: learn to reset to tree
On Thu, Nov 29, 2012 at 2:00 PM, Martin von Zweigbergk martinv...@gmail.com wrote: Slightly off topic, but another difference (or somehow another aspect of the same difference?) that has tripped me up a few times is that git checkout $rev . only affects added and modified files (in $rev compared to HEAD), but git reset $rev . would also delete deleted files from the index. Actually, what is the reasoning behind this difference? It almost seems like a bug. I think I have just thought it was too obvious to be a bug before, but thinking more about it, I can't see any reason why git checkout $rev should delete files, but git checkout $rev . shouldn't. I hope I'm just hallucinating or missing something. Can someone shed some light on this? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/2] reset: learn to reset to tree
On Sat, Dec 1, 2012 at 1:24 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: On Thu, Nov 29, 2012 at 2:00 PM, Martin von Zweigbergk martinv...@gmail.com wrote: Slightly off topic, but another difference (or somehow another aspect of the same difference?) that has tripped me up a few times is that git checkout $rev . only affects added and modified files... checkout $commit pathspec has always been about ... I suppose the has always been is meant to say that it's hard to change at this point, not that it's more intuitive the way it works..? ...checking out the contents stored in the paths that match the pathspec from the named commit to the index and also o the working tree. I think I have always thought that git checkout $commit $pathspec would replace the section(s) of the tree defined by $pathspec. (I'm using tree in the more general sense here, as I'm understood the index is not stored as a tree.) When pathspec is dir/, it does not match the directory whose name is dir. The pathspec matches the paths that store blobs under that directory. Ah, right. Unlike git reset dir/, IIUC. More importantly, when is it desirable not to delete deleted entries? I find it much easier to imagine uses a git checkout $commit $pathspec that does delete deleted entries. It seems like this must have been discussed in depth before, so feel free to point me to an old thread. If it doesn't seem too strange to you and others if I make git reset --hard [$commit] $pathspec work just like had expected git checkout $commit $pathspec, I might look into that when I get some time. ...The please remove everything in dir/ part is not the job of checkout; of course, you can do it as a separate step (e.g. rm -fr dir/). rm -rf dir/ would of course delete everything in there, including e.g. build artifacts -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/2] reset: learn to reset to tree
On Tue, Dec 4, 2012 at 9:46 PM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: More importantly, when is it desirable not to delete deleted entries? When I am trying to check out contents of Documentation/ directory as of an older edition because we made mistakes updating the files in recent versions, with git checkout v1.9.0 Documentation/, for example. Perhaps somebody had this bright idea of reformatting our docs with = Newer Style = section headers, replacing the underline style, and we found our toolchain depend on the underline style, or something. The new files in the same directory added since v1.9.0 may share the same mistake as the files whose recent such changes I am nuking with this operation, but that does not mean I want to retype the contents of them from scratch; I'd rather keep them around so that I can fix them up by hand. I think I follow, but why, then, would you not have the save problem with hunks *within* files that have been added in the new version? Or is the only change to Documentation/ since the broken commit that a new file has been added? That seems like a rather narrow use case in that case? git checkout -p seems more generally useful (whether that command deleted deleted files or not). It feels like I'm missing something... I would have to say that it is more common; I do not recall a time I wanted to replace everything in a directory (and only there without touching other parts of the tree) with an old version, removing new ones. It has happened a few times for me. I think this usually happens when I realize that I had a better solution for some subsystem (under some path) in some other branch (perhaps from a previous attempt at the same problem) or an in older commit. Knowing that git checkout $rev $path doesn't do what I expect and that git reset --hard $rev $path is not allowed, I think I would usually do git reset $rev $path git checkout $path. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: exit code from git reset
On Sun, Dec 9, 2012 at 3:04 PM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martinv...@gmail.com writes: git reset currently returns 0 (if successful) while git reset $pathspec returns 0 iff the index matches HEAD after resetting (on all paths, not just those matching $pathspec). So in short, you observed that either of them reports with its exit code if the resulting index (not just any subpart, but always the entire thing) matches the HEAD, e.g. do we have change that will be listed on 'will be committed' section in git status output? Sounds like one sane and consistent semantics to me. Heh, true, the behavior according to my description does make sense, it's just that my description was wrong; sorry :-(. What git reset $pathspec returns is not whether HEAD differs from index (as I wrote), but whether worktree differs from index. So git reset and git reset . will return different exit codes if there are changes in the worktree as compared to HEAD before the invocation. I hope that's clearer. -- 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: Proposal: create meaningful aliases for git reset's hard/soft/mixed
On Wed, Nov 23, 2011 at 12:49 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Philippe Vaucher philippe.vauc...@gmail.com writes: Optional: a new mode would be introduced for consistency: --worktree (or maybe --tree): only updates the worktree but not the index That would be an alias for git checkout rev -- path, right? Not quite, in two ways, I think. First, it _would_ update the index, wouldn't it? Second, git checkout rev -- path doesn't delete files that are deleted in rev as compared to head. I'm considering implementing support for an operation that would do what I expected git checkout rev -- path and git reset --hard rev -- path to do. I'm currently planning for it to be exactly git reset --hard rev -- path (which is currently simply not allowed), but perhaps it would be more natural as an option to checkout (--also-deleted or 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