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 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: [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 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" > 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 >> 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 wrote: >> >> >> >> On Fri, Nov 07, 2014 at 09:14:42AM -0800, Junio C Hamano wrote: >> >> >> >> > Jeff King 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
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 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 wrote: >> >> On Fri, Nov 07, 2014 at 09:14:42AM -0800, Junio C Hamano wrote: >> >> > Jeff King 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/PATCH] rebase: use reflog to find common base with upstream
On Sun, Dec 8, 2013 at 12:06 PM, John Keeping wrote:
> Commit 15a147e (rebase: use @{upstream} if no upstream specified,
> 2011-02-09) says:
>
> Make it default to 'git rebase @{upstream}'. That is also what
> 'git pull [--rebase]' defaults to, so it only makes sense that
> 'git rebase' defaults to the same thing.
>
> but that isn't actually the case. Since commit d44e712 (pull: support
> rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually
> chosen the most recent reflog entry which is an ancestor of the current
> branch if it can find one.
In my mind, 'git pull --rebase' does default to @{u}, it just started
interpreting it differently in d44e712, but maybe I'm just being
defensive :-).
In a similar way, I think your patch is about interpreting the
upstream argument differently, not about changing the default upstream
argument. This is why I think "git rebase" and "git rebase
origin/master" should be the same (when origin/master is the
configured upstream).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 16/16] add: avoid yoda conditions
I was recently confused by the yoda condition in this block of code from [1] + for (i = 0; i < revs.nr; i++) + if (&bases->item->object == &revs.commit[i]->object) + break; /* found */ + if (revs.nr <= i) I think I was particularly surprised because it came so soon after the "i < revs.nr". I didn't bother commenting because it seemed too subjective and the code base has tons of these. Something as simple as git grep '[0-9] [<>]' *.c finds a bunch (probably with lots of false positives and negatives). I guess what I'm trying to say is that either we accept them and get used to reading them without being surprised, or we can change a bit more than one at a time perhaps? I understand that this was an occurrence you just happened to run into, and I'm not saying that a patch has to deal with _all_ occurrences. I'm more just wondering if we want mention our position, whatever it is, in CodingGuidelines. Martin [1] http://thread.gmane.org/gmane.comp.version-control.git/236252/focus=236716 On Thu, Oct 31, 2013 at 2:25 AM, Felipe Contreras wrote: > Signed-off-by: Felipe Contreras > --- > builtin/add.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin/add.c b/builtin/add.c > index 226f758..9b30356 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -429,7 +429,7 @@ int cmd_add(int argc, const char **argv, const char > *prefix) > argc--; > argv++; > > - if (0 <= addremove_explicit) > + if (addremove_explicit >= 0) > addremove = addremove_explicit; > else if (take_worktree_changes && ADDREMOVE_DEFAULT) > addremove = 0; /* "-u" was given but not "-A" */ > -- > 1.8.4.2+fc1 > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] merge-base: teach "--fork-point" mode
Thanks for taking care of this! Maybe John or I can finally get the
changes to rebase done after this.
A few comments below. Sorry I didn't find time to review the earlier revisions.
On Fri, Oct 25, 2013 at 2:38 PM, Junio C Hamano wrote:
> +
> +where `origin/master` used to point at commits B3, B2, B1 and now it
> +points at B, and your `topic` branch was stated on top of it back
s/stated/started/
> + bases = get_merge_bases_many(derived, revs.nr, revs.commit, 0);
> +
> + /*
> +* There should be one and only one merge base, when we found
> +* a common ancestor among reflog entries.
> +*/
> + if (!bases || bases->next)
> + return 1;
> +
> + /* And the found one must be one of the reflog entries */
> + for (i = 0; i < revs.nr; i++)
> + if (&bases->item->object == &revs.commit[i]->object)
> + break; /* found */
> + if (revs.nr <= i)
> + return 1; /* not found */
> +
> + printf("%s\n", sha1_to_hex(bases->item->object.sha1));
> + free_commit_list(bases);
> + return 0;
Should free_commit_list also be called in the two "return 1" cases
above? I suppose the process will exit soon after this, but that seems
to be true for all three cases.
> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh
> index f80bba8..4f09db0 100755
> --- a/t/t6010-merge-base.sh
> +++ b/t/t6010-merge-base.sh
> @@ -230,4 +230,31 @@ test_expect_success 'criss-cross merge-base for
> octopus-step' '
> test_cmp expected.sorted actual.sorted
> '
>
> +test_expect_success 'using reflog to find the fork point' '
> + git reset --hard &&
> + git checkout -b base $E &&
> +
> + (
> + for count in 1 2 3 4 5
> + do
> + git commit --allow-empty -m "Base commit #$count" &&
> + git rev-parse HEAD >expect$count &&
> + git checkout -B derived &&
> + git commit --allow-empty -m "Derived #$count" &&
> + git rev-parse HEAD >derived$count &&
> + git checkout base || exit 1
I think this creates a history like
---E---B1--B2--B3--B4--B5 (base)
\ \ \ \ \
D1 D2 D3 D4 D5 (derived)
So I think the following test would pass even if you drop the
--fork-point. Did you mean to create a fan-shaped history by resetting
base to $E on every iteration above?
> + git merge-base --fork-point base $(cat derived$count)
> >actual &&
> + test_cmp expect$count actual || exit 1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re* Bug report: reset -p HEAD
Sorry about the regression and thanks for report and fixes.
On Thu, Oct 24, 2013 at 9:24 PM, Jeff King wrote:
> On Thu, Oct 24, 2013 at 08:40:13PM -0700, Junio C Hamano wrote:
>
>> Maarten de Vries writes:
>>
>> > Some more info: It used to work as intended. Using a bisect shows it
>> > has been broken by commit 166ec2e9.
>>
>> Thanks.
>>
>> A knee-jerk change without thinking what side-effect it has for you
>> to try out.
>>
>> builtin/reset.c | 5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/builtin/reset.c b/builtin/reset.c
>> index f2f9d55..a3088d9 100644
>> --- a/builtin/reset.c
>> +++ b/builtin/reset.c
>> @@ -304,7 +304,10 @@ int cmd_reset(int argc, const char **argv, const char
>> *prefix)
>> if (patch_mode) {
>> if (reset_type != NONE)
>> die(_("--patch is incompatible with
>> --{hard,mixed,soft}"));
>> - return run_add_interactive(sha1_to_hex(sha1), "--patch=reset",
>> &pathspec);
>> + return run_add_interactive(
>> + (unborn || strcmp(rev, "HEAD"))
>> + ? sha1_to_hex(sha1)
>> + : "HEAD", "--patch=reset", &pathspec);
>> }
>
> I think that's the correct fix for the regression. You are restoring
> the original, pre-166ec2e9 behavior for just the HEAD case. I do not
> think add--interactive does any other magic between a symbolic rev and
> its sha1, except for recognizing HEAD specially. However, if you wanted
> to minimize the potential impact of 166ec2e9, you could pass the sha1
> _only_ in the unborn case, like this:
Plus, the end result is more readable, IMHO.
> diff --git a/builtin/reset.c b/builtin/reset.c
> index f2f9d55..bfdd8d3 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -283,6 +283,7 @@ int cmd_reset(int argc, const char **argv, const char
> *prefix)
> if (unborn) {
> /* reset on unborn branch: treat as reset to empty tree */
> hashcpy(sha1, EMPTY_TREE_SHA1_BIN);
> + rev = EMPTY_TREE_SHA1_HEX;
> } else if (!pathspec.nr) {
> struct commit *commit;
> if (get_sha1_committish(rev, sha1))
> @@ -304,7 +305,7 @@ int cmd_reset(int argc, const char **argv, const char
> *prefix)
> if (patch_mode) {
> if (reset_type != NONE)
> die(_("--patch is incompatible with
> --{hard,mixed,soft}"));
> - return run_add_interactive(sha1_to_hex(sha1),
> "--patch=reset", &pathspec);
> + return run_add_interactive(rev, "--patch=reset", &pathspec);
> }
>
> /* git reset tree [--] paths... can be used to
>
> That fixes any possible regression from add--interactive treating the
> two cases differently. On an unborn branch, we will still say "apply
> this hunk" rather than "unstage this hunk". That's not a regression,
> because it simply didn't work before, but it's not ideal. To fix that,
> we need to somehow tell add--interactive "this is HEAD, but use the
> empty tree because it's unborn". I can think of a few simple-ish ways:
>
> 1. Pass the head/not-head flag as a separate option.
>
> 2. Pass HEAD even in the unborn case; teach add--interactive to
> convert an unborn HEAD to the empty tree.
>
> 3. Teach add--interactive to recognize the empty tree sha1 as an
> "unstage" path.
>
> I kind of like (3). At first glance, it is wrong; we will also treat
> "git reset -p $(git hash-object -t tree /dev/null)" as if "HEAD" had
> been passed. But if you are explicitly passing the empty tree like that,
> I think saying "unstage" makes a lot of sense.
Makes sense to me. I'm sure others can implement that much faster than
I can, but I feel a little guilty, so I'm happy to do it if no one
else wants to, as long as we agree this is the way we want to go.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase: use reflog to find common base with upstream
On Mon, Oct 21, 2013 at 4:24 AM, John Keeping wrote:
> On Sun, Oct 20, 2013 at 10:03:29PM -0700, Martin von Zweigbergk wrote:
>> On Wed, Oct 16, 2013 at 11:53 AM, John Keeping wrote:
>> > Commit 15a147e (rebase: use @{upstream} if no upstream specified,
>> > 2011-02-09) says:
>> >
>> > Make it default to 'git rebase @{upstream}'. That is also what
>> > 'git pull [--rebase]' defaults to, so it only makes sense that
>> > 'git rebase' defaults to the same thing.
>> >
>> > but that isn't actually the case. Since commit d44e712 (pull: support
>> > rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually
>> > chosen the most recent reflog entry which is an ancestor of the current
>> > branch if it can find one.
>>
>> It is exactly this inconsistency between "git rebase" and "git pull
>> --rebase" that confused me enough to make me send my first email to
>> this list almost 4 years ago [1], so thanks for working on this! I
>> finished that thread with:
>>
>> Would it make sense to teach "git rebase" the same tricks as "git
>> pull --rebase"?
>>
>> Then it took me a year before I sent a patch not unlike this one [2].
>> To summarize, the patch did not get accepted then because it makes
>> rebase a little slower (or a lot slower in some cases). "git pull
>> --rebase" is of course at least as slow in the same cases, but because
>> it often involves connecting to a remote host, people would probably
>> blame the connection rather than git itself even in those rare (?)
>> cases.
>>
>> I think
>>
>> git merge-base HEAD $(git rev-list -g "$upstream_name")
>>
>> is roughly correct and hopefully fast enough. That can lead to too
>> long a command line, so I was planning on teaching merge-base a
>> --stdin option, but never got around to it.
>
> I'm not sure we should worry about the additional overhead here. In the
> common case, we should hit a common ancestor within the first couple of
> reflog entries; and in the case that will be slow, it's likely that
> there are a lot of differences between the branches so the cherry
> comparison phase will take a while anyway.
Perhaps true. I created a simple commit based on my origin/master@{1}
in git.git, which happened to be 136 commits behind origin/master.
Before (a modified version of) your patch, it took 0.756s to rebase it
(best of 5) and afterwards it took 0.720s.
And in a worse case: The same test with one commit off my
origin/master@{13}, 2910 behind origin/master, shows an increase from
2.75s to 4.04s.
And a degenerate case: I created a test branch (called u) with
1000-entry reflog from the output of "git rev-list --first-parent
origin/master | head -1000 | tac" and created the same simple commit
as before off of the end of this reflog (u@{999}). This ended up 3769
commits behind u@{0} (aka origin/master). In this case it went from
3.43s to 3m32s. Obviously, this was a degenerate case designed to be
slow, but I think it's still worth noting that one can get such O(n^2)
behavior e.g. if one lets a branch get out of sync with an upstream
that's very frequently fetches (I've heard of people running
short-interval cron jobs that fetch from a remote).
I do like the feature, but I'm still concerned about this last case.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase: use reflog to find common base with upstream
On Wed, Oct 16, 2013 at 11:53 AM, John Keeping wrote:
> Commit 15a147e (rebase: use @{upstream} if no upstream specified,
> 2011-02-09) says:
>
> Make it default to 'git rebase @{upstream}'. That is also what
> 'git pull [--rebase]' defaults to, so it only makes sense that
> 'git rebase' defaults to the same thing.
>
> but that isn't actually the case. Since commit d44e712 (pull: support
> rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually
> chosen the most recent reflog entry which is an ancestor of the current
> branch if it can find one.
>
> Change rebase so that it uses the same logic.
>
> Signed-off-by: John Keeping
> ---
> git-rebase.sh | 8
> t/t3400-rebase.sh | 6 --
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 226752f..fd36cf7 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -437,6 +437,14 @@ then
> error_on_missing_default_upstream "rebase" "rebase" \
> "against" "git rebase "
> fi
> + for reflog in $(git rev-list -g "$upstream_name" 2>/dev/null)
> + do
> + if test "$reflog" = "$(git merge-base "$reflog" HEAD)"
> + then
> + upstream_name=$reflog
> + break
> + fi
> + done
> ;;
> *) upstream_name="$1"
> shift
A little later, "onto_name" gets assigned like so:
onto_name=${onto-"$upstream_name"}
So if upstream_name was set above, then onto would get the same value,
which is not what we want, right? It seems like this block of code
should come a bit later.
I also think it not be run only when rebase was run without a given
upstream. If the configured upstream is "origin/master", it seems like
it would be surprising to get different behavior from "git rebase" and
"git rebase origin/master".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase: use reflog to find common base with upstream
On Wed, Oct 16, 2013 at 11:53 AM, John Keeping wrote:
> Commit 15a147e (rebase: use @{upstream} if no upstream specified,
> 2011-02-09) says:
>
> Make it default to 'git rebase @{upstream}'. That is also what
> 'git pull [--rebase]' defaults to, so it only makes sense that
> 'git rebase' defaults to the same thing.
>
> but that isn't actually the case. Since commit d44e712 (pull: support
> rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually
> chosen the most recent reflog entry which is an ancestor of the current
> branch if it can find one.
It is exactly this inconsistency between "git rebase" and "git pull
--rebase" that confused me enough to make me send my first email to
this list almost 4 years ago [1], so thanks for working on this! I
finished that thread with:
Would it make sense to teach "git rebase" the same tricks as "git
pull --rebase"?
Then it took me a year before I sent a patch not unlike this one [2].
To summarize, the patch did not get accepted then because it makes
rebase a little slower (or a lot slower in some cases). "git pull
--rebase" is of course at least as slow in the same cases, but because
it often involves connecting to a remote host, people would probably
blame the connection rather than git itself even in those rare (?)
cases.
I think
git merge-base HEAD $(git rev-list -g "$upstream_name")
is roughly correct and hopefully fast enough. That can lead to too
long a command line, so I was planning on teaching merge-base a
--stdin option, but never got around to it.
Martin
[1] http://thread.gmane.org/gmane.comp.version-control.git/136339
[2] http://thread.gmane.org/gmane.comp.version-control.git/166710
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/13] Use "git merge" instead of "git pull ."
On Sat, Aug 24, 2013 at 9:19 PM, Jonathan Nieder wrote: > Thomas Ackermann wrote: >> --- a/Documentation/user-manual.txt >> +++ b/Documentation/user-manual.txt >> @@ -1784,17 +1784,6 @@ repository that you pulled from. >> <>; 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
Re: detached HEAD before root commit - possible?
On Sun, Jun 23, 2013 at 4:54 PM, Jonathan Nieder 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: [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 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: [PATCH 1/3] rebase: guard against missing files in read_basic_state()
On Thu, Jun 13, 2013 at 3:29 PM, Junio C Hamano wrote: > Ramkumar Ramachandra 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] reset: trivial refactoring
On Thu, Jun 13, 2013 at 11:15 AM, Felipe Contreras
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 v4 45/45] tests: update topology tests
On Sun, Jun 9, 2013 at 9:40 AM, Felipe Contreras
wrote:
> Signed-off-by: Felipe Contreras
> ---
> 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: 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 wrote: > On Mon, Jun 10, 2013 at 2:11 PM, Martin von Zweigbergk > wrote: >> On Mon, Jun 10, 2013 at 9:58 AM, Felipe Contreras >> wrote: >>> On Mon, Jun 10, 2013 at 4:05 AM, Stefano Lattarini >>> 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: 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 wrote: > On Mon, Jun 10, 2013 at 4:05 AM, Stefano Lattarini > 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
[PATCH v6 5/7] add tests for rebasing merged history
Signed-off-by: Martin von Zweigbergk --- 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_C
[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 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
---
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 2/7] add tests for rebasing with patch-equivalence present
Signed-off-by: Martin von Zweigbergk
---
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
Signed-off-by: Martin von Zweigbergk
---
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 --- 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
---
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 3/7] add tests for rebasing of empty commits
Signed-off-by: Martin von Zweigbergk
---
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
Re: [PATCH v5 5/7] add tests for rebasing merged history
On Tue, Jun 4, 2013 at 10:18 AM, Junio C Hamano wrote: > Martin von Zweigbergk 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
Re: [PATCH v5 1/7] add simple tests of consistency across rebase types
On Mon, Jun 3, 2013 at 11:15 PM, Johannes Sixt wrote:
> Am 6/4/2013 7:14, schrieb Martin von Zweigbergk:
>> On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano 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 1/7] add simple tests of consistency across rebase types
On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano 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
[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 -# #
[PATCH v5 1/7] add simple tests of consistency across rebase types
Helped-by: Johannes Sixt
---
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 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 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 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 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 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
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 wrote: > Martin von Zweigbergk writes: > >> On Tue, May 28, 2013 at 11:39 PM, Martin von Zweigbergk >> 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
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 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
[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.s
[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 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 -# #
[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 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 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
---
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
Re: [PATCH v2 5/7] add tests for rebasing merged history
On Wed, May 29, 2013 at 12:57 AM, Johannes Sixt 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
Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present
On Thu, May 30, 2013 at 5:54 AM, Johannes Sixt wrote:
> Am 30.05.2013 07:30, schrieb Martin von Zweigbergk:
>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt 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 2/7] add tests for rebasing with patch-equivalence present
On Wed, May 29, 2013 at 11:40 PM, Felipe Contreras
wrote:
> On Thu, May 30, 2013 at 1:14 AM, Martin von Zweigbergk
> wrote:
>> On Wed, May 29, 2013 at 10:41 PM, Felipe Contreras
>> wrote:
>>> On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk
>>> wrote:
>>>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt
>>>> 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 Wed, May 29, 2013 at 10:41 PM, Felipe Contreras
wrote:
> On Thu, May 30, 2013 at 12:30 AM, Martin von Zweigbergk
> wrote:
>> On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt 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 4/7] add tests for rebasing root
On Wed, May 29, 2013 at 12:31 AM, Johannes Sixt 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 12:09 AM, Johannes Sixt 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
[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
[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 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 -# #
[PATCH v2 1/7] add simple tests of consistency across rebase types
Helped-by: Johannes Sixt
---
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 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 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
Re: [RFC/PATCH v2 3/8] rebase: cherry-pick: fix sequence continuation
:-) On Tue, May 28, 2013 at 11:05 PM, Felipe Contreras wrote: > On Wed, May 29, 2013 at 12:51 AM, Martin von Zweigbergk > wrote: >> On Tue, May 28, 2013 at 10:41 PM, Felipe Contreras >> 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
Re: [RFC/PATCH v2 3/8] rebase: cherry-pick: fix sequence continuation
On Tue, May 28, 2013 at 10:41 PM, Felipe Contreras wrote: > On Wed, May 29, 2013 at 12:33 AM, Martin von Zweigbergk > 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 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 wrote: > We don't use the 'rebase-apply'. > > Signed-off-by: Felipe Contreras > --- > 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 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 wrote: > Signed-off-by: Felipe Contreras > --- > 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 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 wrote: > We are not in am mode. > > Signed-off-by: Felipe Contreras > --- > 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: [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 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: [PATCH 5/7] rebase -i: return control to the caller, for housekeeping
On Tue, Apr 23, 2013 at 7:02 AM, Ramkumar Ramachandra
wrote:
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index cc3a9a7..9514e31 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -597,7 +597,7 @@ do_next () {
> fi
> ;;
> esac
> - test -s "$todo" && return
> + test -s "$todo" && return 1
Unlike the other cases below, this seems to be replacing success by
failure. What does it mean in practice that $todo is empty?
> comment_for_reflog finish &&
> newhead=$(git rev-parse HEAD) &&
> @@ -623,17 +623,15 @@ do_next () {
> "$GIT_DIR"/hooks/post-rewrite rebase < "$rewritten_list"
> true # we don't care if this hook failed
> fi &&
> - rm -rf "$state_dir" &&
> - git gc --auto &&
> warn "Successfully rebased and updated $head_name."
>
> - exit
> + return 0
So after this patch, the "warning" will coming before gc is run. It's
a change, but it seems fine. gc usually only prints a few line, right?
> }
>
> do_rest () {
> while :
> do
> - do_next
> + do_next && break
> done
> }
Normally one would break if unsuccessful. What would fail if this was
replaced by "do_next || break" and the above ".. && return 1" was "..
&& return". I assume that was your first attempt, but why did it not
work?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] rebase -i: don't error out if $state_dir already exists
On Tue, Apr 23, 2013 at 7:01 AM, Ramkumar Ramachandra wrote: > Practically speaking, the only reason why a `mkdir $state_dir` would > fail is because $state_dir already exists. Would we ever get to this point in the code if it already exists? Also, I had the feeling that the check it might fail if the user does not have permissions to create the directory. Is there always something else that will fail first? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] am: suppress error output from a conditional
On Tue, Apr 23, 2013 at 7:01 AM, Ramkumar Ramachandra wrote: > When testing if the $dotest directory exists, and if $next is greater > than $last When can that happen? If one edits the todo? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git checkout --orpan` leaves a dirty worktree
I'm curious what your use case is. The behavior has been inconvenient for me too, but I have only used it in test cases; I have no real use case where I wanted to create an unborn/orphan branch. On Fri, Feb 8, 2013 at 11:50 AM, Ramkumar Ramachandra wrote: > Hi, > > Why should I have to `git rm -rf .` after a `git checkout --orphan`? > What sort of misfeature/ incomplete feature is this? > > Ram > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase --preserve-merges keeps empty merge commits
On Fri, Feb 1, 2013 at 1:05 PM, Phil Hord wrote: > > This is probably right, but it is not exactly the case that caused my itch. > I think my branch looked like [...] That also makes sense. I'll add tests for both cases. Your patch makes both of them pass. >> # a---b---c >> # \ \ >> # d \ >> #\ \ >> # e \ >> # \ \ >> # C---l >> >> As you say, your patch doesn't try to handle this case, but at least >> the new behavior seems better. I think we would ideally want the >> recreated 'l' to have only 'C' as parent in this case. Does that make >> sense? > > This is not what I meant, but it is a very interesting corner case. I > am not sure I have a solid opinion on what the result should be here. Neither do I, so I'll just drop the test case. Thanks. > Here is the corner case I was thinking of. I did not test this to see > if this will happen, but I conceived that it might. Suppose you have > this tree where > > # a---b---c > # \ > # d---g---l > #\ / > # C > > where 'C' introduced the same changes as 'c'. > > When I execute 'git rebase -p l c', I expect that I will end up with > this: > > # a---b---c---d--- > # \ \ > # ---g---l > > That is, 'C' gets skipped because it introduces the same changes already > seen in 'c'. So 'g' now has two parents: 'd' and 'C^'. But 'C^' is 'd', > so 'g' now has two parents, both of whom are 'd'. > > I think it should collapse to this instead: > > # a---b---c---d---g---l I think this is actually what you will get. But I think it will only be linearized if the branch that should be dropped is the second parent. I have two tests for this, but I need to simplify them a little to see that that (parent number) is the only difference. > I hope this is clear, but please let me know if I made it too confusing. Very clear. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase --preserve-merges keeps empty merge commits
I'm working on a re-roll of http://thread.gmane.org/gmane.comp.version-control.git/205796 and finally got around to including test cases for what you fixed in this patch. I want to make sure I'm testing what you fixed here. See questions below. On Sat, Jan 12, 2013 at 12:46 PM, Phil Hord wrote: > Since 90e1818f9a (git-rebase: add keep_empty flag, 2012-04-20) > 'git rebase --preserve-merges' fails to preserve empty merge commits > unless --keep-empty is also specified. Merge commits should be > preserved in order to preserve the structure of the rebased graph, > even if the merge commit does not introduce changes to the parent. > > Teach rebase not to drop merge commits only because they are empty. Consider a history like # a---b---c # \ \ # d---l #\ # e # \ # C where 'l' is tree-same with 'd' and 'C' introduces the same change as 'c'. My test case runs 'git rebase -p e l' and expects the result to look like # a---b---c # \ \ # d \ #\ \ # e---l > A special case which is not handled by this change is for a merge commit > whose parents are now the same commit because all the previous different > parents have been dropped as a result of this rebase or some previous > operation. And for this case, the test case runs 'git rebase -p C l'. Is that what you meant here? Before your patch, git would just say "Nothing to do" and after your patch, we get # a---b---c # \ \ # d \ #\ \ # e \ # \ \ # C---l As you say, your patch doesn't try to handle this case, but at least the new behavior seems better. I think we would ideally want the recreated 'l' to have only 'C' as parent in this case. Does that make sense? Martin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] git-am: record full index line in the patch used while rebasing
On Thu, Jan 31, 2013 at 8:32 PM, Junio C Hamano wrote: > Earlier, a230949 (am --rebasing: get patch body from commit, not > from mailbox, 2012-06-26) learned to regenerate patch body from the > commit object while rebasing, instead of reading from the rebase-am > front-end. While doing so, it used "git diff-tree" but without > giving it the "--full-index" option. > > This does not matter for in-repository objects; during rebasing, any > abbreviated object name should uniquely identify them. > > But we may be rebasing a commit that contains a change to a gitlink, > in which case we usually should not have the object (it names a > commit in the submodule). A full object name is necessary to later > reconstruct a fake ancestor index for them. >From what I can understand, this all makes sense. I didn't notice the emails about the breakage until now, but I wouldn't have known where to even start looking anyway, so thanks a lot for taking care of it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 08/31] parse_pathspec: add PATHSPEC_EMPTY_MATCH_ALL
Hi, I was tempted to ask this before, and the recent thread regarding "add -u/A" [1] convinced me to. On Sun, Jan 13, 2013 at 4:35 AM, Nguyễn Thái Ngọc Duy wrote: > We have two ways of dealing with empty pathspec: > > 1. limit it to current prefix > 2. match the entire working directory > > Some commands go with #1, some with #2. get_pathspec() and > parse_pathspec() only supports #1. Make it support #2 too via > PATHSPEC_EMPTY_MATCH_ALL flag. If #2 is indeed the direction we want to go, then maybe we should make that the default behavior from parse_pathspec()? I.e. rename the flag "PATHSPEC_EMPTY_MATCH_PREFIX" (or something). Makes sense? Btw, Matthieu was asking where we use #1. If you do invert the name and meaning of the flag, then the answer to that question should be (mostly?) obvious from a re-roll of your series (i.e. all the places where PATHSPEC_EMPTY_MATCH_PREFIX is used). Martin [1] http://thread.gmane.org/gmane.comp.version-control.git/213988/focus=214113 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] git rm -u
On Sun, Jan 20, 2013 at 1:27 PM, Junio C Hamano wrote: > Junio C Hamano writes: > >> Matthieu Moy writes: >> >>> "git add -u" is one of the only exceptions (with "git grep"). I consider >>> this as a bug, and think this should be changed. This has been discussed >>> several times here, but no one took the time to actually do the change > > - As we have the "from the root" magic pathspec these days, >requiring "git add -u :/" when the user really means to add >everything is no longer too much of a burden, but if we suddenly >changed "git add -u" to mean "git add -u .", that is too much of >a change in the semantics. And I think someone (Jeff?) pointed out that that last part is even more true for "git clean", which also currently works on the current directory if not told otherwise. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 17/19] fixup! reset $sha1 $pathspec: require $sha1 only to be treeish
On Wed, Jan 16, 2013 at 10:00 AM, Martin von Zweigbergk wrote: > --- > > Sorry, I forgot the documentation updates. I hope this looks ok. Can > you squash this in, Junio? Thanks. I see the series just entered 'next', so I guess it would have to go on top then. Perhaps with a commit message like as simple as the following. Let me know if you prefer it to be resent as a proper patch. Sorry about the noise. reset: update documentation to require only tree-ish with paths When resetting with paths, we no longer require a commit argument, but only a tree-ish. Update the documentation and synopsis accordingly. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 17/19] fixup! reset $sha1 $pathspec: require $sha1 only to be treeish
---
Sorry, I forgot the documentation updates. I hope this looks ok. Can
you squash this in, Junio? Thanks.
I don't think any documentation update is necessary for the "reset on
unborn branch" patch. Let me know if you think differently.
Documentation/git-reset.txt | 18 +-
builtin/reset.c | 4 ++--
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 978d8da..a404b47 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -8,20 +8,20 @@ git-reset - Reset current HEAD to the specified state
SYNOPSIS
[verse]
-'git reset' [-q] [] [--] ...
-'git reset' (--patch | -p) [] [--] [...]
+'git reset' [-q] [] [--] ...
+'git reset' (--patch | -p) [] [--] [...]
'git reset' [--soft | --mixed | --hard | --merge | --keep] [-q] []
DESCRIPTION
---
-In the first and second form, copy entries from to the index.
+In the first and second form, copy entries from to the index.
In the third form, set the current branch head (HEAD) to , optionally
-modifying index and working tree to match. The defaults to HEAD
-in all forms.
+modifying index and working tree to match. The / defaults
+to HEAD in all forms.
-'git reset' [-q] [] [--] ...::
+'git reset' [-q] [] [--] ...::
This form resets the index entries for all to their
- state at . (It does not affect the working tree, nor
+ state at . (It does not affect the working tree, nor
the current branch.)
+
This means that `git reset ` is the opposite of `git add
@@ -34,9 +34,9 @@ Alternatively, using linkgit:git-checkout[1] and specifying a
commit, you
can copy the contents of a path out of a commit to the index and to the
working tree in one go.
-'git reset' (--patch | -p) [] [--] [...]::
+'git reset' (--patch | -p) [] [--] [...]::
Interactively select hunks in the difference between the index
- and (defaults to HEAD). The chosen hunks are applied
+ and (defaults to HEAD). The chosen hunks are applied
in reverse to the index.
+
This means that `git reset -p` is the opposite of `git add -p`, i.e.
diff --git a/builtin/reset.c b/builtin/reset.c
index b776867..cb84f1b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -23,8 +23,8 @@
static const char * const git_reset_usage[] = {
N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q]
[]"),
- N_("git reset [-q] [--] ..."),
- N_("git reset --patch [] [--] [...]"),
+ N_("git reset [-q] [--] ..."),
+ N_("git reset --patch [] [--] [...]"),
NULL
};
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/19] reset.c: remove unnecessary variable 'i'
I suppose this was meant for everyone. Adding back the others. On Tue, Jan 15, 2013 at 10:27 AM, Holding, Lawrence (NZ) wrote: > Maybe use *argv instead of argv[0]? Sure. Everywhere? Also in the lines added in patch 17/19 that refer to both argv[0] and argv[1], such as "argv[1] && !get_sha1_treeish(argv[0], unused)"? Or is this just a sign that I'm making the code _more_ confusing to those who are more familiar with C? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/19] reset.c: remove unnecessary variable 'i'
Throughout most of parse_args(), the variable 'i' remains at 0. Many
references are still made to the variable even when it could only have
the value 0. This made at least me, who has relatively little
experience with C programming styles, think that parts of the function
was meant to be part of a loop. To avoid such confusion, remove the
variable and also the 'argc' parameter and check for NULL trailing
argv instead.
Signed-off-by: Martin von Zweigbergk
---
I explained a bit more why I was confused by the current style, but
I'm also perfectly happy if you just drop the patch (there would be
some minor conflicts in a later patch, though).
builtin/reset.c | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 58f0f61..d89cf4d 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -198,9 +198,8 @@ static void die_if_unmerged_cache(int reset_type)
}
-static const char **parse_args(int argc, const char **argv, const char
*prefix, const char **rev_ret)
+static const char **parse_args(const char **argv, const char *prefix, const
char **rev_ret)
{
- int i = 0;
const char *rev = "HEAD";
unsigned char unused[20];
/*
@@ -211,34 +210,34 @@ static const char **parse_args(int argc, const char
**argv, const char *prefix,
* git reset [-opts] -- ...
* git reset [-opts] ...
*
-* At this point, argv[i] points immediately after [-opts].
+* At this point, argv points immediately after [-opts].
*/
- if (i < argc) {
- if (!strcmp(argv[i], "--")) {
- i++; /* reset to HEAD, possibly with paths */
- } else if (i + 1 < argc && !strcmp(argv[i+1], "--")) {
- rev = argv[i];
- i += 2;
+ if (argv[0]) {
+ if (!strcmp(argv[0], "--")) {
+ argv++; /* reset to HEAD, possibly with paths */
+ } else if (argv[1] && !strcmp(argv[1], "--")) {
+ rev = argv[0];
+ argv += 2;
}
/*
-* Otherwise, argv[i] could be either or and
+* Otherwise, argv[0] could be either or and
* has to be unambiguous.
*/
- else if (!get_sha1_committish(argv[i], unused)) {
+ else if (!get_sha1_committish(argv[0], unused)) {
/*
-* Ok, argv[i] looks like a rev; it should not
+* Ok, argv[0] looks like a rev; it should not
* be a filename.
*/
- verify_non_filename(prefix, argv[i]);
- rev = argv[i++];
+ verify_non_filename(prefix, argv[0]);
+ rev = *argv++;
} else {
/* Otherwise we treat this as a filename */
- verify_filename(prefix, argv[i], 1);
+ verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
- return i < argc ? get_pathspec(prefix, argv + i) : NULL;
+ return argv[0] ? get_pathspec(prefix, argv) : NULL;
}
int cmd_reset(int argc, const char **argv, const char *prefix)
@@ -270,7 +269,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, git_reset_usage,
PARSE_OPT_KEEP_DASHDASH);
- pathspec = parse_args(argc, argv, prefix, &rev);
+ pathspec = parse_args(argv, prefix, &rev);
if (get_sha1_committish(rev, sha1))
die(_("Failed to resolve '%s' as a valid ref."), rev);
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/19] reset.c: share call to die_if_unmerged_cache()
Use a single condition to guard the call to die_if_unmerged_cache for
both --soft and --keep. This avoids the small distraction of the
precondition check from the logic following it.
Also change an instance of
if (e)
err = err || f();
to the almost as short, but clearer
if (e && !err)
err = f();
(which is equivalent since we only care whether exit code is 0)
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 2187d64..4e34195 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -337,15 +337,13 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
/* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order. Other resets reset
* the index file to the tree object we are switching to. */
- if (reset_type == SOFT)
+ if (reset_type == SOFT || reset_type == KEEP)
die_if_unmerged_cache(reset_type);
- else {
- int err;
- if (reset_type == KEEP)
- die_if_unmerged_cache(reset_type);
- err = reset_index_file(sha1, reset_type, quiet);
- if (reset_type == KEEP)
- err = err || reset_index_file(sha1, MIXED, quiet);
+
+ if (reset_type != SOFT) {
+ int err = reset_index_file(sha1, reset_type, quiet);
+ if (reset_type == KEEP && !err)
+ err = reset_index_file(sha1, MIXED, quiet);
if (err)
die(_("Could not reset index file to revision '%s'."),
rev);
}
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 12/19] reset.c: move update_index_refresh() call out of read_from_tree()
The final part of cmd_reset() essentially looks like:
if (pathspec) {
...
read_from_tree(...);
} else {
...
reset_index(...);
update_index_refresh(...);
...
}
where read_from_tree() internally also calls
update_index_refresh(). Move the call to update_index_refresh() out of
read_from_tree for symmetry with the 'else' block, making
read_from_tree() and reset_index() closer in functionality.
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index c3eb2eb..70733c2 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -145,11 +145,8 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
}
}
-static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
- int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1)
{
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd;
struct diff_options opt;
memset(&opt, 0, sizeof(opt));
@@ -157,7 +154,6 @@ static int read_from_tree(const char **pathspec, unsigned
char *tree_sha1,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- index_fd = hold_locked_index(lock, 1);
read_cache();
if (do_diff_cache(tree_sha1, &opt))
return 1;
@@ -165,7 +161,7 @@ static int read_from_tree(const char **pathspec, unsigned
char *tree_sha1,
diff_flush(&opt);
diff_tree_release_paths(&opt);
- return update_index_refresh(index_fd, lock, refresh_flags);
+ return 0;
}
static void set_reflog_message(struct strbuf *sb, const char *action,
@@ -322,9 +318,13 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_("%s reset is not allowed in a bare repository"),
_(reset_type_names[reset_type]));
- if (pathspec)
- return read_from_tree(pathspec, sha1,
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (pathspec) {
+ struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+ int index_fd = hold_locked_index(lock, 1);
+ return read_from_tree(pathspec, sha1) ||
+ update_index_refresh(index_fd, lock,
+ quiet ? REFRESH_QUIET :
REFRESH_IN_PORCELAIN);
+ }
/* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order. Other resets reset
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/19] reset improvements
Changes since v1:
- Spelling fixes.
- Explained how "git reset -- $pathspec" in bare repo is broken.
- Provided motivation for replacement of switch by if-else
- Fixed argv/argc handling by removing use of argc.
- Replaced "don't refresh index on --quiet" patch by one that just
inlines update_index_refresh()
- Incorporated fixes from Junio's repo
- Provided some motivation for "replace switch by if-else" amd moved
the patch later in the series.
Thanks for reviewing!
Martin von Zweigbergk (19):
reset $pathspec: no need to discard index
reset $pathspec: exit with code 0 if successful
reset.c: pass pathspec around instead of (prefix, argv) pair
reset: don't allow "git reset -- $pathspec" in bare repo
reset.c: extract function for parsing arguments
reset.c: remove unnecessary variable 'i'
reset.c: extract function for updating {ORIG_,}HEAD
reset.c: share call to die_if_unmerged_cache()
reset --keep: only write index file once
reset: avoid redundant error message
reset.c: replace switch by if-else
reset.c: move update_index_refresh() call out of read_from_tree()
reset.c: move lock, write and commit out of update_index_refresh()
reset [--mixed]: only write index file once
reset.c: finish entire cmd_reset() whether or not pathspec is given
reset.c: inline update_index_refresh()
reset $sha1 $pathspec: require $sha1 only to be treeish
reset: allow reset on unborn branch
reset [--mixed]: use diff-based reset whether or not pathspec was
given
builtin/reset.c| 283 +++--
t/t2013-checkout-submodule.sh | 2 +-
t/t7102-reset.sh | 26 +++-
t/t7106-reset-unborn-branch.sh | 52
4 files changed, 203 insertions(+), 160 deletions(-)
create mode 100755 t/t7106-reset-unborn-branch.sh
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/19] reset --keep: only write index file once
"git reset --keep" calls reset_index_file() twice, first doing a
two-way merge to the target revision, updating the index and worktree,
and then resetting the index. After each call, we write the index
file.
In the unlikely event that the second call to reset_index_file()
fails, the index will have been merged to the target revision, but
HEAD will not be updated, leaving the user with a dirty index.
By moving the locking, writing and committing out of
reset_index_file() and into the caller, we can avoid writing the index
twice, thereby making the sure we don't end up in the half-way reset
state. As a bonus, we speed up "git reset --keep" a little on the
linux-2.6 repo (best of five, warm cache):
Before After
real0m0.315s0m0.296s
user0m0.290s0m0.280s
sys 0m0.020s0m0.010s
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4e34195..7c440ad 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -38,14 +38,12 @@ static inline int is_merge(void)
return !access(git_path("MERGE_HEAD"), F_OK);
}
-static int reset_index_file(const unsigned char *sha1, int reset_type, int
quiet)
+static int reset_index(const unsigned char *sha1, int reset_type, int quiet)
{
int nr = 1;
- int newfd;
struct tree_desc desc[2];
struct tree *tree;
struct unpack_trees_options opts;
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
memset(&opts, 0, sizeof(opts));
opts.head_idx = 1;
@@ -67,8 +65,6 @@ static int reset_index_file(const unsigned char *sha1, int
reset_type, int quiet
opts.reset = 1;
}
- newfd = hold_locked_index(lock, 1);
-
read_cache_unmerged();
if (reset_type == KEEP) {
@@ -91,10 +87,6 @@ static int reset_index_file(const unsigned char *sha1, int
reset_type, int quiet
prime_cache_tree(&active_cache_tree, tree);
}
- if (write_cache(newfd, active_cache, active_nr) ||
- commit_locked_index(lock))
- return error(_("Could not write new index file."));
-
return 0;
}
@@ -341,9 +333,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die_if_unmerged_cache(reset_type);
if (reset_type != SOFT) {
- int err = reset_index_file(sha1, reset_type, quiet);
+ struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+ int newfd = hold_locked_index(lock, 1);
+ int err = reset_index(sha1, reset_type, quiet);
if (reset_type == KEEP && !err)
- err = reset_index_file(sha1, MIXED, quiet);
+ err = reset_index(sha1, MIXED, quiet);
+ if (!err &&
+ (write_cache(newfd, active_cache, active_nr) ||
+commit_locked_index(lock))) {
+ err = error(_("Could not write new index file."));
+ }
if (err)
die(_("Could not reset index file to revision '%s'."),
rev);
}
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/19] reset.c: extract function for updating {ORIG_,}HEAD
By extracting the code for updating the HEAD and ORIG_HEAD symbolic
references to a separate function, we declutter cmd_reset() a bit and
we make it clear that e.g. the four variables {,sha1_}{,old_}orig are
only used by this code.
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index d89cf4d..2187d64 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -240,16 +240,35 @@ static const char **parse_args(const char **argv, const
char *prefix, const char
return argv[0] ? get_pathspec(prefix, argv) : NULL;
}
+static int update_refs(const char *rev, const unsigned char *sha1)
+{
+ int update_ref_status;
+ struct strbuf msg = STRBUF_INIT;
+ unsigned char *orig = NULL, sha1_orig[20],
+ *old_orig = NULL, sha1_old_orig[20];
+
+ if (!get_sha1("ORIG_HEAD", sha1_old_orig))
+ old_orig = sha1_old_orig;
+ if (!get_sha1("HEAD", sha1_orig)) {
+ orig = sha1_orig;
+ set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
+ update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+ } else if (old_orig)
+ delete_ref("ORIG_HEAD", old_orig, 0);
+ set_reflog_message(&msg, "updating HEAD", rev);
+ update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0,
MSG_ON_ERR);
+ strbuf_release(&msg);
+ return update_ref_status;
+}
+
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
int patch_mode = 0;
const char *rev;
- unsigned char sha1[20], *orig = NULL, sha1_orig[20],
- *old_orig = NULL, sha1_old_orig[20];
+ unsigned char sha1[20];
const char **pathspec = NULL;
struct commit *commit;
- struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_SET_INT(0, "mixed", &reset_type,
@@ -333,17 +352,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
/* Any resets update HEAD to the head being switched to,
* saving the previous head in ORIG_HEAD before. */
- if (!get_sha1("ORIG_HEAD", sha1_old_orig))
- old_orig = sha1_old_orig;
- if (!get_sha1("HEAD", sha1_orig)) {
- orig = sha1_orig;
- set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
- update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
- }
- else if (old_orig)
- delete_ref("ORIG_HEAD", old_orig, 0);
- set_reflog_message(&msg, "updating HEAD", rev);
- update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0,
MSG_ON_ERR);
+ update_ref_status = update_refs(rev, sha1);
switch (reset_type) {
case HARD:
@@ -360,7 +369,5 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
remove_branch_state();
- strbuf_release(&msg);
-
return update_ref_status;
}
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 16/19] reset.c: inline update_index_refresh()
Now that there is only one caller left to the single-line method
update_index_refresh(), inline it.
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index c316d9b..520c1a5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,12 +109,6 @@ static void print_new_head_line(struct commit *commit)
printf("\n");
}
-static void update_index_refresh(int flags)
-{
- refresh_index(&the_index, (flags), NULL, NULL,
- _("Unstaged changes after reset:"));
-}
-
static void update_index_from_diff(struct diff_queue_struct *q,
struct diff_options *opt, void *data)
{
@@ -329,9 +323,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_("Could not reset index file to revision
'%s'."), rev);
}
- if (reset_type == MIXED) /* Report what has not been updated. */
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (reset_type == MIXED) { /* Report what has not been updated.
*/
+ int flags = quiet ? REFRESH_QUIET :
REFRESH_IN_PORCELAIN;
+ refresh_index(&the_index, flags, NULL, NULL,
+ _("Unstaged changes after reset:"));
+ }
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/19] reset.c: replace switch by if-else
The switch statement towards the end of reset.c is missing case arms
for KEEP and MERGE for no obvious reason, and soon the only non-empty
case arm will be the one for HARD. So let's proactively replace it by
if-else, which will let us move one if statement out without leaving
funny-looking left-overs.
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 97fa9f7..c3eb2eb 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -349,18 +349,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
* saving the previous head in ORIG_HEAD before. */
update_ref_status = update_refs(rev, sha1);
- switch (reset_type) {
- case HARD:
- if (!update_ref_status && !quiet)
- print_new_head_line(commit);
- break;
- case SOFT: /* Nothing else to do. */
- break;
- case MIXED: /* Report what has not been updated. */
+ if (reset_type == HARD && !update_ref_status && !quiet)
+ print_new_head_line(commit);
+ else if (reset_type == MIXED) /* Report what has not been updated. */
update_index_refresh(0, NULL,
quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- break;
- }
remove_branch_state();
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/19] reset.c: extract function for parsing arguments
Declutter cmd_reset() a bit by moving out the argument parsing to its
own function.
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 70 +++--
1 file changed, 38 insertions(+), 32 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 664fad9..58f0f61 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -198,36 +198,11 @@ static void die_if_unmerged_cache(int reset_type)
}
-int cmd_reset(int argc, const char **argv, const char *prefix)
+static const char **parse_args(int argc, const char **argv, const char
*prefix, const char **rev_ret)
{
- int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
- int patch_mode = 0;
+ int i = 0;
const char *rev = "HEAD";
- unsigned char sha1[20], *orig = NULL, sha1_orig[20],
- *old_orig = NULL, sha1_old_orig[20];
- const char **pathspec = NULL;
- struct commit *commit;
- struct strbuf msg = STRBUF_INIT;
- const struct option options[] = {
- OPT__QUIET(&quiet, N_("be quiet, only report errors")),
- OPT_SET_INT(0, "mixed", &reset_type,
- N_("reset HEAD and index"),
MIXED),
- OPT_SET_INT(0, "soft", &reset_type, N_("reset only HEAD"),
SOFT),
- OPT_SET_INT(0, "hard", &reset_type,
- N_("reset HEAD, index and working tree"), HARD),
- OPT_SET_INT(0, "merge", &reset_type,
- N_("reset HEAD, index and working tree"),
MERGE),
- OPT_SET_INT(0, "keep", &reset_type,
- N_("reset HEAD but keep local changes"), KEEP),
- OPT_BOOLEAN('p', "patch", &patch_mode, N_("select hunks
interactively")),
- OPT_END()
- };
-
- git_config(git_default_config, NULL);
-
- argc = parse_options(argc, argv, prefix, options, git_reset_usage,
- PARSE_OPT_KEEP_DASHDASH);
-
+ unsigned char unused[20];
/*
* Possible arguments are:
*
@@ -250,7 +225,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
* Otherwise, argv[i] could be either or and
* has to be unambiguous.
*/
- else if (!get_sha1_committish(argv[i], sha1)) {
+ else if (!get_sha1_committish(argv[i], unused)) {
/*
* Ok, argv[i] looks like a rev; it should not
* be a filename.
@@ -262,6 +237,40 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
verify_filename(prefix, argv[i], 1);
}
}
+ *rev_ret = rev;
+ return i < argc ? get_pathspec(prefix, argv + i) : NULL;
+}
+
+int cmd_reset(int argc, const char **argv, const char *prefix)
+{
+ int reset_type = NONE, update_ref_status = 0, quiet = 0;
+ int patch_mode = 0;
+ const char *rev;
+ unsigned char sha1[20], *orig = NULL, sha1_orig[20],
+ *old_orig = NULL, sha1_old_orig[20];
+ const char **pathspec = NULL;
+ struct commit *commit;
+ struct strbuf msg = STRBUF_INIT;
+ const struct option options[] = {
+ OPT__QUIET(&quiet, N_("be quiet, only report errors")),
+ OPT_SET_INT(0, "mixed", &reset_type,
+ N_("reset HEAD and index"),
MIXED),
+ OPT_SET_INT(0, "soft", &reset_type, N_("reset only HEAD"),
SOFT),
+ OPT_SET_INT(0, "hard", &reset_type,
+ N_("reset HEAD, index and working tree"), HARD),
+ OPT_SET_INT(0, "merge", &reset_type,
+ N_("reset HEAD, index and working tree"),
MERGE),
+ OPT_SET_INT(0, "keep", &reset_type,
+ N_("reset HEAD but keep local changes"), KEEP),
+ OPT_BOOLEAN('p', "patch", &patch_mode, N_("select hunks
interactively")),
+ OPT_END()
+ };
+
+ git_config(git_default_config, NULL);
+
+ argc = parse_options(argc, argv, prefix, options, git_reset_usage,
+ PARSE_OPT_KEEP_DASHDASH);
+ pathspec = parse_args(argc, argv, prefix, &rev);
if (get_sha1_committish(rev, sha1))
die(_("Failed to resolve '%s' as a valid ref."), rev);
@@ -277,9 +286,6 @@ int cmd_reset(i
[PATCH v2 10/19] reset: avoid redundant error message
If writing or committing the new index file fails, we print "Could not
write new index file." followed by "Could not reset index file to
revision $rev.". The first message seems to imply the second, so print
only the first message.
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 7c440ad..97fa9f7 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -338,13 +338,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
int err = reset_index(sha1, reset_type, quiet);
if (reset_type == KEEP && !err)
err = reset_index(sha1, MIXED, quiet);
- if (!err &&
- (write_cache(newfd, active_cache, active_nr) ||
-commit_locked_index(lock))) {
- err = error(_("Could not write new index file."));
- }
if (err)
die(_("Could not reset index file to revision '%s'."),
rev);
+ if (write_cache(newfd, active_cache, active_nr) ||
+ commit_locked_index(lock))
+ die(_("Could not write new index file."));
}
/* Any resets update HEAD to the head being switched to,
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 14/19] reset [--mixed]: only write index file once
When doing a mixed reset without paths, the index is locked, read,
reset, and written back as part of the actual reset operation (in
reset_index()). Then, when showing the list of worktree modifications,
we lock the index again, refresh it, and write it.
Change this so we only write the index once, making "git reset" a
little faster. It does mean that the index lock will be held a little
longer, but the difference is small compared to the time spent
refreshing the index.
There is one minor functional difference: We used to say "Could not
write new index file." if the first write failed, and "Could not
refresh index" if the second write failed. Now, we will only use the
first message.
This speeds up "git reset" a little on the linux-2.6 repo (best of
five, warm cache):
Before After
real0m0.239s0m0.214s
user0m0.160s0m0.130s
sys 0m0.070s0m0.080s
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index c1d6ef2..e8a3e41 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -336,6 +336,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
err = reset_index(sha1, MIXED, quiet);
if (err)
die(_("Could not reset index file to revision '%s'."),
rev);
+
+ if (reset_type == MIXED) /* Report what has not been updated. */
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
die(_("Could not write new index file."));
@@ -347,15 +352,6 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type == HARD && !update_ref_status && !quiet)
print_new_head_line(commit);
- else if (reset_type == MIXED) { /* Report what has not been updated. */
- struct lock_file *index_lock = xcalloc(1, sizeof(struct
lock_file));
- int fd = hold_locked_index(index_lock, 1);
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(index_lock))
- error("Could not refresh index");
- }
remove_branch_state();
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given
By not returning from inside the "if (pathspec)" block, we can let the
pathspec-aware and pathspec-less code share a bit more, making it
easier to make future changes that should affect both cases. This also
highlights the similarity between read_from_tree() and reset_index().
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 42 ++
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index e8a3e41..c316d9b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -309,19 +309,6 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_("%s reset is not allowed in a bare repository"),
_(reset_type_names[reset_type]));
- if (pathspec) {
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd = hold_locked_index(lock, 1);
- if (read_from_tree(pathspec, sha1))
- return 1;
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- if (write_cache(index_fd, active_cache, active_nr) ||
- commit_locked_index(lock))
- return error("Could not write new index file.");
- return 0;
- }
-
/* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order. Other resets reset
* the index file to the tree object we are switching to. */
@@ -331,11 +318,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type != SOFT) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
- int err = reset_index(sha1, reset_type, quiet);
- if (reset_type == KEEP && !err)
- err = reset_index(sha1, MIXED, quiet);
- if (err)
- die(_("Could not reset index file to revision '%s'."),
rev);
+ if (pathspec) {
+ if (read_from_tree(pathspec, sha1))
+ return 1;
+ } else {
+ int err = reset_index(sha1, reset_type, quiet);
+ if (reset_type == KEEP && !err)
+ err = reset_index(sha1, MIXED, quiet);
+ if (err)
+ die(_("Could not reset index file to revision
'%s'."), rev);
+ }
if (reset_type == MIXED) /* Report what has not been updated. */
update_index_refresh(
@@ -346,14 +338,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_("Could not write new index file."));
}
- /* Any resets update HEAD to the head being switched to,
-* saving the previous head in ORIG_HEAD before. */
- update_ref_status = update_refs(rev, sha1);
+ if (!pathspec) {
+ /* Any resets without paths update HEAD to the head being
+* switched to, saving the previous head in ORIG_HEAD before. */
+ update_ref_status = update_refs(rev, sha1);
- if (reset_type == HARD && !update_ref_status && !quiet)
- print_new_head_line(commit);
+ if (reset_type == HARD && !update_ref_status && !quiet)
+ print_new_head_line(commit);
- remove_branch_state();
+ remove_branch_state();
+ }
return update_ref_status;
}
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/19] reset.c: move lock, write and commit out of update_index_refresh()
In preparation for the/a following patch, move the locking, writing
and committing of the index file out of update_index_refresh(). The
code duplication caused will soon be taken care of. What remains of
update_index_refresh() is just one line, but it is still called from
two places, so let's leave it for now.
In the process, we expose and fix the minor UI bug that makes us print
"Could not refresh index" when we fail to write the index file when
invoked with a pathspec. Copy the error message from the pathspec-less
codepath ("Could not write new index file.").
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 34 ++
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 70733c2..c1d6ef2 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,19 +109,10 @@ static void print_new_head_line(struct commit *commit)
printf("\n");
}
-static int update_index_refresh(int fd, struct lock_file *index_lock, int
flags)
+static void update_index_refresh(int flags)
{
- if (!index_lock) {
- index_lock = xcalloc(1, sizeof(struct lock_file));
- fd = hold_locked_index(index_lock, 1);
- }
-
refresh_index(&the_index, (flags), NULL, NULL,
_("Unstaged changes after reset:"));
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(index_lock))
- return error ("Could not refresh index");
- return 0;
}
static void update_index_from_diff(struct diff_queue_struct *q,
@@ -321,9 +312,14 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (pathspec) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd = hold_locked_index(lock, 1);
- return read_from_tree(pathspec, sha1) ||
- update_index_refresh(index_fd, lock,
- quiet ? REFRESH_QUIET :
REFRESH_IN_PORCELAIN);
+ if (read_from_tree(pathspec, sha1))
+ return 1;
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (write_cache(index_fd, active_cache, active_nr) ||
+ commit_locked_index(lock))
+ return error("Could not write new index file.");
+ return 0;
}
/* Soft reset does not touch the index file nor the working tree
@@ -351,9 +347,15 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type == HARD && !update_ref_status && !quiet)
print_new_head_line(commit);
- else if (reset_type == MIXED) /* Report what has not been updated. */
- update_index_refresh(0, NULL,
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ else if (reset_type == MIXED) { /* Report what has not been updated. */
+ struct lock_file *index_lock = xcalloc(1, sizeof(struct
lock_file));
+ int fd = hold_locked_index(index_lock, 1);
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (write_cache(fd, active_cache, active_nr) ||
+ commit_locked_index(index_lock))
+ error("Could not refresh index");
+ }
remove_branch_state();
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 19/19] reset [--mixed]: use diff-based reset whether or not pathspec was given
Thanks to b65982b (Optimize "diff-index --cached" using cache-tree,
2009-05-20), resetting with paths is much faster than resetting
without paths. Some timings for the linux-2.6 repo to illustrate this
(best of five, warm cache):
reset reset .
real0m0.219s0m0.080s
user0m0.140s0m0.040s
sys 0m0.070s0m0.030s
These two commands should do the same thing, so instead of having the
user type the trailing " ." to get the faster do_diff_cache()-based
implementation, always use it when doing a mixed reset, with or
without paths (so "git reset $rev" would also be faster).
Timing "git reset" shows that it indeed becomes as fast as
"git reset ." after this patch.
Signed-off-by: Martin von Zweigbergk
---
It seems like a better solution would be for unpack_trees() learn the
same tricks as do_diff_cache(). I'm leaving that a challange for the
reader :-). I did have a look a unpack_trees(), but it looked rather
overwhelming.
builtin/reset.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 45b01eb..921afbe 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -322,7 +322,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (reset_type != SOFT) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
- if (pathspec) {
+ if (reset_type == MIXED) {
if (read_from_tree(pathspec, sha1))
return 1;
} else {
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 18/19] reset: allow reset on unborn branch
Some users seem to think, knowingly or not, that being on an unborn
branch is like having a commit with an empty tree checked out, but
when run on an unborn branch, "git reset" currently fails with:
fatal: Failed to resolve 'HEAD' as a valid ref.
Instead of making users figure out that they should run
git rm --cached -r .
, let's teach "git reset" without a revision argument, when on an
unborn branch, to behave as if the user asked to reset to an empty
tree. Don't take the analogy with an empty commit too far, though, but
still disallow explictly referring to HEAD in "git reset HEAD".
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c| 16 -
t/t7106-reset-unborn-branch.sh | 52 ++
2 files changed, 62 insertions(+), 6 deletions(-)
create mode 100755 t/t7106-reset-unborn-branch.sh
diff --git a/builtin/reset.c b/builtin/reset.c
index b776867..45b01eb 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -240,7 +240,7 @@ static int update_refs(const char *rev, const unsigned char
*sha1)
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
- int patch_mode = 0;
+ int patch_mode = 0, unborn;
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
@@ -265,7 +265,11 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argv, prefix, &rev);
- if (!pathspec) {
+ unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", sha1);
+ if (unborn) {
+ /* reset on unborn branch: treat as reset to empty tree */
+ hashcpy(sha1, EMPTY_TREE_SHA1_BIN);
+ } else if (!pathspec) {
struct commit *commit;
if (get_sha1_committish(rev, sha1))
die(_("Failed to resolve '%s' as a valid revision."),
rev);
@@ -286,7 +290,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
if (patch_mode) {
if (reset_type != NONE)
die(_("--patch is incompatible with
--{hard,mixed,soft}"));
- return run_add_interactive(rev, "--patch=reset", pathspec);
+ return run_add_interactive(sha1_to_hex(sha1), "--patch=reset",
pathspec);
}
/* git reset tree [--] paths... can be used to
@@ -340,16 +344,16 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_("Could not write new index file."));
}
- if (!pathspec) {
+ if (!pathspec && !unborn) {
/* Any resets without paths update HEAD to the head being
* switched to, saving the previous head in ORIG_HEAD before. */
update_ref_status = update_refs(rev, sha1);
if (reset_type == HARD && !update_ref_status && !quiet)
print_new_head_line(lookup_commit_reference(sha1));
-
- remove_branch_state();
}
+ if (!pathspec)
+ remove_branch_state();
return update_ref_status;
}
diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
new file mode 100755
index 000..8062cf5
--- /dev/null
+++ b/t/t7106-reset-unborn-branch.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='git reset should work on unborn branch'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo a >a &&
+ echo b >b
+'
+
+test_expect_success 'reset' '
+ git add a b &&
+ git reset &&
+ test "$(git ls-files)" = ""
+'
+
+test_expect_success 'reset HEAD' '
+ rm .git/index &&
+ git add a b &&
+ test_must_fail git reset HEAD
+'
+
+test_expect_success 'reset $file' '
+ rm .git/index &&
+ git add a b &&
+ git reset a &&
+ test "$(git ls-files)" = "b"
+'
+
+test_expect_success 'reset -p' '
+ rm .git/index &&
+ git add a &&
+ echo y | git reset -p &&
+ test "$(git ls-files)" = ""
+'
+
+test_expect_success 'reset --soft is a no-op' '
+ rm .git/index &&
+ git add a &&
+ git reset --soft
+ test "$(git ls-files)" = "a"
+'
+
+test_expect_success 'reset --hard' '
+ rm .git/index &&
+ git add a &&
+ git reset --hard &&
+ test "$(git ls-files)" = "" &&
+ test_path_is_missing a
+'
+
+test_done
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 17/19] reset $sha1 $pathspec: require $sha1 only to be treeish
Resetting with paths does not update HEAD and there is nothing else
that a commit should be needed for. Relax the argument parsing so only
a tree is required.
The sha1 is only passed to read_from_tree(), which already only
requires a tree.
The "rev" variable we pass to run_add_interactive() will resolve to a
tree. This is fine since interactive_reset only needs the parameter to
be a treeish and doesn't use it for display purposes.
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 48 +++-
t/t7102-reset.sh | 8
2 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 520c1a5..b776867 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -178,9 +178,10 @@ static const char **parse_args(const char **argv, const
char *prefix, const char
/*
* Possible arguments are:
*
-* git reset [-opts] ...
-* git reset [-opts] -- ...
-* git reset [-opts] -- ...
+* git reset [-opts] []
+* git reset [-opts] [...]
+* git reset [-opts] -- [...]
+* git reset [-opts] -- [...]
* git reset [-opts] ...
*
* At this point, argv points immediately after [-opts].
@@ -195,11 +196,13 @@ static const char **parse_args(const char **argv, const
char *prefix, const char
}
/*
* Otherwise, argv[0] could be either or and
-* has to be unambiguous.
+* has to be unambiguous. If there is a single argument, it
+* can not be a tree
*/
- else if (!get_sha1_committish(argv[0], unused)) {
+ else if ((!argv[1] && !get_sha1_committish(argv[0], unused)) ||
+(argv[1] && !get_sha1_treeish(argv[0], unused))) {
/*
-* Ok, argv[0] looks like a rev; it should not
+* Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
verify_non_filename(prefix, argv[0]);
@@ -241,7 +244,6 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
- struct commit *commit;
const struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_SET_INT(0, "mixed", &reset_type,
@@ -263,19 +265,23 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argv, prefix, &rev);
- if (get_sha1_committish(rev, sha1))
- die(_("Failed to resolve '%s' as a valid ref."), rev);
-
- /*
-* NOTE: As "git reset $treeish -- $path" should be usable on
-* any tree-ish, this is not strictly correct. We are not
-* moving the HEAD to any commit; we are merely resetting the
-* entries in the index to that of a treeish.
-*/
- commit = lookup_commit_reference(sha1);
- if (!commit)
- die(_("Could not parse object '%s'."), rev);
- hashcpy(sha1, commit->object.sha1);
+ if (!pathspec) {
+ struct commit *commit;
+ if (get_sha1_committish(rev, sha1))
+ die(_("Failed to resolve '%s' as a valid revision."),
rev);
+ commit = lookup_commit_reference(sha1);
+ if (!commit)
+ die(_("Could not parse object '%s'."), rev);
+ hashcpy(sha1, commit->object.sha1);
+ } else {
+ struct tree *tree;
+ if (get_sha1_treeish(rev, sha1))
+ die(_("Failed to resolve '%s' as a valid tree."), rev);
+ tree = parse_tree_indirect(sha1);
+ if (!tree)
+ die(_("Could not parse object '%s'."), rev);
+ hashcpy(sha1, tree->object.sha1);
+ }
if (patch_mode) {
if (reset_type != NONE)
@@ -340,7 +346,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
update_ref_status = update_refs(rev, sha1);
if (reset_type == HARD && !update_ref_status && !quiet)
- print_new_head_line(commit);
+ print_new_head_line(lookup_commit_reference(sha1));
remove_branch_state();
}
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 81b2570..1fa2a5f 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -497,4 +497,12 @@ test_expect_
[PATCH v2 01/19] reset $pathspec: no need to discard index
Since 34110cd (Make 'unpack_trees()' have a separate source and
destination index, 2008-03-06), the index no longer gets clobbered by
do_diff_cache() and we can remove the code for discarding and
re-reading it.
There are two paths to update_index_refresh() from cmd_reset(), but on
both paths, either read_cache() or read_cache_unmerged() will have
been called, so the call to read_cache() in this method is redundant
(although practically free).
This speeds up "git reset -- ." a little on the linux-2.6 repo (best
of five, warm cache):
Before After
real0m0.093s0m0.080s
user0m0.040s0m0.020s
sys 0m0.050s0m0.050s
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 915cc9f..8cc7c72 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -126,9 +126,6 @@ static int update_index_refresh(int fd, struct lock_file
*index_lock, int flags)
fd = hold_locked_index(index_lock, 1);
}
- if (read_cache() < 0)
- return error(_("Could not read index"));
-
result = refresh_index(&the_index, (flags), NULL, NULL,
_("Unstaged changes after reset:")) ? 1 : 0;
if (write_cache(fd, active_cache, active_nr) ||
@@ -141,12 +138,6 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
struct diff_options *opt, void *data)
{
int i;
- int *discard_flag = data;
-
- /* do_diff_cache() mangled the index */
- discard_cache();
- *discard_flag = 1;
- read_cache();
for (i = 0; i < q->nr; i++) {
struct diff_filespec *one = q->queue[i]->one;
@@ -179,17 +170,15 @@ static int read_from_tree(const char *prefix, const char
**argv,
unsigned char *tree_sha1, int refresh_flags)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd, index_was_discarded = 0;
+ int index_fd;
struct diff_options opt;
memset(&opt, 0, sizeof(opt));
diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- opt.format_callback_data = &index_was_discarded;
index_fd = hold_locked_index(lock, 1);
- index_was_discarded = 0;
read_cache();
if (do_diff_cache(tree_sha1, &opt))
return 1;
@@ -197,9 +186,6 @@ static int read_from_tree(const char *prefix, const char
**argv,
diff_flush(&opt);
diff_tree_release_paths(&opt);
- if (!index_was_discarded)
- /* The index is still clobbered from do_diff_cache() */
- discard_cache();
return update_index_refresh(index_fd, lock, refresh_flags);
}
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/19] reset $pathspec: exit with code 0 if successful
"git reset $pathspec" currently exits with a non-zero exit code if the
worktree is dirty after resetting, which is inconsistent with reset
without pathspec, and it makes it harder to know whether the command
really failed. Change it to exit with code 0 regardless of whether the
worktree is dirty so that non-zero indicates an error.
This makes the 4 "disambiguation" test cases in t7102 clearer since
they all used to "fail", 3 of which "failed" due to changes in the
work tree. Now only the ambiguous one fails.
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 8 +++-
t/t2013-checkout-submodule.sh | 2 +-
t/t7102-reset.sh | 18 --
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 8cc7c72..65413d0 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -119,19 +119,17 @@ static void print_new_head_line(struct commit *commit)
static int update_index_refresh(int fd, struct lock_file *index_lock, int
flags)
{
- int result;
-
if (!index_lock) {
index_lock = xcalloc(1, sizeof(struct lock_file));
fd = hold_locked_index(index_lock, 1);
}
- result = refresh_index(&the_index, (flags), NULL, NULL,
- _("Unstaged changes after reset:")) ? 1 : 0;
+ refresh_index(&the_index, (flags), NULL, NULL,
+ _("Unstaged changes after reset:"));
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(index_lock))
return error ("Could not refresh index");
- return result;
+ return 0;
}
static void update_index_from_diff(struct diff_queue_struct *q,
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 70edbb3..06b18f8 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -23,7 +23,7 @@ test_expect_success '"reset " updates the index' '
git update-index --refresh &&
git diff-files --quiet &&
git diff-index --quiet --cached HEAD &&
- test_must_fail git reset HEAD^ submodule &&
+ git reset HEAD^ submodule &&
test_must_fail git diff-files --quiet &&
git reset submodule &&
git diff-files --quiet
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index b096dc8..81b2570 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -388,7 +388,8 @@ test_expect_success 'test --mixed ' '
echo 4 > file4 &&
echo 5 > file1 &&
git add file1 file3 file4 &&
- test_must_fail git reset HEAD -- file1 file2 file3 &&
+ git reset HEAD -- file1 file2 file3 &&
+ test_must_fail git diff --quiet &&
git diff > output &&
test_cmp output expect &&
git diff --cached > output &&
@@ -402,7 +403,8 @@ test_expect_success 'test resetting the index at give
paths' '
>sub/file2 &&
git update-index --add sub/file1 sub/file2 &&
T=$(git write-tree) &&
- test_must_fail git reset HEAD sub/file2 &&
+ git reset HEAD sub/file2 &&
+ test_must_fail git diff --quiet &&
U=$(git write-tree) &&
echo "$T" &&
echo "$U" &&
@@ -440,7 +442,8 @@ test_expect_success 'resetting specific path that is
unmerged' '
echo "100644 $F3 3 file2"
} | git update-index --index-info &&
git ls-files -u &&
- test_must_fail git reset HEAD file2 &&
+ git reset HEAD file2 &&
+ test_must_fail git diff --quiet &&
git diff-index --exit-code --cached HEAD
'
@@ -449,7 +452,8 @@ test_expect_success 'disambiguation (1)' '
git reset --hard &&
>secondfile &&
git add secondfile &&
- test_must_fail git reset secondfile &&
+ git reset secondfile &&
+ test_must_fail git diff --quiet -- secondfile &&
test -z "$(git diff --cached --name-only)" &&
test -f secondfile &&
test ! -s secondfile
@@ -474,7 +478,8 @@ test_expect_success 'disambiguation (3)' '
>secondfile &&
git add secondfile &&
rm -f secondfile &&
- test_must_fail git reset HEAD secondfile &&
+ git reset HEAD secondfile &&
+ test_must_fail git diff --quiet &&
test -z "$(git diff --cached --name-only)" &&
test ! -f secondfile
@@ -486,7 +491,8 @@ test_expect_success 'disambiguation (4)' '
>secondfile &&
git add secondfile &&
rm -f secondfile &&
- test_must_fail git reset -- secondfile &&
+ git reset -- secondfile &&
+ test_must_fail git diff --quiet &&
test -z "$(git diff --cached --name-only)" &&
test ! -f secondfile
'
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair
We use the path arguments in two places in reset.c: in
interactive_reset() and read_from_tree(). Both of these call
get_pathspec(), so we pass the (prefix, argv) pair to both
functions. Move the call to get_pathspec() out of these methods, for
two reasons: 1) One argument is simpler than two. 2) It lets us use
the (arguably clearer) "if (pathspec)" in place of "if (i < argc)".
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 27 ++-
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 65413d0..045c960 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -153,26 +153,15 @@ static void update_index_from_diff(struct
diff_queue_struct *q,
}
}
-static int interactive_reset(const char *revision, const char **argv,
-const char *prefix)
-{
- const char **pathspec = NULL;
-
- if (*argv)
- pathspec = get_pathspec(prefix, argv);
-
- return run_add_interactive(revision, "--patch=reset", pathspec);
-}
-
-static int read_from_tree(const char *prefix, const char **argv,
- unsigned char *tree_sha1, int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
+ int refresh_flags)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd;
struct diff_options opt;
memset(&opt, 0, sizeof(opt));
- diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
+ diff_tree_setup_paths(pathspec, &opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
@@ -216,6 +205,7 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
const char *rev = "HEAD";
unsigned char sha1[20], *orig = NULL, sha1_orig[20],
*old_orig = NULL, sha1_old_orig[20];
+ const char **pathspec = NULL;
struct commit *commit;
struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
@@ -287,22 +277,25 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_("Could not parse object '%s'."), rev);
hashcpy(sha1, commit->object.sha1);
+ if (i < argc)
+ pathspec = get_pathspec(prefix, argv + i);
+
if (patch_mode) {
if (reset_type != NONE)
die(_("--patch is incompatible with
--{hard,mixed,soft}"));
- return interactive_reset(rev, argv + i, prefix);
+ return run_add_interactive(rev, "--patch=reset", pathspec);
}
/* git reset tree [--] paths... can be used to
* load chosen paths from the tree into the index without
* affecting the working tree nor HEAD. */
- if (i < argc) {
+ if (pathspec) {
if (reset_type == MIXED)
warning(_("--mixed with paths is deprecated; use 'git
reset -- ' instead."));
else if (reset_type != NONE)
die(_("Cannot do %s reset with paths."),
_(reset_type_names[reset_type]));
- return read_from_tree(prefix, argv + i, sha1,
+ return read_from_tree(pathspec, sha1,
quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
}
if (reset_type == NONE)
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/19] reset: don't allow "git reset -- $pathspec" in bare repo
Running e.g. "git reset ." in a bare repo results in an index file
being created from the HEAD commit. The differences compared to the
index are then printed as usual, but since there is no worktree, it
will appear as if all files are deleted. For example, in a bare clone
of git.git:
Unstaged changes after reset:
D .gitattributes
D .gitignore
D .mailmap
...
This happens because the check for is_bare_repository() happens after
we branch off into read_from_tree() to reset with paths. Fix by moving
the branching point after the check.
Signed-off-by: Martin von Zweigbergk
---
builtin/reset.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 045c960..664fad9 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -295,8 +295,6 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
else if (reset_type != NONE)
die(_("Cannot do %s reset with paths."),
_(reset_type_names[reset_type]));
- return read_from_tree(pathspec, sha1,
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
}
if (reset_type == NONE)
reset_type = MIXED; /* by default */
@@ -308,6 +306,10 @@ int cmd_reset(int argc, const char **argv, const char
*prefix)
die(_("%s reset is not allowed in a bare repository"),
_(reset_type_names[reset_type]));
+ if (pathspec)
+ return read_from_tree(pathspec, sha1,
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+
/* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order. Other resets reset
* the index file to the tree object we are switching to. */
--
1.8.1.1.454.gce43f05
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
