* Re: Please support add -p with a new file, to add only part of the file
From: Jeff King @ 2012-01-10 18:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Rast, Jonathan Nieder, Josh Triplett, git,
Wincent Colaiuta
In-Reply-To: <7vpqer9znv.fsf@alter.siamese.dyndns.org>
On Tue, Jan 10, 2012 at 10:32:20AM -0800, Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
>
> > A not-so-proper solution might of course start by looking at which files
> > are untracked, and only run the 'git add -N' immediately before patch
> > application.
>
> Isn't the real issue that we mistakenly gave an impression that "add -p"
> is the primary interface to the users, and forgot to tell them about the
> more general "add -i", which "add -p" is a small subset of?
Maybe it is just me, but I find "add -p" insanely useful, and the rest
of "add -i" to be worthless clutter. The "add -p" functionality is not
easily available anywhere else, but the rest of "add -i" can be easily
(and often more efficiently) mimicked using existing git commands.
> Even if you start with "add -N", there won't be individual "hunks" you can
> pick and choose from diffing emptiness and the whole new file, so you end
> up using "edit hunk" interface.
I don't think the main impetus for this is that people necessarily want
to pick and choose hunks from added files. I think it is simply a nice
workflow to do:
$ hack hack hack
$ git add -p ;# inspect and add changes
$ git commit
which is very similar to the traditional git workflow:
$ hack hack hack
$ git diff ;# inspect changes
$ git add foo ;# add changes
$ git commit
I find myself using "add -p" almost exclusively these days, as it
combines the two middle steps (and even though I usually am just hitting
"y" after inspection, when I _do_ want to make a change, I am right
there in the hunk selection loop already).
-Peff
^ permalink raw reply
* Re: [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Jonathan Nieder @ 2012-01-10 18:38 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <1326212039-13806-3-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Currently, 'git cherry-pick' fills up the '.git/sequencer/todo'
> instruction sheet with "pick" actions, while 'git revert' fills it up
> with "revert" actions. Inspired by the way 'rebase -i' works, we
> would like to permit mixing arbitrary actions in the same instruction
> sheet.
Ok.
> To do this, we first have to decouple the notion of an action
> in the instruction sheet from builtin commands.
Wha?
I guess at this point I get lost because you are using jargon. What
is the difference between an action in the instruction sheet and a
builtin command? How does being a busybox-style hard link to the main
"git" binary as opposed to a separate binary (which is what "builtin"
means) have anything to do with this at all?
And I don't have any sense of the impact. Will this change the
behavior of "git cherry-pick"? Will it avoid some confusion on the
part of future people modifying the code? Is it a necessary step
before some future change we want? Does it just make the code
prettier, and help the sanity of future readers in the process (which
is definitely valuable, too)? The above doesn't make it clear to me.
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
[...]
> @@ -52,7 +57,7 @@ enum replay_subcommand {
> };
>
> struct replay_opts {
> - enum replay_action action;
> + enum replay_command command;
> enum replay_subcommand subcommand;
This part seems to be renaming "action" to "command". A cosmetic
change, which I don't have an immediate opinion about either way.
[...]
> -static const char *action_name(const struct replay_opts *opts)
> +static const char *command_name(struct replay_opts *opts)
This part does a similar renaming, and drops a const while at it for
no intelligible reason.
[...]
> @@ -142,7 +147,7 @@ static void verify_opt_mutually_compatible(const char *me, ...)
> 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);
> + const char *me = command_name(opts);
The rest is stuff like this, which follows from the first part.
Stepping back, I think the idea is that "enum replay_action" is not a
good way to identify the command name in error messages like
fatal: cherry-pick: --abort cannot be used with --continue
So you introduce a _new_ enum to represent the command name. Why not
just use a string, so commands using the nice and generic sequencer
library do not have to register themselves in a global callers list to
use it?
Plus the renaming of the ->action field and action_name() function
feel gratuitous, and drowned out the actual changes in the noise.
Does that clarify?
I guess I agree with the problem you are solving, but I don't agree
with the solution at all.
Thanks,
Jonathan
^ permalink raw reply
* Re: Please support add -p with a new file, to add only part of the file
From: Junio C Hamano @ 2012-01-10 18:32 UTC (permalink / raw)
To: Thomas Rast
Cc: Jonathan Nieder, Josh Triplett, git, Wincent Colaiuta, Jeff King
In-Reply-To: <87ty43fy7f.fsf@thomas.inf.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> A not-so-proper solution might of course start by looking at which files
> are untracked, and only run the 'git add -N' immediately before patch
> application.
Isn't the real issue that we mistakenly gave an impression that "add -p"
is the primary interface to the users, and forgot to tell them about the
more general "add -i", which "add -p" is a small subset of?
Even if you start with "add -N", there won't be individual "hunks" you can
pick and choose from diffing emptiness and the whole new file, so you end
up using "edit hunk" interface.
Whatever you do, you should aim to make it easier to use than
$ cp newfile.c newfile.c-saved
$ edit newfile.c ;# delete all the things you do not want for now
$ git add newfile.c
$ mv newfile.c-saved newfile.c
Personally, I think it is a very tall order to tweak "add -p / edit"
interface to make it easier to use than the above four command sequence,
the second one among which is the most time and braincell consuming part
of the process, especially when "add -p / edit" gives you has to be
unnecessarily distracting by having '+' prefix.
^ permalink raw reply
* Re: git grep doesn't follow symbolic link
From: Junio C Hamano @ 2012-01-10 18:22 UTC (permalink / raw)
To: Thomas Rast; +Cc: Ramkumar Ramachandra, Bertrand BENOIT, git
In-Reply-To: <877h0zlvwd.fsf@thomas.inf.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
>> I'd imagine so: symbolic links are not portable across different file
>> systems; Git's internal representation of a symbolic link is a file
>> containing the path of the file to be linked to.
>
> I'd actually welcome a fix to this general area,...
Even though some platforms may lack symbolic links, where they are
supported, they have a clear and defined meaning and that is what Git
tracks as contents: where the link points at.
So we would want our "git diff" to tell us, even if you moved without
content modification the symbolic link target that lives somewhere on your
filesystem but is outside the control of Git, and updated a symbolic link
that is tracked by Git to point to a new location, that you updated the
link. On the other hand, if you did not update a tracked symbolic link,
even if the location the link points at that may or may not be under the
control of Git, we do not want "git diff" to show anything. As far as that
link is concerned, nothing has changed.
Changing this would not be a fix; it would be butchering.
Having said that...
> But I lose all the cute features of git-diff. I *could* say
>
> git diff --no-index <(ls) <(cd elsewhere && ls)
... "--no-index" is specifically _not_ about tracked contents of Git, but
was bolted on as a poor-man's substitute for GNU diff (think of it as
somebody wanted to have the nifty "git diff" features like renames and
coloring, but did not want to bother to port them to GNU diff codebase,
but instead hacked Git codebase to work outside Git tracked contents). In
that context, I would agree that it _might_ make sense to treat special
files and symbolic links in a way that is different from how tracked
contents are handled.
^ permalink raw reply
* Re: [PATCH 1/8] revert: prepare to move replay_action to header
From: Jonathan Nieder @ 2012-01-10 18:27 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <1326212039-13806-2-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Later in the series, we will build a generalized sequencer by
> factoring code out of the revert builtin, and leaving it with just
> argument parsing work. This involves moving "replay_action" to
> sequencer.h, so that both sequencer.c and builtin/revert.c can use it.
> Unfortunately, "REVERT" and "CHERRY_PICK" are unsuitable variable
> names, as their purpose is unclear in the global context; in
> anticipation of the move, rename them to "REPLAY_REVERT" and
> "REPLAY_PICK" respectively.
This could be pruned to just the final bit. Something like:
REVERT and CHERRY_PICK and are unsuitable names for an enumerator in
a public interface, because they are generic enough to be likely to
clash with identifiers with other meanings. Rename to REPLAY_REVERT
and REPLAY_PICK as preparation for exposing them.
The patch proper looks safe and the effect positive to me, so I would
be all for applying it as long as the commit message is cleaned up.
^ permalink raw reply
* Re: [PATCH] attr: don't confuse prefixes with leading directories
From: Jeff King @ 2012-01-10 18:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git, Henrik Grubbström, git-dev
In-Reply-To: <20120110180820.GA15273@sigill.intra.peff.net>
On Tue, Jan 10, 2012 at 01:08:21PM -0500, Jeff King wrote:
> diff --git a/attr.c b/attr.c
> index 76b079f..fa975da 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -582,7 +582,8 @@ static void prepare_attr_stack(const char *path)
>
> elem = attr_stack;
> if (namelen <= dirlen &&
> - !strncmp(elem->origin, path, namelen))
> + !strncmp(elem->origin, path, namelen) &&
> + (!namelen || path[namelen] == '/'))
> break;
Side note. One thing that confused me about this code is that
prepare_attr_stack does a popping loop like this:
while (attr_stack && attr_stack->origin) {
if (/* attr_stack->origin is a prefix */)
break;
/* otherwise, pop */
elem = attr_stack;
attr_stack = elem->prev;
free(elem);
}
/* now push our new ones */
...
len = strlen(attr_stack->origin);
IOW, our loop breaks out when attr_stack is NULL, but then we go on to
assume that attr_stack is _not_ NULL. This isn't a bug, because it turns
out that we always leave something in the attr_stack: the root
gitattributes file (and the builtins). But it is slightly confusing to
a reader because of the useless loop condition.
I'm not sure if the right solution is to change the popping loop to:
/* we will never run out of stack, because we always have the root */
while (attr_stack->origin) {
...
Or to be extra defensive and put:
if (!attr_stack)
die("BUG: we ran out of attr stack!?");
after the loop, or to somehow handle the case of an empty attr stack
below (which is hard to do, because it can't be triggered, so I have no
idea what it would mean).
-Peff
^ permalink raw reply
* [PATCH] attr: don't confuse prefixes with leading directories
From: Jeff King @ 2012-01-10 18:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, git, Henrik Grubbström, git-dev
In-Reply-To: <20120110171100.GA18962@sigill.intra.peff.net>
When we prepare the attribute stack for a lookup on a path,
we start with the cached stack from the previous lookup
(because it is common to do several lookups in the same
directory hierarchy). So the first thing we must do in
preparing the stack is to pop any entries that point to
directories we are no longer interested in.
For example, if our stack contains gitattributes for:
foo/bar/baz
foo/bar
foo
but we want to do a lookup in "foo/bar/bleep", then we want
to pop the top element, but retain the others.
To do this we walk down the stack from the top, popping
elements that do not match our lookup directory. However,
the test do this simply checked strncmp, meaning we would
mistake "foo/bar/baz" as a leading directory of
"foo/bar/baz_plus". We must also check that the character
after our match is '/', meaning we matched the whole path
component.
There are two special cases to consider:
1. The top of our attr stack has the empty path. So we
must not check for '/', but rather special-case the
empty path, which always matches.
2. Typically when matching paths in this way, you would
also need to check for a full string match (i.e., the
character after is '\0'). We don't need to do so in
this case, though, because our path string is actually
just the directory component of the path to a file
(i.e., we know that it terminates with "/", because the
filename comes after that).
Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jeff King <peff@peff.net>
---
I wrote it in the minimally-intrusive way, but you could also pull
the "!namelen" case before the strncmp (since a strncmp of size 0 is
always true, anyway). I don't know if it would be more obvious that way.
I prepared this on top of master, but the patch applies (with some
shifted line counts) on older releases, too.
attr.c | 3 ++-
t/t0003-attributes.sh | 10 ++++++++++
2 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/attr.c b/attr.c
index 76b079f..fa975da 100644
--- a/attr.c
+++ b/attr.c
@@ -582,7 +582,8 @@ static void prepare_attr_stack(const char *path)
elem = attr_stack;
if (namelen <= dirlen &&
- !strncmp(elem->origin, path, namelen))
+ !strncmp(elem->origin, path, namelen) &&
+ (!namelen || path[namelen] == '/'))
break;
debug_pop(elem);
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index dbb2623..51f3045 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -159,6 +159,16 @@ test_expect_success 'relative paths' '
(cd b && attr_check ../a/b/g a/b/g)
'
+test_expect_success 'prefixes are not confused with leading directories' '
+ attr_check a_plus/g unspecified &&
+ cat >expect <<-\EOF &&
+ a/g: test: a/g
+ a_plus/g: test: unspecified
+ EOF
+ git check-attr test a/g a_plus/g >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'core.attributesfile' '
attr_check global unspecified &&
git config core.attributesfile "$HOME/global-gitattributes" &&
--
1.7.9.rc0.33.gd3c17
^ permalink raw reply related
* Re: [PATCHv3 10/13] credentials: add "cache" helper
From: Jeff King @ 2012-01-10 17:53 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, git
In-Reply-To: <20120110174420.GA22184@burratino>
On Tue, Jan 10, 2012 at 11:44:21AM -0600, Jonathan Nieder wrote:
> Jeff King wrote:
>
> > 2. A hack in credential-cache won't help any unix-socket
> > users who come along later.
> >
> > 3. The chdir trickery isn't that likely to fail (basically
> > it's only a problem if your cwd is missing or goes away
> > while you're running). And because we only enable the
> > hack when we get a too-long name, it can only fail in
> > cases that would have failed under the previous code
> > anyway.
> >
> > Signed-off-by: Jeff King <peff@peff.net>
>
> A part of me worries that this assumption (3), and the additional
> assumption that nothing running concurrently cares about the cwd,
> might come back to bite us when the future (2) arrives. But I don't
> see a better approach.
The problem is that when future (2) arrives, a credential-cache specific
hack won't be helping it at all. :)
To be honest, I don't really expect a lot of future unix-socket users.
It's not portable, and most of our IPC happens over pipes. The design of
the cache daemon is very specific in requiring a true
many-clients-to-one-server model, but also caring deeply about access
controls (making TCP sockets less good[1]).
[1] One could in theory do a loopback TCP socket and provide a random
token read from an access-controlled file. But that ends up a lot
more complicated and introduces new attack surfaces. Which is a bad
thing for security-sensitive code like this.
> Follow-on ideas: on platforms that support it, it would be nice to use
>
> ctx->orig_dirfd = open(".", O_RDONLY);
> if (ctx->orig_dirfd < 0)
> ... error handling ...
> ...
> if (ctx->orig_dirfd >= 0) {
> if (fchdir(ctx->orig_dirfd))
> die_errno("unable to restore original working directory");
> close(ctx->orig_dirfd);
> }
Yeah, I started by using fchdir, but noticed that we don't use it
anywhere else and didn't want to create a portability problem. It does
fix the "somebody deleted your cwd while you were gone from it" problem.
But if you have no cwd at all, the open call will still fail.
Still, it would be slightly more robust. I wonder how portable fchdir
is in practice (I guess we could always fall back to the getcwd code
path). Do you want to prepare a patch on top?
> [...]
> > + dir = path;
> > + dir_len = slash - path;
> [...]
> > + if (chdir_len(dir, dir_len) < 0)
> > + return -1;
>
> Could avoid some complication by eliminating the dir_len var.
>
> if (chdir_len(dir, slash - dir))
> return -1;
Ah, yeah. Originally I had written it so that "slash" didn't survive
untouched to there, but in the current code, that would work.
Junio, if you haven't merged it to next yet, it might be worth squashing
in the patch below. Otherwise I wouldn't worry about it.
> Neither seems worth holding up the patch, so
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks for the review (and for, as usual, a well-written bug report).
---
diff --git a/unix-socket.c b/unix-socket.c
index 91ed9dc..e8f19c6 100644
--- a/unix-socket.c
+++ b/unix-socket.c
@@ -43,7 +43,6 @@ static int unix_sockaddr_init(struct sockaddr_un *sa, const char *path,
if (size > sizeof(sa->sun_path)) {
const char *slash = find_last_dir_sep(path);
const char *dir;
- int dir_len;
if (!slash) {
errno = ENAMETOOLONG;
@@ -51,8 +50,6 @@ static int unix_sockaddr_init(struct sockaddr_un *sa, const char *path,
}
dir = path;
- dir_len = slash - path;
-
path = slash + 1;
size = strlen(path) + 1;
if (size > sizeof(sa->sun_path)) {
@@ -64,7 +61,7 @@ static int unix_sockaddr_init(struct sockaddr_un *sa, const char *path,
errno = ENAMETOOLONG;
return -1;
}
- if (chdir_len(dir, dir_len) < 0)
+ if (chdir_len(dir, slash - dir) < 0)
return -1;
}
^ permalink raw reply related
* Re: [PATCHv3 10/13] credentials: add "cache" helper
From: Jonathan Nieder @ 2012-01-10 17:44 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20120110044430.GA23619@sigill.intra.peff.net>
Jeff King wrote:
> 2. A hack in credential-cache won't help any unix-socket
> users who come along later.
>
> 3. The chdir trickery isn't that likely to fail (basically
> it's only a problem if your cwd is missing or goes away
> while you're running). And because we only enable the
> hack when we get a too-long name, it can only fail in
> cases that would have failed under the previous code
> anyway.
>
> Signed-off-by: Jeff King <peff@peff.net>
A part of me worries that this assumption (3), and the additional
assumption that nothing running concurrently cares about the cwd,
might come back to bite us when the future (2) arrives. But I don't
see a better approach.
Follow-on ideas: on platforms that support it, it would be nice to use
ctx->orig_dirfd = open(".", O_RDONLY);
if (ctx->orig_dirfd < 0)
... error handling ...
...
if (ctx->orig_dirfd >= 0) {
if (fchdir(ctx->orig_dirfd))
die_errno("unable to restore original working directory");
close(ctx->orig_dirfd);
}
> --- a/unix-socket.c
> +++ b/unix-socket.c
> @@ -9,27 +9,86 @@ static int unix_stream_socket(void)
[...]
> + dir = path;
> + dir_len = slash - path;
[...]
> + if (chdir_len(dir, dir_len) < 0)
> + return -1;
Could avoid some complication by eliminating the dir_len var.
if (chdir_len(dir, slash - dir))
return -1;
Neither seems worth holding up the patch, so
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply
* Re: [BUG] gitattribute macro expansion oddity
From: Junio C Hamano @ 2012-01-10 17:22 UTC (permalink / raw)
To: Jeff King; +Cc: git, Henrik Grubbström, git-dev
In-Reply-To: <20120110070300.GA17086@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Bisection points to ec775c4 (attr: Expand macros immediately when
> encountered., 2010-04-06), but it's too late for me to dig further
> tonight. Cc'ing Junio as the author of the attr code and Henrik as the
> author of ec775c4.
Thanks for getting the ball rolling.
Regardless of this unrelated regression, after looking at what ec775c4
wanted to do again, I am very much tempted to just revert it.
It aimed to take these three
* ident
foo mybin
bar mybin ident
and wanted to omit 'ident' from "foo" when there is this macro definition
elsewhere:
[attr] mybin binary -ident
But the real point of the macro was that the users do not have to know
their internals, iow, if you explicitly specify a pattern that overrides
the contents of the macro, that explicit pattern should win. When deciding
the value of "ident" attribute for path "foo", "* ident" is stronger than
"foo mybin" (the latter of which does not say anything about 'ident'
explicitly).
^ permalink raw reply
* Re: [BUG] gitattribute macro expansion oddity
From: Jeff King @ 2012-01-10 17:11 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git, Junio C Hamano, Henrik Grubbström, git-dev
In-Reply-To: <4F0BFE6E.6080904@alum.mit.edu>
On Tue, Jan 10, 2012 at 10:01:34AM +0100, Michael Haggerty wrote:
> On 01/10/2012 08:03 AM, Jeff King wrote:
> > I'm seeing some very odd behavior with git's attribute expansion for
> > diffs. You can see it with this repository:
> >
> > git clone git://github.com/libgit2/libgit2sharp.git
> >
> > Try a diff of a non-binary file: [...]
>
> The problem has nothing with diffing; simply interrogating the attribute
> values gives different results depending on the order of the files:
>
> $ git check-attr --all Lib/NativeBinaries/x86/git2.dll
> LibGit2Sharp/Configuration.cs
> Lib/NativeBinaries/x86/git2.dll: binary: set
> Lib/NativeBinaries/x86/git2.dll: diff: unset
> Lib/NativeBinaries/x86/git2.dll: text: unset
> LibGit2Sharp/Configuration.cs: binary: set
> LibGit2Sharp/Configuration.cs: diff: unset
> LibGit2Sharp/Configuration.cs: text: unset
> LibGit2Sharp/Configuration.cs: crlf: set
> $ git check-attr --all LibGit2Sharp/Configuration.cs
> Lib/NativeBinaries/x86/git2.dll
> LibGit2Sharp/Configuration.cs: diff: csharp
> LibGit2Sharp/Configuration.cs: crlf: set
> Lib/NativeBinaries/x86/git2.dll: binary: set
> Lib/NativeBinaries/x86/git2.dll: diff: unset
> Lib/NativeBinaries/x86/git2.dll: text: unset
Thanks. I tried to test it with check-attr but for some reason wasn't
able to provoke the bug (I think I probably just screwed up the
invocation).
> It also doesn't depend on the fact that Lib/.gitattributes uses CRLF as
> its EOL, nor does it depend on the use of the "binary" macro. However,
> it does depend on the fact that the directory name "Lib" matches the
> first part of the directory name "LibGit2Sharp". Here is a simplified
> demonstration of the problem:
Ah, very helpful. That allowed me to find the problem very quickly by
grepping for "strncmp". :)
The patch below seem to fix it for me. I'll do a bit more testing before
posting it for real, though.
-Peff
diff --git a/attr.c b/attr.c
index 7467baf..f4beb62 100644
--- a/attr.c
+++ b/attr.c
@@ -528,7 +528,8 @@ static void prepare_attr_stack(const char *path, int dirlen)
elem = attr_stack;
if (namelen <= dirlen &&
- !strncmp(elem->origin, path, namelen))
+ !strncmp(elem->origin, path, namelen) &&
+ (!namelen || path[namelen] == '/' || path[namelen] == '\0'))
break;
debug_pop(elem);
^ permalink raw reply related
* Re: [PATCHv3 10/13] credentials: add "cache" helper
From: Junio C Hamano @ 2012-01-10 16:59 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Nieder, git
In-Reply-To: <20120110045733.GA12460@sigill.intra.peff.net>
Thanks, these look sane.
Will queue both to 'next' aiming for 'master' before 1.7.9-rc1.
^ permalink raw reply
* git svn dcommit sends to wrong branch
From: Victor Engmark @ 2012-01-10 16:18 UTC (permalink / raw)
To: git
Commands:
git svn clone -s -r 1:HEAD http://svn/repo
cd repo
git commit [thrice, in master only]
git rebase --interactive HEAD~20 [i.e., started rebase in commits before
the clone]
[Merged two commits I had made *after* the clone]
git commit ...
git dcommit
This created commits on
<http://svn/repo/branches/branch_name>! Why? Is it because HEAD~20's
git-svn-id <http://svn/repo/branches/branch_name@22481> is on that
branch?
And more importantly, how do I "replay" my commits on trunk?
--
terreActive AG
Kasinostrasse 30
CH-5001 Aarau
Tel: +41 62 834 00 55
Fax: +41 62 823 93 56
www.terreactive.ch
Wir sichern Ihren Erfolg - seit 15 Jahren
^ permalink raw reply
* [PATCH 8/8] sequencer: factor code out of revert builtin
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
Expose the cherry-picking machinery through a public
sequencer_pick_revisions() (renamed from pick_revisions() in
builtin/revert.c), so that cherry-picking and reverting are special
cases of a general sequencer operation. The cherry-pick builtin is
now a thin wrapper that does command-line argument parsing before
calling into sequencer_pick_revisions(). So now, your "foo" builtin
can use the sequencer machinery by implementing a
parse_args_populate_opts() function and then running the following:
memset(&opts, 0, sizeof(opts));
opts.command = REPLAY_CMD_FOO;
opts.revisions = xmalloc(sizeof(*opts.revs));
parse_args_populate_opts(argc, argv, &opts);
init_revisions(opts.revs);
sequencer_pick_revisions(&opts);
This patch does not intend to make any functional changes. Check
with:
$ git blame -s -C HEAD^..HEAD -- sequencer.c | grep -C3 '^[^^]'
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Documentation/technical/api-sequencer.txt | 22 +
builtin/revert.c | 966 +----------------------------
sequencer.c | 925 +++++++++++++++++++++++++++-
sequencer.h | 48 ++
4 files changed, 996 insertions(+), 965 deletions(-)
create mode 100644 Documentation/technical/api-sequencer.txt
diff --git a/Documentation/technical/api-sequencer.txt b/Documentation/technical/api-sequencer.txt
new file mode 100644
index 0000000..22e9a95
--- /dev/null
+++ b/Documentation/technical/api-sequencer.txt
@@ -0,0 +1,22 @@
+sequencer API
+=============
+
+The sequencer API makes it easy for builtins to execute a sequence of
+commands operating on a range of commits. It provides uniform
+semantics to '--continue', '--quit', and '--abort' the sequence when a
+conflict is encountered.
+
+Currently, the 'git cherry-pick' and 'git revert' builtins utilize the
+API. A new builtin "foo" can use the sequencer machinery by
+implementing a parse_args_populate_opts() function and then running
+the following:
+
+ memset(&opts, 0, sizeof(opts));
+ opts.command = REPLAY_CMD_FOO;
+ opts.revisions = xmalloc(sizeof(*opts.revs));
+ parse_args_populate_opts(argc, argv, &opts);
+ init_revisions(opts.revs);
+ sequencer_pick_revisions(&opts);
+
+
+(Ram)
diff --git a/builtin/revert.c b/builtin/revert.c
index 187c317..54a1c50 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1,18 +1,9 @@
#include "cache.h"
#include "builtin.h"
-#include "object.h"
-#include "commit.h"
-#include "tag.h"
-#include "run-command.h"
-#include "exec_cmd.h"
-#include "utf8.h"
#include "parse-options.h"
-#include "cache-tree.h"
#include "diff.h"
#include "revision.h"
#include "rerere.h"
-#include "merge-recursive.h"
-#include "refs.h"
#include "dir.h"
#include "sequencer.h"
@@ -39,61 +30,11 @@ static const char * const cherry_pick_usage[] = {
NULL
};
-enum replay_action {
- REPLAY_REVERT,
- REPLAY_PICK
-};
-
-enum replay_command {
- REPLAY_CMD_REVERT,
- REPLAY_CMD_CHERRY_PICK
-};
-
-enum replay_subcommand {
- REPLAY_NONE,
- REPLAY_REMOVE_STATE,
- REPLAY_CONTINUE,
- REPLAY_ROLLBACK
-};
-
-struct replay_insn_list {
- struct commit *operand;
- enum replay_action action;
- struct replay_insn_list *next;
-};
-
-struct replay_opts {
- enum replay_command command;
- enum replay_subcommand subcommand;
-
- /* Boolean options */
- int edit;
- int record_origin;
- int no_commit;
- int signoff;
- int allow_ff;
- int allow_rerere_auto;
-
- int mainline;
-
- /* Merge strategy */
- const char *strategy;
- const char **xopts;
- size_t xopts_nr, xopts_alloc;
-
- /* Only used by REPLAY_NONE */
- struct rev_info *revs;
-};
-
-#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
-
static const char *command_name(struct replay_opts *opts)
{
return opts->command == REPLAY_CMD_REVERT ? "revert" : "cherry-pick";
}
-static char *get_encoding(const char *message);
-
static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
{
return opts->command == REPLAY_CMD_REVERT ? revert_usage : cherry_pick_usage;
@@ -252,909 +193,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
usage_with_options(usage_str, options);
}
-struct commit_message {
- char *parent_label;
- const char *label;
- const char *subject;
- char *reencoded_message;
- const char *message;
-};
-
-static int get_message(struct commit *commit, struct commit_message *out)
-{
- const char *encoding;
- const char *abbrev, *subject;
- int abbrev_len, subject_len;
- char *q;
-
- if (!commit->buffer)
- return -1;
- encoding = get_encoding(commit->buffer);
- if (!encoding)
- encoding = "UTF-8";
- if (!git_commit_encoding)
- git_commit_encoding = "UTF-8";
-
- out->reencoded_message = NULL;
- out->message = commit->buffer;
- if (strcmp(encoding, git_commit_encoding))
- out->reencoded_message = reencode_string(commit->buffer,
- git_commit_encoding, encoding);
- if (out->reencoded_message)
- out->message = out->reencoded_message;
-
- abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
- abbrev_len = strlen(abbrev);
-
- subject_len = find_commit_subject(out->message, &subject);
-
- out->parent_label = xmalloc(strlen("parent of ") + abbrev_len +
- strlen("... ") + subject_len + 1);
- q = out->parent_label;
- q = mempcpy(q, "parent of ", strlen("parent of "));
- out->label = q;
- q = mempcpy(q, abbrev, abbrev_len);
- q = mempcpy(q, "... ", strlen("... "));
- out->subject = q;
- q = mempcpy(q, subject, subject_len);
- *q = '\0';
- return 0;
-}
-
-static void free_message(struct commit_message *msg)
-{
- free(msg->parent_label);
- free(msg->reencoded_message);
-}
-
-static char *get_encoding(const char *message)
-{
- const char *p = message, *eol;
-
- while (*p && *p != '\n') {
- for (eol = p + 1; *eol && *eol != '\n'; eol++)
- ; /* do nothing */
- if (!prefixcmp(p, "encoding ")) {
- char *result = xmalloc(eol - 8 - p);
- strlcpy(result, p + 9, eol - 8 - p);
- return result;
- }
- p = eol;
- if (*p == '\n')
- p++;
- }
- return NULL;
-}
-
-static void write_cherry_pick_head(struct commit *commit, const char *pseudoref)
-{
- const char *filename;
- int fd;
- struct strbuf buf = STRBUF_INIT;
-
- strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
-
- filename = git_path("%s", pseudoref);
- fd = open(filename, O_WRONLY | O_CREAT, 0666);
- if (fd < 0)
- die_errno(_("Could not open '%s' for writing"), filename);
- if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
- die_errno(_("Could not write to '%s'"), filename);
- strbuf_release(&buf);
-}
-
-static void print_advice(int show_hint)
-{
- char *msg = getenv("GIT_CHERRY_PICK_HELP");
-
- if (msg) {
- fprintf(stderr, "%s\n", msg);
- /*
- * A conflict has occured but the porcelain
- * (typically rebase --interactive) wants to take care
- * of the commit itself so remove CHERRY_PICK_HEAD
- */
- unlink(git_path("CHERRY_PICK_HEAD"));
- return;
- }
-
- if (show_hint) {
- advise("after resolving the conflicts, mark the corrected paths");
- advise("with 'git add <paths>' or 'git rm <paths>'");
- advise("and commit the result with 'git commit'");
- }
-}
-
-static void write_message(struct strbuf *msgbuf, const char *filename)
-{
- static struct lock_file msg_file;
-
- int msg_fd = hold_lock_file_for_update(&msg_file, filename,
- LOCK_DIE_ON_ERROR);
- if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
- die_errno(_("Could not write to %s"), filename);
- strbuf_release(msgbuf);
- if (commit_lock_file(&msg_file) < 0)
- die(_("Error wrapping up %s"), filename);
-}
-
-static struct tree *empty_tree(void)
-{
- return lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN);
-}
-
-static int error_dirty_index(struct replay_opts *opts)
-{
- if (read_cache_unmerged())
- return error_resolve_conflict(command_name(opts));
-
- /* Different translation strings for cherry-pick and revert */
- if (opts->command == REPLAY_CMD_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)
-{
- struct ref_lock *ref_lock;
-
- read_cache();
- if (checkout_fast_forward(from, to))
- exit(1); /* the callee should have complained already */
- ref_lock = lock_any_ref_for_update("HEAD", from, 0);
- return write_ref_sha1(ref_lock, to, "cherry-pick");
-}
-
-static int do_recursive_merge(struct commit *base, struct commit *next,
- const char *base_label, const char *next_label,
- unsigned char *head, struct strbuf *msgbuf,
- struct replay_opts *opts)
-{
- struct merge_options o;
- struct tree *result, *next_tree, *base_tree, *head_tree;
- int clean, index_fd;
- const char **xopt;
- static struct lock_file index_lock;
-
- index_fd = hold_locked_index(&index_lock, 1);
-
- read_cache();
-
- init_merge_options(&o);
- o.ancestor = base ? base_label : "(empty tree)";
- o.branch1 = "HEAD";
- o.branch2 = next ? next_label : "(empty tree)";
-
- head_tree = parse_tree_indirect(head);
- next_tree = next ? next->tree : empty_tree();
- base_tree = base ? base->tree : empty_tree();
-
- for (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)
- parse_merge_opt(&o, *xopt);
-
- clean = merge_trees(&o,
- head_tree,
- next_tree, base_tree, &result);
-
- if (active_cache_changed &&
- (write_cache(index_fd, active_cache, active_nr) ||
- commit_locked_index(&index_lock)))
- /* TRANSLATORS: %s will be "revert" or "cherry-pick" */
- die(_("%s: Unable to write new index file"), command_name(opts));
- rollback_lock_file(&index_lock);
-
- if (!clean) {
- int i;
- strbuf_addstr(msgbuf, "\nConflicts:\n\n");
- for (i = 0; i < active_nr;) {
- struct cache_entry *ce = active_cache[i++];
- if (ce_stage(ce)) {
- strbuf_addch(msgbuf, '\t');
- strbuf_addstr(msgbuf, ce->name);
- strbuf_addch(msgbuf, '\n');
- while (i < active_nr && !strcmp(ce->name,
- active_cache[i]->name))
- i++;
- }
- }
- }
-
- return !clean;
-}
-
-/*
- * If we are cherry-pick, and if the merge did not result in
- * hand-editing, we will hit this commit and inherit the original
- * author date and name.
- * If we are revert, or if our cherry-pick results in a hand merge,
- * we had better say that the current user is responsible for that.
- */
-static int run_git_commit(const char *defmsg, struct replay_opts *opts)
-{
- /* 6 is max possible length of our args array including NULL */
- const char *args[6];
- int i = 0;
-
- args[i++] = "commit";
- args[i++] = "-n";
- if (opts->signoff)
- args[i++] = "-s";
- if (!opts->edit) {
- args[i++] = "-F";
- args[i++] = defmsg;
- }
- args[i] = NULL;
-
- return run_command_v_opt(args, RUN_GIT_CMD);
-}
-
-static int do_pick_commit(struct commit *commit, enum replay_action action,
- struct replay_opts *opts)
-{
- unsigned char head[20];
- struct commit *base, *next, *parent;
- const char *base_label, *next_label;
- struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
- char *defmsg = NULL;
- struct strbuf msgbuf = STRBUF_INIT;
- int res;
-
- if (opts->no_commit) {
- /*
- * We do not intend to commit immediately. We just want to
- * merge the differences in, so let's compute the tree
- * that represents the "current" state for merge-recursive
- * to work on.
- */
- if (write_cache_as_tree(head, 0, NULL))
- die (_("Your index file is unmerged."));
- } else {
- if (get_sha1("HEAD", head))
- return error(_("You do not have a valid HEAD"));
- if (index_differs_from("HEAD", 0))
- return error_dirty_index(opts);
- }
- discard_cache();
-
- if (!commit->parents) {
- parent = NULL;
- }
- else if (commit->parents->next) {
- /* Reverting or cherry-picking a merge commit */
- int cnt;
- struct commit_list *p;
-
- if (!opts->mainline)
- 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)
- 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)
- return error(_("Mainline was specified but commit %s is not a merge."),
- sha1_to_hex(commit->object.sha1));
- else
- parent = commit->parents->item;
-
- if (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))
- return fast_forward_to(commit->object.sha1, head);
-
- if (parent && parse_commit(parent) < 0)
- /* TRANSLATORS: The first %s will be "revert" or
- "cherry-pick", the second %s a SHA1 */
- return error(_("%s: cannot parse parent commit %s"),
- command_name(opts), sha1_to_hex(parent->object.sha1));
-
- if (get_message(commit, &msg) != 0)
- return error(_("Cannot get commit message for %s"),
- sha1_to_hex(commit->object.sha1));
-
- /*
- * "commit" is an existing commit. We would want to apply
- * the difference it introduces since its first parent "prev"
- * on top of the current HEAD if we are cherry-pick. Or the
- * reverse of it if we are revert.
- */
-
- defmsg = git_pathdup("MERGE_MSG");
-
- if (action == REPLAY_REVERT) {
- base = commit;
- base_label = msg.label;
- next = parent;
- next_label = msg.parent_label;
- strbuf_addstr(&msgbuf, "Revert \"");
- strbuf_addstr(&msgbuf, msg.subject);
- strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
- strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
-
- if (commit->parents && commit->parents->next) {
- strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
- strbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));
- }
- strbuf_addstr(&msgbuf, ".\n");
- } else {
- const char *p;
-
- base = parent;
- base_label = msg.parent_label;
- next = commit;
- next_label = msg.label;
-
- /*
- * Append the commit log message to msgbuf; it starts
- * after the tree, parent, author, committer
- * information followed by "\n\n".
- */
- p = strstr(msg.message, "\n\n");
- if (p) {
- p += 2;
- strbuf_addstr(&msgbuf, p);
- }
-
- if (opts->record_origin) {
- strbuf_addstr(&msgbuf, "(cherry picked from commit ");
- strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
- strbuf_addstr(&msgbuf, ")\n");
- }
- }
-
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
- res = do_recursive_merge(base, next, base_label, next_label,
- head, &msgbuf, opts);
- write_message(&msgbuf, defmsg);
- } else {
- struct commit_list *common = NULL;
- struct commit_list *remotes = NULL;
-
- write_message(&msgbuf, defmsg);
-
- commit_list_insert(base, &common);
- commit_list_insert(next, &remotes);
- res = try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
- common, sha1_to_hex(head), remotes);
- free_commit_list(common);
- free_commit_list(remotes);
- }
-
- /*
- * If the merge was clean or if it failed due to conflict, we write
- * CHERRY_PICK_HEAD for the subsequent invocation of commit to use.
- * However, if the merge did not even start, then we don't want to
- * write it at all.
- */
- if (action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
- write_cherry_pick_head(commit, "CHERRY_PICK_HEAD");
- if (action == REPLAY_REVERT && ((opts->no_commit && res == 0) || res == 1))
- write_cherry_pick_head(commit, "REVERT_HEAD");
-
- if (res) {
- error(action == REPLAY_REVERT
- ? _("could not revert %s... %s")
- : _("could not apply %s... %s"),
- find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
- msg.subject);
- print_advice(res == 1);
- rerere(opts->allow_rerere_auto);
- } else {
- if (!opts->no_commit)
- res = run_git_commit(defmsg, opts);
- }
-
- free_message(&msg);
- free(defmsg);
-
- return res;
-}
-
-static void prepare_revs(struct replay_opts *opts)
-{
- if (opts->command != REPLAY_CMD_REVERT)
- opts->revs->reverse ^= 1;
-
- if (prepare_revision_walk(opts->revs))
- die(_("revision walk setup failed"));
-
- if (!opts->revs->commits)
- die(_("empty commit set passed"));
-}
-
-static void read_and_refresh_cache(struct replay_opts *opts)
-{
- static struct lock_file index_lock;
- int index_fd = hold_locked_index(&index_lock, 0);
- if (read_index_preload(&the_index, NULL) < 0)
- die(_("git %s: failed to read the index"), command_name(opts));
- refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
- if (the_index.cache_changed) {
- if (write_index(&the_index, index_fd) ||
- commit_locked_index(&index_lock))
- die(_("git %s: failed to refresh the index"),
- command_name(opts));
- }
- rollback_lock_file(&index_lock);
-}
-
-/*
- * Append a (commit, action) to the end of the replay_insn_list.
- *
- * next starts by pointing to the variable that holds the head of an
- * empty replay_insn_list, and is updated to point to the "next" field of
- * the last item on the list as new (commit, action) pairs are appended.
- *
- * Usage example:
- *
- * struct replay_insn_list *list;
- * struct replay_insn_list **next = &list;
- *
- * next = replay_insn_list_append(c1, a1, next);
- * next = replay_insn_list_append(c2, a2, next);
- * assert(len(list) == 2);
- * return list;
- */
-static struct replay_insn_list **replay_insn_list_append(struct commit *operand,
- enum replay_action action,
- struct replay_insn_list **next)
-{
- struct replay_insn_list *new = xmalloc(sizeof(*new));
- new->action = action;
- new->operand = operand;
- *next = new;
- new->next = NULL;
- return &new->next;
-}
-
-static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
-{
- struct replay_insn_list *cur;
-
- for (cur = todo_list; cur; cur = cur->next) {
- const char *sha1_abbrev, *action_str, *subject;
- int subject_len;
-
- action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
- sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
- subject_len = find_commit_subject(cur->operand->buffer, &subject);
- strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
- subject_len, subject);
- }
- return 0;
-}
-
-static int parse_error(const char *message, const char *file,
- int lineno, char *error_line)
-{
- const char *suffix = "";
- int error_len = strcspn(error_line, " \t\n");
-
- if (error_len > 20) {
- error_len = 20;
- suffix = "...";
- }
- return error(_("%s:%d: %s: %.*s%s"), file, lineno, message,
- error_len, error_line, suffix);
-}
-
-static int parse_insn_line(char *bol, char *eol,
- struct replay_insn_list *item, int lineno)
-{
- unsigned char commit_sha1[20];
- int namelen;
-
- if (!prefixcmp(bol, "pick ") || !prefixcmp(bol, "pick\t")) {
- item->action = REPLAY_PICK;
- bol += strlen("pick ");
- } else if (!prefixcmp(bol, "revert ") || !prefixcmp(bol, "revert\t")) {
- item->action = REPLAY_REVERT;
- bol += strlen("revert ");
- } else
- return parse_error(_("unrecognized action"),
- git_path(SEQ_TODO_FILE), lineno, bol);
-
- /* Eat up extra spaces/ tabs before object name */
- bol += strspn(bol, " \t");
-
- namelen = strcspn(bol, " \t\n");
- if (getn_sha1(bol, namelen, commit_sha1))
- return parse_error(_("malformed object name"),
- git_path(SEQ_TODO_FILE), lineno, bol);
-
- item->operand = lookup_commit_reference(commit_sha1);
- if (!item->operand)
- return parse_error(_("not a valid commit"),
- git_path(SEQ_TODO_FILE), lineno, bol);
-
- item->next = NULL;
- return 0;
-}
-
-static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
-{
- struct replay_insn_list **next = todo_list;
- struct replay_insn_list item = { NULL, 0, NULL };
- char *p = buf;
- int i;
-
- for (i = 1; *p; i++) {
- char *eol = strchrnul(p, '\n');
- if (parse_insn_line(p, eol, &item, i))
- return -1;
- next = replay_insn_list_append(item.operand, item.action, next);
- p = *eol ? eol + 1 : eol;
- }
- if (!*todo_list)
- return error(_("No commits parsed."));
- return 0;
-}
-
-static void read_populate_todo(struct replay_insn_list **todo_list)
-{
- 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);
- 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 replay_insn_list **todo_list,
- struct replay_opts *opts)
-{
- struct commit *commit;
- struct replay_insn_list **next;
- enum replay_action action;
-
- prepare_revs(opts);
-
- next = todo_list;
- action = opts->command == REPLAY_CMD_REVERT ? REPLAY_REVERT : REPLAY_PICK;
- while ((commit = get_revision(opts->revs)))
- next = replay_insn_list_append(commit, action, next);
-}
-
-static int create_seq_dir(void)
-{
- const char *seq_dir = git_path(SEQ_DIR);
-
- if (file_exists(seq_dir)) {
- error(_("a cherry-pick or revert is already in progress"));
- advise(_("try \"git cherry-pick (--continue | --quit | --abort)\""));
- return -1;
- }
- 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)
-{
- 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 int reset_for_rollback(const unsigned char *sha1)
-{
- const char *argv[4]; /* reset --merge <arg> + NULL */
- argv[0] = "reset";
- argv[1] = "--merge";
- argv[2] = sha1_to_hex(sha1);
- argv[3] = NULL;
- return run_command_v_opt(argv, RUN_GIT_CMD);
-}
-
-static int rollback_single_pick(void)
-{
- unsigned char head_sha1[20];
-
- if (!file_exists(git_path("CHERRY_PICK_HEAD")) &&
- !file_exists(git_path("REVERT_HEAD")))
- return error(_("no cherry-pick or revert in progress"));
- if (read_ref_full("HEAD", head_sha1, 0, NULL))
- return error(_("cannot resolve HEAD"));
- if (is_null_sha1(head_sha1))
- return error(_("cannot abort from a branch yet to be born"));
- return reset_for_rollback(head_sha1);
-}
-
-static int sequencer_rollback(struct replay_opts *opts)
-{
- const char *filename;
- FILE *f;
- unsigned char sha1[20];
- struct strbuf buf = STRBUF_INIT;
-
- filename = git_path(SEQ_HEAD_FILE);
- f = fopen(filename, "r");
- if (!f && errno == ENOENT) {
- /*
- * There is no multiple-cherry-pick in progress.
- * If CHERRY_PICK_HEAD or REVERT_HEAD indicates
- * a single-cherry-pick in progress, abort that.
- */
- return rollback_single_pick();
- }
- if (!f)
- return error(_("cannot open %s: %s"), filename,
- strerror(errno));
- if (strbuf_getline(&buf, f, '\n')) {
- error(_("cannot read %s: %s"), filename, ferror(f) ?
- strerror(errno) : _("unexpected end of file"));
- fclose(f);
- goto fail;
- }
- fclose(f);
- if (get_sha1_hex(buf.buf, sha1) || buf.buf[40] != '\0') {
- error(_("stored pre-cherry-pick HEAD file '%s' is corrupt"),
- filename);
- goto fail;
- }
- if (reset_for_rollback(sha1))
- goto fail;
- remove_sequencer_state();
- strbuf_release(&buf);
- return 0;
-fail:
- strbuf_release(&buf);
- return -1;
-}
-
-static void save_todo(struct replay_insn_list *todo_list)
-{
- 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) < 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 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_insn_list *todo_list, struct replay_opts *opts)
-{
- struct replay_insn_list *cur;
- int res;
-
- setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
- if (opts->allow_ff)
- assert(!(opts->signoff || opts->no_commit ||
- opts->record_origin || opts->edit));
- read_and_refresh_cache(opts);
-
- for (cur = todo_list; cur; cur = cur->next) {
- save_todo(cur);
- res = do_pick_commit(cur->operand, cur->action, opts);
- if (res)
- return res;
- }
-
- /*
- * Sequence of picks finished successfully; cleanup by
- * removing the .git/sequencer directory
- */
- remove_sequencer_state();
- return 0;
-}
-
-static int continue_single_pick(void)
-{
- const char *argv[] = { "commit", NULL };
-
- if (!file_exists(git_path("CHERRY_PICK_HEAD")) &&
- !file_exists(git_path("REVERT_HEAD")))
- return error(_("no cherry-pick or revert in progress"));
- return run_command_v_opt(argv, RUN_GIT_CMD);
-}
-
-static int sequencer_continue(struct replay_opts *opts)
-{
- struct replay_insn_list *todo_list = NULL;
-
- if (!file_exists(git_path(SEQ_TODO_FILE)))
- return continue_single_pick();
- read_populate_opts(&opts);
- read_populate_todo(&todo_list);
-
- /* Verify that the conflict has been resolved */
- if (file_exists(git_path("CHERRY_PICK_HEAD")) ||
- file_exists(git_path("REVERT_HEAD"))) {
- int ret = continue_single_pick();
- if (ret)
- return ret;
- }
- if (index_differs_from("HEAD", 0))
- return error_dirty_index(opts);
- todo_list = todo_list->next;
- return pick_commits(todo_list, opts);
-}
-
-static int single_pick(struct commit *cmit, struct replay_opts *opts)
-{
- enum replay_action action;
- action = opts->command == REPLAY_CMD_REVERT ? REPLAY_REVERT : REPLAY_PICK;
-
- setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
- return do_pick_commit(cmit, action, opts);
-}
-
-static int pick_revisions(struct replay_opts *opts)
-{
- struct replay_insn_list *todo_list = NULL;
- unsigned char sha1[20];
-
- if (opts->subcommand == REPLAY_NONE)
- assert(opts->revs);
-
- 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
- */
- if (opts->subcommand == REPLAY_REMOVE_STATE) {
- remove_sequencer_state();
- return 0;
- }
- if (opts->subcommand == REPLAY_ROLLBACK)
- return sequencer_rollback(opts);
- if (opts->subcommand == REPLAY_CONTINUE)
- return sequencer_continue(opts);
-
- /*
- * If we were called as "git cherry-pick <commit>", just
- * cherry-pick/revert it, set CHERRY_PICK_HEAD /
- * REVERT_HEAD, and don't touch the sequencer state.
- * This means it is possible to cherry-pick in the middle
- * of a cherry-pick sequence.
- */
- if (opts->revs->cmdline.nr == 1 &&
- opts->revs->cmdline.rev->whence == REV_CMD_REV &&
- opts->revs->no_walk &&
- !opts->revs->cmdline.rev->flags) {
- struct commit *cmit;
- if (prepare_revision_walk(opts->revs))
- die(_("revision walk setup failed"));
- cmit = get_revision(opts->revs);
- if (!cmit || get_revision(opts->revs))
- die("BUG: expected exactly one commit from walk");
- return single_pick(cmit, opts);
- }
-
- /*
- * 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);
- if (create_seq_dir() < 0)
- return -1;
- if (get_sha1("HEAD", sha1)) {
- if (opts->command == REPLAY_CMD_REVERT)
- 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);
-}
-
int cmd_revert(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts;
@@ -1166,7 +204,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
opts.command = REPLAY_CMD_REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- res = pick_revisions(&opts);
+ res = sequencer_pick_revisions(&opts);
if (res < 0)
die(_("revert failed"));
return res;
@@ -1181,7 +219,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
opts.command = REPLAY_CMD_CHERRY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- res = pick_revisions(&opts);
+ res = sequencer_pick_revisions(&opts);
if (res < 0)
die(_("cherry-pick failed"));
return res;
diff --git a/sequencer.c b/sequencer.c
index d1f28a6..a9c3bfa 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1,7 +1,20 @@
#include "cache.h"
#include "sequencer.h"
-#include "strbuf.h"
#include "dir.h"
+#include "object.h"
+#include "commit.h"
+#include "tag.h"
+#include "run-command.h"
+#include "exec_cmd.h"
+#include "utf8.h"
+#include "cache-tree.h"
+#include "diff.h"
+#include "revision.h"
+#include "rerere.h"
+#include "merge-recursive.h"
+#include "refs.h"
+
+#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
void remove_sequencer_state(void)
{
@@ -11,3 +24,913 @@ void remove_sequencer_state(void)
remove_dir_recursively(&seq_dir, 0);
strbuf_release(&seq_dir);
}
+
+static const char *command_name(struct replay_opts *opts)
+{
+ return opts->command == REPLAY_CMD_REVERT ? "revert" : "cherry-pick";
+}
+
+static char *get_encoding(const char *message);
+
+struct commit_message {
+ char *parent_label;
+ const char *label;
+ const char *subject;
+ char *reencoded_message;
+ const char *message;
+};
+
+static int get_message(struct commit *commit, struct commit_message *out)
+{
+ const char *encoding;
+ const char *abbrev, *subject;
+ int abbrev_len, subject_len;
+ char *q;
+
+ if (!commit->buffer)
+ return -1;
+ encoding = get_encoding(commit->buffer);
+ if (!encoding)
+ encoding = "UTF-8";
+ if (!git_commit_encoding)
+ git_commit_encoding = "UTF-8";
+
+ out->reencoded_message = NULL;
+ out->message = commit->buffer;
+ if (strcmp(encoding, git_commit_encoding))
+ out->reencoded_message = reencode_string(commit->buffer,
+ git_commit_encoding, encoding);
+ if (out->reencoded_message)
+ out->message = out->reencoded_message;
+
+ abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
+ abbrev_len = strlen(abbrev);
+
+ subject_len = find_commit_subject(out->message, &subject);
+
+ out->parent_label = xmalloc(strlen("parent of ") + abbrev_len +
+ strlen("... ") + subject_len + 1);
+ q = out->parent_label;
+ q = mempcpy(q, "parent of ", strlen("parent of "));
+ out->label = q;
+ q = mempcpy(q, abbrev, abbrev_len);
+ q = mempcpy(q, "... ", strlen("... "));
+ out->subject = q;
+ q = mempcpy(q, subject, subject_len);
+ *q = '\0';
+ return 0;
+}
+
+static void free_message(struct commit_message *msg)
+{
+ free(msg->parent_label);
+ free(msg->reencoded_message);
+}
+
+static char *get_encoding(const char *message)
+{
+ const char *p = message, *eol;
+
+ while (*p && *p != '\n') {
+ for (eol = p + 1; *eol && *eol != '\n'; eol++)
+ ; /* do nothing */
+ if (!prefixcmp(p, "encoding ")) {
+ char *result = xmalloc(eol - 8 - p);
+ strlcpy(result, p + 9, eol - 8 - p);
+ return result;
+ }
+ p = eol;
+ if (*p == '\n')
+ p++;
+ }
+ return NULL;
+}
+
+static void write_cherry_pick_head(struct commit *commit, const char *pseudoref)
+{
+ const char *filename;
+ int fd;
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
+
+ filename = git_path("%s", pseudoref);
+ fd = open(filename, O_WRONLY | O_CREAT, 0666);
+ if (fd < 0)
+ die_errno(_("Could not open '%s' for writing"), filename);
+ if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
+ die_errno(_("Could not write to '%s'"), filename);
+ strbuf_release(&buf);
+}
+
+static void print_advice(int show_hint)
+{
+ char *msg = getenv("GIT_CHERRY_PICK_HELP");
+
+ if (msg) {
+ fprintf(stderr, "%s\n", msg);
+ /*
+ * A conflict has occured but the porcelain
+ * (typically rebase --interactive) wants to take care
+ * of the commit itself so remove CHERRY_PICK_HEAD
+ */
+ unlink(git_path("CHERRY_PICK_HEAD"));
+ return;
+ }
+
+ if (show_hint) {
+ advise("after resolving the conflicts, mark the corrected paths");
+ advise("with 'git add <paths>' or 'git rm <paths>'");
+ advise("and commit the result with 'git commit'");
+ }
+}
+
+static void write_message(struct strbuf *msgbuf, const char *filename)
+{
+ static struct lock_file msg_file;
+
+ int msg_fd = hold_lock_file_for_update(&msg_file, filename,
+ LOCK_DIE_ON_ERROR);
+ if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
+ die_errno(_("Could not write to %s"), filename);
+ strbuf_release(msgbuf);
+ if (commit_lock_file(&msg_file) < 0)
+ die(_("Error wrapping up %s"), filename);
+}
+
+static struct tree *empty_tree(void)
+{
+ return lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN);
+}
+
+static int error_dirty_index(struct replay_opts *opts)
+{
+ if (read_cache_unmerged())
+ return error_resolve_conflict(command_name(opts));
+
+ /* Different translation strings for cherry-pick and revert */
+ if (opts->command == REPLAY_CMD_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)
+{
+ struct ref_lock *ref_lock;
+
+ read_cache();
+ if (checkout_fast_forward(from, to))
+ exit(1); /* the callee should have complained already */
+ ref_lock = lock_any_ref_for_update("HEAD", from, 0);
+ return write_ref_sha1(ref_lock, to, "cherry-pick");
+}
+
+static int do_recursive_merge(struct commit *base, struct commit *next,
+ const char *base_label, const char *next_label,
+ unsigned char *head, struct strbuf *msgbuf,
+ struct replay_opts *opts)
+{
+ struct merge_options o;
+ struct tree *result, *next_tree, *base_tree, *head_tree;
+ int clean, index_fd;
+ const char **xopt;
+ static struct lock_file index_lock;
+
+ index_fd = hold_locked_index(&index_lock, 1);
+
+ read_cache();
+
+ init_merge_options(&o);
+ o.ancestor = base ? base_label : "(empty tree)";
+ o.branch1 = "HEAD";
+ o.branch2 = next ? next_label : "(empty tree)";
+
+ head_tree = parse_tree_indirect(head);
+ next_tree = next ? next->tree : empty_tree();
+ base_tree = base ? base->tree : empty_tree();
+
+ for (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)
+ parse_merge_opt(&o, *xopt);
+
+ clean = merge_trees(&o,
+ head_tree,
+ next_tree, base_tree, &result);
+
+ if (active_cache_changed &&
+ (write_cache(index_fd, active_cache, active_nr) ||
+ commit_locked_index(&index_lock)))
+ /* TRANSLATORS: %s will be "revert" or "cherry-pick" */
+ die(_("%s: Unable to write new index file"), command_name(opts));
+ rollback_lock_file(&index_lock);
+
+ if (!clean) {
+ int i;
+ strbuf_addstr(msgbuf, "\nConflicts:\n\n");
+ for (i = 0; i < active_nr;) {
+ struct cache_entry *ce = active_cache[i++];
+ if (ce_stage(ce)) {
+ strbuf_addch(msgbuf, '\t');
+ strbuf_addstr(msgbuf, ce->name);
+ strbuf_addch(msgbuf, '\n');
+ while (i < active_nr && !strcmp(ce->name,
+ active_cache[i]->name))
+ i++;
+ }
+ }
+ }
+
+ return !clean;
+}
+
+/*
+ * If we are cherry-pick, and if the merge did not result in
+ * hand-editing, we will hit this commit and inherit the original
+ * author date and name.
+ * If we are revert, or if our cherry-pick results in a hand merge,
+ * we had better say that the current user is responsible for that.
+ */
+static int run_git_commit(const char *defmsg, struct replay_opts *opts)
+{
+ /* 6 is max possible length of our args array including NULL */
+ const char *args[6];
+ int i = 0;
+
+ args[i++] = "commit";
+ args[i++] = "-n";
+ if (opts->signoff)
+ args[i++] = "-s";
+ if (!opts->edit) {
+ args[i++] = "-F";
+ args[i++] = defmsg;
+ }
+ args[i] = NULL;
+
+ return run_command_v_opt(args, RUN_GIT_CMD);
+}
+
+static int do_pick_commit(struct commit *commit, enum replay_action action,
+ struct replay_opts *opts)
+{
+ unsigned char head[20];
+ struct commit *base, *next, *parent;
+ const char *base_label, *next_label;
+ struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
+ char *defmsg = NULL;
+ struct strbuf msgbuf = STRBUF_INIT;
+ int res;
+
+ if (opts->no_commit) {
+ /*
+ * We do not intend to commit immediately. We just want to
+ * merge the differences in, so let's compute the tree
+ * that represents the "current" state for merge-recursive
+ * to work on.
+ */
+ if (write_cache_as_tree(head, 0, NULL))
+ die (_("Your index file is unmerged."));
+ } else {
+ if (get_sha1("HEAD", head))
+ return error(_("You do not have a valid HEAD"));
+ if (index_differs_from("HEAD", 0))
+ return error_dirty_index(opts);
+ }
+ discard_cache();
+
+ if (!commit->parents) {
+ parent = NULL;
+ }
+ else if (commit->parents->next) {
+ /* Reverting or cherry-picking a merge commit */
+ int cnt;
+ struct commit_list *p;
+
+ if (!opts->mainline)
+ 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)
+ 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)
+ return error(_("Mainline was specified but commit %s is not a merge."),
+ sha1_to_hex(commit->object.sha1));
+ else
+ parent = commit->parents->item;
+
+ if (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))
+ return fast_forward_to(commit->object.sha1, head);
+
+ if (parent && parse_commit(parent) < 0)
+ /* TRANSLATORS: The first %s will be "revert" or
+ "cherry-pick", the second %s a SHA1 */
+ return error(_("%s: cannot parse parent commit %s"),
+ command_name(opts), sha1_to_hex(parent->object.sha1));
+
+ if (get_message(commit, &msg) != 0)
+ return error(_("Cannot get commit message for %s"),
+ sha1_to_hex(commit->object.sha1));
+
+ /*
+ * "commit" is an existing commit. We would want to apply
+ * the difference it introduces since its first parent "prev"
+ * on top of the current HEAD if we are cherry-pick. Or the
+ * reverse of it if we are revert.
+ */
+
+ defmsg = git_pathdup("MERGE_MSG");
+
+ if (action == REPLAY_REVERT) {
+ base = commit;
+ base_label = msg.label;
+ next = parent;
+ next_label = msg.parent_label;
+ strbuf_addstr(&msgbuf, "Revert \"");
+ strbuf_addstr(&msgbuf, msg.subject);
+ strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
+
+ if (commit->parents && commit->parents->next) {
+ strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));
+ }
+ strbuf_addstr(&msgbuf, ".\n");
+ } else {
+ const char *p;
+
+ base = parent;
+ base_label = msg.parent_label;
+ next = commit;
+ next_label = msg.label;
+
+ /*
+ * Append the commit log message to msgbuf; it starts
+ * after the tree, parent, author, committer
+ * information followed by "\n\n".
+ */
+ p = strstr(msg.message, "\n\n");
+ if (p) {
+ p += 2;
+ strbuf_addstr(&msgbuf, p);
+ }
+
+ if (opts->record_origin) {
+ strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
+ strbuf_addstr(&msgbuf, ")\n");
+ }
+ }
+
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
+ res = do_recursive_merge(base, next, base_label, next_label,
+ head, &msgbuf, opts);
+ write_message(&msgbuf, defmsg);
+ } else {
+ struct commit_list *common = NULL;
+ struct commit_list *remotes = NULL;
+
+ write_message(&msgbuf, defmsg);
+
+ commit_list_insert(base, &common);
+ commit_list_insert(next, &remotes);
+ res = try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
+ common, sha1_to_hex(head), remotes);
+ free_commit_list(common);
+ free_commit_list(remotes);
+ }
+
+ /*
+ * If the merge was clean or if it failed due to conflict, we write
+ * CHERRY_PICK_HEAD for the subsequent invocation of commit to use.
+ * However, if the merge did not even start, then we don't want to
+ * write it at all.
+ */
+ if (action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
+ write_cherry_pick_head(commit, "CHERRY_PICK_HEAD");
+ if (action == REPLAY_REVERT && ((opts->no_commit && res == 0) || res == 1))
+ write_cherry_pick_head(commit, "REVERT_HEAD");
+
+ if (res) {
+ error(action == REPLAY_REVERT
+ ? _("could not revert %s... %s")
+ : _("could not apply %s... %s"),
+ find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
+ msg.subject);
+ print_advice(res == 1);
+ rerere(opts->allow_rerere_auto);
+ } else {
+ if (!opts->no_commit)
+ res = run_git_commit(defmsg, opts);
+ }
+
+ free_message(&msg);
+ free(defmsg);
+
+ return res;
+}
+
+static void prepare_revs(struct replay_opts *opts)
+{
+ if (opts->command != REPLAY_CMD_REVERT)
+ opts->revs->reverse ^= 1;
+
+ if (prepare_revision_walk(opts->revs))
+ die(_("revision walk setup failed"));
+
+ if (!opts->revs->commits)
+ die(_("empty commit set passed"));
+}
+
+static void read_and_refresh_cache(struct replay_opts *opts)
+{
+ static struct lock_file index_lock;
+ int index_fd = hold_locked_index(&index_lock, 0);
+ if (read_index_preload(&the_index, NULL) < 0)
+ die(_("git %s: failed to read the index"), command_name(opts));
+ refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
+ if (the_index.cache_changed) {
+ if (write_index(&the_index, index_fd) ||
+ commit_locked_index(&index_lock))
+ die(_("git %s: failed to refresh the index"),
+ command_name(opts));
+ }
+ rollback_lock_file(&index_lock);
+}
+
+/*
+ * Append a (commit, action) to the end of the replay_insn_list.
+ *
+ * next starts by pointing to the variable that holds the head of an
+ * empty replay_insn_list, and is updated to point to the "next" field of
+ * the last item on the list as new (commit, action) pairs are appended.
+ *
+ * Usage example:
+ *
+ * struct replay_insn_list *list;
+ * struct replay_insn_list **next = &list;
+ *
+ * next = replay_insn_list_append(c1, a1, next);
+ * next = replay_insn_list_append(c2, a2, next);
+ * assert(len(list) == 2);
+ * return list;
+ */
+static struct replay_insn_list **replay_insn_list_append(struct commit *operand,
+ enum replay_action action,
+ struct replay_insn_list **next)
+{
+ struct replay_insn_list *new = xmalloc(sizeof(*new));
+ new->action = action;
+ new->operand = operand;
+ *next = new;
+ new->next = NULL;
+ return &new->next;
+}
+
+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
+{
+ struct replay_insn_list *cur;
+
+ for (cur = todo_list; cur; cur = cur->next) {
+ const char *sha1_abbrev, *action_str, *subject;
+ int subject_len;
+
+ action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
+ sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
+ subject_len = find_commit_subject(cur->operand->buffer, &subject);
+ strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
+ subject_len, subject);
+ }
+ return 0;
+}
+
+static int parse_error(const char *message, const char *file,
+ int lineno, char *error_line)
+{
+ const char *suffix = "";
+ int error_len = strcspn(error_line, " \t\n");
+
+ if (error_len > 20) {
+ error_len = 20;
+ suffix = "...";
+ }
+ return error(_("%s:%d: %s: %.*s%s"), file, lineno, message,
+ error_len, error_line, suffix);
+}
+
+static int parse_insn_line(char *bol, char *eol,
+ struct replay_insn_list *item, int lineno)
+{
+ unsigned char commit_sha1[20];
+ int namelen;
+
+ if (!prefixcmp(bol, "pick ") || !prefixcmp(bol, "pick\t")) {
+ item->action = REPLAY_PICK;
+ bol += strlen("pick ");
+ } else if (!prefixcmp(bol, "revert ") || !prefixcmp(bol, "revert\t")) {
+ item->action = REPLAY_REVERT;
+ bol += strlen("revert ");
+ } else
+ return parse_error(_("unrecognized action"),
+ git_path(SEQ_TODO_FILE), lineno, bol);
+
+ /* Eat up extra spaces/ tabs before object name */
+ bol += strspn(bol, " \t");
+
+ namelen = strcspn(bol, " \t\n");
+ if (getn_sha1(bol, namelen, commit_sha1))
+ return parse_error(_("malformed object name"),
+ git_path(SEQ_TODO_FILE), lineno, bol);
+
+ item->operand = lookup_commit_reference(commit_sha1);
+ if (!item->operand)
+ return parse_error(_("not a valid commit"),
+ git_path(SEQ_TODO_FILE), lineno, bol);
+
+ item->next = NULL;
+ return 0;
+}
+
+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
+{
+ struct replay_insn_list **next = todo_list;
+ struct replay_insn_list item = { NULL, 0, NULL };
+ char *p = buf;
+ int i;
+
+ for (i = 1; *p; i++) {
+ char *eol = strchrnul(p, '\n');
+ if (parse_insn_line(p, eol, &item, i))
+ return -1;
+ next = replay_insn_list_append(item.operand, item.action, next);
+ p = *eol ? eol + 1 : eol;
+ }
+ if (!*todo_list)
+ return error(_("No commits parsed."));
+ return 0;
+}
+
+static void read_populate_todo(struct replay_insn_list **todo_list)
+{
+ 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);
+ 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 replay_insn_list **todo_list,
+ struct replay_opts *opts)
+{
+ struct commit *commit;
+ struct replay_insn_list **next;
+ enum replay_action action;
+
+ prepare_revs(opts);
+
+ next = todo_list;
+ action = opts->command == REPLAY_CMD_REVERT ? REPLAY_REVERT : REPLAY_PICK;
+ while ((commit = get_revision(opts->revs)))
+ next = replay_insn_list_append(commit, action, next);
+}
+
+static int create_seq_dir(void)
+{
+ const char *seq_dir = git_path(SEQ_DIR);
+
+ if (file_exists(seq_dir)) {
+ error(_("a cherry-pick or revert is already in progress"));
+ advise(_("try \"git cherry-pick (--continue | --quit | --abort)\""));
+ return -1;
+ }
+ 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)
+{
+ 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 int reset_for_rollback(const unsigned char *sha1)
+{
+ const char *argv[4]; /* reset --merge <arg> + NULL */
+ argv[0] = "reset";
+ argv[1] = "--merge";
+ argv[2] = sha1_to_hex(sha1);
+ argv[3] = NULL;
+ return run_command_v_opt(argv, RUN_GIT_CMD);
+}
+
+static int rollback_single_pick(void)
+{
+ unsigned char head_sha1[20];
+
+ if (!file_exists(git_path("CHERRY_PICK_HEAD")) &&
+ !file_exists(git_path("REVERT_HEAD")))
+ return error(_("no cherry-pick or revert in progress"));
+ if (read_ref_full("HEAD", head_sha1, 0, NULL))
+ return error(_("cannot resolve HEAD"));
+ if (is_null_sha1(head_sha1))
+ return error(_("cannot abort from a branch yet to be born"));
+ return reset_for_rollback(head_sha1);
+}
+
+static int sequencer_rollback(struct replay_opts *opts)
+{
+ const char *filename;
+ FILE *f;
+ unsigned char sha1[20];
+ struct strbuf buf = STRBUF_INIT;
+
+ filename = git_path(SEQ_HEAD_FILE);
+ f = fopen(filename, "r");
+ if (!f && errno == ENOENT) {
+ /*
+ * There is no multiple-cherry-pick in progress.
+ * If CHERRY_PICK_HEAD or REVERT_HEAD indicates
+ * a single-cherry-pick in progress, abort that.
+ */
+ return rollback_single_pick();
+ }
+ if (!f)
+ return error(_("cannot open %s: %s"), filename,
+ strerror(errno));
+ if (strbuf_getline(&buf, f, '\n')) {
+ error(_("cannot read %s: %s"), filename, ferror(f) ?
+ strerror(errno) : _("unexpected end of file"));
+ fclose(f);
+ goto fail;
+ }
+ fclose(f);
+ if (get_sha1_hex(buf.buf, sha1) || buf.buf[40] != '\0') {
+ error(_("stored pre-cherry-pick HEAD file '%s' is corrupt"),
+ filename);
+ goto fail;
+ }
+ if (reset_for_rollback(sha1))
+ goto fail;
+ remove_sequencer_state();
+ strbuf_release(&buf);
+ return 0;
+fail:
+ strbuf_release(&buf);
+ return -1;
+}
+
+static void save_todo(struct replay_insn_list *todo_list)
+{
+ 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) < 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 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_insn_list *todo_list, struct replay_opts *opts)
+{
+ struct replay_insn_list *cur;
+ int res;
+
+ setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
+ if (opts->allow_ff)
+ assert(!(opts->signoff || opts->no_commit ||
+ opts->record_origin || opts->edit));
+ read_and_refresh_cache(opts);
+
+ for (cur = todo_list; cur; cur = cur->next) {
+ save_todo(cur);
+ res = do_pick_commit(cur->operand, cur->action, opts);
+ if (res)
+ return res;
+ }
+
+ /*
+ * Sequence of picks finished successfully; cleanup by
+ * removing the .git/sequencer directory
+ */
+ remove_sequencer_state();
+ return 0;
+}
+
+static int continue_single_pick(void)
+{
+ const char *argv[] = { "commit", NULL };
+
+ if (!file_exists(git_path("CHERRY_PICK_HEAD")) &&
+ !file_exists(git_path("REVERT_HEAD")))
+ return error(_("no cherry-pick or revert in progress"));
+ return run_command_v_opt(argv, RUN_GIT_CMD);
+}
+
+static int sequencer_continue(struct replay_opts *opts)
+{
+ struct replay_insn_list *todo_list = NULL;
+
+ if (!file_exists(git_path(SEQ_TODO_FILE)))
+ return continue_single_pick();
+ read_populate_opts(&opts);
+ read_populate_todo(&todo_list);
+
+ /* Verify that the conflict has been resolved */
+ if (file_exists(git_path("CHERRY_PICK_HEAD")) ||
+ file_exists(git_path("REVERT_HEAD"))) {
+ int ret = continue_single_pick();
+ if (ret)
+ return ret;
+ }
+ if (index_differs_from("HEAD", 0))
+ return error_dirty_index(opts);
+ todo_list = todo_list->next;
+ return pick_commits(todo_list, opts);
+}
+
+static int single_pick(struct commit *cmit, struct replay_opts *opts)
+{
+ enum replay_action action;
+ action = opts->command == REPLAY_CMD_REVERT ? REPLAY_REVERT : REPLAY_PICK;
+
+ setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
+ return do_pick_commit(cmit, action, opts);
+}
+
+int sequencer_pick_revisions(struct replay_opts *opts)
+{
+ struct replay_insn_list *todo_list = NULL;
+ unsigned char sha1[20];
+
+ if (opts->subcommand == REPLAY_NONE)
+ assert(opts->revs);
+
+ 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
+ */
+ if (opts->subcommand == REPLAY_REMOVE_STATE) {
+ remove_sequencer_state();
+ return 0;
+ }
+ if (opts->subcommand == REPLAY_ROLLBACK)
+ return sequencer_rollback(opts);
+ if (opts->subcommand == REPLAY_CONTINUE)
+ return sequencer_continue(opts);
+
+ /*
+ * If we were called as "git cherry-pick <commit>", just
+ * cherry-pick/revert it, set CHERRY_PICK_HEAD /
+ * REVERT_HEAD, and don't touch the sequencer state.
+ * This means it is possible to cherry-pick in the middle
+ * of a cherry-pick sequence.
+ */
+ if (opts->revs->cmdline.nr == 1 &&
+ opts->revs->cmdline.rev->whence == REV_CMD_REV &&
+ opts->revs->no_walk &&
+ !opts->revs->cmdline.rev->flags) {
+ struct commit *cmit;
+ if (prepare_revision_walk(opts->revs))
+ die(_("revision walk setup failed"));
+ cmit = get_revision(opts->revs);
+ if (!cmit || get_revision(opts->revs))
+ die("BUG: expected exactly one commit from walk");
+ return single_pick(cmit, opts);
+ }
+
+ /*
+ * 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);
+ if (create_seq_dir() < 0)
+ return -1;
+ if (get_sha1("HEAD", sha1)) {
+ if (opts->command == REPLAY_CMD_REVERT)
+ 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);
+}
diff --git a/sequencer.h b/sequencer.h
index 2d4528f..7035908 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -6,7 +6,55 @@
#define SEQ_TODO_FILE "sequencer/todo"
#define SEQ_OPTS_FILE "sequencer/opts"
+enum replay_action {
+ REPLAY_REVERT,
+ REPLAY_PICK
+};
+
+enum replay_command {
+ REPLAY_CMD_REVERT,
+ REPLAY_CMD_CHERRY_PICK
+};
+
+enum replay_subcommand {
+ REPLAY_NONE,
+ REPLAY_REMOVE_STATE,
+ REPLAY_CONTINUE,
+ REPLAY_ROLLBACK
+};
+
+struct replay_insn_list {
+ struct commit *operand;
+ enum replay_action action;
+ struct replay_insn_list *next;
+};
+
+struct replay_opts {
+ enum replay_command command;
+ enum replay_subcommand subcommand;
+
+ /* Boolean options */
+ int edit;
+ int record_origin;
+ int no_commit;
+ int signoff;
+ int allow_ff;
+ int allow_rerere_auto;
+
+ int mainline;
+
+ /* Merge strategy */
+ const char *strategy;
+ const char **xopts;
+ size_t xopts_nr, xopts_alloc;
+
+ /* Only used by REPLAY_NONE */
+ struct rev_info *revs;
+};
+
/* Removes SEQ_DIR. */
extern void remove_sequencer_state(void);
+extern int sequencer_pick_revisions(struct replay_opts *opts);
+
#endif
--
1.7.8.2
^ permalink raw reply related
* [PATCH 7/8] revert: use getn_sha1() to simplify insn parsing
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
To read the object name in the instruction sheet, we currently
manipulate the buffer to artificially introduce a NUL after the
supposed object name, and then use get_sha1() to read the object name
before restoring the buffer. Get rid of this ugliness by using
getn_sha1(), a function introduced in the previous patch.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 0954d22..187c317 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -750,8 +750,7 @@ static int parse_insn_line(char *bol, char *eol,
struct replay_insn_list *item, int lineno)
{
unsigned char commit_sha1[20];
- char *end_of_object_name;
- int saved, status;
+ int namelen;
if (!prefixcmp(bol, "pick ") || !prefixcmp(bol, "pick\t")) {
item->action = REPLAY_PICK;
@@ -766,13 +765,8 @@ static int parse_insn_line(char *bol, char *eol,
/* Eat up extra spaces/ tabs before object name */
bol += strspn(bol, " \t");
- end_of_object_name = bol + strcspn(bol, " \t\n");
- saved = *end_of_object_name;
- *end_of_object_name = '\0';
- status = get_sha1(bol, commit_sha1);
- *end_of_object_name = saved;
-
- if (status < 0)
+ namelen = strcspn(bol, " \t\n");
+ if (getn_sha1(bol, namelen, commit_sha1))
return parse_error(_("malformed object name"),
git_path(SEQ_TODO_FILE), lineno, bol);
--
1.7.8.2
^ permalink raw reply related
* [PATCH 6/8] sha1_name: introduce getn_sha1() to take length
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
Introduce a variant of get_sha1() that additionally takes the length
of the buffer, so it can parse object names from buffers that don't
necessarily terminate with NUL.
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
cache.h | 1 +
sha1_name.c | 23 +++++++++++++++++++----
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index 10afd71..8bb6759 100644
--- a/cache.h
+++ b/cache.h
@@ -812,6 +812,7 @@ struct object_context {
};
extern int get_sha1(const char *str, unsigned char *sha1);
+extern int getn_sha1(const char *name, int namelen, unsigned char *sha1);
extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int only_to_die, const char *prefix);
static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
{
diff --git a/sha1_name.c b/sha1_name.c
index 03ffc2c..31d412e 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1019,12 +1019,11 @@ static char *resolve_relative_path(const char *rel)
rel);
}
-int get_sha1_with_context_1(const char *name, unsigned char *sha1,
- struct object_context *oc,
- int only_to_die, const char *prefix)
+static int getn_sha1_with_context_1(const char *name, int namelen,
+ unsigned char *sha1, struct object_context *oc,
+ int only_to_die, const char *prefix)
{
int ret, bracket_depth;
- int namelen = strlen(name);
const char *cp;
memset(oc, 0, sizeof(*oc));
@@ -1134,3 +1133,19 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
}
return ret;
}
+
+int get_sha1_with_context_1(const char *name, unsigned char *sha1,
+ struct object_context *oc,
+ int only_to_die, const char *prefix)
+{
+ int namelen = strlen(name);
+ return getn_sha1_with_context_1(name, namelen, sha1,
+ oc, only_to_die, prefix);
+}
+
+/* A variant of get_sha1 that takes a length. */
+int getn_sha1(const char *name, int namelen, unsigned char *sha1)
+{
+ struct object_context unused;
+ return getn_sha1_with_context_1(name, namelen, sha1, &unused, 0, NULL);
+}
--
1.7.8.2
^ permalink raw reply related
* [PATCH 5/8] revert: report fine-grained errors from insn parser
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
The infrastructure that parses '.git/sequencer/todo' is meant to
handle arbitrary user input some day, so it can be used as the
implementation of 'git rebase --interactive' and 'git sequence
--edit'. It is currently sub-optimal for that purpose because the
parse error messages just say:
error: Could not parse line 5.
This patch shifts responsibility to parse_insn_line(), which can come
up with a more detailed message like:
error: .git/sequencer/todo:5: unrecognized action: frobnicate
Once the operator is allowed to edit the sequence, the message might
be adjusted to something like:
error: <sequence you just gave me>:5: unrecognized action: frobnicate
instead of exposing an implementation detail. Some day "git sequence
--edit" could even re-launch the editor with an error message in a
comment before the problematic line and the cursor pointing there.
For now, pointing to the explicit filename is useful since this should
only happen if there was filesystem corruption, tampering, or a git
bug.
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 30 ++++++++++++++++++++++++------
1 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 9a09471..0954d22 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -732,7 +732,22 @@ static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
return 0;
}
-static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
+static int parse_error(const char *message, const char *file,
+ int lineno, char *error_line)
+{
+ const char *suffix = "";
+ int error_len = strcspn(error_line, " \t\n");
+
+ if (error_len > 20) {
+ error_len = 20;
+ suffix = "...";
+ }
+ return error(_("%s:%d: %s: %.*s%s"), file, lineno, message,
+ error_len, error_line, suffix);
+}
+
+static int parse_insn_line(char *bol, char *eol,
+ struct replay_insn_list *item, int lineno)
{
unsigned char commit_sha1[20];
char *end_of_object_name;
@@ -745,7 +760,8 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
item->action = REPLAY_REVERT;
bol += strlen("revert ");
} else
- return -1;
+ return parse_error(_("unrecognized action"),
+ git_path(SEQ_TODO_FILE), lineno, bol);
/* Eat up extra spaces/ tabs before object name */
bol += strspn(bol, " \t");
@@ -757,11 +773,13 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
*end_of_object_name = saved;
if (status < 0)
- return -1;
+ return parse_error(_("malformed object name"),
+ git_path(SEQ_TODO_FILE), lineno, bol);
item->operand = lookup_commit_reference(commit_sha1);
if (!item->operand)
- return -1;
+ return parse_error(_("not a valid commit"),
+ git_path(SEQ_TODO_FILE), lineno, bol);
item->next = NULL;
return 0;
@@ -776,8 +794,8 @@ static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
for (i = 1; *p; i++) {
char *eol = strchrnul(p, '\n');
- if (parse_insn_line(p, eol, &item))
- return error(_("Could not parse line %d."), i);
+ if (parse_insn_line(p, eol, &item, i))
+ return -1;
next = replay_insn_list_append(item.operand, item.action, next);
p = *eol ? eol + 1 : eol;
}
--
1.7.8.2
^ permalink raw reply related
* [PATCH 4/8] revert: separate out parse errors logically
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
Three kinds of errors can arise from parsing the instruction sheet:
1. Unrecognized action
2. Malformed object name
3. Object name does not refer to a valid commit
The next patch makes an attempt to make the parser report meaningful
errors by replacing the "return -1" with "return error(...)"
appropriately. For the first kind of error, it is insufficient to
check if the buffer beings with a "pick" or "revert", otherwise the
following insn sheet would be interpreted as having a malformed object
name:
pickle a1fe57~2
In reality, the issue is that "pickle" is an unrecognized instruction.
So, check that the buffer starts with ("pick " or "pick\t") and
("revert " or "revert\t").
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 15 ++++++---------
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 1841ffa..9a09471 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -736,22 +736,19 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
{
unsigned char commit_sha1[20];
char *end_of_object_name;
- int saved, status, padding;
+ int saved, status;
- if (!prefixcmp(bol, "pick")) {
+ if (!prefixcmp(bol, "pick ") || !prefixcmp(bol, "pick\t")) {
item->action = REPLAY_PICK;
- bol += strlen("pick");
- } else if (!prefixcmp(bol, "revert")) {
+ bol += strlen("pick ");
+ } else if (!prefixcmp(bol, "revert ") || !prefixcmp(bol, "revert\t")) {
item->action = REPLAY_REVERT;
- bol += strlen("revert");
+ bol += strlen("revert ");
} else
return -1;
/* Eat up extra spaces/ tabs before object name */
- padding = strspn(bol, " \t");
- if (!padding)
- return -1;
- bol += padding;
+ bol += strspn(bol, " \t");
end_of_object_name = bol + strcspn(bol, " \t\n");
saved = *end_of_object_name;
--
1.7.8.2
^ permalink raw reply related
* [PATCH 3/8] revert: allow mixing "pick" and "revert" actions
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
Parse the instruction sheet in '.git/sequencer/todo' as a list of
(action, operand) pairs, instead of assuming that all lines have the
same action. Now, an instruction sheet like the following is
perfectly valid:
pick fdc0b12 picked
revert 965fed4 anotherpick
The operator can use this feature by hand-editing the instruction
sheet and using '--continue' as appropriate:
$ git cherry-pick foo..bar
[conflict occurs]
$ edit problematicfile
$ git add problematicfile
$ edit .git/sequencer/todo
$ git revert --continue
[finishes successfully]
Consequently, this means that a 'git cherry-pick --continue' can
continue an ongoing 'git revert' operation, and viceversa.
Helped-by: Jonathan Nieder <jrnider@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 142 ++++++++++++++++++++-------------------
t/t3510-cherry-pick-sequence.sh | 46 +++++++++----
2 files changed, 105 insertions(+), 83 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 9bca9c7..1841ffa 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -56,6 +56,12 @@ enum replay_subcommand {
REPLAY_ROLLBACK
};
+struct replay_insn_list {
+ struct commit *operand;
+ enum replay_action action;
+ struct replay_insn_list *next;
+};
+
struct replay_opts {
enum replay_command command;
enum replay_subcommand subcommand;
@@ -487,7 +493,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)
return run_command_v_opt(args, RUN_GIT_CMD);
}
-static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
+static int do_pick_commit(struct commit *commit, enum replay_action action,
+ struct replay_opts *opts)
{
unsigned char head[20];
struct commit *base, *next, *parent;
@@ -562,7 +569,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
defmsg = git_pathdup("MERGE_MSG");
- if (opts->command == REPLAY_CMD_REVERT) {
+ if (action == REPLAY_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -603,7 +610,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
}
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->command == REPLAY_CMD_REVERT) {
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
write_message(&msgbuf, defmsg);
@@ -627,13 +634,13 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
* However, if the merge did not even start, then we don't want to
* write it at all.
*/
- if (opts->command == REPLAY_CMD_CHERRY_PICK && !opts->no_commit && (res == 0 || res == 1))
+ if (action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
write_cherry_pick_head(commit, "CHERRY_PICK_HEAD");
- if (opts->command == REPLAY_CMD_REVERT && ((opts->no_commit && res == 0) || res == 1))
+ if (action == REPLAY_REVERT && ((opts->no_commit && res == 0) || res == 1))
write_cherry_pick_head(commit, "REVERT_HEAD");
if (res) {
- error(opts->command == REPLAY_CMD_REVERT
+ error(action == REPLAY_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -680,70 +687,70 @@ static void read_and_refresh_cache(struct replay_opts *opts)
}
/*
- * Append a commit to the end of the commit_list.
+ * Append a (commit, action) to the end of the replay_insn_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.
+ * empty replay_insn_list, and is updated to point to the "next" field of
+ * the last item on the list as new (commit, action) pairs are appended.
*
* Usage example:
*
- * struct commit_list *list;
- * struct commit_list **next = &list;
+ * struct replay_insn_list *list;
+ * struct replay_insn_list **next = &list;
*
- * next = commit_list_append(c1, next);
- * next = commit_list_append(c2, next);
- * assert(commit_list_count(list) == 2);
+ * next = replay_insn_list_append(c1, a1, next);
+ * next = replay_insn_list_append(c2, a2, next);
+ * assert(len(list) == 2);
* return list;
*/
-static struct commit_list **commit_list_append(struct commit *commit,
- struct commit_list **next)
+static struct replay_insn_list **replay_insn_list_append(struct commit *operand,
+ enum replay_action action,
+ struct replay_insn_list **next)
{
- struct commit_list *new = xmalloc(sizeof(struct commit_list));
- new->item = commit;
+ struct replay_insn_list *new = xmalloc(sizeof(*new));
+ new->action = action;
+ new->operand = operand;
*next = new;
new->next = NULL;
return &new->next;
}
-static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
- struct replay_opts *opts)
+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
{
- struct commit_list *cur = NULL;
- const char *sha1_abbrev = NULL;
- const char *action_str = opts->command == REPLAY_CMD_REVERT ? "revert" : "pick";
- const char *subject;
- int subject_len;
+ struct replay_insn_list *cur;
for (cur = todo_list; cur; cur = cur->next) {
- sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
- subject_len = find_commit_subject(cur->item->buffer, &subject);
+ const char *sha1_abbrev, *action_str, *subject;
+ int subject_len;
+
+ action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
+ sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
+ subject_len = find_commit_subject(cur->operand->buffer, &subject);
strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
subject_len, subject);
}
return 0;
}
-static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
+static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
{
unsigned char commit_sha1[20];
- enum replay_action action;
char *end_of_object_name;
int saved, status, padding;
if (!prefixcmp(bol, "pick")) {
- action = REPLAY_PICK;
+ item->action = REPLAY_PICK;
bol += strlen("pick");
} else if (!prefixcmp(bol, "revert")) {
- action = REPLAY_REVERT;
+ item->action = REPLAY_REVERT;
bol += strlen("revert");
} else
- return NULL;
+ return -1;
/* Eat up extra spaces/ tabs before object name */
padding = strspn(bol, " \t");
if (!padding)
- return NULL;
+ return -1;
bol += padding;
end_of_object_name = bol + strcspn(bol, " \t\n");
@@ -752,38 +759,29 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
status = get_sha1(bol, commit_sha1);
*end_of_object_name = saved;
- /*
- * Verify that the action matches up with the one in
- * opts; we don't support arbitrary instructions
- */
- if ((action == REPLAY_PICK && opts->command == REPLAY_CMD_REVERT) ||
- (action == REPLAY_REVERT && opts->command == REPLAY_CMD_CHERRY_PICK)) {
- error(_("Cannot %s during a %s"),
- action == REPLAY_REVERT ? "revert" : "pick",
- command_name(opts));
- return NULL;
- }
-
if (status < 0)
- return NULL;
+ return -1;
+
+ item->operand = lookup_commit_reference(commit_sha1);
+ if (!item->operand)
+ return -1;
- return lookup_commit_reference(commit_sha1);
+ item->next = NULL;
+ return 0;
}
-static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
- struct replay_opts *opts)
+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
{
- struct commit_list **next = todo_list;
- struct commit *commit;
+ struct replay_insn_list **next = todo_list;
+ struct replay_insn_list item = { NULL, 0, NULL };
char *p = buf;
int i;
for (i = 1; *p; i++) {
char *eol = strchrnul(p, '\n');
- commit = parse_insn_line(p, eol, opts);
- if (!commit)
+ if (parse_insn_line(p, eol, &item))
return error(_("Could not parse line %d."), i);
- next = commit_list_append(commit, next);
+ next = replay_insn_list_append(item.operand, item.action, next);
p = *eol ? eol + 1 : eol;
}
if (!*todo_list)
@@ -791,8 +789,7 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
return 0;
}
-static void read_populate_todo(struct commit_list **todo_list,
- struct replay_opts *opts)
+static void read_populate_todo(struct replay_insn_list **todo_list)
{
const char *todo_file = git_path(SEQ_TODO_FILE);
struct strbuf buf = STRBUF_INIT;
@@ -808,7 +805,7 @@ static void read_populate_todo(struct commit_list **todo_list,
}
close(fd);
- res = parse_insn_buffer(buf.buf, todo_list, opts);
+ res = parse_insn_buffer(buf.buf, todo_list);
strbuf_release(&buf);
if (res)
die(_("Unusable instruction sheet: %s"), todo_file);
@@ -857,17 +854,19 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
die(_("Malformed options sheet: %s"), opts_file);
}
-static void walk_revs_populate_todo(struct commit_list **todo_list,
+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
struct replay_opts *opts)
{
struct commit *commit;
- struct commit_list **next;
+ struct replay_insn_list **next;
+ enum replay_action action;
prepare_revs(opts);
next = todo_list;
+ action = opts->command == REPLAY_CMD_REVERT ? REPLAY_REVERT : REPLAY_PICK;
while ((commit = get_revision(opts->revs)))
- next = commit_list_append(commit, next);
+ next = replay_insn_list_append(commit, action, next);
}
static int create_seq_dir(void)
@@ -965,7 +964,7 @@ fail:
return -1;
}
-static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+static void save_todo(struct replay_insn_list *todo_list)
{
const char *todo_file = git_path(SEQ_TODO_FILE);
static struct lock_file todo_lock;
@@ -973,7 +972,7 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
int fd;
fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
- if (format_todo(&buf, todo_list, opts) < 0)
+ if (format_todo(&buf, todo_list) < 0)
die(_("Could not format %s."), todo_file);
if (write_in_full(fd, buf.buf, buf.len) < 0) {
strbuf_release(&buf);
@@ -1017,9 +1016,9 @@ static void save_opts(struct replay_opts *opts)
}
}
-static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
+static int pick_commits(struct replay_insn_list *todo_list, struct replay_opts *opts)
{
- struct commit_list *cur;
+ struct replay_insn_list *cur;
int res;
setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
@@ -1029,8 +1028,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
read_and_refresh_cache(opts);
for (cur = todo_list; cur; cur = cur->next) {
- save_todo(cur, opts);
- res = do_pick_commit(cur->item, opts);
+ save_todo(cur);
+ res = do_pick_commit(cur->operand, cur->action, opts);
if (res)
return res;
}
@@ -1055,12 +1054,12 @@ static int continue_single_pick(void)
static int sequencer_continue(struct replay_opts *opts)
{
- struct commit_list *todo_list = NULL;
+ struct replay_insn_list *todo_list = NULL;
if (!file_exists(git_path(SEQ_TODO_FILE)))
return continue_single_pick();
read_populate_opts(&opts);
- read_populate_todo(&todo_list, opts);
+ read_populate_todo(&todo_list);
/* Verify that the conflict has been resolved */
if (file_exists(git_path("CHERRY_PICK_HEAD")) ||
@@ -1077,13 +1076,16 @@ static int sequencer_continue(struct replay_opts *opts)
static int single_pick(struct commit *cmit, struct replay_opts *opts)
{
+ enum replay_action action;
+ action = opts->command == REPLAY_CMD_REVERT ? REPLAY_REVERT : REPLAY_PICK;
+
setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
- return do_pick_commit(cmit, opts);
+ return do_pick_commit(cmit, action, opts);
}
static int pick_revisions(struct replay_opts *opts)
{
- struct commit_list *todo_list = NULL;
+ struct replay_insn_list *todo_list = NULL;
unsigned char sha1[20];
if (opts->subcommand == REPLAY_NONE)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 97f3710..af747c8 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -454,7 +454,7 @@ test_expect_success 'sign-off needs to be reaffirmed after conflict resolution,
! grep Signed-off-by: msg
'
-test_expect_success 'malformed instruction sheet 1' '
+test_expect_success 'malformed instruction sheet, action' '
pristine_detach initial &&
test_expect_code 1 git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
@@ -465,23 +465,12 @@ test_expect_success 'malformed instruction sheet 1' '
test_expect_code 128 git cherry-pick --continue
'
-test_expect_success 'malformed instruction sheet 2' '
- pristine_detach initial &&
- test_expect_code 1 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_expect_code 128 git cherry-pick --continue
-'
-
test_expect_success 'empty commit set' '
pristine_detach initial &&
test_expect_code 128 git cherry-pick base..base
'
-test_expect_success 'malformed instruction sheet 3' '
+test_expect_success 'malformed instruction sheet, object name' '
pristine_detach initial &&
test_expect_code 1 git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
@@ -517,4 +506,35 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
test_line_count = 4 commits
'
+test_expect_success 'mixed pick and revert instructions' '
+ pristine_detach initial &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ oldsha=`git rev-parse --short HEAD~1` &&
+ echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
+ git revert --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 unrelated
+ 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_done
--
1.7.8.2
^ permalink raw reply related
* [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
Currently, 'git cherry-pick' fills up the '.git/sequencer/todo'
instruction sheet with "pick" actions, while 'git revert' fills it up
with "revert" actions. Inspired by the way 'rebase -i' works, we
would like to permit mixing arbitrary actions in the same instruction
sheet. To do this, we first have to decouple the notion of an action
in the instruction sheet from builtin commands. While a future
instruction sheet would look like:
pickle next~4
action3 b74fea
revert rr/moo^2~34
the actions "pickle", "action3" and "revert" need not necessarily
correspond to the specific builtins. Introduce a "replay_command",
and replace instances of "replay_action" with "replay_command"
everywhere except in parse_insn_line(), the function that parses the
instruction sheet.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 63 ++++++++++++++++++++++++++++++------------------------
1 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 2739405..9bca9c7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -44,6 +44,11 @@ enum replay_action {
REPLAY_PICK
};
+enum replay_command {
+ REPLAY_CMD_REVERT,
+ REPLAY_CMD_CHERRY_PICK
+};
+
enum replay_subcommand {
REPLAY_NONE,
REPLAY_REMOVE_STATE,
@@ -52,7 +57,7 @@ enum replay_subcommand {
};
struct replay_opts {
- enum replay_action action;
+ enum replay_command command;
enum replay_subcommand subcommand;
/* Boolean options */
@@ -76,16 +81,16 @@ struct replay_opts {
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
-static const char *action_name(const struct replay_opts *opts)
+static const char *command_name(struct replay_opts *opts)
{
- return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
+ return opts->command == REPLAY_CMD_REVERT ? "revert" : "cherry-pick";
}
static char *get_encoding(const char *message);
static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
{
- return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
+ return opts->command == REPLAY_CMD_REVERT ? revert_usage : cherry_pick_usage;
}
static int option_parse_x(const struct option *opt,
@@ -142,7 +147,7 @@ static void verify_opt_mutually_compatible(const char *me, ...)
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);
+ const char *me = command_name(opts);
int remove_state = 0;
int contin = 0;
int rollback = 0;
@@ -164,7 +169,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
OPT_END(),
};
- if (opts->action == REPLAY_PICK) {
+ if (opts->command == REPLAY_CMD_CHERRY_PICK) {
struct option cp_extra[] = {
OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"),
OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"),
@@ -375,10 +380,10 @@ static struct tree *empty_tree(void)
static int error_dirty_index(struct replay_opts *opts)
{
if (read_cache_unmerged())
- return error_resolve_conflict(action_name(opts));
+ return error_resolve_conflict(command_name(opts));
/* Different translation strings for cherry-pick and revert */
- if (opts->action == REPLAY_PICK)
+ if (opts->command == REPLAY_CMD_CHERRY_PICK)
error(_("Your local changes would be overwritten by cherry-pick."));
else
error(_("Your local changes would be overwritten by revert."));
@@ -434,7 +439,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
(write_cache(index_fd, active_cache, active_nr) ||
commit_locked_index(&index_lock)))
/* TRANSLATORS: %s will be "revert" or "cherry-pick" */
- die(_("%s: Unable to write new index file"), action_name(opts));
+ die(_("%s: Unable to write new index file"), command_name(opts));
rollback_lock_file(&index_lock);
if (!clean) {
@@ -542,7 +547,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
/* TRANSLATORS: The first %s will be "revert" or
"cherry-pick", the second %s a SHA1 */
return error(_("%s: cannot parse parent commit %s"),
- action_name(opts), sha1_to_hex(parent->object.sha1));
+ command_name(opts), sha1_to_hex(parent->object.sha1));
if (get_message(commit, &msg) != 0)
return error(_("Cannot get commit message for %s"),
@@ -557,7 +562,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
defmsg = git_pathdup("MERGE_MSG");
- if (opts->action == REPLAY_REVERT) {
+ if (opts->command == REPLAY_CMD_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -598,7 +603,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
}
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REPLAY_REVERT) {
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->command == REPLAY_CMD_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
write_message(&msgbuf, defmsg);
@@ -622,13 +627,13 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
* However, if the merge did not even start, then we don't want to
* write it at all.
*/
- if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
+ if (opts->command == REPLAY_CMD_CHERRY_PICK && !opts->no_commit && (res == 0 || res == 1))
write_cherry_pick_head(commit, "CHERRY_PICK_HEAD");
- if (opts->action == REPLAY_REVERT && ((opts->no_commit && res == 0) || res == 1))
+ if (opts->command == REPLAY_CMD_REVERT && ((opts->no_commit && res == 0) || res == 1))
write_cherry_pick_head(commit, "REVERT_HEAD");
if (res) {
- error(opts->action == REPLAY_REVERT
+ error(opts->command == REPLAY_CMD_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -648,7 +653,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
static void prepare_revs(struct replay_opts *opts)
{
- if (opts->action != REPLAY_REVERT)
+ if (opts->command != REPLAY_CMD_REVERT)
opts->revs->reverse ^= 1;
if (prepare_revision_walk(opts->revs))
@@ -663,12 +668,13 @@ static void read_and_refresh_cache(struct replay_opts *opts)
static struct lock_file index_lock;
int index_fd = hold_locked_index(&index_lock, 0);
if (read_index_preload(&the_index, NULL) < 0)
- die(_("git %s: failed to read the index"), action_name(opts));
+ die(_("git %s: failed to read the index"), command_name(opts));
refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
if (the_index.cache_changed) {
if (write_index(&the_index, index_fd) ||
commit_locked_index(&index_lock))
- die(_("git %s: failed to refresh the index"), action_name(opts));
+ die(_("git %s: failed to refresh the index"),
+ command_name(opts));
}
rollback_lock_file(&index_lock);
}
@@ -705,7 +711,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
{
struct commit_list *cur = NULL;
const char *sha1_abbrev = NULL;
- const char *action_str = opts->action == REPLAY_REVERT ? "revert" : "pick";
+ const char *action_str = opts->command == REPLAY_CMD_REVERT ? "revert" : "pick";
const char *subject;
int subject_len;
@@ -750,10 +756,11 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
* 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 == REPLAY_REVERT ? "revert" : "cherry-pick";
- error(_("Cannot %s during a %s"), action_str, action_name(opts));
+ if ((action == REPLAY_PICK && opts->command == REPLAY_CMD_REVERT) ||
+ (action == REPLAY_REVERT && opts->command == REPLAY_CMD_CHERRY_PICK)) {
+ error(_("Cannot %s during a %s"),
+ action == REPLAY_REVERT ? "revert" : "pick",
+ command_name(opts));
return NULL;
}
@@ -1015,7 +1022,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
struct commit_list *cur;
int res;
- setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
+ setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
if (opts->allow_ff)
assert(!(opts->signoff || opts->no_commit ||
opts->record_origin || opts->edit));
@@ -1070,7 +1077,7 @@ static int sequencer_continue(struct replay_opts *opts)
static int single_pick(struct commit *cmit, struct replay_opts *opts)
{
- setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
+ setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
return do_pick_commit(cmit, opts);
}
@@ -1128,7 +1135,7 @@ static int pick_revisions(struct replay_opts *opts)
if (create_seq_dir() < 0)
return -1;
if (get_sha1("HEAD", sha1)) {
- if (opts->action == REPLAY_REVERT)
+ if (opts->command == REPLAY_CMD_REVERT)
return error(_("Can't revert as initial commit"));
return error(_("Can't cherry-pick into empty head"));
}
@@ -1145,7 +1152,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
memset(&opts, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
- opts.action = REPLAY_REVERT;
+ opts.command = REPLAY_CMD_REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
@@ -1160,7 +1167,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
int res;
memset(&opts, 0, sizeof(opts));
- opts.action = REPLAY_PICK;
+ opts.command = REPLAY_CMD_CHERRY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
--
1.7.8.2
^ permalink raw reply related
* [PATCH 1/8] revert: prepare to move replay_action to header
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
Later in the series, we will build a generalized sequencer by
factoring code out of the revert builtin, and leaving it with just
argument parsing work. This involves moving "replay_action" to
sequencer.h, so that both sequencer.c and builtin/revert.c can use it.
Unfortunately, "REVERT" and "CHERRY_PICK" are unsuitable variable
names, as their purpose is unclear in the global context; in
anticipation of the move, rename them to "REPLAY_REVERT" and
"REPLAY_PICK" respectively.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 40 ++++++++++++++++++++++------------------
1 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 0d8020c..2739405 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -39,7 +39,11 @@ static const char * const cherry_pick_usage[] = {
NULL
};
-enum replay_action { REVERT, CHERRY_PICK };
+enum replay_action {
+ REPLAY_REVERT,
+ REPLAY_PICK
+};
+
enum replay_subcommand {
REPLAY_NONE,
REPLAY_REMOVE_STATE,
@@ -74,14 +78,14 @@ struct replay_opts {
static const char *action_name(const struct replay_opts *opts)
{
- return opts->action == REVERT ? "revert" : "cherry-pick";
+ return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
}
static char *get_encoding(const char *message);
static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
{
- return opts->action == REVERT ? revert_usage : cherry_pick_usage;
+ return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
}
static int option_parse_x(const struct option *opt,
@@ -160,7 +164,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
OPT_END(),
};
- if (opts->action == CHERRY_PICK) {
+ if (opts->action == REPLAY_PICK) {
struct option cp_extra[] = {
OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"),
OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"),
@@ -374,7 +378,7 @@ static int error_dirty_index(struct replay_opts *opts)
return error_resolve_conflict(action_name(opts));
/* Different translation strings for cherry-pick and revert */
- if (opts->action == CHERRY_PICK)
+ if (opts->action == REPLAY_PICK)
error(_("Your local changes would be overwritten by cherry-pick."));
else
error(_("Your local changes would be overwritten by revert."));
@@ -553,7 +557,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
defmsg = git_pathdup("MERGE_MSG");
- if (opts->action == REVERT) {
+ if (opts->action == REPLAY_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -594,7 +598,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
}
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REVERT) {
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REPLAY_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
write_message(&msgbuf, defmsg);
@@ -618,13 +622,13 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
* However, if the merge did not even start, then we don't want to
* write it at all.
*/
- if (opts->action == CHERRY_PICK && !opts->no_commit && (res == 0 || res == 1))
+ if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
write_cherry_pick_head(commit, "CHERRY_PICK_HEAD");
- if (opts->action == REVERT && ((opts->no_commit && res == 0) || res == 1))
+ if (opts->action == REPLAY_REVERT && ((opts->no_commit && res == 0) || res == 1))
write_cherry_pick_head(commit, "REVERT_HEAD");
if (res) {
- error(opts->action == REVERT
+ error(opts->action == REPLAY_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -644,7 +648,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
static void prepare_revs(struct replay_opts *opts)
{
- if (opts->action != REVERT)
+ if (opts->action != REPLAY_REVERT)
opts->revs->reverse ^= 1;
if (prepare_revision_walk(opts->revs))
@@ -701,7 +705,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
{
struct commit_list *cur = NULL;
const char *sha1_abbrev = NULL;
- const char *action_str = opts->action == REVERT ? "revert" : "pick";
+ const char *action_str = opts->action == REPLAY_REVERT ? "revert" : "pick";
const char *subject;
int subject_len;
@@ -722,10 +726,10 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
int saved, status, padding;
if (!prefixcmp(bol, "pick")) {
- action = CHERRY_PICK;
+ action = REPLAY_PICK;
bol += strlen("pick");
} else if (!prefixcmp(bol, "revert")) {
- action = REVERT;
+ action = REPLAY_REVERT;
bol += strlen("revert");
} else
return NULL;
@@ -748,7 +752,7 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
*/
if (action != opts->action) {
const char *action_str;
- action_str = action == REVERT ? "revert" : "cherry-pick";
+ action_str = action == REPLAY_REVERT ? "revert" : "cherry-pick";
error(_("Cannot %s during a %s"), action_str, action_name(opts));
return NULL;
}
@@ -1124,7 +1128,7 @@ static int pick_revisions(struct replay_opts *opts)
if (create_seq_dir() < 0)
return -1;
if (get_sha1("HEAD", sha1)) {
- if (opts->action == REVERT)
+ if (opts->action == REPLAY_REVERT)
return error(_("Can't revert as initial commit"));
return error(_("Can't cherry-pick into empty head"));
}
@@ -1141,7 +1145,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
memset(&opts, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
- opts.action = REVERT;
+ opts.action = REPLAY_REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
@@ -1156,7 +1160,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
int res;
memset(&opts, 0, sizeof(opts));
- opts.action = CHERRY_PICK;
+ opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
--
1.7.8.2
^ permalink raw reply related
* [PATCH v2 0/8] The move to sequencer.c
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326025653-11922-1-git-send-email-artagnon@gmail.com>
Hi,
The big changes in this round are:
1. Dropped "revert: don't let revert continue a cherry-pick" from the
last round after a quick discussion with Jonathan.
2. Separated out "revert: separate out parse errors logically" from
"revert: report fine-grained errors from insn parser". Definitely
looks clearer.
3. Improved "revert: report fine-grained errors from insn parser" by
eliminating repetition. 20 is a bit arbitrary, but it looks pretty
enough on my terminal.
4. Added "sha1_name: introduce getn_sha1() to take length" and
"revert: use getn_sha1() to simplify insn parsing". I'm happy with
them. Name is inspired from the strncmp() variant of strcmp().
5. Included minimal API documentation with "sequencer: factor code out
of revert builtin".
Thanks for reading. I think I'll work on fixing the memory leaks now.
Ramkumar Ramachandra (8):
revert: prepare to move replay_action to header
revert: decouple sequencer actions from builtin commands
revert: allow mixing "pick" and "revert" actions
revert: separate out parse errors logically
revert: report fine-grained errors from insn parser
sha1_name: introduce getn_sha1() to take length
revert: use getn_sha1() to simplify insn parsing
sequencer: factor code out of revert builtin
Documentation/technical/api-sequencer.txt | 22 +
builtin/revert.c | 958 +----------------------------
cache.h | 1 +
sequencer.c | 925 ++++++++++++++++++++++++++++-
sequencer.h | 48 ++
sha1_name.c | 23 +-
t/t3510-cherry-pick-sequence.sh | 46 +-
7 files changed, 1056 insertions(+), 967 deletions(-)
create mode 100644 Documentation/technical/api-sequencer.txt
--
1.7.8.2
^ permalink raw reply
* Re: Auto-refresh git-gui
From: Uri Okrent @ 2012-01-10 16:14 UTC (permalink / raw)
To: victor.engmark; +Cc: git
In-Reply-To: <20120105080322.GD3484@victor>
Not to muddy the waters, but if you're open to alternatives, if you
have inotify installed, git cola will automatically update it's status
whenever files in your repository change.
--
Uri
Please consider the environment before printing this message.
http://wwf.panda.org/savepaper/
^ permalink raw reply
* Re: [PATCH 5/6] revert: report fine-grained error messages from insn parser
From: Ramkumar Ramachandra @ 2012-01-10 15:24 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <20120108213318.GQ1942@burratino>
Jonathan Nieder wrote:
> My "alternatively" was bogus --- lookup_commit_reference takes a (raw)
> full commit name as its argument.
>
> I dunno. The question was not actually rhetorical --- I just meant
> that it's worth thinking about these cases. There are a few cases:
>
> - missing object
> - object is present but corrupt
> - object is a blob, not a commit
>
> In the second case, there's an error message printed describing the
> problem, but in the other two there isn't. The other callers tend to
> say "not a valid <foo>" or "could not lookup commit <foo>, so I guess
>
> error: .git/sequencer/todo:5: not a valid commit: 78a89f493
>
> would be fine.
>
> Except that this focusses on the .git/sequencer/todo filename which
> would leave the person debugging astray. It is not that
> .git/sequencer/todo contains a typo (that would have been caught by
> get_sha1), but that it referred to a bad object or non-commit. Maybe
> something in the direction of
>
> error: cannot pick 78a89f493 because it is not a valid commit
>
> would be more helpful.
>
> Is this the right moment to report that error? Will the operator be
> happy that we errored out right away before cherry-picking anything
> and wasting the human's time in assisting with that process, or will
> she be unhappy that inability to do something later that she might
> have been planning on skipping anyway prevented making progress right
> away? (I'm not sure what the best thing to do is --- I guess some
> advice like
>
> hint: to abort, use cherry-pick --abort
> hint: to skip or replace that commit, use cherry-pick --edit
>
> would help.)
Ignoring this in the re-roll: I'd be inclined to put these changes in
a separate series that begins with bolting on some advice
configuration to the sequencer.
Thanks for thinking about these things.
-- Ram
^ permalink raw reply
* Re: [PATCH 6/6] sequencer: factor code out of revert builtin
From: Ramkumar Ramachandra @ 2012-01-10 15:21 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <20120108203800.GM1942@burratino>
Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>> memset(&opts, 0, sizeof(opts));
>> opts.command = REPLAY_CMD_FOO;
>> opts.revisions = xmalloc(sizeof(*opts.revs));
>> parse_args_populate_opts(argc, argv, &opts);
>> init_revisions(opts.revs);
>> sequencer_pick_revisions(&opts);
>
> Hm, I wonder if opts.command should be a string so each new caller
> does not have to add to the enum and switch statements...
The new caller would have to add enum and switch statements to define
a new action anyway, so I think this should be an enum too.
-- Ram
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox