* Re: How to run http server tests?
From: Jeff King @ 2011-12-14 21:39 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <4EE91307.6080504@kdbg.org>
On Wed, Dec 14, 2011 at 10:20:07PM +0100, Johannes Sixt wrote:
> I have a hard time running tests that use lib-httpd.sh.
>
> I run the tests like this:
>
> LIB_HTTPD_MODULE_PATH=/usr/lib64/apache2 GIT_TEST_HTTPD=Yes \
> sh -x t5541-http-push.sh -v -i
>
> and I have to apply this patch because I do not have mod_cgi on my
> system (OpenSuse 11.4):
>
> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> index 0a4cdfa..e5cb3f9 100644
> --- a/t/lib-httpd/apache.conf
> +++ b/t/lib-httpd/apache.conf
> @@ -13,4 +13,4 @@ ErrorLog error.log
> </IfModule>
> -<IfModule !mod_cgi.c>
> - LoadModule cgi_module modules/mod_cgi.so
> +<IfModule !mod_scgi.c>
> + LoadModule scgi_module modules/mod_scgi.so
> </IfModule>
Hmm. I know nothing about scgi, but a quick google indicates that it is
a separate protocol from CGI and is more like FastCGI (i.e., it wants to
spawn a long-running CGI server and contact it over a separate
protocol).
So I suspect you are not able to run http-backend, and thus you have no
smart-http support in your setup.
> I see t5541-http-push.sh #2 fail with:
>
> ++ cd '/home/jsixt/Src/git/git/t/trash directory.t5541-http-push'
> ++ git clone http://127.0.0.1:5541/smart/test_repo.git/ test_repo_clone
> Cloning into 'test_repo_clone'...
> fatal: http://127.0.0.1:5541/smart/test_repo.git/info/refs not found:
> did you run git update-server-info on the server?
That's would I would expect if you have no smart-http support. The git
client will fall back to trying dumb http, but that should fail (because
we haven't run update-server-info).
The other errors you see probably stem from the same issue.
-Peff
^ permalink raw reply
* How to run http server tests?
From: Johannes Sixt @ 2011-12-14 21:20 UTC (permalink / raw)
To: Git Mailing List
I have a hard time running tests that use lib-httpd.sh.
I run the tests like this:
LIB_HTTPD_MODULE_PATH=/usr/lib64/apache2 GIT_TEST_HTTPD=Yes \
sh -x t5541-http-push.sh -v -i
and I have to apply this patch because I do not have mod_cgi on my
system (OpenSuse 11.4):
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 0a4cdfa..e5cb3f9 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -13,4 +13,4 @@ ErrorLog error.log
</IfModule>
-<IfModule !mod_cgi.c>
- LoadModule cgi_module modules/mod_cgi.so
+<IfModule !mod_scgi.c>
+ LoadModule scgi_module modules/mod_scgi.so
</IfModule>
I see t5541-http-push.sh #2 fail with:
++ cd '/home/jsixt/Src/git/git/t/trash directory.t5541-http-push'
++ git clone http://127.0.0.1:5541/smart/test_repo.git/ test_repo_clone
Cloning into 'test_repo_clone'...
fatal: http://127.0.0.1:5541/smart/test_repo.git/info/refs not found:
did you run git update-server-info on the server?
t5551-http-fetch.sh fails at #3 here:
++ GIT_CURL_VERBOSE=1
++ git clone --quiet http://127.0.0.1:5551/smart/repo.git clone
+ eval_ret=128
In this case, 'git clone' output is redirected to file 'err'; it has the
same error as above.
t5561-http-backend.sh fails at #3 like so:
+++ diff -u exp act
--- exp 2011-12-14 21:14:49.000000000 +0000
+++ act 2011-12-14 21:14:49.000000000 +0000
@@ -1 +1 @@
-HTTP/1.1 200 OK
+HTTP/1.1 404 Not Found
Can someone help?
Thanks,
-- Hannes
^ permalink raw reply related
* Re: Question about commit message wrapping
From: Sidney San Martín @ 2011-12-14 21:07 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <201112102030.15504.jnareb@gmail.com>
On Dec 10, 2011, at 2:30 PM, Jakub Narebski wrote:
> BTW. In Polish (and Czech) typography there is rule that you don't leave
> single-letter prepositions like 'i' ('and' in English) hanging at the end
> of the line... automatic wrapper cannot ensure that.
I meant to ask… how is that rule followed by browsers/on the web?
^ permalink raw reply
* Re: Question about commit message wrapping
From: Sidney San Martín @ 2011-12-14 21:04 UTC (permalink / raw)
To: Michael Haggerty
Cc: Frans Klaver, Holger Hellmuth, Andrew Ardill, JakubNarebski,
git@vger.kernel.org
In-Reply-To: <4EE6C31C.60909@alum.mit.edu>
On Dec 12, 2011, at 10:14 PM, Michael Haggerty wrote:
> FWIW I think automatic wrapping of commit messages is a bad idea. I
> wrap my commit messages deliberately to make them look the way I want
> them to look. The assumption of an 80-character display has historical
> reasons, but it is also a relatively comfortable line-width to read
> (even on wider displays).
A lot of Git repos live in heterogeneous environments. I played a little with some of the popular Git interfaces I can use on my computer. The majority of them…
- compose and show commit messages in a proportional font (where the width of and formatting in "80 characters" means nothing),
- don’t insert line breaks when you write a commit message (and don't provide a way to do so automatically),
- do wrap commit messages when showing them.
Jakub, you said that education was the answer to getting some consistency in line wrapping, but I have trouble imagining the makers of new tools using fixed-width text for anything other than code.
> And given that commit messages sometimes
> contain "flowable" paragraph text, sometimes code snippets, sometimes
> ASCII art, etc, no automatic wrapping will work correctly unless
> everybody agrees that commit messages must be written in some specific
> form of markup (or lightweight markup). And I can't imagine such a
> thing ever happening.
The two biggest websites I know of for talking about code, GitHub and Stack Overflow, both adopted flavors of Markdown. It is basically the formatting syntax already used for commit messages in the Git project itself (this email too), so can be formatted to look good in a specific environment (i.e. proportional fonts) and looks good by itself.
(Actually, as far as I can tell commit messages are the only place GitHub doesn’t currently render user-entered text as Markdown.)
I think, now and in the future, consistency will be found most easily in in Markdown-like formatting and least in 80 columns of fixed-width text.
- - -
## Gitbox 1.5.1
- Commit messages written and displayed in a proportional font
- Does not insert line breaks when you commit
- When displaying a commit message, turns single line breaks into spaces and keeps double line breaks, wraps as needed based on the size of the viewing area (it's somewhat intelligent about this, it does preserve some line breaks; it doesn't preserve spaces used for formatting)
## Tower 1.0.3
- Commit messages written and displayed in a proportional font
- Does not insert line breaks when you commit. Collapses multiple newlines into a single newline
- When displaying a commit message, wraps at 100 characters and collapses multiple newlines into a single one
## GitHub (Mac app) 1.1.1
- Commit messages written and displayed in a proportional font
- Inserts line breaks after 73 characters when you commit (incl. in the middle of a long identifier)
- Does not wrap lines when displaying commit messages, and doesn’t provide a way to scroll to read them
## GitHub (website)
- Commit messages written and displayed in a fixed-width font
- Does not insert line breaks when you commit, input wraps, visually, at 113 characters
- When displaying a commit message, uses CSS to wrap the commit message to 700 pixels, 100 characters
## git (default configuration in a fresh Linux Mint live cd)
- Does not insert line breaks when you commit (using the default editor (nano) and pager (less, IIRC)
- Does not wrap lines when displaying commit messages.
^ permalink raw reply
* Re: [BUG] in rev-parse
From: Jeff King @ 2011-12-14 21:01 UTC (permalink / raw)
To: nathan.panike; +Cc: git
In-Reply-To: <20111214184926.GB18335@llunet.cs.wisc.edu>
On Wed, Dec 14, 2011 at 12:49:27PM -0600, nathan.panike@gmail.com wrote:
> In my local git.git:
>
> $ git rev-parse 73c6b3575bc638b7096ec913bd91193707e2265d^@
> 57526fde5df201a99afa6d122c3266b3a1c5673a
> 942e6baa92846e5628752c65a22bc4957d8de4d0
>
> $ git rev-parse --short 73c6b3575bc638b7096ec913bd91193707e2265d^@
> 57526fd
> 942e6ba
> fatal: Needed a single revision
>
> ^^^ I don't believe this "fatal" message should be here
It looks like "--short" implies "--verify", and you cannot "--verify"
multiple sha1s.
But the documentation for "--short" says only:
--short::
--short=number::
Instead of outputting the full SHA1 values of object names try to
abbreviate them to a shorter unique name. When no length is specified
7 is used. The minimum length is 4.
which would imply to me that not only should your example work, but this
should, too:
$ git rev-parse HEAD HEAD^
803b1a83b0ec5f04dfb770e83e11211e0015630f
28c2058b6048d32abc0a23b827ab0b26a0332b9b
$ git rev-parse --short HEAD HEAD^
fatal: Needed a single revision
On the other hand, it has been like this since it was introduced in
2006, and I wonder if scripts rely on the --verify side effect.
As a work-around, you can get what you want with:
git rev-list --no-walk --abbrev-commit $sha1^@
-Peff
^ permalink raw reply
* [BUG] in rev-parse
From: nathan.panike @ 2011-12-14 18:49 UTC (permalink / raw)
To: git
In my local git.git:
$ git rev-parse 73c6b3575bc638b7096ec913bd91193707e2265d^@
57526fde5df201a99afa6d122c3266b3a1c5673a
942e6baa92846e5628752c65a22bc4957d8de4d0
$ git rev-parse --short 73c6b3575bc638b7096ec913bd91193707e2265d^@
57526fd
942e6ba
fatal: Needed a single revision
^^^ I don't believe this "fatal" message should be here
$ git version
git version 1.7.8
Nathan Panike
^ permalink raw reply
* Re: [PATCH 3/3] Do not create commits whose message contains NUL
From: Jeff King @ 2011-12-14 18:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git, Miles Bader
In-Reply-To: <7vzkevow2j.fsf@alter.siamese.dyndns.org>
On Wed, Dec 14, 2011 at 10:19:16AM -0800, Junio C Hamano wrote:
> Limiting the Porcelain layer to deal only with reasonable text encodings
> (yes, I am declaring that utf16 is not among them) is perfectly fine, but
> I was somehow hoping that you would allow the option for the low-level
> function commit_tree() to create a commit object with binary blob in the
> body part, especially after seeing the patch 1/3 to do so.
>
> Certainly that kind of usage would not give the binary blob literally in
> "git log" output, but it is with or without the issue around NUL byte. A
> custom program linked with commit.c to call commit_tree() may not be using
> the data structure to store anything that is meant to be read by "git log"
> to begin with.
I'm happy to ignore custom programs linking against internal git code,
but what should "git commit-tree" do?
My gut feeling is that it should store the literal binary contents.
However, I don't think this has ever been the case. Even in the initial
version of commit-tree.c, we read the input line-by-line and sprintf it
into place.
-Peff
^ permalink raw reply
* Re: [PATCH 3/3] Do not create commits whose message contains NUL
From: Junio C Hamano @ 2011-12-14 18:19 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Jeff King, Miles Bader
In-Reply-To: <1323871699-8839-4-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> We assume that the commit log messages are uninterpreted sequences of
> non-NUL bytes (see Documentation/i18n.txt). However the assumption
> does not really stand out and it's quite easy to set an editor to save
> in a NUL-included encoding. Currently we silently cut at the first NUL
> we see.
>
> Make it more obvious that NUL is not welcome by refusing to create
> such commits. Those who deliberately want to create them can still do
> with hash-object.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Limiting the Porcelain layer to deal only with reasonable text encodings
(yes, I am declaring that utf16 is not among them) is perfectly fine, but
I was somehow hoping that you would allow the option for the low-level
function commit_tree() to create a commit object with binary blob in the
body part, especially after seeing the patch 1/3 to do so.
Certainly that kind of usage would not give the binary blob literally in
"git log" output, but it is with or without the issue around NUL byte. A
custom program linked with commit.c to call commit_tree() may not be using
the data structure to store anything that is meant to be read by "git log"
to begin with.
Not a strong veto at all, just throwing out something to think about.
^ permalink raw reply
* Re: [PATCH v2 4/4] use wrapper for unchecked setenv/putenv calls
From: Jeff King @ 2011-12-14 18:16 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, gitster, schwab
In-Reply-To: <1323871631-2872-5-git-send-email-kusmabite@gmail.com>
On Wed, Dec 14, 2011 at 03:07:11PM +0100, Erik Faye-Lund wrote:
> This avoids us from accidentally dropping state, possibly leading
> to unexpected behaviour.
I do think this is fine in a "be extra cautious" kind of way.
> This is especially important on Windows, where the maximum size of
> the environment is 32 kB.
But does your patch actually detect that? As Andreas pointed out, these
limits don't typically come into play at setenv time. Instead, the
environment is allocated on the heap, and then the result is passed to
exec/spawn, which will fail.
So your patch is really detecting a failure to malloc, not an overflow
of the environment size, and Windows is just as (un)likely to run out of
heap as any other platform.
You can check how your platform behaves by applying this patch:
diff --git a/git.c b/git.c
index f10e434..57f6b12 100644
--- a/git.c
+++ b/git.c
@@ -223,6 +223,16 @@ static int handle_alias(int *argcp, const char ***argv)
alias_argv[i] = (*argv)[i];
alias_argv[argc] = NULL;
+ /* make gigantic environment */
+ {
+ int len = 256 * 1024;
+ char *buf = xmalloc(len);
+ memset(buf, 'z', len);
+ buf[len-1] = '\0';
+ if (setenv("FOO", buf, 1))
+ die("setenv failed");
+ }
+
ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
if (ret >= 0) /* normal exit */
exit(ret);
and then running:
git -c alias.foo='!echo ok' foo
which yields:
fatal: cannot exec 'echo ok': Argument list too long
on Linux.
-Peff
PS I tried to come up with an invocation of git that would demonstrate
this, but it turns out it's really hard. The contents of environment
variables we set are either constants, come from the environment (so
they can't be too big already!), or come from filesystem paths. So
it's possible to overflow now, but you have to have a nearly-full
environment in the first place, and then have a long path that tips
it over the limit.
^ permalink raw reply related
* Re: [PATCH 2/3] merge: abort if fails to commit
From: Junio C Hamano @ 2011-12-14 18:13 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Jeff King, Miles Bader
In-Reply-To: <1323871699-8839-3-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> builtin/merge.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index df4548a..e57eefa 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -913,7 +913,9 @@ static int merge_trivial(struct commit *head)
> parent->next->item = remoteheads->item;
> parent->next->next = NULL;
> prepare_to_commit();
> - commit_tree(merge_msg.buf, merge_msg.len, result_tree, parent, result_commit, NULL);
> + if (commit_tree(merge_msg.buf, merge_msg.len,
> + result_tree, parent, result_commit, NULL))
> + die(_("failed to write commit object"));
> finish(head, result_commit, "In-index merge");
> drop_save();
> return 0;
Should we die immediately, or should we do some clean-ups after ourselves
before doing so?
In any case, this is a good change that shouldn't be taken hostage to the
unrelated change in patch [1/3].
Thanks.
> @@ -944,7 +946,9 @@ static int finish_automerge(struct commit *head,
> strbuf_addch(&merge_msg, '\n');
> prepare_to_commit();
> free_commit_list(remoteheads);
> - commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents, result_commit, NULL);
> + if (commit_tree(merge_msg.buf, merge_msg.len,
> + result_tree, parents, result_commit, NULL))
> + die(_("failed to write commit object"));
> strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
> finish(head, result_commit, buf.buf);
> strbuf_release(&buf);
^ permalink raw reply
* Re: [PATCH 1/3] Make commit_tree() take message length in addition to the commit message
From: Junio C Hamano @ 2011-12-14 18:12 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Jeff King, Miles Bader
In-Reply-To: <1323871699-8839-2-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Justification?
As all 3 primary users of this API feed strbuf.buf to the function, it
would make more sense to change the first parameter to a pointer to a
strbuf, no?
^ permalink raw reply
* Re: Revisiting metadata storage
From: Richard Hartmann @ 2011-12-14 17:59 UTC (permalink / raw)
To: Ronan Keryell; +Cc: Git List
In-Reply-To: <87sjkx8gll.fsf@an-dro.info.enstb.org>
On Tue, Dec 6, 2011 at 23:45, Ronan Keryell
<Ronan.Keryell@hpc-project.com> wrote:
> At least I'm interested and began to dig into it but I do not have a lot
> of time to work on it...
If we can agree on Perl, I can try to help. I don't think I speak
enough Python to be of use with that.
Other people who have an interest in this: Please pipe up so we can
hammer out a rough consensus & roadmap.
Richard
^ permalink raw reply
* Re: [bug?] checkout -m doesn't work without a base version
From: Junio C Hamano @ 2011-12-14 17:54 UTC (permalink / raw)
To: Michael Schubert; +Cc: Pete Harlan, git
In-Reply-To: <4EE8782A.9040507@elegosoft.com>
Michael Schubert <mschub@elegosoft.com> writes:
>> ...
>> + memset(threeway, 0, sizeof(threeway));
>> + while (pos < active_nr) {
>> + int stage;
>> + stage = ce_stage(ce);
>> + if (!stage || strcmp(path, ce->name))
>> + break;
>> + hashcpy(threeway[stage - 1], ce->sha1);
>> + if (stage == 2)
>> + mode = create_ce_mode(ce->ce_mode);
>> + pos++;
>> + ce = active_cache[pos];
>> + }
>> + if (is_null_sha1(threeway[1]) || is_null_sha1(threeway[2]))
>> + return error(_("path '%s' does not have necessary versions"), path);
>> ...
>> + ce = make_cache_entry(mode, sha1, path, 2, 0);
>> if (!ce)
>> die(_("make_cache_entry failed for path '%s'"), path);
>> status = checkout_entry(ce, state, NULL);
>
> gcc 4.6.2:
>
> builtin/checkout.c: In function ‘cmd_checkout’:
> builtin/checkout.c:210:5: warning: ‘mode’ may be used uninitialized in this function [-Wuninitialized]
> builtin/checkout.c:160:11: note: ‘mode’ was declared here
Isn't this just your gcc being overly cautious (aka "silly")?
The variable "mode" is assigned to when we see an stage #2 entry in the
loop, and we should have updated threeway[1] immediately before doing so.
If threeway[1] is not updated, we would have already returned before using
the variable in make_cache_entry().
^ permalink raw reply
* Re: Git difftool / mergetool on directory tree
From: Junio C Hamano @ 2011-12-14 17:50 UTC (permalink / raw)
To: Thomas Rast; +Cc: Daniele Segato, Git Mailing List
In-Reply-To: <201112141725.14729.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Daniele Segato wrote:
>> Hi,
>>
>> many diff / merge tool around have the ability to compare a directory
>> tree (meld is one, but there are many).
>>
>> Is there a way to start a difftool or a mergetool on a folder instead of
>> the single file?
>>
>> It would be an handsome feature to git.
>>
>> I googled a little before popping out this question and I only found
>> suggestion on how to open "many" file at once instead of opening them
>> serially but that's not the same thing not as powerful as directory
>> comparison.
>
> Maybe this helps:
>
> http://thread.gmane.org/gmane.comp.version-control.git/144839
>
> It was never applied however.
A later reincarnation of a similar idea was in
http://thread.gmane.org/gmane.comp.version-control.git/184458/focus=184462
but the interest petered out, it seems.
^ permalink raw reply
* Re: tr/pty-all (Re: What's cooking in git.git (Dec 2011, #04; Tue, 13))
From: Junio C Hamano @ 2011-12-14 17:42 UTC (permalink / raw)
To: Thomas Rast
Cc: Jonathan Nieder, git, Jeff King,
Nguyễn Thái Ngọc Duy, Michael Haggerty
In-Reply-To: <201112141717.15021.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Jonathan Nieder wrote:
>> Junio C Hamano wrote:
>> > Will merge to 'next' after taking another look.
>>
>> The middle commit looks good. The bottom commit could be improved as
>> discussed at [1], but I guess that can happen in-tree.
>>
>> However, the top commit ("test test-terminal's sanity") still does not
>> seem right to me.
>
> I wasn't under the impression that we were done with this, either :-)
>
>> It makes the same test run three times. Probably I should send an
>> alternate patch to get that sanity-check to run once, but I am also
>> not convinced the sanity-check is needed at all --- wouldn't any test
>> that is relying on output from test_terminal act as a sanity check for
>> it already?
>
> It didn't. Or more precisely, Michael Haggerty ran into the behavior
> of
>
> git rev-parse ... | while read sha; do git checkout $sha; make test; done
>
> couldn't make any sense of it, and reported it on IRC. So in some
> sense, it took infrequent circumstances and two developers' time; next
> time around I'd prefer it to be detected automatically.
>
>> As an aside, I also still believe that running "git shortlog" without
>> explicitly passing "HEAD" when testing how it reacts to [core] pager
>> configuration was a bug and a distraction, hence the patch at [2].
>
> Why not. Some other test should verify how shortlog reacts to the
> tty-ness of stdin, but that's yet another direction.
>
>> I also find Jeff's patch [3] appealing.
>
> Me too, though wonder whether feeding a file full of garbage wouldn't
> be better, so as to trip up commands that try to read only from a
> non-tty stdin.
Well, I guess I was too quick to pull the trigger after sending the
"What's cooking" out. Sorry about that.
On the other hand, I think these require relatively low impact changes
that can be handled in-tree and the downsides of the series like running
prerequisite tests more than once are not serious show stoppers, so it
isn't a disaster ;-)
Thanks both for noticing and commenting. Very much appreciated.
^ permalink raw reply
* [PATCH 10/10] revert: report fine-grained error messages from insn parser
From: Ramkumar Ramachandra @ 2011-12-14 16:54 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano
In-Reply-To: <1323881677-11117-1-git-send-email-artagnon@gmail.com>
Three kinds of errors can arise from parsing '.git/sequencer/todo':
1. Unrecognized action
2. Missing space after valid action prefix
3. Malformed object name
4. Object name does not refer to a valid commit
Since we would like to make the instruction sheet user-editable in the
future (much like the 'rebase -i' sheet), report more fine-grained
parse errors prefixed with the filename and line number.
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 | 37 ++++++++++++++++++++++++++-----------
1 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 50af439..d106c3c 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -705,11 +705,14 @@ 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_insn_line(char *bol, char *eol,
+ struct replay_insn_list *item, int lineno)
{
+ const char *todo_file = git_path(SEQ_TODO_FILE);
unsigned char commit_sha1[20];
char *end_of_object_name;
int saved, status, padding;
+ size_t error_len;
if (!prefixcmp(bol, "pick")) {
item->action = REPLAY_PICK;
@@ -717,13 +720,19 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
} else if (!prefixcmp(bol, "revert")) {
item->action = REPLAY_REVERT;
bol += strlen("revert");
- } else
- return -1;
+ } else {
+ error_len = eol - bol > 255 ? 255 : eol - bol;
+ return error(_("%s:%d: Unrecognized action: %.*s"),
+ todo_file, lineno, (int)error_len, bol);
+ }
/* Eat up extra spaces/ tabs before object name */
padding = strspn(bol, " \t");
- if (!padding)
- return -1;
+ if (!padding) {
+ error_len = eol - bol > 255 ? 255 : eol - bol;
+ return error(_("%s:%d: Missing space after action: %.*s"),
+ todo_file, lineno, (int)error_len, bol);
+ }
bol += padding;
end_of_object_name = bol + strcspn(bol, " \t\n");
@@ -732,12 +741,18 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
status = get_sha1(bol, commit_sha1);
*end_of_object_name = saved;
- if (status < 0)
- return -1;
+ if (status < 0) {
+ error_len = eol - bol > 255 ? 255 : eol - bol;
+ return error(_("%s:%d: Malformed object name: %.*s"),
+ todo_file, lineno, (int)error_len, bol);
+ }
item->operand = lookup_commit_reference(commit_sha1);
- if (!item->operand)
- return -1;
+ if (!item->operand) {
+ error_len = eol - bol > 255 ? 255 : eol - bol;
+ return error(_("%s:%d: Not a valid commit: %.*s"),
+ todo_file, lineno, (int)error_len, bol);
+ }
item->next = NULL;
return 0;
@@ -752,8 +767,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) < 0)
- return error(_("Could not parse line %d."), i);
+ if (parse_insn_line(p, eol, &item, i) < 0)
+ return -1;
next = replay_insn_list_append(item.action, item.operand, next);
p = *eol ? eol + 1 : eol;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 09/10] revert: allow mixed pick and revert instructions
From: Ramkumar Ramachandra @ 2011-12-14 16:54 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano
In-Reply-To: <1323881677-11117-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 instructions use
the same action. Now you can do:
pick fdc0b12 picked
revert 965fed4 anotherpick
For cherry-pick and revert, this means that a 'git cherry-pick
--continue' can continue an ongoing revert operation and viceversa.
Helped-by: Jonathan Nieder <jrnider@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 107 ++++++++++++++++++---------------------
sequencer.h | 6 ++
t/t3510-cherry-pick-sequence.sh | 58 +++++++++++++++++++++
3 files changed, 114 insertions(+), 57 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index ed062ea..50af439 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -459,7 +459,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;
@@ -534,7 +535,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
defmsg = git_pathdup("MERGE_MSG");
- if (opts->action == REPLAY_REVERT) {
+ if (action == REPLAY_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -575,7 +576,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") || action == REPLAY_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
write_message(&msgbuf, defmsg);
@@ -605,7 +606,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
write_cherry_pick_head(commit, "REVERT_HEAD");
if (res) {
- error(opts->action == REPLAY_REVERT
+ error(action == REPLAY_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -675,54 +676,54 @@ static void read_and_refresh_cache(struct replay_opts *opts)
* assert(commit_list_count(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(enum replay_action action,
+ struct commit *operand,
+ 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->action == REPLAY_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");
@@ -731,37 +732,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 != 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));
- return NULL;
- }
-
if (status < 0)
- return NULL;
+ return -1;
- return lookup_commit_reference(commit_sha1);
+ item->operand = lookup_commit_reference(commit_sha1);
+ if (!item->operand)
+ return -1;
+
+ 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 = { 0, NULL, 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) < 0)
return error(_("Could not parse line %d."), i);
- next = commit_list_append(commit, next);
+ next = replay_insn_list_append(item.action, item.operand, next);
p = *eol ? eol + 1 : eol;
}
if (!*todo_list)
@@ -769,8 +762,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;
@@ -786,7 +778,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);
@@ -835,18 +827,18 @@ 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 rev_info revs;
struct commit *commit;
- struct commit_list **next;
+ struct replay_insn_list **next;
prepare_revs(&revs, opts);
next = todo_list;
while ((commit = get_revision(&revs)))
- next = commit_list_append(commit, next);
+ next = replay_insn_list_append(opts->action, commit, next);
}
static int create_seq_dir(void)
@@ -944,7 +936,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;
@@ -952,7 +944,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);
@@ -996,9 +988,10 @@ 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, action_name(opts), 0);
@@ -1008,8 +1001,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) {
if (!cur->next)
/*
@@ -1034,7 +1027,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *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];
read_and_refresh_cache(opts);
@@ -1054,7 +1047,7 @@ static int pick_revisions(struct replay_opts *opts)
if (!file_exists(git_path(SEQ_TODO_FILE)))
return error(_("No %s in progress"), action_name(opts));
read_populate_opts(&opts);
- read_populate_todo(&todo_list, opts);
+ read_populate_todo(&todo_list);
/* Verify that the conflict has been resolved */
if (!index_differs_from("HEAD", 0))
diff --git a/sequencer.h b/sequencer.h
index 9b1b94d..648f7bf 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -19,6 +19,12 @@ enum replay_subcommand {
REPLAY_ROLLBACK
};
+struct replay_insn_list {
+ enum replay_action action;
+ struct commit *operand;
+ struct replay_insn_list *next;
+};
+
/*
* Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
* any errors. Intended to be used by 'git reset'.
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 1bfbe41..9b19917 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -356,4 +356,62 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
test_line_count = 4 commits
'
+test_expect_success 'revert --continue continues after cherry-pick' '
+ pristine_detach initial &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ 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 foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '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 cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M 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.4.1
^ permalink raw reply related
* [PATCH 08/10] revert: move replay_action, replay_subcommand to header
From: Ramkumar Ramachandra @ 2011-12-14 16:54 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano
In-Reply-To: <1323881677-11117-1-git-send-email-artagnon@gmail.com>
In our vision to build a generalized sequencer, the revert builtin
will be left only with argument parsing work before calling into the
sequencer. Since the enums replay_action and replay_subcommand have
nothing to do with this argument parsing, move them to sequencer.h.
However, REVERT and CHERRY_PICK are unsuitable variable names for
publicly exposing, as their purpose is unclear in the global context:
rename them to REPLAY_REVERT and REPLAY_PICK respectively.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 42 +++++++++++++++++-------------------------
sequencer.h | 12 ++++++++++++
2 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 0a4b215..ed062ea 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -39,14 +39,6 @@ static const char * const cherry_pick_usage[] = {
NULL
};
-enum replay_action { REVERT, CHERRY_PICK };
-enum replay_subcommand {
- REPLAY_NONE,
- REPLAY_REMOVE_STATE,
- REPLAY_CONTINUE,
- REPLAY_ROLLBACK
-};
-
struct replay_opts {
enum replay_action action;
enum replay_subcommand subcommand;
@@ -73,14 +65,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,
@@ -159,7 +151,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"),
@@ -363,7 +355,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."));
@@ -542,7 +534,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;
@@ -583,7 +575,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);
@@ -607,13 +599,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),
@@ -637,7 +629,7 @@ static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
init_revisions(revs, NULL);
revs->no_walk = 1;
- if (opts->action != REVERT)
+ if (opts->action != REPLAY_REVERT)
revs->reverse = 1;
argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
@@ -698,7 +690,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;
@@ -719,10 +711,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;
@@ -745,7 +737,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;
}
@@ -1080,7 +1072,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"));
}
@@ -1097,7 +1089,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);
@@ -1112,7 +1104,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);
diff --git a/sequencer.h b/sequencer.h
index f435fdb..9b1b94d 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -7,6 +7,18 @@
#define SEQ_TODO_FILE "sequencer/todo"
#define SEQ_OPTS_FILE "sequencer/opts"
+enum replay_action {
+ REPLAY_REVERT,
+ REPLAY_PICK
+};
+
+enum replay_subcommand {
+ REPLAY_NONE,
+ REPLAY_REMOVE_STATE,
+ REPLAY_CONTINUE,
+ REPLAY_ROLLBACK
+};
+
/*
* Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
* any errors. Intended to be used by 'git reset'.
--
1.7.4.1
^ permalink raw reply related
* [PATCH 07/10] t3510 (cherry-pick-sequencer): remove malformed sheet 2
From: Ramkumar Ramachandra @ 2011-12-14 16:54 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano
In-Reply-To: <1323881677-11117-1-git-send-email-artagnon@gmail.com>
The next patch allows mixing "pick" and "revert" instruction in the
same instruction sheet. By removing the "malformed instruction sheet
2" test in advance, it'll be easier to see the changes made by the
future patches that touch this file.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t3510-cherry-pick-sequence.sh | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 163ce7d..1bfbe41 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -309,7 +309,7 @@ test_expect_success '--signoff is not automatically propagated to resolved confl
grep "Signed-off-by:" anotherpick_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 &&
@@ -320,18 +320,7 @@ 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 '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 &&
--
1.7.4.1
^ permalink raw reply related
* [PATCH 06/10] t3502, t3510: clarify cherry-pick -m failure
From: Ramkumar Ramachandra @ 2011-12-14 16:54 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano
In-Reply-To: <1323881677-11117-1-git-send-email-artagnon@gmail.com>
The "cherry-pick persists opts correctly" test in t3510
(cherry-pick-sequence) can cause some confusion, because the command
actually has two points of failure:
1. "-m 1" is specified on the command-line despite the base commit
"initial" not being a merge-commit.
2. The revision range indicates that there will be a conflict that
needs to be resolved.
Although the former error is trapped, and cherry-pick die()s with the
exit status 128, the reader may be distracted by the latter. Fix this
by changing the revision range to something that wouldn't cause a
conflict. Additionally, explicitly check the exit code in
"cherry-pick a non-merge with -m should fail" in t3502
(cherry-pick-merge) to reassure the reader that this failure has
nothing to do with the sequencer itself.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t3502-cherry-pick-merge.sh | 2 +-
t/t3510-cherry-pick-sequence.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t3502-cherry-pick-merge.sh b/t/t3502-cherry-pick-merge.sh
index 0ab52da..e37547f 100755
--- a/t/t3502-cherry-pick-merge.sh
+++ b/t/t3502-cherry-pick-merge.sh
@@ -35,7 +35,7 @@ test_expect_success 'cherry-pick a non-merge with -m should fail' '
git reset --hard &&
git checkout a^0 &&
- test_must_fail git cherry-pick -m 1 b &&
+ test_expect_code 128 git cherry-pick -m 1 b &&
git diff --exit-code a --
'
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index b6e822e..163ce7d 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -53,7 +53,7 @@ test_expect_success 'cherry-pick persists data on failure' '
test_expect_success 'cherry-pick persists opts correctly' '
pristine_detach initial &&
- test_expect_code 128 git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours base..anotherpick &&
+ test_expect_code 128 git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours initial..anotherpick &&
test_path_is_dir .git/sequencer &&
test_path_is_file .git/sequencer/head &&
test_path_is_file .git/sequencer/todo &&
--
1.7.4.1
^ permalink raw reply related
* [PATCH 05/10] t3510 (cherry-pick-sequencer): use exit status
From: Ramkumar Ramachandra @ 2011-12-14 16:54 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano
In-Reply-To: <1323881677-11117-1-git-send-email-artagnon@gmail.com>
All the tests asserting failure use 'test_must_fail', which simply
checks for a non-zero exit status, potentially hiding underlying bugs.
So, replace instances of 'test_must_fail' with 'test_expect_code' to
check the exit status explicitly, where appropriate.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t3510-cherry-pick-sequence.sh | 58 +++++++++++++++++++-------------------
1 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 781c5ac..b6e822e 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -44,7 +44,7 @@ test_expect_success setup '
test_expect_success 'cherry-pick persists data on failure' '
pristine_detach initial &&
- test_must_fail git cherry-pick -s base..anotherpick &&
+ test_expect_code 1 git cherry-pick -s base..anotherpick &&
test_path_is_dir .git/sequencer &&
test_path_is_file .git/sequencer/head &&
test_path_is_file .git/sequencer/todo &&
@@ -53,7 +53,7 @@ test_expect_success 'cherry-pick persists data on failure' '
test_expect_success 'cherry-pick persists opts correctly' '
pristine_detach initial &&
- test_must_fail git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours base..anotherpick &&
+ test_expect_code 128 git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours base..anotherpick &&
test_path_is_dir .git/sequencer &&
test_path_is_file .git/sequencer/head &&
test_path_is_file .git/sequencer/todo &&
@@ -93,7 +93,7 @@ test_expect_success '--abort requires cherry-pick in progress' '
test_expect_success '--quit cleans up sequencer state' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..picked &&
+ test_expect_code 1 git cherry-pick base..picked &&
git cherry-pick --quit &&
test_path_is_missing .git/sequencer
'
@@ -107,7 +107,7 @@ test_expect_success '--quit keeps HEAD and conflicted index intact' '
:000000 100644 OBJID OBJID A foo
:000000 100644 OBJID OBJID A unrelated
EOF
- test_must_fail git cherry-pick base..picked &&
+ test_expect_code 1 git cherry-pick base..picked &&
git cherry-pick --quit &&
test_path_is_missing .git/sequencer &&
test_must_fail git update-index --refresh &&
@@ -121,7 +121,7 @@ test_expect_success '--quit keeps HEAD and conflicted index intact' '
test_expect_success '--abort to cancel multiple cherry-pick' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
git cherry-pick --abort &&
test_path_is_missing .git/sequencer &&
test_cmp_rev initial HEAD &&
@@ -131,7 +131,7 @@ test_expect_success '--abort to cancel multiple cherry-pick' '
test_expect_success '--abort to cancel single cherry-pick' '
pristine_detach initial &&
- test_must_fail git cherry-pick picked &&
+ test_expect_code 1 git cherry-pick picked &&
git cherry-pick --abort &&
test_path_is_missing .git/sequencer &&
test_cmp_rev initial HEAD &&
@@ -141,7 +141,7 @@ test_expect_success '--abort to cancel single cherry-pick' '
test_expect_success 'cherry-pick --abort to cancel multiple revert' '
pristine_detach anotherpick &&
- test_must_fail git revert base..picked &&
+ test_expect_code 1 git revert base..picked &&
git cherry-pick --abort &&
test_path_is_missing .git/sequencer &&
test_cmp_rev anotherpick HEAD &&
@@ -151,7 +151,7 @@ test_expect_success 'cherry-pick --abort to cancel multiple revert' '
test_expect_success 'revert --abort works, too' '
pristine_detach anotherpick &&
- test_must_fail git revert base..picked &&
+ test_expect_code 1 git revert base..picked &&
git revert --abort &&
test_path_is_missing .git/sequencer &&
test_cmp_rev anotherpick HEAD
@@ -159,7 +159,7 @@ test_expect_success 'revert --abort works, too' '
test_expect_success '--abort to cancel single revert' '
pristine_detach anotherpick &&
- test_must_fail git revert picked &&
+ test_expect_code 1 git revert picked &&
git revert --abort &&
test_path_is_missing .git/sequencer &&
test_cmp_rev anotherpick HEAD &&
@@ -170,7 +170,7 @@ test_expect_success '--abort to cancel single revert' '
test_expect_success '--abort keeps unrelated change, easy case' '
pristine_detach unrelatedpick &&
echo changed >expect &&
- test_must_fail git cherry-pick picked..yetanotherpick &&
+ test_expect_code 1 git cherry-pick picked..yetanotherpick &&
echo changed >unrelated &&
git cherry-pick --abort &&
test_cmp expect unrelated
@@ -179,7 +179,7 @@ test_expect_success '--abort keeps unrelated change, easy case' '
test_expect_success '--abort refuses to clobber unrelated change, harder case' '
pristine_detach initial &&
echo changed >expect &&
- test_must_fail git cherry-pick base..anotherpick &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
echo changed >unrelated &&
test_must_fail git cherry-pick --abort &&
test_cmp expect unrelated &&
@@ -194,7 +194,7 @@ test_expect_success '--abort refuses to clobber unrelated change, harder case' '
test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..picked &&
+ test_expect_code 1 git cherry-pick base..picked &&
test_path_is_missing .git/sequencer &&
echo "resolved" >foo &&
git add foo &&
@@ -218,7 +218,7 @@ test_expect_success 'cherry-pick cleans up sequencer state when one commit is le
test_expect_failure '--abort after last commit in sequence' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..picked &&
+ test_expect_code 1 git cherry-pick base..picked &&
git cherry-pick --abort &&
test_path_is_missing .git/sequencer &&
test_cmp_rev initial HEAD &&
@@ -228,27 +228,27 @@ test_expect_failure '--abort after last commit in sequence' '
test_expect_success 'cherry-pick does not implicitly stomp an existing operation' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
test-chmtime -v +0 .git/sequencer >expect &&
- test_must_fail git cherry-pick unrelatedpick &&
+ test_expect_code 128 git cherry-pick unrelatedpick &&
test-chmtime -v +0 .git/sequencer >actual &&
test_cmp expect actual
'
test_expect_success '--continue complains when no cherry-pick is in progress' '
pristine_detach initial &&
- test_must_fail git cherry-pick --continue
+ test_expect_code 128 git cherry-pick --continue
'
test_expect_success '--continue complains when there are unresolved conflicts' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- test_must_fail git cherry-pick --continue
+ test_expect_code 1 git cherry-pick base..anotherpick &&
+ test_expect_code 128 git cherry-pick --continue
'
test_expect_success '--continue continues after conflicts are resolved' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
echo "c" >foo &&
git add foo &&
git commit &&
@@ -275,7 +275,7 @@ test_expect_success '--continue continues after conflicts are resolved' '
test_expect_success '--continue respects opts' '
pristine_detach initial &&
- test_must_fail git cherry-pick -x base..anotherpick &&
+ test_expect_code 1 git cherry-pick -x base..anotherpick &&
echo "c" >foo &&
git add foo &&
git commit &&
@@ -293,7 +293,7 @@ test_expect_success '--continue respects opts' '
test_expect_success '--signoff is not automatically propagated to resolved conflict' '
pristine_detach initial &&
- test_must_fail git cherry-pick --signoff base..anotherpick &&
+ test_expect_code 1 git cherry-pick --signoff base..anotherpick &&
echo "c" >foo &&
git add foo &&
git commit &&
@@ -311,40 +311,40 @@ test_expect_success '--signoff is not automatically propagated to resolved confl
test_expect_success 'malformed instruction sheet 1' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
git add foo &&
git commit &&
sed "s/pick /pick/" .git/sequencer/todo >new_sheet &&
cp new_sheet .git/sequencer/todo &&
- test_must_fail git cherry-pick --continue
+ test_expect_code 128 git cherry-pick --continue
'
test_expect_success 'malformed instruction sheet 2' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
+ 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_must_fail git cherry-pick --continue
+ test_expect_code 128 git cherry-pick --continue
'
test_expect_success 'malformed instruction sheet 3' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
git add foo &&
git commit &&
sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
cp new_sheet .git/sequencer/todo &&
- test_must_fail git cherry-pick --continue
+ test_expect_code 128 git cherry-pick --continue
'
test_expect_success 'instruction sheet, fat-fingers version' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
echo "c" >foo &&
git add foo &&
git commit &&
@@ -355,7 +355,7 @@ test_expect_success 'instruction sheet, fat-fingers version' '
test_expect_success 'commit descriptions in insn sheet are optional' '
pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
echo "c" >foo &&
git add foo &&
git commit &&
--
1.7.4.1
^ permalink raw reply related
* [PATCH 04/10] revert: simplify getting commit subject in format_todo()
From: Ramkumar Ramachandra @ 2011-12-14 16:54 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano
In-Reply-To: <1323881677-11117-1-git-send-email-artagnon@gmail.com>
format_todo() calls get_message(), but uses only the subject line of
the commit message. As a minor optimization, save work and
unnecessary memory allocations by using find_commit_subject() instead.
Also, remove the unnecessary check on cur->item->buffer: the
lookup_commit_reference() call in parse_insn_line() has already made
sure of this.
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index be0686d..0a4b215 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -697,16 +697,16 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
struct replay_opts *opts)
{
struct commit_list *cur = NULL;
- struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
const char *sha1_abbrev = NULL;
const char *action_str = opts->action == REVERT ? "revert" : "pick";
+ const char *subject;
+ int subject_len;
for (cur = todo_list; cur; cur = cur->next) {
sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
- if (get_message(cur->item, &msg))
- return error(_("Cannot get commit message for %s"), sha1_abbrev);
- strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
- free_message(&msg);
+ subject_len = find_commit_subject(cur->item->buffer, &subject);
+ strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
+ subject_len, subject);
}
return 0;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 03/10] revert: tolerate extra spaces, tabs in insn sheet
From: Ramkumar Ramachandra @ 2011-12-14 16:54 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano
In-Reply-To: <1323881677-11117-1-git-send-email-artagnon@gmail.com>
Tolerate extra spaces and tabs as part of the the field separator in
'.git/sequencer/todo', for people with fat fingers.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 18 ++++++++++++------
t/t3510-cherry-pick-sequence.sh | 11 +++++++++++
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 70055e5..be0686d 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -716,18 +716,24 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
unsigned char commit_sha1[20];
enum replay_action action;
char *end_of_object_name;
- int saved, status;
+ int saved, status, padding;
- if (!prefixcmp(bol, "pick ")) {
+ if (!prefixcmp(bol, "pick")) {
action = CHERRY_PICK;
- bol += strlen("pick ");
- } else if (!prefixcmp(bol, "revert ")) {
+ bol += strlen("pick");
+ } else if (!prefixcmp(bol, "revert")) {
action = REVERT;
- bol += strlen("revert ");
+ bol += strlen("revert");
} else
return NULL;
- end_of_object_name = bol + strcspn(bol, " \n");
+ /* Eat up extra spaces/ tabs before object name */
+ padding = strspn(bol, " \t");
+ if (!padding)
+ return NULL;
+ bol += padding;
+
+ 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);
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 6390f2a..781c5ac 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -342,6 +342,17 @@ test_expect_success 'malformed instruction sheet 3' '
test_must_fail git cherry-pick --continue
'
+test_expect_success 'instruction sheet, fat-fingers version' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick \([0-9a-f]*\)/pick \1 /" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ git cherry-pick --continue
+'
+
test_expect_success 'commit descriptions in insn sheet are optional' '
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
--
1.7.4.1
^ permalink raw reply related
* [PATCH 02/10] revert: make commit subjects in insn sheet optional
From: Ramkumar Ramachandra @ 2011-12-14 16:54 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano
In-Reply-To: <1323881677-11117-1-git-send-email-artagnon@gmail.com>
Change the instruction sheet format subtly so that the subject of the
commit message that follows the object name is optional. As a result,
an instruction sheet like this is now perfectly valid:
pick 35b0426
pick fbd5bbcbc2e
pick 7362160f
While at it, also fix a bug introduced by 5a5d80f4 (revert: Introduce
--continue to continue the operation, 2011-08-04) that failed to read
lines that are too long to fit on the commit-id-shaped buffer we
currently use; eliminate the need for the buffer altogether. In
addition to literal SHA-1 hexes, you can now safely use expressions
like the following in the instruction sheet:
featurebranch~4
rr/revert-cherry-pick-continue^2~12@{12 days ago}
[jc: simplify parsing]
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 37 ++++++++++++++++---------------------
t/t3510-cherry-pick-sequence.sh | 28 ++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 21 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 0c6d3d8..70055e5 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -711,31 +711,27 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
return 0;
}
-static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
+static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
{
unsigned char commit_sha1[20];
- char sha1_abbrev[40];
enum replay_action action;
- int insn_len = 0;
- char *p, *q;
+ char *end_of_object_name;
+ int saved, status;
- if (!prefixcmp(start, "pick ")) {
+ if (!prefixcmp(bol, "pick ")) {
action = CHERRY_PICK;
- insn_len = strlen("pick");
- p = start + insn_len + 1;
- } else if (!prefixcmp(start, "revert ")) {
+ bol += strlen("pick ");
+ } else if (!prefixcmp(bol, "revert ")) {
action = REVERT;
- insn_len = strlen("revert");
- p = start + insn_len + 1;
+ bol += strlen("revert ");
} else
return NULL;
- q = strchr(p, ' ');
- if (!q)
- return NULL;
- q++;
-
- strlcpy(sha1_abbrev, p, q - p);
+ end_of_object_name = bol + strcspn(bol, " \n");
+ saved = *end_of_object_name;
+ *end_of_object_name = '\0';
+ status = get_sha1(bol, commit_sha1);
+ *end_of_object_name = saved;
/*
* Verify that the action matches up with the one in
@@ -748,7 +744,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
return NULL;
}
- if (get_sha1(sha1_abbrev, commit_sha1) < 0)
+ if (status < 0)
return NULL;
return lookup_commit_reference(commit_sha1);
@@ -763,13 +759,12 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
int i;
for (i = 1; *p; i++) {
- commit = parse_insn_line(p, opts);
+ char *eol = strchrnul(p, '\n');
+ commit = parse_insn_line(p, eol, opts);
if (!commit)
return error(_("Could not parse line %d."), i);
next = commit_list_append(commit, next);
- p = strchrnul(p, '\n');
- if (*p)
- p++;
+ p = *eol ? eol + 1 : eol;
}
if (!*todo_list)
return error(_("No commits parsed."));
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 2c4c1c8..6390f2a 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -13,6 +13,9 @@ test_description='Test cherry-pick continuation features
. ./test-lib.sh
+# Repeat first match 10 times
+_r10='\1\1\1\1\1\1\1\1\1\1'
+
pristine_detach () {
git cherry-pick --quit &&
git checkout -f "$1^0" &&
@@ -328,4 +331,29 @@ test_expect_success 'malformed instruction sheet 2' '
test_must_fail git cherry-pick --continue
'
+test_expect_success 'malformed instruction sheet 3' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git cherry-pick --continue
+'
+
+test_expect_success 'commit descriptions in insn sheet are optional' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ git rev-list HEAD >commits &&
+ test_line_count = 4 commits
+'
+
test_done
--
1.7.4.1
^ permalink raw reply related
* [PATCH 01/10] revert: free msg in format_todo()
From: Ramkumar Ramachandra @ 2011-12-14 16:54 UTC (permalink / raw)
To: Git List; +Cc: Jonathan Nieder, Junio C Hamano
In-Reply-To: <1323881677-11117-1-git-send-email-artagnon@gmail.com>
Memory allocated to the fields of msg by get_message() isn't freed.
This is potentially a big leak, because fresh memory is allocated to
store the commit message for each commit. Fix this using
free_message().
Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 1ea525c..0c6d3d8 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -706,6 +706,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
if (get_message(cur->item, &msg))
return error(_("Cannot get commit message for %s"), sha1_abbrev);
strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
+ free_message(&msg);
}
return 0;
}
--
1.7.4.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox