* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Jonathan Nieder @ 2012-01-12 20:11 UTC (permalink / raw)
To: Jeff King
Cc: Ramkumar Ramachandra, Junio C Hamano, SZEDER Gábor,
Christian Couder, Christian Couder, git
In-Reply-To: <20120112194710.GA28148@sigill.intra.peff.net>
Jeff King wrote:
> I am tempted to suggest
[...]
> That would make all of these work as most people would
> expect:
>
> git cherry-pick A B C
> git cherry-pick A..B
> git cherry-pick A..B B..C
>
> but would be a regression for:
>
> git cherry-pick B ^A
>
> versus the current code. I suspect that the latter form is not all that
> commonly used, though, and certainly I would accept it as a casualty of
> making the "A B C" form work. My only hesitation is that it is in fact a
> regression.
I find myself using such complicated expressions as
list-revs-to-skip |
xargs git cherry-pick --cherry-pick --right-only HEAD...topic --not
so yeah, that would be a pretty serious loss in functionality.
However, moving to something like the far future semantics that Junio
hinted at, for cherry-pick/revert and other --no-walk style commands
only, would not be a regression for me. The multi-pick feature is
still young, and I _suspect_ changing the meaning of A..B B..C for it
would not inconvenience anybody.
I would even welcome a change in the meaning of B ^A: the most
intuitive thing for it to do would be to cherry-pick the single commit B
when and only when it is not an ancestor of A.
Jonathan
^ permalink raw reply
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Jeff King @ 2012-01-12 19:47 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Junio C Hamano, SZEDER Gábor, Christian Couder,
Christian Couder, git, Jonathan Nieder
In-Reply-To: <CALkWK0kk0mVNaetr=triuVYva7inyx2aZvam81qTVA9=Q=UzGw@mail.gmail.com>
On Fri, Jan 13, 2012 at 12:55:58AM +0530, Ramkumar Ramachandra wrote:
> Junio C Hamano wrote:
> > You should be able to look at revs->cmdline and tell if you need to let
> > cherry-pick walk (i.e. "cherry-pick master..next"), or if the user wants
> > individual commits (i.e. "cherry-pick A B C").
> >
> > And you do prepare_revision_walk() only when you need to walk; otherwise
> > you use the contents of revs->pending in order.
I am tempted to suggest that cherry-pick should not feed its arguments
to the revision machinery in the first place, but instead accept a set
of arguments, each argument of which is either a single commit
(interpreted by get_sha1) or a range specifier (which can be fed to
setup-revisions). And then get a linearized set of commits for _each_
argument independently and concatenate them (possibly eliminating
duplicates). That would make all of these work as most people would
expect:
git cherry-pick A B C
git cherry-pick A..B
git cherry-pick A..B B..C
but would be a regression for:
git cherry-pick B ^A
versus the current code. I suspect that the latter form is not all that
commonly used, though, and certainly I would accept it as a casualty of
making the "A B C" form work. My only hesitation is that it is in fact a
regression.
> Okay, just to make sure I understand this correctly: if more than one
> argument is literally specified, I should not set up the revision
> walker and pick the commits listed in revs->pending, correct? Then,
> when I encounter the following command,
>
> $ git cherry-pick maint ^master
>
> I should just pick two commits: maint, and ^master.
But ^master is not a commit, it is a negation. So it is nonsensical if
the arguments are considered independent of each other (you _could_
use a heuristic to guess that they are not independent, but I'd rather
not go there). So you'd probably end up just rejecting the arguments.
-Peff
^ permalink raw reply
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Jonathan Nieder @ 2012-01-12 19:34 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Johannes Sixt, Jeff King, SZEDER Gábor, Christian Couder,
Christian Couder, git
In-Reply-To: <CALkWK0=NVUd629FgkPfgi8ZgTuO+a10t+iwbSrAvONCSmeq2rQ@mail.gmail.com>
Ramkumar Ramachandra wrote:
> That was my first reaction too -- then I saw builtin/push.c (the
> builtin show is quite similar), and found out that it doesn't use the
> revision walker at all.
"git push", unlike "git show", does not accept arguments like
maint..master.
^ permalink raw reply
* Re: [PATCH v2] cherry-pick: add failing test for out-of-order pick
From: Jonathan Nieder @ 2012-01-12 19:33 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: SZEDER Gábor, Christian Couder, Christian Couder, git,
Jeff King
In-Reply-To: <1326395132-18947-1-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Had some weird compulsion to conform to the style of the other tests
> in the previous iteration.
The tests you're talking about were introduced in commit 7b53b92f to
check for a buglet that made --strategy suppress the progress
reporting ("Finished one cherry-pick.") output cherry-pick normally
would emit.
So no inconsistency here --- those tests are _intending_ to check the
output format and that cherry-pick, unlike cherry-pick --ff, produces
new commits (though it would probably be clearer to put checks for
these behaviors in separate test assertions), while the new failing
test you are introducing is not about those things.
Striving for a consistent style is certainly not weird.
> --- a/t/t3508-cherry-pick-many-commits.sh
> +++ b/t/t3508-cherry-pick-many-commits.sh
> @@ -59,6 +59,23 @@ test_expect_success 'cherry-pick first..fourth works' '
[...]
> + git cherry-pick fourth second third &&
> + {
> + git rev-list --reverse HEAD |
> + git diff-tree --stdin -s --format=%s
> + } >actual &&
> + cat >expect <<-\EOF &&
> + fourth
> + second
> + third
> + EOF
> + test_cmp expect actual
This still feels more convoluted than expected (e.g., why --reverse?).
Something like
printf "%s\n" third second fourth >expect &&
...
git log --format=%s >actual &&
test_cmp expect actual
should be plenty.
^ permalink raw reply
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Ramkumar Ramachandra @ 2012-01-12 19:29 UTC (permalink / raw)
To: Johannes Sixt
Cc: Jeff King, SZEDER Gábor, Christian Couder, Christian Couder,
git, Jonathan Nieder
In-Reply-To: <4F0F32CC.8040404@kdbg.org>
Hi Johannes,
Johannes Sixt wrote:
> Why do we need a new flag?
>
> git show origin/master origin/maint
> git show origin/maint origin/master
>
> show the revisions in different order, in particular, in the order
> requested on the command line. Shoudn't cherry-pick be able to do the
> same without new hacks?
That was my first reaction too -- then I saw builtin/push.c (the
builtin show is quite similar), and found out that it doesn't use the
revision walker at all. It operates on refs, which has different
semantics altogether (called "refspec" in some places I think).
-- Ram
^ permalink raw reply
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Ramkumar Ramachandra @ 2012-01-12 19:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, SZEDER Gábor, Christian Couder, Christian Couder,
git, Jonathan Nieder
In-Reply-To: <7vaa5s3hiq.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> What are your thoughts on making it a flag in the revision API to be
>> activated with "cherry-pick --literal-order commit1 commit3 commit2"
>> or similar?
>
> That is an insane UI for the sake of flexibility.
>
> You should be able to look at revs->cmdline and tell if you need to let
> cherry-pick walk (i.e. "cherry-pick master..next"), or if the user wants
> individual commits (i.e. "cherry-pick A B C").
>
> And you do prepare_revision_walk() only when you need to walk; otherwise
> you use the contents of revs->pending in order.
Okay, just to make sure I understand this correctly: if more than one
argument is literally specified, I should not set up the revision
walker and pick the commits listed in revs->pending, correct? Then,
when I encounter the following command,
$ git cherry-pick maint ^master
I should just pick two commits: maint, and ^master. But won't this
introduce some kind of regression for those who expect me to pick the
master..maint range instead? Has this double-interpretation issue
come up in other commands? Have we documented this somewhere?
Thanks.
-- Ram
^ permalink raw reply
* Re: [PATCH] word-diff: ignore '\ No newline at eof' marker
From: Junio C Hamano @ 2012-01-12 19:23 UTC (permalink / raw)
To: Thomas Rast; +Cc: Ivan Shirokoff, git
In-Reply-To: <902665ee053876c2684f5b935ee4f81e77122802.1326366909.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> A proper fix would defer such markers until the end of the hunk.
> However, word-diff is inherently whitespace-ignoring, so as a cheap
> fix simply ignore the marker (and hide it from the output).
Sounds like a very sensible simplification of the issue.
> We use a prefix match for '\ ' to parallel the logic in
> apply.c:parse_fragment(). We currently do not localize this string
> (just accept other variants of it in git-apply), but this should be
> future-proof.
>
> Noticed-by: Ivan Shirokoff <shirokoff@yandex-team.ru>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>
> diff.c | 8 ++++++++
> t/t4034-diff-words.sh | 14 ++++++++++++++
> 2 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index a65223a..996cc60 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1113,6 +1113,14 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
> diff_words_append(line, len,
> &ecbdata->diff_words->plus);
> return;
> + } else if (!prefixcmp(line, "\\ ")) {
> + /*
> + * Silently eat the "no newline at eof" marker
> + * (we are diffing without regard to
> + * whitespace anyway), and defer processing:
> + * more '+' lines could be after it.
> + */
> + return;
> }
> diff_words_flush(ecbdata);
It took me a while to realize "defer processing" in the comment was meant
to justify the placement of the new block _before_ this flush. Perhaps
rephrasing it to "return without calling diff_words_flush()" would make it
more readable?
Otherwise the patch looks good.
Thanks.
> if (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) {
> diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
> index 6f1e5a2..5c20121 100755
> --- a/t/t4034-diff-words.sh
> +++ b/t/t4034-diff-words.sh
> @@ -334,4 +334,18 @@ test_expect_success 'word-diff with diff.sbe' '
> word_diff --word-diff=plain
> '
>
> +test_expect_success 'word-diff with no newline at EOF' '
> + cat >expect <<-\EOF &&
> + diff --git a/pre b/post
> + index 7bf316e..3dd0303 100644
> + --- a/pre
> + +++ b/post
> + @@ -1 +1 @@
> + a a [-a-]{+ab+} a a
> + EOF
> + printf "%s" "a a a a a" >pre &&
> + printf "%s" "a a ab a a" >post &&
> + word_diff --word-diff=plain
> +'
> +
> test_done
^ permalink raw reply
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Johannes Sixt @ 2012-01-12 19:21 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Jeff King, SZEDER Gábor, Christian Couder, Christian Couder,
git, Jonathan Nieder
In-Reply-To: <CALkWK0=Mv_tzNw-hN_9fAr+vABappndEK5iSWQHDk8Yk6Z-stw@mail.gmail.com>
Am 12.01.2012 18:09, schrieb Ramkumar Ramachandra:
> @@ -2054,7 +2054,10 @@ int prepare_revision_walk(struct rev_info *revs)
> if (commit) {
> if (!(commit->object.flags & SEEN)) {
> commit->object.flags |= SEEN;
> - commit_list_insert_by_date(commit,
> &revs->commits
> + if (revs->literal_order)
> + commit_list_insert(commit,
> &revs->commits
> + else
> +
> commit_list_insert_by_date(commit, &revs-
Why do we need a new flag?
git show origin/master origin/maint
git show origin/maint origin/master
show the revisions in different order, in particular, in the order
requested on the command line. Shoudn't cherry-pick be able to do the
same without new hacks?
-- Hannes
^ permalink raw reply
* Re: [PATCH] stash show: use default pretty format
From: Junio C Hamano @ 2012-01-12 19:06 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: Git Mailing List
In-Reply-To: <1326351953-3724-1-git-send-email-rctay89@gmail.com>
Tay Ray Chuan <rctay89@gmail.com> writes:
> By default (ie. when stash show is invoked without any arguments), the
> diff stat of the stashed changes is displayed. Let git-diff decide the
> default pretty format to use.
>
> This gives git more consistency, as users who have set their
> pretty.format config would naturally expect `git-stash show` to display
> the diff in the same pretty format as the other diff-producing procelain
> like git-log and git-show.
A handful of issues:
- The stash entries, unlike the usual commits you store on branches and
inspect with "show", are designed to be quick escapes for emergency
interruption, and "--stat" is a good default to remind the user what
she was working on before she was interrupted _without_ scrolling the
top of the screen away by showing the full diff. Careful design
decisions far outweigh mechanical application of "consistency for the
sake of consistency".
- What does "pretty.format" has anything to do with "stash"?
- If it does, why doesn't the script read from it?
- How does this justify the UI regression for people who are used to the
good default "--stat" they have been seeing?
^ permalink raw reply
* [PATCH v2] cherry-pick: add failing test for out-of-order pick
From: Ramkumar Ramachandra @ 2012-01-12 19:05 UTC (permalink / raw)
To: Jonathan Nieder
Cc: SZEDER Gábor, Christian Couder, Christian Couder, git,
Jeff King
In-Reply-To: <20120112183246.GB6038@burratino>
The invocation
$ git cherry-pick commit3 commit1 commit2
picks commits after sorting by date order, which is counter-intuitive.
Add a failing test to t3508 (cherry-pick-many-commits) documenting
this behavior.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Had some weird compulsion to conform to the style of the other tests
in the previous iteration.
t/t3508-cherry-pick-many-commits.sh | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index 8e09fd0..d9d632d 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -59,6 +59,23 @@ test_expect_success 'cherry-pick first..fourth works' '
check_head_differs_from fourth
'
+test_expect_failure 'cherry-pick picks commits in the order requested' '
+ git checkout -f master &&
+ git reset --hard first &&
+ test_tick &&
+ git cherry-pick fourth second third &&
+ {
+ git rev-list --reverse HEAD |
+ git diff-tree --stdin -s --format=%s
+ } >actual &&
+ cat >expect <<-\EOF &&
+ fourth
+ second
+ third
+ EOF
+ test_cmp expect actual
+'
+
test_expect_success 'cherry-pick --strategy resolve first..fourth works' '
cat <<-\EOF >expected &&
Trying simple merge.
--
1.7.8.2
^ permalink raw reply related
* Bug? Git checkout fails with a wrong error message
From: Yves Goergen @ 2012-01-12 18:44 UTC (permalink / raw)
To: git
Hi,
I am using Git alone for my local software project in Visual Studio 2010. I've
been on the master branch most of the time. Recently I created a new branch to
do a larger refactoring of one of the dialogue windows. I did the following
modifications:
* Rename Form1 to Form1a (including all depending files)
* Add new Form1
I checked this change into the branch, say form-refactoring. Interestingly, Git
didn't notice that I renamed the file Form1.cs into Form1a.cs and created a
brand new, totally different Form1.cs, but instead it noticed a new Form1a.cs
file and found a whole lot of differences between the previous and new Form1.cs
files. This will of course lead to totally garbaged diffs, but I don't care in
this case as long as all files are handled correctly in the end.
Then I switched back to master to do some other small changes. Nothing
conflicting. Until now, everything worked fine.
Today, I wanted to switch back to my branch form-refactoring to continue that
work. But all I get is the following message:
-----
git.exe checkout form-refactoring
Aborting
error: The following untracked working tree files would be overwritten by
checkout:
Form1.Designer.cs
Please move or remove them before you can switch branches.
-----
What is that supposed to be? The mentioned file is not untracked. Neither in the
master branch, nor in the form-refactoring branch. It is part of both branches,
but one is not a descendent of the other (because it was recreated on the
form-refactoring branch, if that matters). What would happen if I delete it, is
it gone for good then? I don't trust Git to bring back the correct file if I
delete something now. I did not play with any file at all outside of my
mentioned Git operations, so why should I play around with any file to continue
using Git operations now? Git broke it, Git's supposed to handle it now!
Here's some other input:
There are no uncommitted changes in my working directory. 'git status' doesn't
list anything.
The file in question is not untracked. Right now on the master branch, it has a
green checkmark in Explorer (provided by TortoiseGit) and it has a history as
well. There are more Form....Designer.cs files that don't cause any trouble.
'git clean -f -d', 'git reset --hard HEAD', 'git stash' do nothing and don't
help resolving the issue.
Right now, I cannot continue with my work because I cannot switch branches. Is
there an easy solution to this? Is my Git repository broken, all by standard
operations?
^ permalink raw reply
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Junio C Hamano @ 2012-01-12 18:41 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Jeff King, SZEDER Gábor, Christian Couder, Christian Couder,
git, Ramkumar Ramachandra
In-Reply-To: <20120112174731.GA6038@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Yes, exactly. Another question: what should
>
> git cherry-pick master..next maint..master
>
> do?
Revision ranges are not defined as a union of sets, but a single set as a
range between zero or more bottom (UNINTERESTING) commits and zero or more
top commits, the ones reachable from the top but not from the bottom, so
the above will work as if you said "^master ^maint master next", which is
the same as "master..next" (if you assume all of "maint" is contained in
"master" all of which is contained in "next", of course).
And it is not likely to change soon.
In the longer term (or in an alternate universe where we were inventing
Git from scratch today without any existing users), we may want to revamp
the revision machinery, taking advantage of the recent addition of the
"cmdline" facility to it, so that we would walk ranges "master..next" and
"maint..master" independently, clearing the object flags as needed between
the separate traversals as needed, and then take a union of these ranges,
before returning results from get_revision() calls.
^ permalink raw reply
* Re: [PATCH] cherry-pick: add failing test for out-of-order pick
From: Jonathan Nieder @ 2012-01-12 18:32 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Jeff King, SZEDER Gábor, Christian Couder, Christian Couder,
git
In-Reply-To: <1326390647-21446-1-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> $ git cherry-pick master..topic
> $ git cherry-pick topic ^master
> $ git cherry-pick ^master topic
>
> So the order of the arguments specified on the command-line is
> irrelevant in these cases. However, there are cases where it is worth
> paying attention to the order. For instance:
>
This segue feels a bit unnatural. I think the relevant point was that
early output from revision traversal (and perhaps some other things
--- I haven't checked) relies on commits having been inserted in a
topologically sorted order.
Anyway, I don't think the background is necessary --- the
one-paragraph description below stands well enough alone.
> $ git cherry-pick commit3 commit1 commit2
>
> picks commits after sorting by date order, which is counter-intuitive.
> Add a failing test to t3508 (cherry-pick-many-commits) documenting
> this behavior.
>
> Reported-by: SZEDER Gábor <szeder@ira.uka.de>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
[...]
> --- a/t/t3508-cherry-pick-many-commits.sh
> +++ b/t/t3508-cherry-pick-many-commits.sh
> @@ -59,6 +59,31 @@ test_expect_success 'cherry-pick first..fourth works' '
> check_head_differs_from fourth
> '
>
> +test_expect_failure 'cherry-pick picks commits in the right order' '
I would say "in the order requested" instead of the right order, since
it is not completely obvious to me what the right order is.
> + cat <<-\EOF >expected &&
> + [master OBJID] fourth
> + Author: A U Thor <author@example.com>
> + 1 files changed, 1 insertions(+), 0 deletions(-)
> + [master OBJID] second
> + Author: A U Thor <author@example.com>
> + 1 files changed, 1 insertions(+), 0 deletions(-)
> + [master OBJID] third
> + Author: A U Thor <author@example.com>
> + 1 files changed, 1 insertions(+), 0 deletions(-)
> + EOF
Why check all these details of formatting, instead of e.g. using "git
rev-list | git diff-tree -s --format=%s"?
[...]
> + test_cmp expected actual.fuzzy &&
> + check_head_differs_from second
Why make the same check twice?
Hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCH] diff --no-index: support more than one file pair
From: Matthieu Moy @ 2012-01-12 18:26 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8B460CwtACV+o0wnwqi1On_oEavLfDAL8f=w6kyfktKcQ@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>>> - hell, i might even benefit from git copy/modify detection
>>
>> I don't see how, if you specify explicitely the pairs (old, new). You
>> may have such benefit if you let the command-line express "here's a
>> bunch of old files, and a bunch of new ones", but not with your proposed
>> syntax.
>
> That's what git gives to diff machinery: a set of file pairs, and the
> diff machinery has to figure out copy/modify pairs, shuffling them up
> if necessary. I simply cut of tree traversal part out and feed file
> pairs directly to diff machinery.
If you want to benefit from copy detection, you cannot hardcode the fact
that you have as many source and destination files. And even to benefit
from rename detection, I find the user interface really weird. If I
provide files in pairs, I really don't expect Git to shuffle them like
git diff --no-index A1 B1 A2 B2
--- A1
+++ B2
..
--- A2
+++ B1
..
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Junio C Hamano @ 2012-01-12 18:25 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Jeff King, SZEDER Gábor, Christian Couder, Christian Couder,
git, Jonathan Nieder
In-Reply-To: <CALkWK0=Mv_tzNw-hN_9fAr+vABappndEK5iSWQHDk8Yk6Z-stw@mail.gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> What are your thoughts on making it a flag in the revision API to be
> activated with "cherry-pick --literal-order commit1 commit3 commit2"
> or similar?
That is an insane UI for the sake of flexibility.
You should be able to look at revs->cmdline and tell if you need to let
cherry-pick walk (i.e. "cherry-pick master..next"), or if the user wants
individual commits (i.e. "cherry-pick A B C").
And you do prepare_revision_walk() only when you need to walk; otherwise
you use the contents of revs->pending in order.
^ permalink raw reply
* Re: git svn clone terminating prematurely (I think)
From: Steven Line @ 2012-01-12 18:25 UTC (permalink / raw)
To: git
In-Reply-To: <20120111224833.GA29654@burratino>
Thank you Jonathan.
I had a breakthrough yesterday on this problem. To make a long story
short my ssh connection to the server where I was running 'nohup git
svn clone' was timing out. Additionally the nohup I was using wasn't
really protecting the git svn clone process so an hour or two after
the ssh disconnected, the git would terminate leaving me with an
imcomplete repository. I don't understand why the nohup wasn't
working yet. So my problem wasn't due to git itself.
I started my most recent git svn clone and it's now been running for
18 hours. I'm optimistic that I've solved the problem. If my git
does terminate early then I'll try a 'git svn fetch' to complete the
clone, based on what you said in your post.
--
Steven Line
303-910-1212
sline00@gmail.com
^ permalink raw reply
* [PATCH] cherry-pick: add failing test for out-of-order pick
From: Ramkumar Ramachandra @ 2012-01-12 17:50 UTC (permalink / raw)
To: Jeff King
Cc: SZEDER Gábor, Christian Couder, Christian Couder, git,
Jonathan Nieder
In-Reply-To: <CALkWK0nJM2wUE9qzp38qjFFqCdwX9w0Jckxi1G=1=7adMxg2rw@mail.gmail.com>
Due to the way traditional revision arguments work, the following
invocations of 'git cherry-pick' are equivalent:
$ git cherry-pick master..topic
$ git cherry-pick topic ^master
$ git cherry-pick ^master topic
So the order of the arguments specified on the command-line is
irrelevant in these cases. However, there are cases where it is worth
paying attention to the order. For instance:
$ git cherry-pick commit3 commit1 commit2
picks commits after sorting by date order, which is counter-intuitive.
Add a failing test to t3508 (cherry-pick-many-commits) documenting
this behavior.
Reported-by: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Irrespective of how far we get with the '--literal-order' idea, I
think this quirk is worth documenting.
t/t3508-cherry-pick-many-commits.sh | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index 8e09fd0..dd65835 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -59,6 +59,31 @@ test_expect_success 'cherry-pick first..fourth works' '
check_head_differs_from fourth
'
+test_expect_failure 'cherry-pick picks commits in the right order' '
+ cat <<-\EOF >expected &&
+ [master OBJID] fourth
+ Author: A U Thor <author@example.com>
+ 1 files changed, 1 insertions(+), 0 deletions(-)
+ [master OBJID] second
+ Author: A U Thor <author@example.com>
+ 1 files changed, 1 insertions(+), 0 deletions(-)
+ [master OBJID] third
+ Author: A U Thor <author@example.com>
+ 1 files changed, 1 insertions(+), 0 deletions(-)
+ EOF
+
+ git checkout -f master &&
+ git reset --hard first &&
+ test_tick &&
+ git cherry-pick fourth second third >actual &&
+ git diff --quiet other &&
+ git diff --quiet HEAD other &&
+
+ sed -e "s/$_x05[0-9a-f][0-9a-f]/OBJID/" <actual >actual.fuzzy &&
+ test_cmp expected actual.fuzzy &&
+ check_head_differs_from second
+'
+
test_expect_success 'cherry-pick --strategy resolve first..fourth works' '
cat <<-\EOF >expected &&
Trying simple merge.
--
1.7.8.2
^ permalink raw reply related
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Jonathan Nieder @ 2012-01-12 17:47 UTC (permalink / raw)
To: Jeff King
Cc: SZEDER Gábor, Christian Couder, Christian Couder, git,
Ramkumar Ramachandra
In-Reply-To: <20120112165329.GA17173@sigill.intra.peff.net>
Jeff King wrote:
> I agree it would be nice to make:
>
> git cherry-pick commit1 commit3 commit2
>
> work in the order specified, but how does that interact with existing
> cases that provide more traditional revision arguments?
Yes, exactly. Another question: what should
git cherry-pick master..next maint..master
do?
^ permalink raw reply
* Re: [PATCH] diff --no-index: support more than one file pair
From: Nguyen Thai Ngoc Duy @ 2012-01-12 17:40 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqmx9te08z.fsf@bauges.imag.fr>
On Thu, Jan 12, 2012 at 4:30 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
>> - might be easier to script (just throw them all to xargs)
>
> I don't see a use-case where a command produces old1 new1 old2 new2, but
> if there is one, then "| xargs -n 2 diff" is the solution. You don't
> need your patch.
Unthorough thought. I agree with you.
>> - hell, i might even benefit from git copy/modify detection
>
> I don't see how, if you specify explicitely the pairs (old, new). You
> may have such benefit if you let the command-line express "here's a
> bunch of old files, and a bunch of new ones", but not with your proposed
> syntax.
That's what git gives to diff machinery: a set of file pairs, and the
diff machinery has to figure out copy/modify pairs, shuffling them up
if necessary. I simply cut of tree traversal part out and feed file
pairs directly to diff machinery. I remember long long time ago Junio
asked for assistance about code moving support within a file. It has
not come up (at least in public), but one can hope it'll come someday.
--
Duy
^ permalink raw reply
* Re: git diff <file> HEAD^:<file> error message
From: Nguyen Thai Ngoc Duy @ 2012-01-12 17:34 UTC (permalink / raw)
To: Carlos Martín Nieto, git; +Cc: Jonathan Nieder
In-Reply-To: <20120111111831.GB15232@beez.lab.cmartin.tk>
On Wed, Jan 11, 2012 at 6:18 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> Hello,
>
> I was trying to figure out why running
>
> git diff HEAD^:RelNotes RelNotes
>
> gives the expected output (on maint it tells me that the stable
> version changed from 1.7.8.3 to 1.7.8.4) but swapping the arguments
> doesn't.
>
> git diff RelNotes HEAD^:RelNotes
>
> doesn't show the opposite patch but tells me that RelNotes doesn't
> exist in HEAD^ which is clearly a lie (it sounds like it's a
> misunderstanding on git's part, but it's certainly not the truth).
I find Jonathan's comment [1] interesting: "Meanwhile, there is no
plumbing command to compare two blobs. Strange".
I _think_ the main purpose of git diff is to compare a stage (a
revision, index, worktree) with another stage, filtered by path and
blob-to-blob diff is a minor thing that is needed to support "git diff
<tag> <tag>" where both tags point to a tag. It'd be better to start a
new command that diff between two blobs (or files in worktree/index).
Something pretty close to --no-index. You would not need to mess up
with setup_revisions() or verify_filename().
[1] ed84e6d (Documentation: diff can compare blobs - 2010-10-11)
--
Duy
^ permalink raw reply
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Ramkumar Ramachandra @ 2012-01-12 17:26 UTC (permalink / raw)
To: Jeff King
Cc: SZEDER Gábor, Christian Couder, Christian Couder, git,
Jonathan Nieder
In-Reply-To: <20120112171536.GA18102@sigill.intra.peff.net>
Hi Peff,
Jeff King wrote:
> It may even be possible to use
> heuristics to identify when --literal-order is needed, and eventually it
> could go away.
Yeah, that would be really nice.
> My only concern is that there are other parts of the revision machinery
> that depend on the date-ordering of the commit list.
Agreed. Looking at it another way, it's an opportunity to read
revision.c, learn, and modernize some older parts :)
> What would happen,
> for example, with:
>
> git rev-list --literal-order --do-walk foo
>
> It probably doesn't make sense to allow literal-order without no-walk,
> anyway (which of course is the default in cherry-pick anyway, so it's
> not a big deal here).
I don't know if that particular case is a problem: there are some
mutually exclusive options in revision.c already like:
cannot combine --reverse with --graph
This'll just be another one of them.
> I'm also not sure what:
>
> git rev-list --literal-order foo..bar
>
> would or should do.
Instead of classifying it as an "ordering" option (as defined in
Documentation/rev-list-options.txt), I think we should give it some
sort of special status for now -- it can be combined with ordering
options (of which date ordering is default anyway). For this specific
question, I suspect that revision.c does a topo-ordering for commit
ranges (I haven't read the code), so we have to make sure that
whatever extra logic we add doesn't disrupt the existing logic in
revision.c.
-- Ram
^ permalink raw reply
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Ramkumar Ramachandra @ 2012-01-12 17:14 UTC (permalink / raw)
To: Jeff King
Cc: SZEDER Gábor, Christian Couder, Christian Couder, git,
Jonathan Nieder
In-Reply-To: <CALkWK0=Mv_tzNw-hN_9fAr+vABappndEK5iSWQHDk8Yk6Z-stw@mail.gmail.com>
Ramkumar Ramachandra wrote:
> My current worktree (WIP):
> [...]
Classic whitespace breakage. How many times am I going to fall for
the same joke?
diff --git a/builtin/revert.c b/builtin/revert.c
index 0d8020c..47da41b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -228,6 +228,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
opts->revs = xmalloc(sizeof(*opts->revs));
init_revisions(opts->revs, NULL);
opts->revs->no_walk = 1;
+ opts->revs->literal_order = 1;
if (argc < 2)
usage_with_options(usage_str, options);
argc = setup_revisions(argc, argv, opts->revs, NULL);
diff --git a/revision.c b/revision.c
index 064e351..301ef58 100644
--- a/revision.c
+++ b/revision.c
@@ -2054,7 +2054,10 @@ int prepare_revision_walk(struct rev_info *revs)
if (commit) {
if (!(commit->object.flags & SEEN)) {
commit->object.flags |= SEEN;
- commit_list_insert_by_date(commit, &revs->commits);
+ if (revs->literal_order)
+ commit_list_insert(commit, &revs->commits);
+ else
+ commit_list_insert_by_date(commit, &revs->commits);
}
}
e++;
diff --git a/revision.h b/revision.h
index b8e9223..65c3dc3 100644
--- a/revision.h
+++ b/revision.h
@@ -67,6 +67,7 @@ struct rev_info {
remove_empty_trees:1,
simplify_history:1,
lifo:1,
+ literal_order:1,
topo_order:1,
simplify_merges:1,
simplify_by_decoration:1,
^ permalink raw reply related
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Jeff King @ 2012-01-12 17:15 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: SZEDER Gábor, Christian Couder, Christian Couder, git,
Jonathan Nieder
In-Reply-To: <CALkWK0=Mv_tzNw-hN_9fAr+vABappndEK5iSWQHDk8Yk6Z-stw@mail.gmail.com>
On Thu, Jan 12, 2012 at 10:39:48PM +0530, Ramkumar Ramachandra wrote:
> Jeff King wrote:
> > I agree it would be nice to make:
> > git cherry-pick commit1 commit3 commit2
> >
> > work in the order specified, but how does that interact with existing
> > cases that provide more traditional revision arguments?
>
> What are your thoughts on making it a flag in the revision API to be
> activated with "cherry-pick --literal-order commit1 commit3 commit2"
> or similar? I'm not sure how to get it to reconcile with the more
> traditional revision arguments yet. My current worktree (WIP):
I think that is a sensible first-cut. It may even be possible to use
heuristics to identify when --literal-order is needed, and eventually it
could go away. But that is a much riskier feature that can be built on
top of the much safer proposal you are making.
> diff --git a/revision.c b/revision.c
> index 064e351..301ef58 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2054,7 +2054,10 @@ int prepare_revision_walk(struct rev_info *revs)
> if (commit) {
> if (!(commit->object.flags & SEEN)) {
> commit->object.flags |= SEEN;
> - commit_list_insert_by_date(commit,
> &revs->commits
> + if (revs->literal_order)
> + commit_list_insert(commit,
> &revs->commits
> + else
> +
> commit_list_insert_by_date(commit, &revs-
My only concern is that there are other parts of the revision machinery
that depend on the date-ordering of the commit list. What would happen,
for example, with:
git rev-list --literal-order --do-walk foo
It probably doesn't make sense to allow literal-order without no-walk,
anyway (which of course is the default in cherry-pick anyway, so it's
not a big deal here).
I'm also not sure what:
git rev-list --literal-order foo..bar
would or should do.
-Peff
^ permalink raw reply
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Ramkumar Ramachandra @ 2012-01-12 17:09 UTC (permalink / raw)
To: Jeff King
Cc: SZEDER Gábor, Christian Couder, Christian Couder, git,
Jonathan Nieder
In-Reply-To: <20120112165329.GA17173@sigill.intra.peff.net>
Hi Peff,
Jeff King wrote:
> I agree it would be nice to make:
> git cherry-pick commit1 commit3 commit2
>
> work in the order specified, but how does that interact with existing
> cases that provide more traditional revision arguments?
What are your thoughts on making it a flag in the revision API to be
activated with "cherry-pick --literal-order commit1 commit3 commit2"
or similar? I'm not sure how to get it to reconcile with the more
traditional revision arguments yet. My current worktree (WIP):
diff --git a/builtin/revert.c b/builtin/revert.c
index 0d8020c..47da41b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -228,6 +228,7 @@ static void parse_args(int argc, const char
**argv, struct re
opts->revs = xmalloc(sizeof(*opts->revs));
init_revisions(opts->revs, NULL);
opts->revs->no_walk = 1;
+ opts->revs->literal_order = 1;
if (argc < 2)
usage_with_options(usage_str, options);
argc = setup_revisions(argc, argv, opts->revs, NULL);
diff --git a/revision.c b/revision.c
index 064e351..301ef58 100644
--- a/revision.c
+++ b/revision.c
@@ -2054,7 +2054,10 @@ int prepare_revision_walk(struct rev_info *revs)
if (commit) {
if (!(commit->object.flags & SEEN)) {
commit->object.flags |= SEEN;
- commit_list_insert_by_date(commit,
&revs->commits
+ if (revs->literal_order)
+ commit_list_insert(commit,
&revs->commits
+ else
+
commit_list_insert_by_date(commit, &revs-
}
}
e++;
diff --git a/revision.h b/revision.h
index b8e9223..65c3dc3 100644
--- a/revision.h
+++ b/revision.h
@@ -67,6 +67,7 @@ struct rev_info {
remove_empty_trees:1,
simplify_history:1,
lifo:1,
+ literal_order:1,
topo_order:1,
simplify_merges:1,
simplify_by_decoration:1,
^ permalink raw reply related
* Re: [PATCH] git-blame.el: Fix compilation warnings.
From: Rüdiger Sonderfeld @ 2012-01-12 17:08 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, davidk, Sergei Organov, Kevin Ryde
In-Reply-To: <20120112162617.GA2479@burratino>
Hi,
On Thursday 12 January 2012 10:26:41 Jonathan Nieder wrote:
> These lines should be left out [*].
Sorry, I wasn't sure whether to remove them or not. I followed the description
in git-format-patch(1) on how to send patches with kmail. I'll remove them in
the future. Thanks for the advice.
> I assume this was prompted by warning messages like this one:
>
> In git-blame-cleanup:
> git-blame.el:306:6:Warning: `mapcar' called for effect; use `mapc' or
> `dolist' instead
>
> Looks reasonable to my very much untrained eyes, and it's consistent
> with the hints Kevin gave at [1].
Yes. I think the warnings are correct and should be addressed. E.g. Using
mapcar compared to mapc is slower due to the required accumulation of the
results and the additional garbage collection costs. It's not very dramatic
but there is no reason not to fix it imho.
Regards,
Rüdiger
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox