* Re: [PATCH 00/48] Handling more corner cases in merge-recursive.c
From: Elijah Newren @ 2011-08-04 17:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, jgfouca
In-Reply-To: <7v4o1y81sv.fsf@alter.siamese.dyndns.org>
On Wed, Aug 3, 2011 at 6:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Unfortunately I seem to have found a regression that manifests in the real
> life.
>
> When this series is merged to 'next', it mismerges a trivial renamed path.
I know the problem, and can think of a couple potential solutions but
I'm not sure which one of those (if any) should be used. I may need
advice from an unpack_trees expert.
This breakage was introduced in d487486 (merge-recursive: When we
detect we can skip an update, actually skip it 2011-06-08), and the
reason for the breakage is that was_tracked(path) is lying to us. Let
me explain...
If you run with GIT_MERGE_VERBOSITY=3, you'll note that you get the message
"Skipped two (merged same as existing)"
which is NOT the path it should be taking. The reason for this is
that the call to was_tracked('two') just before that is returning true
for us when the correct answer is false. Explaining why we get the
wrong answer requires understanding a couple things.
Using your simple reproduction recipe as an example to explain this,
the sha1sums and paths for your merge case are:
Base commit: d00491f... one
HEAD commit: 0cfbf08... one
side commit: d00491f... two
In other words, 'one' is modified in HEAD, and unchanged other than
being renamed (to 'two') on side.
Now, was_tracked() simply checks whether there is a cache entry in the
current index with either stage 0 or stage 2. Clearly, I was
expecting 'two' to only appear with stage 3, but that's not what
happened.
When unpacking these trees, we got three cache entries: one for path
'one' at stage 1, one for path 'one' at stage 2, and one for path
'two' at stage 0. Yes, stage 0 and not stage 3. The reason for this
is that this is case 2ALT from
Documentation/technical/trivial-merge.txt:
case ancest head remote result
----------------------------------------
...
2ALT (empty)+ *empty* remote remote
This means the threeway_merge code considers it a clean merge and
calls merged_entry on it, which clears the CE_STAGEMASK bits from the
cache entry flags.
Since the cache entry for path 'two' has the CE_STAGEMASK bits
cleared, when we call was_tracked on that path, we get back the answer
true. 'two' was not tracked in HEAD, though, so this is the wrong
answer and the source of the bug.
Now, the question is...how do we fix the was_tracked function? It
would be nice to make use of the original index we had before
unpacking, but that is overwritten at the end of unpack_trees. Should
we save it somewhere? Or should we save off all the tracked filenames
before unpacking into a linked list and then have was_tracked() use
that instead of looking it up in the current cache? (I'm slightly
worried that may be slow given the number of lookups that exist).
Thoughts?
Elijah
^ permalink raw reply
* Re: [PATCH] Tolerate zlib deflation with window size < 32Kb
From: Junio C Hamano @ 2011-08-04 17:11 UTC (permalink / raw)
To: Roberto Tyley; +Cc: Junio C Hamano, git, Roberto Tyley
In-Reply-To: <CAFY1edZyO7oYDi+tV2mxbhBHY_cf2F0bD7+KF9rxmKYygSFAjA@mail.gmail.com>
Roberto Tyley <roberto.tyley@gmail.com> writes:
> On 4 August 2011 00:56, Junio C Hamano <gitster@pobox.com> wrote:
>
>> roberto.tyley@gmail.com writes:
>> > - if (map[0] == 0x78 && !(word % 31))
>> > + if ((map[0] & 0x88) == 0x08 && !(word % 31))
>>
>> Are you sure about this 0x88? Isn't it 0x8F or something?
>
>
> Ah- yes, you're right - the bitmask should be 0x8F, because the
> lower nibble of a deflated zlib stream is 8 (8=deflate) and we want
> to match all 4 bits.
>
> Thankfully the rest of the logic still applies - I'll submit an
> updated patch in a minute, once I've conquered my embarrassment :-)
Heh, don't get embarrassed. It was your well-written in-code comment that
helped me to follow the new logic well enough to spot it.
^ permalink raw reply
* Re: [RFC 4/6] git-check-attr: Normalize paths
From: Junio C Hamano @ 2011-08-04 17:05 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git
In-Reply-To: <4E3A12B9.1010800@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
>> I think passing down "prefix" (i.e. where your $(cwd) was relative to the
>> root level of the working tree) and the user-supplied "pathspec" (which
>> typically is relative to that original $(cwd)) is the canonical approach.
>> The very original git worked only at the root level of the working tree,
>> with paths specified relative to the root level of the tree, so many code
>> do:
>>
>> - find out the root of the working tree;
>> - note where the $(cwd) was in "prefix";
>> - chdir to the root of the working tree;
>> - prepend the "prefix" to user supplied pathspec;
>> - forget all the complexity and work on the whole tree.
>>
>> Then the "prefix" gets stripped away from the beginning of the paths when
>> reporting.
>>
>> Your patch from a cursory look seems to follow that pattern, which is
>> good.
>
> Thanks for the explanation.
>
> Yes, my code follows the pattern, except that in this case it is
> unnecessary to chdir to the root of the working tree.
Just to make sure there is no misunderstanding. The chdir() should not be
in the core part of the system that you may want to libify.
The above pattern was developed primarily so that older utility functions
in the system that were written back when nobody ran git from anywhere
other than the top level of the working tree can be easily adapted to main
programs that can be launched from a subdirectory. The initial set-up part
of the program is responsible for figuring out "prefix", turning relative
paths given by the user into paths relative to the top of the working
tree, and then chdir'ing to the top.
After all that happens, the library-ish parts of the system only have to
deal with full paths relative to the root of the working tree. "prefix"
comes into play when reporting the results (i.e. showing paths relative to
user's $(cwd) in the output or in the error messages).
^ permalink raw reply
* [PATCH] fix gitweb syntax highlight utf-8 encode bug
From: zzs @ 2011-08-04 15:52 UTC (permalink / raw)
To: artagnon; +Cc: git, 张忠山
In-Reply-To: <CALkWK0kOTXyevPy0Ys+pWwGjr2NSo3+ifhWuPVU+=FL=LC=J7A@mail.gmail.com>
From: 张忠山 <zzs213@126.com>
Signed-off-by: 张忠山 <zzs213@126.com>
---
gitweb/gitweb.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index dab89f2..48def38 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6465,7 +6465,7 @@ sub git_blob {
$nr++;
$line = untabify($line);
printf qq!<div class="pre"><a id="l%i" href="%s#l%i" class="linenr">%4i</a> %s</div>\n!,
- $nr, esc_attr(href(-replay => 1)), $nr, $nr, $syntax ? $line : esc_html($line, -nbsp=>1);
+ $nr, esc_attr(href(-replay => 1)), $nr, $nr, $syntax ? to_utf8($line) : esc_html($line, -nbsp=>1);
}
}
close $fd
--
1.7.4.4
^ permalink raw reply related
* Re: Cleaning up history with git rebase
From: Michael Witten @ 2011-08-04 14:35 UTC (permalink / raw)
To: Patricia Bittencourt Egeland; +Cc: Ricky Egeland, git
In-Reply-To: <80e62eb56a83a138a79e112e382da4ae@linea.gov.br>
[Put your reply text below the quoted text.]
On Wed, 03 Aug 2011 17:58:26 -0300, Patricia Egeland wrote:
> On Mon, 01 Aug 2011 01:07:33 -0000, Michael Witten wrote:
>
>> Michael Witten wrote:
>>
>>> On Sun, 31 Jul 2011 18:44:43 -0300, Ricky, Egeland wrote:
>>>
>>>> On Jul 31, 2011, at 6:33 PM, Michael Witten wrote:
>>>>
>>>>> On Sun, Jul 31, 2011 at 20:21, Michael Witten <mfwitten@gmail.com>
>>>>> wrote:
>>>>>> Why are there conflicts anyway?
>>>>>
>>>>> Oh...
>>>>>
>>>>> I guess there were conflicts when the merge commit was made in
>>>>> the original repository, and these conflicts were resolved by
>>>>> the merge commit itself. Hence, when rebase tries to split up
>>>>> a merge by dealing with just the non-merge parents, you end up
>>>>> having to deal with the conflict again.
>>>>
>>>> Yes, I thought it was something like this going on, too. In the
>>>> pre-rebase history, when there is a commit with "Conflict:" and
>>>> listing file which is in the sub-repository history, this is a
>>>> point where rebase stops with a conflict.
>>>>
>>>>> Shouldn't rebase take this into account?
>>>>
>>>> Not sure. Seems that it does not, it makes me resolve the conflict
>>>> =
>>>> again.
>>>
>>> I think git rebase should take this into account is what I'm saying.
>>>
>>> The following implements what I think `git rebase' should be doing;
>>> run it instead of `git rebase' in your repo:
>>>
>>> git branch saved
>>> git rev-list HEAD --reverse --first-parent --parents |
>>> {
>>> read root
>>> git reset --hard $root
>>> rebase_head=$root
>>>
>>> while read commit first_parent other_parents; do
>>>
>>> if [ -z "$other_parents" ]; then
>>>
>>> git cherry-pick $commit
>>> rebase_head=$commit
>>>
>>> else
>>>
>>> for parent in $other_parents; do
>>>
>>> if ! git cherry-pick $parent; then
>>>
>>> git reset --hard $rebase_head
>>> git merge $other_parents
>>> git rm -rf .
>>> git checkout -- $commit
>>> git commit -aC $commit
>>> break
>>>
>>> fi
>>>
>>> done
>>>
>>> rebase_head=$(git rev-parse HEAD)
>>>
>>> fi
>>>
>>> done
>>> }
>>
>> Woops!
>>
>> This line:
>>
>> git checkout -- $commit
>>
>> should be:
>>
>> git checkout $commit -- .
>
> I tried to run the script in my repo. However, seems like the 'git
> merge $other_parents' process fails. In the script output I see some
> lines saying that files were not able to be merged, ie:
>
> warning: Cannot merge binary files:
> scienceportal/images/tabs/tabs-gray.png (HEAD vs.
> 84f6fc283861aa7c5798f58769789dd0b91a5e9d)
> warning: Cannot merge binary files: scienceportal/images/waiting.gif
> (HEAD vs. e033cbbf1e9d24b66cb55a04701c059dc945c1c3)
>
> Do you have some suggestion?
That's probably as expected; the script is coming across the conflict, but
it should be taking care of the conflict automatically.
Unfortunately, though, the results probably end up being almost completely
similar to the original un-rebased branch because the original script
actually has ANOTHER mistake (sorry!). See the updated version here (or
in your inbox):
http://marc.info/?l=git&m=131246773005168&w=2
Message-ID: <d62225a3cc5740cda7cb163a94d55892-mfwitten@gmail.com>
^ permalink raw reply
* Re: Cleaning up history with git rebase
From: Michael Witten @ 2011-08-04 14:18 UTC (permalink / raw)
To: Patricia Bittencourt Egeland; +Cc: Ricky Egeland, git
In-Reply-To: <1b66c8efe0214915be2c52a5aacd22de-mfwitten@gmail.com>
On Mon, 01 Aug 2011 01:07:33 -0000, Michael Witten wrote:
> Michael Witten wrote:
>
>> On Sun, 31 Jul 2011 18:44:43 -0300, Ricky, Egeland wrote:
>>
>>> On Jul 31, 2011, at 6:33 PM, Michael Witten wrote:
>>>
>>>> On Sun, Jul 31, 2011 at 20:21, Michael Witten <mfwitten@gmail.com> wrote:
>>>>> Why are there conflicts anyway?
>>>>
>>>> Oh...
>>>>
>>>> I guess there were conflicts when the merge commit was made in
>>>> the original repository, and these conflicts were resolved by
>>>> the merge commit itself. Hence, when rebase tries to split up
>>>> a merge by dealing with just the non-merge parents, you end up
>>>> having to deal with the conflict again.
>>>
>>> Yes, I thought it was something like this going on, too. In the
>>> pre-rebase history, when there is a commit with "Conflict:" and
>>> listing file which is in the sub-repository history, this is a
>>> point where rebase stops with a conflict.
>>>
>>>> Shouldn't rebase take this into account?
>>>
>>> Not sure. Seems that it does not, it makes me resolve the conflict =
>>> again.
>>
>> I think git rebase should take this into account is what I'm saying.
>>
>> The following implements what I think `git rebase' should be doing;
>> run it instead of `git rebase' in your repo:
>>
>> git branch saved
>> git rev-list HEAD --reverse --first-parent --parents |
>> {
>> read root
>> git reset --hard $root
>> rebase_head=$root
>>
>> while read commit first_parent other_parents; do
>>
>> if [ -z "$other_parents" ]; then
>>
>> git cherry-pick $commit
>> rebase_head=$commit
>>
>> else
>>
>> for parent in $other_parents; do
>>
>> if ! git cherry-pick $parent; then
>>
>> git reset --hard $rebase_head
>> git merge $other_parents
>> git rm -rf .
>> git checkout -- $commit
>> git commit -aC $commit
>> break
>>
>> fi
>>
>> done
>>
>> rebase_head=$(git rev-parse HEAD)
>>
>> fi
>>
>> done
>> }
>
> Woops!
>
> This line:
>
> git checkout -- $commit
>
> should be:
>
> git checkout $commit -- .
I noticed that my script has another problem; the line:
rebase_head=$commit
should be:
rebase_head=$(git rev-parse HEAD)
I was trying to make an optimization, but it's the wrong
logic :-/
Sorry for the confusion. Here is an updated version of
the entire script:
git branch saved
git rev-list HEAD --reverse --first-parent --parents |
{
read root
git reset --hard $root
rebase_head=$root
while read commit first_parent other_parents; do
if [ -z "$other_parents" ]; then
git cherry-pick $commit
else
for parent in $other_parents; do
if ! git cherry-pick $parent; then
git reset --hard $rebase_head
git merge $other_parents
git rm -rf .
git checkout $commit -- .
git commit -aC $commit
break
fi
done
fi
rebase_head=$(git rev-parse HEAD)
done
}
^ permalink raw reply
* Re: [PATCH] rebase -i: fix has_action
From: Sverre Rabbelier @ 2011-08-04 12:15 UTC (permalink / raw)
To: Noe Rubinstein; +Cc: git, gitster
In-Reply-To: <1312450780-5021-1-git-send-email-nrubinstein@proformatique.com>
Heya,
On Thu, Aug 4, 2011 at 11:39, Noe Rubinstein
<nrubinstein@proformatique.com> wrote:
> This patch fixes the bug by changing has_action to grep any line
> containing anything that is not a space nor a #.
Probably maint-worthy? Should this have a test too?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* [PATCH v17 7/7] bisect: add documentation for --no-checkout option.
From: Jon Seymour @ 2011-08-04 12:01 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, jrnieder, Jon Seymour
In-Reply-To: <1312459263-16911-1-git-send-email-jon.seymour@gmail.com>
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
Documentation/git-bisect.txt | 32 +++++++++++++++++++++++++++++++-
1 files changed, 31 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index ab60a18..41e6ca8 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending
on the subcommand:
git bisect help
- git bisect start [<bad> [<good>...]] [--] [<paths>...]
+ git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<paths>...]
git bisect bad [<rev>]
git bisect good [<rev>...]
git bisect skip [(<rev>|<range>)...]
@@ -263,6 +263,17 @@ rewind the tree to the pristine state. Finally the script should exit
with the status of the real test to let the "git bisect run" command loop
determine the eventual outcome of the bisect session.
+OPTIONS
+-------
+--no-checkout::
++
+Do not checkout the new working tree at each iteration of the bisection
+process. Instead just update a special reference named 'BISECT_HEAD' to make
+it point to the commit that should be tested.
++
+This option may be useful when the test you would perform in each step
+does not require a checked out tree.
+
EXAMPLES
--------
@@ -343,6 +354,25 @@ $ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
This shows that you can do without a run script if you write the test
on a single line.
+* Locate a good region of the object graph in a damaged repository
++
+------------
+$ git bisect start HEAD <known-good-commit> [ <boundary-commit> ... ] --no-checkout
+$ git bisect run sh -c '
+ GOOD=$(git for-each-ref "--format=%(objectname)" refs/bisect/good-*) &&
+ git rev-list --objects BISECT_HEAD --not $GOOD >tmp.$$ &&
+ git pack-objects --stdout >/dev/null <tmp.$$
+ rc=$?
+ rm -f tmp.$$
+ test $rc = 0'
+
+------------
++
+In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit that
+has at least one parent whose reachable graph is fully traversable in the sense
+required by 'git pack objects'.
+
+
SEE ALSO
--------
link:git-bisect-lk2009.html[Fighting regressions with git bisect],
--
1.7.6.353.g50d6f
^ permalink raw reply related
* [PATCH v17 6/7] bisect: add tests for the --no-checkout option.
From: Jon Seymour @ 2011-08-04 12:01 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, jrnieder, Jon Seymour
In-Reply-To: <1312459263-16911-1-git-send-email-jon.seymour@gmail.com>
These tests verify that git-bisect --no-checkout can successfully
bisect commit histories that reference damaged trees.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
t/t6030-bisect-porcelain.sh | 82 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 82 insertions(+), 0 deletions(-)
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 9ae2de8..4fb7d11 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -126,6 +126,18 @@ test_expect_success 'bisect reset removes packed refs' '
test -z "$(git for-each-ref "refs/heads/bisect")"
'
+test_expect_success 'bisect reset removes bisect state after --no-checkout' '
+ git bisect reset &&
+ git bisect start --no-checkout &&
+ git bisect good $HASH1 &&
+ git bisect bad $HASH3 &&
+ git bisect next &&
+ git bisect reset &&
+ test -z "$(git for-each-ref "refs/bisect/*")" &&
+ test -z "$(git for-each-ref "refs/heads/bisect")" &&
+ test -z "$(git for-each-ref "BISECT_HEAD")"
+'
+
test_expect_success 'bisect start: back in good branch' '
git branch > branch.output &&
grep "* other" branch.output > /dev/null &&
@@ -630,4 +642,74 @@ test_expect_success 'bisect fails if tree is broken on trial commit' '
test_cmp expected.missing-tree.default error.txt
'
+check_same()
+{
+ echo "Checking $1 is the same as $2" &&
+ git rev-parse "$1" > expected.same &&
+ git rev-parse "$2" > expected.actual &&
+ test_cmp expected.same expected.actual
+}
+
+test_expect_success 'bisect: --no-checkout - start commit bad' '
+ git bisect reset &&
+ git bisect start BROKEN_HASH7 BROKEN_HASH4 --no-checkout &&
+ check_same BROKEN_HASH6 BISECT_HEAD &&
+ git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - trial commit bad' '
+ git bisect reset &&
+ git bisect start broken BROKEN_HASH4 --no-checkout &&
+ check_same BROKEN_HASH6 BISECT_HEAD &&
+ git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - target before breakage' '
+ git bisect reset &&
+ git bisect start broken BROKEN_HASH4 --no-checkout &&
+ check_same BROKEN_HASH6 BISECT_HEAD &&
+ git bisect bad BISECT_HEAD &&
+ check_same BROKEN_HASH5 BISECT_HEAD &&
+ git bisect bad BISECT_HEAD &&
+ check_same BROKEN_HASH5 bisect/bad &&
+ git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - target in breakage' '
+ git bisect reset &&
+ git bisect start broken BROKEN_HASH4 --no-checkout &&
+ check_same BROKEN_HASH6 BISECT_HEAD &&
+ git bisect bad BISECT_HEAD &&
+ check_same BROKEN_HASH5 BISECT_HEAD &&
+ git bisect good BISECT_HEAD &&
+ check_same BROKEN_HASH6 bisect/bad &&
+ git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - target after breakage' '
+ git bisect reset &&
+ git bisect start broken BROKEN_HASH4 --no-checkout &&
+ check_same BROKEN_HASH6 BISECT_HEAD &&
+ git bisect good BISECT_HEAD &&
+ check_same BROKEN_HASH8 BISECT_HEAD &&
+ git bisect good BISECT_HEAD &&
+ check_same BROKEN_HASH9 bisect/bad &&
+ git bisect reset
+'
+
+test_expect_success 'bisect: demonstrate identification of damage boundary' "
+ git bisect reset &&
+ git checkout broken &&
+ git bisect start broken master --no-checkout &&
+ git bisect run sh -c '
+ GOOD=\$(git for-each-ref \"--format=%(objectname)\" refs/bisect/good-*) &&
+ git rev-list --objects BISECT_HEAD --not \$GOOD >tmp.\$\$ &&
+ git pack-objects --stdout >/dev/null < tmp.\$\$
+ rc=\$?
+ rm -f tmp.\$\$
+ test \$rc = 0' &&
+ check_same BROKEN_HASH6 bisect/bad &&
+ git bisect reset
+"
+
test_done
--
1.7.6.353.g50d6f
^ permalink raw reply related
* [PATCH v17 5/7] bisect: introduce --no-checkout support into porcelain.
From: Jon Seymour @ 2011-08-04 12:01 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, jrnieder, Jon Seymour
In-Reply-To: <1312459263-16911-1-git-send-email-jon.seymour@gmail.com>
git-bisect can now perform bisection of a history without performing
a checkout at each stage of the bisection process. Instead, HEAD is updated.
One use-case for this function is allow git bisect to be used with
damaged repositories where git checkout would fail because the tree
referenced by the commit is damaged.
It can also be used in other cases where actual checkout of the tree
is not required to progress the bisection.
Improved-by: Christian Couder <chriscool@tuxfamily.org>
Improved-by: Junio C Hamano <gitster@pobox.com>
Improved-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
git-bisect.sh | 48 +++++++++++++++++++++++++++++++++++++-----------
1 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index a44ffe1..b9c18dd 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -3,7 +3,7 @@
USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
LONG_USAGE='git bisect help
print this long help message.
-git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
+git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<pathspec>...]
reset bisect state and start bisection.
git bisect bad [<rev>]
mark <rev> a known-bad revision.
@@ -34,6 +34,16 @@ require_work_tree
_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+bisect_head()
+{
+ if test -f "$GIT_DIR/BISECT_HEAD"
+ then
+ echo BISECT_HEAD
+ else
+ echo HEAD
+ fi
+}
+
bisect_autostart() {
test -s "$GIT_DIR/BISECT_START" || {
(
@@ -69,6 +79,7 @@ bisect_start() {
orig_args=$(git rev-parse --sq-quote "$@")
bad_seen=0
eval=''
+ mode=''
while [ $# -gt 0 ]; do
arg="$1"
case "$arg" in
@@ -76,6 +87,11 @@ bisect_start() {
shift
break
;;
+ --no-checkout)
+ mode=--no-checkout
+ shift ;;
+ --*)
+ die "$(eval_gettext "unrecognised option: '\$arg'")" ;;
*)
rev=$(git rev-parse -q --verify "$arg^{commit}") || {
test $has_double_dash -eq 1 &&
@@ -107,7 +123,10 @@ bisect_start() {
then
# Reset to the rev from where we started.
start_head=$(cat "$GIT_DIR/BISECT_START")
- git checkout "$start_head" -- || exit
+ if test "z$mode" != "z--no-checkout"
+ then
+ git checkout "$start_head" --
+ fi
else
# Get rev from where we start.
case "$head" in
@@ -143,7 +162,10 @@ bisect_start() {
#
# Write new start state.
#
- echo "$start_head" >"$GIT_DIR/BISECT_START" &&
+ echo "$start_head" >"$GIT_DIR/BISECT_START" && {
+ test "z$mode" != "z--no-checkout" ||
+ git update-ref --no-deref BISECT_HEAD "$start_head"
+ } &&
git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
eval "$eval true" &&
echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
@@ -206,8 +228,8 @@ bisect_state() {
0,*)
die "$(gettext "Please call 'bisect_state' with at least one argument.")" ;;
1,bad|1,good|1,skip)
- rev=$(git rev-parse --verify HEAD) ||
- die "$(gettext "Bad rev input: HEAD")"
+ rev=$(git rev-parse --verify $(bisect_head)) ||
+ die "$(gettext "Bad rev input: $(bisect_head)")"
bisect_write "$state" "$rev"
check_expected_revs "$rev" ;;
2,bad|*,good|*,skip)
@@ -291,7 +313,7 @@ bisect_next() {
bisect_next_check good
# Perform all bisection computation, display and checkout
- git bisect--helper --next-all
+ git bisect--helper --next-all $(test -f "$GIT_DIR/BISECT_HEAD" && echo --no-checkout)
res=$?
# Check if we should exit because bisection is finished
@@ -340,12 +362,15 @@ bisect_reset() {
*)
usage ;;
esac
- if git checkout "$branch" -- ; then
- bisect_clean_state
- else
- die "$(eval_gettext "Could not check out original HEAD '\$branch'.
+ if ! test -f "$GIT_DIR/BISECT_HEAD"
+ then
+ if ! git checkout "$branch" --
+ then
+ die "$(eval_gettext "Could not check out original HEAD '\$branch'.
Try 'git bisect reset <commit>'.")"
+ fi
fi
+ bisect_clean_state
}
bisect_clean_state() {
@@ -362,7 +387,8 @@ bisect_clean_state() {
rm -f "$GIT_DIR/BISECT_RUN" &&
# Cleanup head-name if it got left by an old version of git-bisect
rm -f "$GIT_DIR/head-name" &&
-
+ git update-ref -d --no-deref BISECT_HEAD &&
+ # clean up BISECT_START last
rm -f "$GIT_DIR/BISECT_START"
}
--
1.7.6.353.g50d6f
^ permalink raw reply related
* [PATCH v17 4/7] bisect: introduce support for --no-checkout option.
From: Jon Seymour @ 2011-08-04 12:01 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, jrnieder, Jon Seymour
In-Reply-To: <1312459263-16911-1-git-send-email-jon.seymour@gmail.com>
If --no-checkout is specified, then the bisection process uses:
git update-ref --no-deref HEAD <trial>
at each trial instead of:
git checkout <trial>
Improved-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
bisect.c | 33 ++++++++++++++++++++++-----------
bisect.h | 2 +-
builtin/bisect--helper.c | 7 +++++--
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/bisect.c b/bisect.c
index dd7e8ed..c7b7d79 100644
--- a/bisect.c
+++ b/bisect.c
@@ -24,6 +24,7 @@ struct argv_array {
static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
+static const char *argv_update_ref[] = {"update-ref", "--no-deref", "BISECT_HEAD", NULL, NULL};
/* bits #0-15 in revision.h */
@@ -707,16 +708,23 @@ static void mark_expected_rev(char *bisect_rev_hex)
die("closing file %s: %s", filename, strerror(errno));
}
-static int bisect_checkout(char *bisect_rev_hex)
+static int bisect_checkout(char *bisect_rev_hex, int no_checkout)
{
int res;
mark_expected_rev(bisect_rev_hex);
argv_checkout[2] = bisect_rev_hex;
- res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
- if (res)
- exit(res);
+ if (no_checkout) {
+ argv_update_ref[3] = bisect_rev_hex;
+ if (run_command_v_opt(argv_update_ref, RUN_GIT_CMD))
+ die("update-ref --no-deref HEAD failed on %s",
+ bisect_rev_hex);
+ } else {
+ res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
+ if (res)
+ exit(res);
+ }
argv_show_branch[1] = bisect_rev_hex;
return run_command_v_opt(argv_show_branch, RUN_GIT_CMD);
@@ -788,7 +796,7 @@ static void handle_skipped_merge_base(const unsigned char *mb)
* - If one is "skipped", we can't know but we should warn.
* - If we don't know, we should check it out and ask the user to test.
*/
-static void check_merge_bases(void)
+static void check_merge_bases(int no_checkout)
{
struct commit_list *result;
int rev_nr;
@@ -806,7 +814,7 @@ static void check_merge_bases(void)
handle_skipped_merge_base(mb);
} else {
printf("Bisecting: a merge base must be tested\n");
- exit(bisect_checkout(sha1_to_hex(mb)));
+ exit(bisect_checkout(sha1_to_hex(mb), no_checkout));
}
}
@@ -849,7 +857,7 @@ static int check_ancestors(const char *prefix)
* If a merge base must be tested by the user, its source code will be
* checked out to be tested by the user and we will exit.
*/
-static void check_good_are_ancestors_of_bad(const char *prefix)
+static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)
{
const char *filename = git_path("BISECT_ANCESTORS_OK");
struct stat st;
@@ -868,7 +876,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix)
/* Check if all good revs are ancestor of the bad rev. */
if (check_ancestors(prefix))
- check_merge_bases();
+ check_merge_bases(no_checkout);
/* Create file BISECT_ANCESTORS_OK. */
fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
@@ -908,8 +916,11 @@ static void show_diff_tree(const char *prefix, struct commit *commit)
* We use the convention that exiting with an exit code 10 means that
* the bisection process finished successfully.
* In this case the calling shell script should exit 0.
+ *
+ * If no_checkout is non-zero, the bisection process does not
+ * checkout the trial commit but instead simply updates BISECT_HEAD.
*/
-int bisect_next_all(const char *prefix)
+int bisect_next_all(const char *prefix, int no_checkout)
{
struct rev_info revs;
struct commit_list *tried;
@@ -920,7 +931,7 @@ int bisect_next_all(const char *prefix)
if (read_bisect_refs())
die("reading bisect refs failed");
- check_good_are_ancestors_of_bad(prefix);
+ check_good_are_ancestors_of_bad(prefix, no_checkout);
bisect_rev_setup(&revs, prefix, "%s", "^%s", 1);
revs.limited = 1;
@@ -966,6 +977,6 @@ int bisect_next_all(const char *prefix)
"(roughly %d step%s)\n", nr, (nr == 1 ? "" : "s"),
steps, (steps == 1 ? "" : "s"));
- return bisect_checkout(bisect_rev_hex);
+ return bisect_checkout(bisect_rev_hex, no_checkout);
}
diff --git a/bisect.h b/bisect.h
index 0862ce5..22f2e4d 100644
--- a/bisect.h
+++ b/bisect.h
@@ -27,7 +27,7 @@ struct rev_list_info {
const char *header_prefix;
};
-extern int bisect_next_all(const char *prefix);
+extern int bisect_next_all(const char *prefix, int no_checkout);
extern int estimate_bisect_steps(int all);
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 5b22639..8d325a5 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -4,16 +4,19 @@
#include "bisect.h"
static const char * const git_bisect_helper_usage[] = {
- "git bisect--helper --next-all",
+ "git bisect--helper --next-all [--no-checkout]",
NULL
};
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
int next_all = 0;
+ int no_checkout = 0;
struct option options[] = {
OPT_BOOLEAN(0, "next-all", &next_all,
"perform 'git bisect next'"),
+ OPT_BOOLEAN(0, "no-checkout", &no_checkout,
+ "update BISECT_HEAD instead of checking out the current commit"),
OPT_END()
};
@@ -24,5 +27,5 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
usage_with_options(git_bisect_helper_usage, options);
/* next-all */
- return bisect_next_all(prefix);
+ return bisect_next_all(prefix, no_checkout);
}
--
1.7.6.353.g50d6f
^ permalink raw reply related
* [PATCH v17 3/7] bisect: add tests to document expected behaviour in presence of broken trees.
From: Jon Seymour @ 2011-08-04 12:00 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, jrnieder, Jon Seymour
In-Reply-To: <1312459263-16911-1-git-send-email-jon.seymour@gmail.com>
If the repo is broken, we expect bisect to fail.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
t/t6030-bisect-porcelain.sh | 48 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index b3d1b14..9ae2de8 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -581,5 +581,53 @@ test_expect_success 'erroring out when using bad path parameters' '
'
#
+# This creates a broken branch which cannot be checked out because
+# the tree created has been deleted.
#
+# H1-H2-H3-H4-H5-H6-H7 <--other
+# \
+# S5-S6'-S7'-S8'-S9 <--broken
+#
+# Commits marked with ' have a missing tree.
+#
+test_expect_success 'broken branch creation' '
+ git bisect reset &&
+ git checkout -b broken $HASH4 &&
+ git tag BROKEN_HASH4 $HASH4 &&
+ add_line_into_file "5(broken): first line on a broken branch" hello2 &&
+ git tag BROKEN_HASH5 &&
+ mkdir missing &&
+ :> missing/MISSING &&
+ git add missing/MISSING &&
+ git commit -m "6(broken): Added file that will be deleted"
+ git tag BROKEN_HASH6 &&
+ add_line_into_file "7(broken): second line on a broken branch" hello2 &&
+ git tag BROKEN_HASH7 &&
+ add_line_into_file "8(broken): third line on a broken branch" hello2 &&
+ git tag BROKEN_HASH8 &&
+ git rm missing/MISSING &&
+ git commit -m "9(broken): Remove missing file"
+ git tag BROKEN_HASH9 &&
+ rm .git/objects/39/f7e61a724187ab767d2e08442d9b6b9dab587d
+'
+
+echo "" > expected.ok
+cat > expected.missing-tree.default <<EOF
+fatal: unable to read tree 39f7e61a724187ab767d2e08442d9b6b9dab587d
+EOF
+
+test_expect_success 'bisect fails if tree is broken on start commit' '
+ git bisect reset &&
+ test_must_fail git bisect start BROKEN_HASH7 BROKEN_HASH4 2>error.txt &&
+ test_cmp expected.missing-tree.default error.txt
+'
+
+test_expect_success 'bisect fails if tree is broken on trial commit' '
+ git bisect reset &&
+ test_must_fail git bisect start BROKEN_HASH9 BROKEN_HASH4 2>error.txt &&
+ git reset --hard broken &&
+ git checkout broken &&
+ test_cmp expected.missing-tree.default error.txt
+'
+
test_done
--
1.7.6.353.g50d6f
^ permalink raw reply related
* [PATCH v17 2/7] bisect: use && to connect statements that are deferred with eval.
From: Jon Seymour @ 2011-08-04 12:00 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, jrnieder, Jon Seymour
In-Reply-To: <1312459263-16911-1-git-send-email-jon.seymour@gmail.com>
Christian Couder pointed out that the existing eval strategy
swallows an initial non-zero return. Using && to connect
the statements should fix this.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
git-bisect.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index 20f6dd5..a44ffe1 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -86,7 +86,7 @@ bisect_start() {
0) state='bad' ; bad_seen=1 ;;
*) state='good' ;;
esac
- eval="$eval bisect_write '$state' '$rev' 'nolog'; "
+ eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
shift
;;
esac
@@ -145,7 +145,7 @@ bisect_start() {
#
echo "$start_head" >"$GIT_DIR/BISECT_START" &&
git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
- eval "$eval" &&
+ eval "$eval true" &&
echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
#
# Check if we can proceed to the next bisect state.
--
1.7.6.353.g50d6f
^ permalink raw reply related
* [PATCH v17 1/7] bisect: move argument parsing before state modification.
From: Jon Seymour @ 2011-08-04 12:00 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, jrnieder, Jon Seymour
In-Reply-To: <1312459263-16911-1-git-send-email-jon.seymour@gmail.com>
Currently 'git bisect start' modifies some state prior to checking
that its arguments are valid.
This change moves argument validation before state modification
with the effect that state modification does not occur
unless argument validations succeeds.
An existing test is changed to check that new bisect state
is not created if arguments are invalid.
A new test is added to check that existing bisect state
is not modified if arguments are invalid.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
git-bisect.sh | 66 +++++++++++++++++++++---------------------
t/t6030-bisect-porcelain.sh | 14 +++++++--
2 files changed, 44 insertions(+), 36 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index b2186a8..20f6dd5 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -60,6 +60,39 @@ bisect_autostart() {
bisect_start() {
#
+ # Check for one bad and then some good revisions.
+ #
+ has_double_dash=0
+ for arg; do
+ case "$arg" in --) has_double_dash=1; break ;; esac
+ done
+ orig_args=$(git rev-parse --sq-quote "$@")
+ bad_seen=0
+ eval=''
+ while [ $# -gt 0 ]; do
+ arg="$1"
+ case "$arg" in
+ --)
+ shift
+ break
+ ;;
+ *)
+ rev=$(git rev-parse -q --verify "$arg^{commit}") || {
+ test $has_double_dash -eq 1 &&
+ die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
+ break
+ }
+ case $bad_seen in
+ 0) state='bad' ; bad_seen=1 ;;
+ *) state='good' ;;
+ esac
+ eval="$eval bisect_write '$state' '$rev' 'nolog'; "
+ shift
+ ;;
+ esac
+ done
+
+ #
# Verify HEAD.
#
head=$(GIT_DIR="$GIT_DIR" git symbolic-ref -q HEAD) ||
@@ -98,39 +131,6 @@ bisect_start() {
bisect_clean_state || exit
#
- # Check for one bad and then some good revisions.
- #
- has_double_dash=0
- for arg; do
- case "$arg" in --) has_double_dash=1; break ;; esac
- done
- orig_args=$(git rev-parse --sq-quote "$@")
- bad_seen=0
- eval=''
- while [ $# -gt 0 ]; do
- arg="$1"
- case "$arg" in
- --)
- shift
- break
- ;;
- *)
- rev=$(git rev-parse -q --verify "$arg^{commit}") || {
- test $has_double_dash -eq 1 &&
- die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
- break
- }
- case $bad_seen in
- 0) state='bad' ; bad_seen=1 ;;
- *) state='good' ;;
- esac
- eval="$eval bisect_write '$state' '$rev' 'nolog'; "
- shift
- ;;
- esac
- done
-
- #
# Change state.
# In case of mistaken revs or checkout error, or signals received,
# "bisect_auto_next" below may exit or misbehave.
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index b5063b6..b3d1b14 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -138,15 +138,23 @@ test_expect_success 'bisect start: back in good branch' '
grep "* other" branch.output > /dev/null
'
-test_expect_success 'bisect start: no ".git/BISECT_START" if junk rev' '
- git bisect start $HASH4 $HASH1 -- &&
- git bisect good &&
+test_expect_success 'bisect start: no ".git/BISECT_START" created if junk rev' '
+ git bisect reset &&
test_must_fail git bisect start $HASH4 foo -- &&
git branch > branch.output &&
grep "* other" branch.output > /dev/null &&
test_must_fail test -e .git/BISECT_START
'
+test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if junk rev' '
+ git bisect start $HASH4 $HASH1 -- &&
+ git bisect good &&
+ cp .git/BISECT_START saved &&
+ test_must_fail git bisect start $HASH4 foo -- &&
+ git branch > branch.output &&
+ grep "* (no branch)" branch.output > /dev/null &&
+ test_cmp saved .git/BISECT_START
+'
test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
git bisect start $HASH4 $HASH1 -- &&
git bisect good &&
--
1.7.6.353.g50d6f
^ permalink raw reply related
* [PATCH v17 0/7] bisect: Add support for --no-checkout option
From: Jon Seymour @ 2011-08-04 12:00 UTC (permalink / raw)
To: git; +Cc: chriscool, gitster, j6t, jnareb, jrnieder, Jon Seymour
Motivation
==========
For some bisection tasks, checking out the commit at each stage of the bisection process is unecessary or undesirable.
This series adds support for a --no-checkout option to git-bisect.
If specified on a start command, --no-checkout causes 'git bisect' to update BISECT_HEAD at each stage of the bisection process instead of checking out the commit at that point.
One application of the --no-checkout option is to find, within a partially damaged repository, a commit that has at least one parent whose graph is fully reachable in the sense of 'git pack-objects'.
For example:
git bisect start BISECT_HEAD <some-known-good-commit> <boundary-commits> --no-checkout
git bisect run sh -c '
GOOD=$(git for-each-ref "--format=%(objectname)" refs/bisect/good-*) &&
git rev-list --objects BISECT_HEAD --not $GOOD >tmp.$$ &&
git pack-objects --stdout >/dev/null <tmp.$$
rc=$?
rm -f tmp.$$
test $rc = 0'
<some-known-good-commit> is a known good commit, for which the test passes.
<boundary-commits> are commits chosen to prevent the bisection visiting missing or corrupt commit objects.
Assuming this git bisect run completes successfully, bisect/bad will refer to a commit which has at least one parent that is fully reachable in the sense of 'git pack-objects'.
Patch Synopsis
==============
Remediation
-----------
Patch 1/7 changes existing behaviour in the case that an invalid revision argument is supplied to 'git bisect start'. In particular, in this case, bisection state is neither created or modified if argument validation fails. Previously, existing bisection state would be cleared even if the revision arguments were subsequently determined to be invalid.
Patch 2/7 remediates a potential flaw that might hide a failure in a chain of pasted statements.
Patch 3/7 adds a test which documents the existing behaviour of git bisect in the presence of tree damage.
New Function
------------
Patch 4/7 modifies the C code that supports bisection.
Patch 5/7 modifies porcelain to enable option exposed by 4/7.
Patch 6/7 adds some tests.
Patch 7/7 adds some documentation.
Revision History
----------------
v17:
Removed 2 trailing semi-colons.
Fix errors found by Junio in documented example and test.
Fixed instances of "; then" in new and modified code.
v16:
Use --no-def with update-ref -d.
Ensure update-ref BISECT_HEAD is created after BISECT_START and destroyed before BISECT_START. (Christian Couder)
dash compatability (Jonathan Nieder).
Documentation and test tweaks (Junio Hamano).
v15:
Fixed reset behaviour in --no-checkout case. Added one test for same.
Simplified implementation so that no-checkout mode is inferred by presence of
$GIT_DIR/BISECT_HEAD eliminating the need for a separate BISECT_MODE control file.
Patch 8/8 from v13/14 was redistributed and squashed into earlier commits.
Style and documentation edits based on feedback from Christian Coulder.
v14:
Reverted --bisect-mode aspect of v13 change so C code matches v11.
v13:
Following suggestions from Junio:
* Replaced BISECT_NO_CHECKOUT control file with BISECT_MODE.
* Changed name of internal option on bisect--helper from --no-checkout to --bisect-mode=checkout|update-ref.
* Changed --no-checkout bisections to update BISECT_HEAD instead of HEAD.
v11:
Removed support for --update-ref=<ref>, per Junio's preference.
v10:
Changed the way deferred statements are connected. Reverted some whitespace minimization.
v8:
Further feedback from Christian Couder. Support --update-ref <ref>.
v6:
This series includes numerous improvements suggested by Christian Couder.
Reworks:
"bisect: allow git bisect to be used with repos containing damaged trees."
Replaced --ignore-checkout-failure with --no-checkout option suggested by Junio.
Future series
-------------
* Implement full support for bisection in bare repositories.
* Fix whitespace, "; then" issues in git-bisect.h (patch that applies on v17 is available).
Jon Seymour (7):
bisect: move argument parsing before state modification.
bisect: use && to connect statements that are deferred with eval.
bisect: add tests to document expected behaviour in presence of
broken trees.
bisect: introduce support for --no-checkout option.
bisect: introduce --no-checkout support into porcelain.
bisect: add tests for the --no-checkout option.
bisect: add documentation for --no-checkout option.
Documentation/git-bisect.txt | 32 +++++++++-
bisect.c | 33 +++++++---
bisect.h | 2 +-
builtin/bisect--helper.c | 7 ++-
git-bisect.sh | 116 +++++++++++++++++++++-------------
t/t6030-bisect-porcelain.sh | 144 +++++++++++++++++++++++++++++++++++++++++-
6 files changed, 271 insertions(+), 63 deletions(-)
--
1.7.6.353.g50d6f
^ permalink raw reply
* Re: What's cooking in git.git (Aug 2011, #01; Wed, 3)
From: Ramkumar Ramachandra @ 2011-08-04 10:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzkjq6mpn.fsf@alter.siamese.dyndns.org>
Hi Junio,
Junio C Hamano writes:
> * rr/revert-cherry-pick-continue (2011-08-01) 18 commits
[...]
> Rerolled (v5) and looked reasonably clean.
> Will merge to "next".
Please use v6 instead [1].
Thanks.
[1]: http://mid.gmane.org/1312454356-3070-1-git-send-email-artagnon@gmail.com
-- Ram
^ permalink raw reply
* [PATCH 18/18] revert: Propagate errors upwards from do_pick_commit
From: Ramkumar Ramachandra @ 2011-08-04 10:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312454356-3070-1-git-send-email-artagnon@gmail.com>
Currently, revert_or_cherry_pick can fail in two ways. If it
encounters a conflict, it returns a positive number indicating the
intended exit status for the git wrapper to pass on; for all other
errors, it calls die(). The latter behavior is inconsiderate towards
callers, as it denies them the opportunity to recover from errors and
do other things.
After this patch, revert_or_cherry_pick will still return a positive
return value to indicate an exit status for conflicts as before, while
for some other errors, it will print an error message and return -1
instead of die()-ing. The cmd_revert and cmd_cherry_pick are adjusted
to handle the fatal errors by die()-ing themselves.
While the full benefits of this patch will only be seen once all the
"die" calls are replaced with calls to "error", its immediate impact
is to change some "fatal:" messages to say "error:" and to add a new
"fatal: cherry-pick failed" message at the end when the operation
fails.
Inspired-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 86 +++++++++++++++++++++++++-----------------------------
1 files changed, 40 insertions(+), 46 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 4f0d8f1..8b452e8 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -66,15 +66,6 @@ struct replay_opts {
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
-static void fatal(const char *advice, ...)
-{
- va_list params;
-
- va_start(params, advice);
- vreportf("fatal: ", advice, params);
- va_end(params);
-}
-
static const char *action_name(const struct replay_opts *opts)
{
return opts->action == REVERT ? "revert" : "cherry-pick";
@@ -356,25 +347,20 @@ static struct tree *empty_tree(void)
return tree;
}
-static NORETURN void die_dirty_index(struct replay_opts *opts)
+static int error_dirty_index(struct replay_opts *opts)
{
- if (read_cache_unmerged()) {
- die_resolve_conflict(action_name(opts));
- } else {
- if (advice_commit_before_merge) {
- if (opts->action == REVERT)
- die(_("Your local changes would be overwritten by revert.\n"
- "Please, commit your changes or stash them to proceed."));
- else
- die(_("Your local changes would be overwritten by cherry-pick.\n"
- "Please, commit your changes or stash them to proceed."));
- } else {
- if (opts->action == REVERT)
- die(_("Your local changes would be overwritten by revert.\n"));
- else
- die(_("Your local changes would be overwritten by cherry-pick.\n"));
- }
- }
+ if (read_cache_unmerged())
+ return error_resolve_conflict(action_name(opts));
+
+ /* Different translation strings for cherry-pick and revert */
+ if (opts->action == CHERRY_PICK)
+ error(_("Your local changes would be overwritten by cherry-pick."));
+ else
+ error(_("Your local changes would be overwritten by revert."));
+
+ if (advice_commit_before_merge)
+ advise(_("Commit your changes or stash them to proceed."));
+ return -1;
}
static int fast_forward_to(const unsigned char *to, const unsigned char *from)
@@ -492,9 +478,9 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
die (_("Your index file is unmerged."));
} else {
if (get_sha1("HEAD", head))
- die (_("You do not have a valid HEAD"));
+ return error(_("You do not have a valid HEAD"));
if (index_differs_from("HEAD", 0))
- die_dirty_index(opts);
+ return error_dirty_index(opts);
}
discard_cache();
@@ -507,20 +493,20 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
struct commit_list *p;
if (!opts->mainline)
- die(_("Commit %s is a merge but no -m option was given."),
- sha1_to_hex(commit->object.sha1));
+ return error(_("Commit %s is a merge but no -m option was given."),
+ sha1_to_hex(commit->object.sha1));
for (cnt = 1, p = commit->parents;
cnt != opts->mainline && p;
cnt++)
p = p->next;
if (cnt != opts->mainline || !p)
- die(_("Commit %s does not have parent %d"),
- sha1_to_hex(commit->object.sha1), opts->mainline);
+ return error(_("Commit %s does not have parent %d"),
+ sha1_to_hex(commit->object.sha1), opts->mainline);
parent = p->item;
} else if (0 < opts->mainline)
- die(_("Mainline was specified but commit %s is not a merge."),
- sha1_to_hex(commit->object.sha1));
+ return error(_("Mainline was specified but commit %s is not a merge."),
+ sha1_to_hex(commit->object.sha1));
else
parent = commit->parents->item;
@@ -530,12 +516,12 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
if (parent && parse_commit(parent) < 0)
/* TRANSLATORS: The first %s will be "revert" or
"cherry-pick", the second %s a SHA1 */
- die(_("%s: cannot parse parent commit %s"),
- action_name(opts), sha1_to_hex(parent->object.sha1));
+ return error(_("%s: cannot parse parent commit %s"),
+ action_name(opts), sha1_to_hex(parent->object.sha1));
if (get_message(commit, &msg) != 0)
- die(_("Cannot get commit message for %s"),
- sha1_to_hex(commit->object.sha1));
+ return error(_("Cannot get commit message for %s"),
+ sha1_to_hex(commit->object.sha1));
/*
* "commit" is an existing commit. We would want to apply
@@ -997,27 +983,28 @@ static int pick_revisions(struct replay_opts *opts)
walk_revs_populate_todo(&todo_list, opts);
if (create_seq_dir() < 0) {
- fatal(_("A cherry-pick or revert is in progress."));
+ error(_("A cherry-pick or revert is in progress."));
advise(_("Use --continue to continue the operation"));
advise(_("or --reset to forget about it"));
- exit(128);
+ return -1;
}
if (get_sha1("HEAD", sha1)) {
if (opts->action == REVERT)
- die(_("Can't revert as initial commit"));
- die(_("Can't cherry-pick into empty head"));
+ return error(_("Can't revert as initial commit"));
+ return error(_("Can't cherry-pick into empty head"));
}
save_head(sha1_to_hex(sha1));
save_opts(opts);
}
return pick_commits(todo_list, opts);
error:
- die(_("No %s in progress"), action_name(opts));
+ return error(_("No %s in progress"), action_name(opts));
}
int cmd_revert(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts;
+ int res;
memset(&opts, 0, sizeof(opts));
if (isatty(0))
@@ -1025,16 +1012,23 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
opts.action = REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- return pick_revisions(&opts);
+ res = pick_revisions(&opts);
+ if (res < 0)
+ die(_("revert failed"));
+ return res;
}
int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts;
+ int res;
memset(&opts, 0, sizeof(opts));
opts.action = CHERRY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- return pick_revisions(&opts);
+ res = pick_revisions(&opts);
+ if (res < 0)
+ die(_("cherry-pick failed"));
+ return res;
}
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 17/18] revert: Introduce --continue to continue the operation
From: Ramkumar Ramachandra @ 2011-08-04 10:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312454356-3070-1-git-send-email-artagnon@gmail.com>
Introduce a new "git cherry-pick --continue" command which uses the
information in ".git/sequencer" to continue a cherry-pick that stopped
because of a conflict or other error. It works by dropping the first
instruction from .git/sequencer/todo and performing the remaining
cherry-picks listed there, with options (think "-s" and "-X") from the
initial command listed in ".git/sequencer/opts".
So now you can do:
$ git cherry-pick -Xpatience foo..bar
... description conflict in commit moo ...
$ git cherry-pick --continue
error: 'cherry-pick' is not possible because you have unmerged files.
fatal: failed to resume cherry-pick
$ echo resolved >conflictingfile
$ git add conflictingfile && git commit
$ git cherry-pick --continue; # resumes with the commit after "moo"
During the "git commit" stage, CHERRY_PICK_HEAD will aid by providing
the commit message from the conflicting "moo" commit. Note that the
cherry-pick mechanism has no control at this stage, so the user is
free to violate anything that was specified during the first
cherry-pick invocation. For example, if "-x" was specified during the
first cherry-pick invocation, the user is free to edit out the message
during commit time. Note that the "--signoff" option specified at
cherry-pick invocation time is not reflected in the commit message
provided by CHERRY_PICK_HEAD; the user must take care to add
"--signoff" during the "git commit" invocation.
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Documentation/git-cherry-pick.txt | 1 +
Documentation/git-revert.txt | 1 +
Documentation/sequencer.txt | 5 +
builtin/revert.c | 188 ++++++++++++++++++++++++++++++++++++-
t/t3510-cherry-pick-sequence.sh | 96 +++++++++++++++++++
5 files changed, 287 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 12a85ba..b4a2d74 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -10,6 +10,7 @@ SYNOPSIS
[verse]
'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...
'git cherry-pick' --reset
+'git cherry-pick' --continue
DESCRIPTION
-----------
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 31c85b4..68f8e68 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -10,6 +10,7 @@ SYNOPSIS
[verse]
'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] <commit>...
'git revert' --reset
+'git revert' --continue
DESCRIPTION
-----------
diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt
index 16ce88c..3e6df33 100644
--- a/Documentation/sequencer.txt
+++ b/Documentation/sequencer.txt
@@ -2,3 +2,8 @@
Forget about the current operation in progress. Can be used
to clear the sequencer state after a failed cherry-pick or
revert.
+
+--continue::
+ Continue the operation in progress using the information in
+ '.git/sequencer'. Can be used to continue after resolving
+ conflicts in a failed cherry-pick or revert.
diff --git a/builtin/revert.c b/builtin/revert.c
index 19b6739..4f0d8f1 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -40,7 +40,7 @@ static const char * const cherry_pick_usage[] = {
};
enum replay_action { REVERT, CHERRY_PICK };
-enum replay_subcommand { REPLAY_NONE, REPLAY_RESET };
+enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
struct replay_opts {
enum replay_action action;
@@ -117,14 +117,37 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
}
+static void verify_opt_mutually_compatible(const char *me, ...)
+{
+ const char *opt1, *opt2;
+ va_list ap;
+
+ va_start(ap, me);
+ while ((opt1 = va_arg(ap, const char *))) {
+ if (va_arg(ap, int))
+ break;
+ }
+ if (opt1) {
+ while ((opt2 = va_arg(ap, const char *))) {
+ if (va_arg(ap, int))
+ break;
+ }
+ }
+
+ if (opt1 && opt2)
+ die(_("%s: %s cannot be used with %s"), me, opt1, opt2);
+}
+
static void parse_args(int argc, const char **argv, struct replay_opts *opts)
{
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
const char *me = action_name(opts);
int noop;
int reset = 0;
+ int contin = 0;
struct option options[] = {
OPT_BOOLEAN(0, "reset", &reset, "forget the current operation"),
+ OPT_BOOLEAN(0, "continue", &contin, "continue the current operation"),
OPT_BOOLEAN('n', "no-commit", &opts->no_commit, "don't automatically commit"),
OPT_BOOLEAN('e', "edit", &opts->edit, "edit the commit message"),
{ OPTION_BOOLEAN, 'r', NULL, &noop, NULL, "no-op (backward compatibility)",
@@ -154,15 +177,29 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
PARSE_OPT_KEEP_ARGV0 |
PARSE_OPT_KEEP_UNKNOWN);
+ /* Check for incompatible subcommands */
+ verify_opt_mutually_compatible(me,
+ "--reset", reset,
+ "--continue", contin,
+ NULL);
+
/* Set the subcommand */
if (reset)
opts->subcommand = REPLAY_RESET;
+ else if (contin)
+ opts->subcommand = REPLAY_CONTINUE;
else
opts->subcommand = REPLAY_NONE;
/* Check for incompatible command line arguments */
- if (opts->subcommand == REPLAY_RESET) {
- verify_opt_compatible(me, "--reset",
+ if (opts->subcommand != REPLAY_NONE) {
+ char *this_operation;
+ if (opts->subcommand == REPLAY_RESET)
+ this_operation = "--reset";
+ else
+ this_operation = "--continue";
+
+ verify_opt_compatible(me, this_operation,
"--no-commit", opts->no_commit,
"--signoff", opts->signoff,
"--mainline", opts->mainline,
@@ -668,6 +705,137 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
return 0;
}
+static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
+{
+ unsigned char commit_sha1[20];
+ char sha1_abbrev[40];
+ enum replay_action action;
+ int insn_len = 0;
+ char *p, *q;
+
+ if (!prefixcmp(start, "pick ")) {
+ action = CHERRY_PICK;
+ insn_len = strlen("pick");
+ p = start + insn_len + 1;
+ } else if (!prefixcmp(start, "revert ")) {
+ action = REVERT;
+ insn_len = strlen("revert");
+ p = start + insn_len + 1;
+ } else
+ return NULL;
+
+ q = strchr(p, ' ');
+ if (!q)
+ return NULL;
+ q++;
+
+ strlcpy(sha1_abbrev, p, q - p);
+
+ /*
+ * Verify that the action matches up with the one in
+ * opts; we don't support arbitrary instructions
+ */
+ if (action != opts->action) {
+ const char *action_str;
+ action_str = action == REVERT ? "revert" : "cherry-pick";
+ error(_("Cannot %s during a %s"), action_str, action_name(opts));
+ return NULL;
+ }
+
+ if (get_sha1(sha1_abbrev, commit_sha1) < 0)
+ return NULL;
+
+ return lookup_commit_reference(commit_sha1);
+}
+
+static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
+ struct replay_opts *opts)
+{
+ struct commit_list **next = todo_list;
+ struct commit *commit;
+ char *p = buf;
+ int i;
+
+ for (i = 1; *p; i++) {
+ commit = parse_insn_line(p, opts);
+ if (!commit)
+ return error(_("Could not parse line %d."), i);
+ next = commit_list_append(commit, next);
+ p = strchrnul(p, '\n');
+ if (*p)
+ p++;
+ }
+ if (!*todo_list)
+ return error(_("No commits parsed."));
+ return 0;
+}
+
+static void read_populate_todo(struct commit_list **todo_list,
+ struct replay_opts *opts)
+{
+ const char *todo_file = git_path(SEQ_TODO_FILE);
+ struct strbuf buf = STRBUF_INIT;
+ int fd, res;
+
+ fd = open(todo_file, O_RDONLY);
+ if (fd < 0)
+ die_errno(_("Could not open %s."), todo_file);
+ if (strbuf_read(&buf, fd, 0) < 0) {
+ close(fd);
+ strbuf_release(&buf);
+ die(_("Could not read %s."), todo_file);
+ }
+ close(fd);
+
+ res = parse_insn_buffer(buf.buf, todo_list, opts);
+ strbuf_release(&buf);
+ if (res)
+ die(_("Unusable instruction sheet: %s"), todo_file);
+}
+
+static int populate_opts_cb(const char *key, const char *value, void *data)
+{
+ struct replay_opts *opts = data;
+ int error_flag = 1;
+
+ if (!value)
+ error_flag = 0;
+ else if (!strcmp(key, "options.no-commit"))
+ opts->no_commit = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.edit"))
+ opts->edit = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.signoff"))
+ opts->signoff = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.record-origin"))
+ opts->record_origin = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.allow-ff"))
+ opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.mainline"))
+ opts->mainline = git_config_int(key, value);
+ else if (!strcmp(key, "options.strategy"))
+ git_config_string(&opts->strategy, key, value);
+ else if (!strcmp(key, "options.strategy-option")) {
+ ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
+ opts->xopts[opts->xopts_nr++] = xstrdup(value);
+ } else
+ return error(_("Invalid key: %s"), key);
+
+ if (!error_flag)
+ return error(_("Invalid value for %s: %s"), key, value);
+
+ return 0;
+}
+
+static void read_populate_opts(struct replay_opts **opts_ptr)
+{
+ const char *opts_file = git_path(SEQ_OPTS_FILE);
+
+ if (!file_exists(opts_file))
+ return;
+ if (git_config_from_file(populate_opts_cb, opts_file, *opts_ptr) < 0)
+ die(_("Malformed options sheet: %s"), opts_file);
+}
+
static void walk_revs_populate_todo(struct commit_list **todo_list,
struct replay_opts *opts)
{
@@ -811,6 +979,15 @@ static int pick_revisions(struct replay_opts *opts)
if (opts->subcommand == REPLAY_RESET) {
remove_sequencer_state(1);
return 0;
+ } else if (opts->subcommand == REPLAY_CONTINUE) {
+ if (!file_exists(git_path(SEQ_TODO_FILE)))
+ goto error;
+ read_populate_opts(&opts);
+ read_populate_todo(&todo_list, opts);
+
+ /* Verify that the conflict has been resolved */
+ if (!index_differs_from("HEAD", 0))
+ todo_list = todo_list->next;
} else {
/*
* Start a new cherry-pick/ revert sequence; but
@@ -821,7 +998,8 @@ static int pick_revisions(struct replay_opts *opts)
walk_revs_populate_todo(&todo_list, opts);
if (create_seq_dir() < 0) {
fatal(_("A cherry-pick or revert is in progress."));
- advise(_("Use --reset to forget about it"));
+ advise(_("Use --continue to continue the operation"));
+ advise(_("or --reset to forget about it"));
exit(128);
}
if (get_sha1("HEAD", sha1)) {
@@ -833,6 +1011,8 @@ static int pick_revisions(struct replay_opts *opts)
save_opts(opts);
}
return pick_commits(todo_list, opts);
+error:
+ die(_("No %s in progress"), action_name(opts));
}
int cmd_revert(int argc, const char **argv, const char *prefix)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 566a15e..78f3f01 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -115,4 +115,100 @@ test_expect_success 'cherry-pick does not implicitly stomp an existing operation
test_cmp expect actual
'
+test_expect_success '--continue complains when no cherry-pick is in progress' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick --continue
+'
+
+test_expect_success '--continue complains when there are unresolved conflicts' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ test_must_fail git cherry-pick --continue
+'
+
+test_expect_success '--continue continues after conflicts are resolved' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--continue respects opts' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick -x base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ git cat-file commit HEAD >anotherpick_msg &&
+ git cat-file commit HEAD~1 >picked_msg &&
+ git cat-file commit HEAD~2 >unrelatedpick_msg &&
+ git cat-file commit HEAD~3 >initial_msg &&
+ test_must_fail grep "cherry picked from" initial_msg &&
+ grep "cherry picked from" unrelatedpick_msg &&
+ grep "cherry picked from" picked_msg &&
+ grep "cherry picked from" anotherpick_msg
+'
+
+test_expect_success '--signoff is not automatically propogated to resolved conflict' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick --signoff base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ git cat-file commit HEAD >anotherpick_msg &&
+ git cat-file commit HEAD~1 >picked_msg &&
+ git cat-file commit HEAD~2 >unrelatedpick_msg &&
+ git cat-file commit HEAD~3 >initial_msg &&
+ test_must_fail grep "Signed-off-by:" initial_msg &&
+ grep "Signed-off-by:" unrelatedpick_msg &&
+ test_must_fail grep "Signed-off-by:" picked_msg &&
+ grep "Signed-off-by:" anotherpick_msg
+'
+
+test_expect_success 'malformed instruction sheet 1' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick /pick/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git cherry-pick --continue
+'
+
+test_expect_success 'malformed instruction sheet 2' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick/revert/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git cherry-pick --continue
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 16/18] revert: Don't implicitly stomp pending sequencer operation
From: Ramkumar Ramachandra @ 2011-08-04 10:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312454356-3070-1-git-send-email-artagnon@gmail.com>
Protect the user from forgetting about a pending sequencer operation
by immediately erroring out when an existing cherry-pick or revert
operation is in progress like:
$ git cherry-pick foo
... conflict ...
$ git cherry-pick moo
error: .git/sequencer already exists
hint: A cherry-pick or revert is in progress
hint: Use --reset to forget about it
fatal: cherry-pick failed
A naive version of this would break the following established ways of
working:
$ git cherry-pick foo
... conflict ...
$ git reset --hard # I actually meant "moo" when I said "foo"
$ git cherry-pick moo
$ git cherry-pick foo
... conflict ...
$ git commit # commit the resolution
$ git cherry-pick moo # New operation
However, the previous patches "reset: Make reset remove the sequencer
state" and "revert: Remove sequencer state when no commits are
pending" make sure that this does not happen.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 30 +++++++++++++++++++++++++-----
t/t3510-cherry-pick-sequence.sh | 9 +++++++++
2 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 000806c..19b6739 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -66,6 +66,15 @@ struct replay_opts {
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
+static void fatal(const char *advice, ...)
+{
+ va_list params;
+
+ va_start(params, advice);
+ vreportf("fatal: ", advice, params);
+ va_end(params);
+}
+
static const char *action_name(const struct replay_opts *opts)
{
return opts->action == REVERT ? "revert" : "cherry-pick";
@@ -673,13 +682,15 @@ static void walk_revs_populate_todo(struct commit_list **todo_list,
next = commit_list_append(commit, next);
}
-static void create_seq_dir(void)
+static int create_seq_dir(void)
{
const char *seq_dir = git_path(SEQ_DIR);
- if (!(file_exists(seq_dir) && is_directory(seq_dir))
- && mkdir(seq_dir, 0777) < 0)
+ if (file_exists(seq_dir))
+ return error(_("%s already exists."), seq_dir);
+ else if (mkdir(seq_dir, 0777) < 0)
die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
+ return 0;
}
static void save_head(const char *head)
@@ -801,9 +812,18 @@ static int pick_revisions(struct replay_opts *opts)
remove_sequencer_state(1);
return 0;
} else {
- /* Start a new cherry-pick/ revert sequence */
+ /*
+ * Start a new cherry-pick/ revert sequence; but
+ * first, make sure that an existing one isn't in
+ * progress
+ */
+
walk_revs_populate_todo(&todo_list, opts);
- create_seq_dir();
+ if (create_seq_dir() < 0) {
+ fatal(_("A cherry-pick or revert is in progress."));
+ advise(_("Use --reset to forget about it"));
+ exit(128);
+ }
if (get_sha1("HEAD", sha1)) {
if (opts->action == REVERT)
die(_("Can't revert as initial commit"));
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index a414086..566a15e 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -106,4 +106,13 @@ test_expect_success 'cherry-pick cleans up sequencer state when one commit is le
test_cmp expect actual
'
+test_expect_success 'cherry-pick does not implicitly stomp an existing operation' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ test-chmtime -v +0 .git/sequencer >expect &&
+ test_must_fail git cherry-pick unrelatedpick &&
+ test-chmtime -v +0 .git/sequencer >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 15/18] revert: Remove sequencer state when no commits are pending
From: Ramkumar Ramachandra @ 2011-08-04 10:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312454356-3070-1-git-send-email-artagnon@gmail.com>
When cherry-pick or revert is called on a list of commits, and a
conflict encountered somewhere in the middle, the data in
".git/sequencer" is required to continue the operation. However, when
a conflict is encountered in the very last commit, the user will have
to "continue" after resolving the conflict and committing just so that
the sequencer state is removed. This is how the current "rebase -i"
script works as well.
$ git cherry-pick foo..bar
... conflict encountered while picking "bar" ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
$ git cherry-pick --continue # This would be a no-op
Change this so that the sequencer state is cleared when a conflict is
encountered in the last commit. Incidentally, this patch makes sure
that some existing tests don't break when features like "--reset" and
"--continue" are implemented later in the series.
A better way to implement this feature is to get the last "git commit"
to remove the sequencer state. However, that requires tighter
coupling between "git commit" and the sequencer, a goal that can be
pursued once the sequencer is made more general.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 12 +++++++++++-
t/t3510-cherry-pick-sequence.sh | 24 ++++++++++++++++++++++++
2 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index a8accd6..000806c 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -763,8 +763,18 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
for (cur = todo_list; cur; cur = cur->next) {
save_todo(cur, opts);
res = do_pick_commit(cur->item, opts);
- if (res)
+ if (res) {
+ if (!cur->next)
+ /*
+ * An error was encountered while
+ * picking the last commit; the
+ * sequencer state is useless now --
+ * the user simply needs to resolve
+ * the conflict and commit
+ */
+ remove_sequencer_state(0);
return res;
+ }
}
/*
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index fd69865..a414086 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -82,4 +82,28 @@ test_expect_success '--reset cleans up sequencer state' '
test_path_is_missing .git/sequencer
'
+test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..picked &&
+ test_path_is_missing .git/sequencer &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 14/18] reset: Make reset remove the sequencer state
From: Ramkumar Ramachandra @ 2011-08-04 10:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312454356-3070-1-git-send-email-artagnon@gmail.com>
Years of muscle memory have trained users to use "git reset --hard" to
remove the branch state after any sort operation. Make it also remove
the sequencer state to facilitate this established workflow:
$ git cherry-pick foo..bar
... conflict encountered ...
$ git reset --hard # Oops, I didn't mean that
$ git cherry-pick quux..bar
... cherry-pick succeeded ...
Guard against accidental removal of the sequencer state by providing
one level of "undo". In the first "reset" invocation,
".git/sequencer" is moved to ".git/sequencer-old"; it is completely
removed only in the second invocation.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
branch.c | 2 ++
t/7106-reset-sequence.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+), 0 deletions(-)
create mode 100755 t/7106-reset-sequence.sh
diff --git a/branch.c b/branch.c
index c0c865a..d06aec4 100644
--- a/branch.c
+++ b/branch.c
@@ -3,6 +3,7 @@
#include "refs.h"
#include "remote.h"
#include "commit.h"
+#include "sequencer.h"
struct tracking {
struct refspec spec;
@@ -228,4 +229,5 @@ void remove_branch_state(void)
unlink(git_path("MERGE_MSG"));
unlink(git_path("MERGE_MODE"));
unlink(git_path("SQUASH_MSG"));
+ remove_sequencer_state(0);
}
diff --git a/t/7106-reset-sequence.sh b/t/7106-reset-sequence.sh
new file mode 100755
index 0000000..4956caa
--- /dev/null
+++ b/t/7106-reset-sequence.sh
@@ -0,0 +1,44 @@
+#!/bin/sh
+
+test_description='Test interaction of reset --hard with sequencer
+
+ + anotherpick: rewrites foo to d
+ + picked: rewrites foo to c
+ + unrelatedpick: rewrites unrelated to reallyunrelated
+ + base: rewrites foo to b
+ + initial: writes foo as a, unrelated as unrelated
+'
+
+. ./test-lib.sh
+
+pristine_detach () {
+ git cherry-pick --reset &&
+ git checkout -f "$1^0" &&
+ git read-tree -u --reset HEAD &&
+ git clean -d -f -f -q -x
+}
+
+test_expect_success setup '
+ echo unrelated >unrelated &&
+ git add unrelated &&
+ test_commit initial foo a &&
+ test_commit base foo b &&
+ test_commit unrelatedpick unrelated reallyunrelated &&
+ test_commit picked foo c &&
+ test_commit anotherpick foo d &&
+ git config advice.detachedhead false
+
+'
+
+test_expect_success 'reset --hard cleans up sequencer state, providing one-level undo' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ test_path_is_dir .git/sequencer &&
+ git reset --hard &&
+ test_path_is_missing .git/sequencer &&
+ test_path_is_dir .git/sequencer-old &&
+ git reset --hard &&
+ test_path_is_missing .git/sequencer-old
+'
+
+test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 13/18] revert: Introduce --reset to remove sequencer state
From: Ramkumar Ramachandra @ 2011-08-04 10:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312454356-3070-1-git-send-email-artagnon@gmail.com>
To explicitly remove the sequencer state for a fresh cherry-pick or
revert invocation, introduce a new subcommand called "--reset" to
remove the sequencer state.
Take the opportunity to publicly expose the sequencer paths, and a
generic function called "remove_sequencer_state" that various git
programs can use to remove the sequencer state in a uniform manner;
"git reset" uses it later in this series. Introducing this public API
is also in line with our long-term goal of eventually factoring out
functions from revert.c into a generic commit sequencer.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Documentation/git-cherry-pick.txt | 5 +++
Documentation/git-revert.txt | 5 +++
Documentation/sequencer.txt | 4 ++
Makefile | 2 +
builtin/revert.c | 62 +++++++++++++++++++++++++-----------
sequencer.c | 19 +++++++++++
sequencer.h | 20 ++++++++++++
t/t3510-cherry-pick-sequence.sh | 14 ++++++++-
8 files changed, 111 insertions(+), 20 deletions(-)
create mode 100644 Documentation/sequencer.txt
create mode 100644 sequencer.c
create mode 100644 sequencer.h
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 6c9c2cb..12a85ba 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -9,6 +9,7 @@ SYNOPSIS
--------
[verse]
'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...
+'git cherry-pick' --reset
DESCRIPTION
-----------
@@ -110,6 +111,10 @@ effect to your index in a row.
Pass the merge strategy-specific option through to the
merge strategy. See linkgit:git-merge[1] for details.
+SEQUENCER SUBCOMMANDS
+---------------------
+include::sequencer.txt[]
+
EXAMPLES
--------
git cherry-pick master::
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 3d0a7d1..31c85b4 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -9,6 +9,7 @@ SYNOPSIS
--------
[verse]
'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] <commit>...
+'git revert' --reset
DESCRIPTION
-----------
@@ -91,6 +92,10 @@ effect to your index in a row.
Pass the merge strategy-specific option through to the
merge strategy. See linkgit:git-merge[1] for details.
+SEQUENCER SUBCOMMANDS
+---------------------
+include::sequencer.txt[]
+
EXAMPLES
--------
git revert HEAD~3::
diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt
new file mode 100644
index 0000000..16ce88c
--- /dev/null
+++ b/Documentation/sequencer.txt
@@ -0,0 +1,4 @@
+--reset::
+ Forget about the current operation in progress. Can be used
+ to clear the sequencer state after a failed cherry-pick or
+ revert.
diff --git a/Makefile b/Makefile
index 4ed7996..afd7673 100644
--- a/Makefile
+++ b/Makefile
@@ -554,6 +554,7 @@ LIB_H += rerere.h
LIB_H += resolve-undo.h
LIB_H += revision.h
LIB_H += run-command.h
+LIB_H += sequencer.h
LIB_H += sha1-array.h
LIB_H += sha1-lookup.h
LIB_H += sideband.h
@@ -658,6 +659,7 @@ LIB_OBJS += revision.o
LIB_OBJS += run-command.o
LIB_OBJS += server-info.o
LIB_OBJS += setup.o
+LIB_OBJS += sequencer.o
LIB_OBJS += sha1-array.o
LIB_OBJS += sha1-lookup.o
LIB_OBJS += sha1_file.o
diff --git a/builtin/revert.c b/builtin/revert.c
index f088e1c..a8accd6 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -14,6 +14,7 @@
#include "merge-recursive.h"
#include "refs.h"
#include "dir.h"
+#include "sequencer.h"
/*
* This implements the builtins revert and cherry-pick.
@@ -28,18 +29,22 @@
static const char * const revert_usage[] = {
"git revert [options] <commit-ish>",
+ "git revert <subcommand>",
NULL
};
static const char * const cherry_pick_usage[] = {
"git cherry-pick [options] <commit-ish>",
+ "git cherry-pick <subcommand>",
NULL
};
enum replay_action { REVERT, CHERRY_PICK };
+enum replay_subcommand { REPLAY_NONE, REPLAY_RESET };
struct replay_opts {
enum replay_action action;
+ enum replay_subcommand subcommand;
/* Boolean options */
int edit;
@@ -61,11 +66,6 @@ struct replay_opts {
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
-#define SEQ_DIR "sequencer"
-#define SEQ_HEAD_FILE "sequencer/head"
-#define SEQ_TODO_FILE "sequencer/todo"
-#define SEQ_OPTS_FILE "sequencer/opts"
-
static const char *action_name(const struct replay_opts *opts)
{
return opts->action == REVERT ? "revert" : "cherry-pick";
@@ -113,7 +113,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
const char *me = action_name(opts);
int noop;
+ int reset = 0;
struct option options[] = {
+ OPT_BOOLEAN(0, "reset", &reset, "forget the current operation"),
OPT_BOOLEAN('n', "no-commit", &opts->no_commit, "don't automatically commit"),
OPT_BOOLEAN('e', "edit", &opts->edit, "edit the commit message"),
{ OPTION_BOOLEAN, 'r', NULL, &noop, NULL, "no-op (backward compatibility)",
@@ -142,7 +144,27 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,
PARSE_OPT_KEEP_ARGV0 |
PARSE_OPT_KEEP_UNKNOWN);
- if (opts->commit_argc < 2)
+
+ /* Set the subcommand */
+ if (reset)
+ opts->subcommand = REPLAY_RESET;
+ else
+ opts->subcommand = REPLAY_NONE;
+
+ /* Check for incompatible command line arguments */
+ if (opts->subcommand == REPLAY_RESET) {
+ verify_opt_compatible(me, "--reset",
+ "--no-commit", opts->no_commit,
+ "--signoff", opts->signoff,
+ "--mainline", opts->mainline,
+ "--strategy", opts->strategy ? 1 : 0,
+ "--strategy-option", opts->xopts ? 1 : 0,
+ "-x", opts->record_origin,
+ "--ff", opts->allow_ff,
+ NULL);
+ }
+
+ else if (opts->commit_argc < 2)
usage_with_options(usage_str, options);
if (opts->allow_ff)
@@ -729,7 +751,6 @@ static void save_opts(struct replay_opts *opts)
static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
{
- struct strbuf buf = STRBUF_INIT;
struct commit_list *cur;
int res;
@@ -750,9 +771,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
* Sequence of picks finished successfully; cleanup by
* removing the .git/sequencer directory
*/
- strbuf_addf(&buf, "%s", git_path(SEQ_DIR));
- remove_dir_recursively(&buf, 0);
- strbuf_release(&buf);
+ remove_sequencer_state(1);
return 0;
}
@@ -768,16 +787,21 @@ static int pick_revisions(struct replay_opts *opts)
* cherry-pick should be handled differently from an existing
* one that is being continued
*/
- walk_revs_populate_todo(&todo_list, opts);
- create_seq_dir();
- if (get_sha1("HEAD", sha1)) {
- if (opts->action == REVERT)
- die(_("Can't revert as initial commit"));
- die(_("Can't cherry-pick into empty head"));
+ if (opts->subcommand == REPLAY_RESET) {
+ remove_sequencer_state(1);
+ return 0;
+ } else {
+ /* Start a new cherry-pick/ revert sequence */
+ walk_revs_populate_todo(&todo_list, opts);
+ create_seq_dir();
+ if (get_sha1("HEAD", sha1)) {
+ if (opts->action == REVERT)
+ die(_("Can't revert as initial commit"));
+ die(_("Can't cherry-pick into empty head"));
+ }
+ save_head(sha1_to_hex(sha1));
+ save_opts(opts);
}
- save_head(sha1_to_hex(sha1));
- save_opts(opts);
-
return pick_commits(todo_list, opts);
}
diff --git a/sequencer.c b/sequencer.c
new file mode 100644
index 0000000..bc2c046
--- /dev/null
+++ b/sequencer.c
@@ -0,0 +1,19 @@
+#include "cache.h"
+#include "sequencer.h"
+#include "strbuf.h"
+#include "dir.h"
+
+void remove_sequencer_state(int aggressive)
+{
+ struct strbuf seq_dir = STRBUF_INIT;
+ struct strbuf seq_old_dir = STRBUF_INIT;
+
+ strbuf_addf(&seq_dir, "%s", git_path(SEQ_DIR));
+ strbuf_addf(&seq_old_dir, "%s", git_path(SEQ_OLD_DIR));
+ remove_dir_recursively(&seq_old_dir, 0);
+ rename(git_path(SEQ_DIR), git_path(SEQ_OLD_DIR));
+ if (aggressive)
+ remove_dir_recursively(&seq_old_dir, 0);
+ strbuf_release(&seq_dir);
+ strbuf_release(&seq_old_dir);
+}
diff --git a/sequencer.h b/sequencer.h
new file mode 100644
index 0000000..905d295
--- /dev/null
+++ b/sequencer.h
@@ -0,0 +1,20 @@
+#ifndef SEQUENCER_H
+#define SEQUENCER_H
+
+#define SEQ_DIR "sequencer"
+#define SEQ_OLD_DIR "sequencer-old"
+#define SEQ_HEAD_FILE "sequencer/head"
+#define SEQ_TODO_FILE "sequencer/todo"
+#define SEQ_OPTS_FILE "sequencer/opts"
+
+/*
+ * Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
+ * any errors. Intended to be used by 'git reset'.
+ *
+ * With the aggressive flag, it additionally removes SEQ_OLD_DIR,
+ * ignoring any errors. Inteded to be used by the sequencer's
+ * '--reset' subcommand.
+ */
+void remove_sequencer_state(int aggressive);
+
+#endif
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 8ee0001..fd69865 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -13,7 +13,7 @@ test_description='Test cherry-pick continuation features
. ./test-lib.sh
pristine_detach () {
- rm -rf .git/sequencer &&
+ git cherry-pick --reset &&
git checkout -f "$1^0" &&
git read-tree -u --reset HEAD &&
git clean -d -f -f -q -x
@@ -70,4 +70,16 @@ test_expect_success 'cherry-pick cleans up sequencer state upon success' '
test_path_is_missing .git/sequencer
'
+test_expect_success '--reset does not complain when no cherry-pick is in progress' '
+ pristine_detach initial &&
+ git cherry-pick --reset
+'
+
+test_expect_success '--reset cleans up sequencer state' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..picked &&
+ git cherry-pick --reset &&
+ test_path_is_missing .git/sequencer
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 12/18] revert: Make pick_commits functionally act on a commit list
From: Ramkumar Ramachandra @ 2011-08-04 10:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312454356-3070-1-git-send-email-artagnon@gmail.com>
Apart from its central objective of calling into the picking
mechanism, pick_commits creates a sequencer directory, prepares a todo
list, and even acts upon the "--reset" subcommand. This makes for a
bad API since the central worry of callers is to figure out whether or
not any conflicts were encountered during the cherry picking. The
current API is like:
if (pick_commits(opts) < 0)
print "Something failed, we're not sure what"
So, change pick_commits so that it's only responsible for picking
commits in a loop and reporting any errors, leaving the rest to a new
function called pick_revisions. Consequently, the API of pick_commits
becomes much clearer:
act_on_subcommand(opts->subcommand);
todo_list = prepare_todo_list();
if (pick_commits(todo_list, opts) < 0)
print "Error encountered while picking commits"
Now, callers can easily call-in to the cherry-picking machinery by
constructing an arbitrary todo list along with some options.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 43 ++++++++++++++++++++++++++++---------------
1 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 308c1ce..f088e1c 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -727,11 +727,9 @@ static void save_opts(struct replay_opts *opts)
}
}
-static int pick_commits(struct replay_opts *opts)
+static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
{
- struct commit_list *todo_list = NULL;
struct strbuf buf = STRBUF_INIT;
- unsigned char sha1[20];
struct commit_list *cur;
int res;
@@ -741,16 +739,6 @@ static int pick_commits(struct replay_opts *opts)
opts->record_origin || opts->edit));
read_and_refresh_cache(opts);
- walk_revs_populate_todo(&todo_list, opts);
- create_seq_dir();
- if (get_sha1("HEAD", sha1)) {
- if (opts->action == REVERT)
- die(_("Can't revert as initial commit"));
- die(_("Can't cherry-pick into empty head"));
- }
- save_head(sha1_to_hex(sha1));
- save_opts(opts);
-
for (cur = todo_list; cur; cur = cur->next) {
save_todo(cur, opts);
res = do_pick_commit(cur->item, opts);
@@ -768,6 +756,31 @@ static int pick_commits(struct replay_opts *opts)
return 0;
}
+static int pick_revisions(struct replay_opts *opts)
+{
+ struct commit_list *todo_list = NULL;
+ unsigned char sha1[20];
+
+ read_and_refresh_cache(opts);
+
+ /*
+ * Decide what to do depending on the arguments; a fresh
+ * cherry-pick should be handled differently from an existing
+ * one that is being continued
+ */
+ walk_revs_populate_todo(&todo_list, opts);
+ create_seq_dir();
+ if (get_sha1("HEAD", sha1)) {
+ if (opts->action == REVERT)
+ die(_("Can't revert as initial commit"));
+ die(_("Can't cherry-pick into empty head"));
+ }
+ save_head(sha1_to_hex(sha1));
+ save_opts(opts);
+
+ return pick_commits(todo_list, opts);
+}
+
int cmd_revert(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts;
@@ -778,7 +791,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
opts.action = REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- return pick_commits(&opts);
+ return pick_revisions(&opts);
}
int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
@@ -789,5 +802,5 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
opts.action = CHERRY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- return pick_commits(&opts);
+ return pick_revisions(&opts);
}
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 11/18] revert: Save command-line options for continuing operation
From: Ramkumar Ramachandra @ 2011-08-04 10:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312454356-3070-1-git-send-email-artagnon@gmail.com>
In the same spirit as ".git/sequencer/head" and ".git/sequencer/todo",
introduce ".git/sequencer/opts" to persist the replay_opts structure
for continuing after a conflict resolution. Use the gitconfig format
for this file so that it looks like:
[options]
signoff = true
record-origin = true
mainline = 1
strategy = recursive
strategy-option = patience
strategy-option = ours
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 33 +++++++++++++++++++++++++++++++++
t/t3510-cherry-pick-sequence.sh | 29 +++++++++++++++++++++++++++--
2 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 54df8a2..308c1ce 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -64,6 +64,7 @@ struct replay_opts {
#define SEQ_DIR "sequencer"
#define SEQ_HEAD_FILE "sequencer/head"
#define SEQ_TODO_FILE "sequencer/todo"
+#define SEQ_OPTS_FILE "sequencer/opts"
static const char *action_name(const struct replay_opts *opts)
{
@@ -695,6 +696,37 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
strbuf_release(&buf);
}
+static void save_opts(struct replay_opts *opts)
+{
+ const char *opts_file = git_path(SEQ_OPTS_FILE);
+
+ if (opts->no_commit)
+ git_config_set_in_file(opts_file, "options.no-commit", "true");
+ if (opts->edit)
+ git_config_set_in_file(opts_file, "options.edit", "true");
+ if (opts->signoff)
+ git_config_set_in_file(opts_file, "options.signoff", "true");
+ if (opts->record_origin)
+ git_config_set_in_file(opts_file, "options.record-origin", "true");
+ if (opts->allow_ff)
+ git_config_set_in_file(opts_file, "options.allow-ff", "true");
+ if (opts->mainline) {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(&buf, "%d", opts->mainline);
+ git_config_set_in_file(opts_file, "options.mainline", buf.buf);
+ strbuf_release(&buf);
+ }
+ if (opts->strategy)
+ git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
+ if (opts->xopts) {
+ int i;
+ for (i = 0; i < opts->xopts_nr; i++)
+ git_config_set_multivar_in_file(opts_file,
+ "options.strategy-option",
+ opts->xopts[i], "^$", 0);
+ }
+}
+
static int pick_commits(struct replay_opts *opts)
{
struct commit_list *todo_list = NULL;
@@ -717,6 +749,7 @@ static int pick_commits(struct replay_opts *opts)
die(_("Can't cherry-pick into empty head"));
}
save_head(sha1_to_hex(sha1));
+ save_opts(opts);
for (cur = todo_list; cur; cur = cur->next) {
save_todo(cur, opts);
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index a2c70ad..8ee0001 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -33,10 +33,35 @@ test_expect_success setup '
test_expect_success 'cherry-pick persists data on failure' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
+ test_must_fail git cherry-pick -s base..anotherpick &&
test_path_is_dir .git/sequencer &&
test_path_is_file .git/sequencer/head &&
- test_path_is_file .git/sequencer/todo
+ test_path_is_file .git/sequencer/todo &&
+ test_path_is_file .git/sequencer/opts
+'
+
+test_expect_success 'cherry-pick persists opts correctly' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours base..anotherpick &&
+ test_path_is_dir .git/sequencer &&
+ test_path_is_file .git/sequencer/head &&
+ test_path_is_file .git/sequencer/todo &&
+ test_path_is_file .git/sequencer/opts &&
+ echo "true" >expect &&
+ git config --file=.git/sequencer/opts --get-all options.signoff >actual &&
+ test_cmp expect actual &&
+ echo "1" >expect &&
+ git config --file=.git/sequencer/opts --get-all options.mainline >actual &&
+ test_cmp expect actual &&
+ echo "recursive" >expect &&
+ git config --file=.git/sequencer/opts --get-all options.strategy >actual &&
+ test_cmp expect actual &&
+ cat >expect <<-\EOF &&
+ patience
+ ours
+ EOF
+ git config --file=.git/sequencer/opts --get-all options.strategy-option >actual &&
+ test_cmp expect actual
'
test_expect_success 'cherry-pick cleans up sequencer state upon success' '
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 10/18] revert: Save data for continuing after conflict resolution
From: Ramkumar Ramachandra @ 2011-08-04 10:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Jonathan Nieder, Christian Couder, Daniel Barkalow,
Jeff King
In-Reply-To: <1312454356-3070-1-git-send-email-artagnon@gmail.com>
Ever since v1.7.2-rc1~4^2~7 (revert: allow cherry-picking more than
one commit, 2010-06-02), a single invocation of "git cherry-pick" or
"git revert" can perform picks of several individual commits. To
implement features like "--continue" to continue the whole operation,
we will need to store some information about the state and the plan at
the beginning. Introduce a ".git/sequencer/head" file to store this
state, and ".git/sequencer/todo" file to store the plan. The head
file contains the SHA-1 of the HEAD before the start of the operation,
and the todo file contains an instruction sheet whose format is
inspired by the format of the "rebase -i" instruction sheet. As a
result, a typical todo file looks like:
pick 8537f0e submodule add: test failure when url is not configured
pick 4d68932 submodule add: allow relative repository path
pick f22a17e submodule add: clean up duplicated code
pick 59a5775 make copy_ref globally available
Since SHA-1 hex is abbreviated using an find_unique_abbrev(), it is
unambiguous. This does not guarantee that there will be no ambiguity
when more objects are added to the repository.
These two files alone are not enough to implement a "--continue" that
remembers the command-line options specified; later patches in the
series save them too.
These new files are unrelated to the existing .git/CHERRY_PICK_HEAD,
which will still be useful while committing after a conflict
resolution.
Inspired-by: Christian Couder <chriscool@tuxfamily.org>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 134 +++++++++++++++++++++++++++++++++++++-
t/t3510-cherry-pick-sequence.sh | 48 ++++++++++++++
2 files changed, 178 insertions(+), 4 deletions(-)
create mode 100755 t/t3510-cherry-pick-sequence.sh
diff --git a/builtin/revert.c b/builtin/revert.c
index f75c9cb..54df8a2 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -13,6 +13,7 @@
#include "rerere.h"
#include "merge-recursive.h"
#include "refs.h"
+#include "dir.h"
/*
* This implements the builtins revert and cherry-pick.
@@ -60,6 +61,10 @@ struct replay_opts {
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
+#define SEQ_DIR "sequencer"
+#define SEQ_HEAD_FILE "sequencer/head"
+#define SEQ_TODO_FILE "sequencer/todo"
+
static const char *action_name(const struct replay_opts *opts)
{
return opts->action == REVERT ? "revert" : "cherry-pick";
@@ -587,10 +592,116 @@ static void read_and_refresh_cache(struct replay_opts *opts)
rollback_lock_file(&index_lock);
}
-static int pick_commits(struct replay_opts *opts)
+/*
+ * Append a commit to the end of the commit_list.
+ *
+ * next starts by pointing to the variable that holds the head of an
+ * empty commit_list, and is updated to point to the "next" field of
+ * the last item on the list as new commits are appended.
+ *
+ * Usage example:
+ *
+ * struct commit_list *list;
+ * struct commit_list **next = &list;
+ *
+ * next = commit_list_append(c1, next);
+ * next = commit_list_append(c2, next);
+ * assert(commit_list_count(list) == 2);
+ * return list;
+ */
+struct commit_list **commit_list_append(struct commit *commit,
+ struct commit_list **next)
+{
+ struct commit_list *new = xmalloc(sizeof(struct commit_list));
+ new->item = commit;
+ *next = new;
+ new->next = NULL;
+ return &new->next;
+}
+
+static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
+ struct replay_opts *opts)
+{
+ struct commit_list *cur = NULL;
+ struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
+ const char *sha1_abbrev = NULL;
+ const char *action_str = opts->action == REVERT ? "revert" : "pick";
+
+ for (cur = todo_list; cur; cur = cur->next) {
+ sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
+ if (get_message(cur->item, &msg))
+ return error(_("Cannot get commit message for %s"), sha1_abbrev);
+ strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
+ }
+ return 0;
+}
+
+static void walk_revs_populate_todo(struct commit_list **todo_list,
+ struct replay_opts *opts)
{
struct rev_info revs;
struct commit *commit;
+ struct commit_list **next;
+
+ prepare_revs(&revs, opts);
+
+ next = todo_list;
+ while ((commit = get_revision(&revs)))
+ next = commit_list_append(commit, next);
+}
+
+static void create_seq_dir(void)
+{
+ const char *seq_dir = git_path(SEQ_DIR);
+
+ if (!(file_exists(seq_dir) && is_directory(seq_dir))
+ && mkdir(seq_dir, 0777) < 0)
+ die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
+}
+
+static void save_head(const char *head)
+{
+ const char *head_file = git_path(SEQ_HEAD_FILE);
+ static struct lock_file head_lock;
+ struct strbuf buf = STRBUF_INIT;
+ int fd;
+
+ fd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);
+ strbuf_addf(&buf, "%s\n", head);
+ if (write_in_full(fd, buf.buf, buf.len) < 0)
+ die_errno(_("Could not write to %s."), head_file);
+ if (commit_lock_file(&head_lock) < 0)
+ die(_("Error wrapping up %s."), head_file);
+}
+
+static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+{
+ const char *todo_file = git_path(SEQ_TODO_FILE);
+ static struct lock_file todo_lock;
+ struct strbuf buf = STRBUF_INIT;
+ int fd;
+
+ fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
+ if (format_todo(&buf, todo_list, opts) < 0)
+ die(_("Could not format %s."), todo_file);
+ if (write_in_full(fd, buf.buf, buf.len) < 0) {
+ strbuf_release(&buf);
+ die_errno(_("Could not write to %s."), todo_file);
+ }
+ if (commit_lock_file(&todo_lock) < 0) {
+ strbuf_release(&buf);
+ die(_("Error wrapping up %s."), todo_file);
+ }
+ strbuf_release(&buf);
+}
+
+static int pick_commits(struct replay_opts *opts)
+{
+ struct commit_list *todo_list = NULL;
+ struct strbuf buf = STRBUF_INIT;
+ unsigned char sha1[20];
+ struct commit_list *cur;
+ int res;
setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
if (opts->allow_ff)
@@ -598,14 +709,29 @@ static int pick_commits(struct replay_opts *opts)
opts->record_origin || opts->edit));
read_and_refresh_cache(opts);
- prepare_revs(&revs, opts);
+ walk_revs_populate_todo(&todo_list, opts);
+ create_seq_dir();
+ if (get_sha1("HEAD", sha1)) {
+ if (opts->action == REVERT)
+ die(_("Can't revert as initial commit"));
+ die(_("Can't cherry-pick into empty head"));
+ }
+ save_head(sha1_to_hex(sha1));
- while ((commit = get_revision(&revs))) {
- int res = do_pick_commit(commit, opts);
+ for (cur = todo_list; cur; cur = cur->next) {
+ save_todo(cur, opts);
+ res = do_pick_commit(cur->item, opts);
if (res)
return res;
}
+ /*
+ * Sequence of picks finished successfully; cleanup by
+ * removing the .git/sequencer directory
+ */
+ strbuf_addf(&buf, "%s", git_path(SEQ_DIR));
+ remove_dir_recursively(&buf, 0);
+ strbuf_release(&buf);
return 0;
}
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
new file mode 100755
index 0000000..a2c70ad
--- /dev/null
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+test_description='Test cherry-pick continuation features
+
+ + anotherpick: rewrites foo to d
+ + picked: rewrites foo to c
+ + unrelatedpick: rewrites unrelated to reallyunrelated
+ + base: rewrites foo to b
+ + initial: writes foo as a, unrelated as unrelated
+
+'
+
+. ./test-lib.sh
+
+pristine_detach () {
+ rm -rf .git/sequencer &&
+ git checkout -f "$1^0" &&
+ git read-tree -u --reset HEAD &&
+ git clean -d -f -f -q -x
+}
+
+test_expect_success setup '
+ echo unrelated >unrelated &&
+ git add unrelated &&
+ test_commit initial foo a &&
+ test_commit base foo b &&
+ test_commit unrelatedpick unrelated reallyunrelated &&
+ test_commit picked foo c &&
+ test_commit anotherpick foo d &&
+ git config advice.detachedhead false
+
+'
+
+test_expect_success 'cherry-pick persists data on failure' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ test_path_is_dir .git/sequencer &&
+ test_path_is_file .git/sequencer/head &&
+ test_path_is_file .git/sequencer/todo
+'
+
+test_expect_success 'cherry-pick cleans up sequencer state upon success' '
+ pristine_detach initial &&
+ git cherry-pick initial..picked &&
+ test_path_is_missing .git/sequencer
+'
+
+test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
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