Re: git commit -p with file arguments

[Jacob Keller]

On Sun, Sep 11, 2016 at 6:57 PM, Junio C Hamano  wrote:
>
> You are excused ;-)
>
> In ancient days, "git commit " was to add the contents
> from working tree files that match  to what is already in
> the index and create a commit from that state.  This ran against the
> intuition of many users who knew older systems (e.g. cvs) and we had
> to migrate it to the current behaviour by breaking backward
> compatibility.

I see.

Thanks,
Jake

Re: [PATCH] doc: mention `git -c` in git-config(1)

[David Glasser]

On Sun, Sep 11, 2016 at 6:54 PM, Junio C Hamano  wrote:
> The patch that was commented on in that exchange should be part of
> v2.10.0 already.

My mistake: I was accidentally searching for the paragraph I added in
config.txt instead of git-config.txt. Thanks and sorry for wasting
your time!

--dave

-- 
glas...@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

Re: git commit -p with file arguments

[Junio C Hamano]

Jacob Keller  writes:

> Yes, I'm actually confused by "git commit " *not* usinng what's
> in the index already, so I think that isn't intuitive as is.

You are excused ;-)

In ancient days, "git commit " was to add the contents
from working tree files that match  to what is already in
the index and create a commit from that state.  This ran against the
intuition of many users who knew older systems (e.g. cvs) and we had
to migrate it to the current behaviour by breaking backward
compatibility.

Re: [PATCH] doc: mention `git -c` in git-config(1)

[Junio C Hamano]

David Glasser  writes:

> On Tue, Aug 23, 2016 at 11:02 AM, Junio C Hamano  wrote:
>> David Glasser  writes:
>>
>> That might be something we want to fix up further in later patches;
>> the change we see in this patch is good regardless.
>
>
> Perhaps I am looking at the wrong branch, but I'm not sure that this
> got merged? Is there something I should do to move this along?

Are you asking about "might be something we want to fix up further",
which I do not think anybody did (and you certainly didn't)?

The patch that was commented on in that exchange should be part of
v2.10.0 already.

$ git blame -s -L264,269 v2.10.0 Documentation/git-config.txt
7da9800f 264) values of a key from all files will be used.
7da9800f 265) 
ae1f7094 266) You may override individual configuration parameters when running 
any git
ae1f7094 267) command by using the `-c` option. See linkgit:git[1] for details.
ae1f7094 268) 
17014090 269) All writing options will per default write to the repository 
specific

$ git show ae1f7094
commit ae1f7094f7a68fcff3d07358d83f5f483f0c300c
Author: David Glasser 
Date:   Tue Aug 23 10:33:21 2016 -0700

doc: mention `git -c` in git-config(1)

Signed-off-by: David Glasser 
Signed-off-by: Junio C Hamano 

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 6843114..636b3eb 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -263,6 +263,9 @@ The files are read in the order given above, with last 
value found taking
 precedence over values read earlier.  When multiple values are taken then all
 values of a key from all files will be used.
 
+You may override individual configuration parameters when running any git
+command by using the `-c` option. See linkgit:git[1] for details.
+
 All writing options will per default write to the repository specific
 configuration file. Note that this also affects options like '--replace-all'
 and '--unset'. *'git config' will only ever change one file at a time*.

Re: [RFC/PATCH] ls-files: adding support for submodules

[Junio C Hamano]

Junio C Hamano  writes:

> So a "ls-files" that is done internally in the end-user facing "git
> grep --recurse-submodules" needs to be run _without_ recursing
> itself at least once to learn "lib/" is a submodule.  A flat "here
> are everything we have" does not sound like a good building block.

... unless you are _only_ interested in grepping (or in general
working outside Git repository) in the files in the working tree,
i.e. "git grep" without  nor "--cached".

A lot of the time you are interested in the current state of files,
not even in "the state recorded in the tip of the history" but in
"the state as I have in my working tree, the state as my compiler
sees it".

I am a bit torn.  Clearly this is an important special case, but it
would make the codepath for object database case and working tree
case even further apart between "git grep [--cached | ]"
and the "find in the working tree" codepath, which does not sound
friendly to the codebase.

Re: [RFC/PATCH] ls-files: adding support for submodules

[Jeff King]

On Sun, Sep 11, 2016 at 08:51:06PM -0400, Jeff King wrote:

> On Sun, Sep 11, 2016 at 03:10:13PM -0700, Junio C Hamano wrote:
> 
> > So a "ls-files" that is done internally in the end-user facing "git
> > grep --recurse-submodules" needs to be run _without_ recursing
> > itself at least once to learn "lib/" is a submodule.  A flat "here
> > are everything we have" does not sound like a good building block.
> 
> I do not use submodules myself, but I could imagine that you may have
> scripts outside of git that do not care about the submodule divisions at
> all, and would be happy with the flat block. E.g., our "make tags"
> target uses "git ls-files" so find all of the source files. I could
> imagine projects with submodules that would want to do so recursively (I
> could also imagine projects that do _not_ want to do so; it would depend
> on your workflow and how tightly bound the submodules are). Another
> plausible example would be something grep-like that has features
> git-grep does not (I don't use "ack", but perhaps "git ls-files
> --recurse-submodules -z | xargs --null ack ..." is something people
> would want to do).

None of that negates your point, btw, which is that this does not seem
like a great building block for "git grep --recurse-submodules". Just
that it seems plausible to me that people could find recursive
"ls-files" useful on its own.

-Peff

Re: [RFC/PATCH] ls-files: adding support for submodules

[Jeff King]

On Sun, Sep 11, 2016 at 03:10:13PM -0700, Junio C Hamano wrote:

> So a "ls-files" that is done internally in the end-user facing "git
> grep --recurse-submodules" needs to be run _without_ recursing
> itself at least once to learn "lib/" is a submodule.  A flat "here
> are everything we have" does not sound like a good building block.

I do not use submodules myself, but I could imagine that you may have
scripts outside of git that do not care about the submodule divisions at
all, and would be happy with the flat block. E.g., our "make tags"
target uses "git ls-files" so find all of the source files. I could
imagine projects with submodules that would want to do so recursively (I
could also imagine projects that do _not_ want to do so; it would depend
on your workflow and how tightly bound the submodules are). Another
plausible example would be something grep-like that has features
git-grep does not (I don't use "ack", but perhaps "git ls-files
--recurse-submodules -z | xargs --null ack ..." is something people
would want to do).

-Peff

Re: Git Miniconference at Plumbers

[Jeff King]

On Tue, Sep 06, 2016 at 12:42:04PM -0500, Jon Loeliger wrote:

> I have recently been enlisted by folks at the Linux Foundation to
> help run a Miniconference on Git at the Plumbers Conference [*]
> this fall.

I see the conference runs for 4 days; I assume the Git portion will just
be one day. Do you know yet which day?

-Peff

Re: [PATCH] doc: mention `git -c` in git-config(1)

[David Glasser]

On Tue, Aug 23, 2016 at 11:02 AM, Junio C Hamano  wrote:
> David Glasser  writes:
>
> That might be something we want to fix up further in later patches;
> the change we see in this patch is good regardless.


Perhaps I am looking at the wrong branch, but I'm not sure that this
got merged? Is there something I should do to move this along?

--dave

-- 
glas...@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

Re: [PATCH v2 20/25] sequencer: left-trim lines read from the script

[Junio C Hamano]

Johannes Schindelin  writes:

> Interactive rebase's scripts may be indented; we need to handle this
> case, too, now that we prepare the sequencer to process interactive
> rebases.

Hmph, have we ever given the sequencer instructions indented to the
user to edit?  I do not offhand see why we want to be lenient here,
especially only to the left.

>
> Signed-off-by: Johannes Schindelin 
> ---
>  sequencer.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/sequencer.c b/sequencer.c
> index 7953a05..5e5d113 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -875,6 +875,9 @@ static int parse_insn_line(struct todo_item *item, const 
> char *bol, char *eol)
>   char *end_of_object_name;
>   int i, saved, status, padding;
>  
> + /* left-trim */
> + bol += strspn(bol, " \t");
> +
>   for (i = 0; i < ARRAY_SIZE(todo_command_strings); i++)
>   if (skip_prefix(bol, todo_command_strings[i], )) {
>   item->command = i;

Re: [PATCH v2 21/25] sequencer: refactor write_message()

[Junio C Hamano]

Johannes Schindelin  writes:

> The write_message() function safely writes an strbuf to a file.
> Sometimes it is inconvenient to require an strbuf, though: the text to
> be written may not be stored in a strbuf, or the strbuf should not be
> released after writing.
>
> Let's refactor "safely writing string to a file" into
> write_with_lock_file(), and make write_message() use it. The new
> function makes it easy to create new convenience function
> write_file_gently(); as some of the upcoming callers of this new
> function would want to append a newline character, add a flag for it in
> write_file_gently(), and thus in write_with_lock_file().

Makes sense.  As an abstraction, "give me strbuf for my sole use,
and I'll trash its contents when I am done" feels like a horrible
helper interface; perhaps that was overly aggressively refactored by
noticing that then-current callers all released the strbuf, but
still feels wrong.

And this makes the underlying helper a lot more useful.

>
> Signed-off-by: Johannes Schindelin 
> ---
>  sequencer.c | 23 +++
>  1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 5e5d113..aa949d4 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -242,22 +242,37 @@ static void print_advice(int show_hint, struct 
> replay_opts *opts)
>   }
>  }
>  
> -static int write_message(struct strbuf *msgbuf, const char *filename)
> +static int write_with_lock_file(const char *filename,
> + const void *buf, size_t len, int append_eol)
>  {
>   static struct lock_file msg_file;
>  
>   int msg_fd = hold_lock_file_for_update(_file, filename, 0);
>   if (msg_fd < 0)
>   return error_errno(_("Could not lock '%s'"), filename);
> - if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
> - return error_errno(_("Could not write to %s"), filename);
> - strbuf_release(msgbuf);
> + if (write_in_full(msg_fd, buf, len) < 0)
> + return error_errno(_("Could not write to '%s'"), filename);
> + if (append_eol && write(msg_fd, "\n", 1) < 0)
> + return error_errno(_("Could not write eol to '%s"), filename);
>   if (commit_lock_file(_file) < 0)
>   return error(_("Error wrapping up %s."), filename);
>  
>   return 0;
>  }
>  
> +static int write_message(struct strbuf *msgbuf, const char *filename)
> +{
> + int res = write_with_lock_file(filename, msgbuf->buf, msgbuf->len, 0);
> + strbuf_release(msgbuf);
> + return res;
> +}
> +
> +static int write_file_gently(const char *filename,
> +  const char *text, int append_eol)
> +{
> + return write_with_lock_file(filename, text, strlen(text), append_eol);
> +}
> +
>  /*
>   * Reads a file that was presumably written by a shell script, i.e.
>   * with an end-of-line marker that needs to be stripped.

Re: [PATCH v2 22/25] sequencer: remove overzealous assumption in rebase -i mode

[Junio C Hamano]

Johannes Schindelin  writes:

> While at it, error out early if the "instruction sheet" (i.e. the todo
> script) could not be parsed.

Sounds good.

Re: [PATCH v2 24/25] sequencer: quote filenames in error messages

[Junio C Hamano]

Johannes Schindelin  writes:

> This makes the code consistent by fixing quite a couple of error messages.

Looks OK.  While at it, we may want another one to downcase the
first word, perhaps?

These may not be messages added by your series and can be left
outside this series, but I have to point out that

if (commit_lock_file(_file) < 0)
return error(_("Error wrapping up '%s'."), filename);

results in "error: Error wrapping up", which sounds quite funny.

"failed to finalize" or something would flow a bit better, I'd say.

> diff --git a/sequencer.c b/sequencer.c
> index 1e7f29e..465e018 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -255,7 +255,7 @@ static int write_with_lock_file(const char *filename,
>   if (append_eol && write(msg_fd, "\n", 1) < 0)
>   return error_errno(_("Could not write eol to '%s"), filename);
>   if (commit_lock_file(_file) < 0)
> - return error(_("Error wrapping up %s."), filename);
> + return error(_("Error wrapping up '%s'."), filename);
>  
>   return 0;
>  }

Re: [PATCH v2 25/25] sequencer: remove bogus hint for translators

[Junio C Hamano]

Johannes Schindelin  writes:

> When translating error messages, we need to be careful *not* to translate
> the todo commands such as "pick", "reword", etc because they are commands,
> and Git would not understand translated versions of those commands.
>
> Therefore, translating the commands in the error messages would simply be
> misleading.

This comes from b9c993e0 ("i18n: git-revert literal "me" messages",
2011-02-22) where it said

Author: Ævar Arnfjörð Bjarmason 

i18n: git-revert literal "me" messages

Translate messages that use the `me' variable. These are all error
messages referencing the command name, so the name shouldn't be
translated.

Reported-by: Jonathan Nieder 
Signed-off-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Junio C Hamano 

This looks like a positive attempt to remind translators that their
translation must flow with these literal command names that will not
get translated in place, not a bogus hint at all, at least to me.

>
> Signed-off-by: Johannes Schindelin 
> ---
>  sequencer.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 465e018..cdff0f1 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -697,8 +697,6 @@ static int do_pick_commit(enum todo_command command, 
> struct commit *commit,
>   return fast_forward_to(commit->object.oid.hash, head, unborn, 
> opts);
>  
>   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_to_string(command),
>   oid_to_hex(>object.oid));

Re: [PATCH] Do not record unstaged deleted file upon recursive merge if file was moved outside of working tree with enabled sparse-checkout.

[Junio C Hamano]

Mikhail Filippov  writes:

> ---

You'd need a lot more explanation on why this is needed
(i.e. without it what behaviour you would get, and why that
behaviour is wrong).

>  merge-recursive.c|  9 +---
>  t/t6042-merge-rename-corner-cases.sh | 42 
> 
>  2 files changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index e349126..25dc701 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1724,9 +1724,12 @@ static int merge_content(struct merge_options *o,
>*/
>   path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
>   if (!path_renamed_outside_HEAD) {
> - add_cacheinfo(o, mfi.mode, , path,
> -   0, (!o->call_depth), 0);
> - return mfi.clean;
> + struct stat st;
> + if (lstat(path, ) == 0) {
> + add_cacheinfo(o, mfi.mode, , path,
> +   0, (!o->call_depth), 0);
> + return mfi.clean;
> + }

I cannot guess what you are trying to achieve without explanation in
the proposed log message, but I can say that this unconditional
checking of a working tree file cannot be correct (there may or may
not be other things that are wrong with this change, which cannot be
judged without more information).

Imagine your o->call_depth is not zero, i.e. we are making a virtual
common ancestor with this merge, in which case any of the three
trees involved may have nothing to do with the current working tree
files.

> +test_expect_success 'move file/sparse-checkout/merge should not delete moved 
> file' '
> + git rm -rf . &&
> + git clean -fdqx &&
> + rm -rf .git &&
> + git init &&

Yuck.  This is inherited from existing tests but I think they need
to be cleaned up.  It is not your fault, and it is not in the scope
of this change.

> + git status >output &&
> + cp output /tmp/a &&

Huh?

> + test_i18ngrep "nothing to commit" output
> +'
> +
>  test_done

Re: [PATCH v2 2/4] update-index: use the same structure for chmod as add

[Junio C Hamano]

Thomas Gummerer  writes:

> @@ -955,10 +941,8 @@ int cmd_update_index(int argc, const char **argv, const 
> char *prefix)
>   PARSE_OPT_NOARG | /* disallow --cacheinfo= form */
>   PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
>   (parse_opt_cb *) cacheinfo_callback},
> - {OPTION_CALLBACK, 0, "chmod", _executable_bit, N_("(+/-)x"),
> - N_("override the executable bit of the listed files"),
> - PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
> - chmod_callback},
> + OPT_STRING( 0, "chmod", _arg, N_("(+/-)x"),
> + N_("override the executable bit of the listed files")),
>   {OPTION_SET_INT, 0, "assume-unchanged", _valid_only, NULL,
>   N_("mark files as \"not changing\""),
>   PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, MARK_FLAG},
> @@ -1018,6 +1002,15 @@ int cmd_update_index(int argc, const char **argv, 
> const char *prefix)
>   if (argc == 2 && !strcmp(argv[1], "-h"))
>   usage_with_options(update_index_usage, options);
>  
> + if (!chmod_arg)
> + force_mode = 0;
> + else if (!strcmp(chmod_arg, "-x"))
> + force_mode = 0666;
> + else if (!strcmp(chmod_arg, "+x"))
> + force_mode = 0777;
> + else
> + die(_("option 'chmod' expects \"+x\" or \"-x\""));
> +

I am afraid that this changes the behaviour drastically.

"git update-index" is an oddball command that takes options and then
processes them immediately, exactly because it was designed to take

git update-index --chmod=-x A --chmod=+x B --add C

and say things like "A and B are not in the index and you are
attempting to add them before giving me --add option".

git update-index --add --chmod=-x A --chmod=+x B C

is expected to add A as non-executable, and B and C as executable.
Many exotic parse-options callback mechanisms used in this command
were invented exactly to support its quirky way of not doing "get a
list of options and use the last one".  And this patch breaks it for
only one option without changing the others.

If we were willing to take such a big backward compatiblity hit in
the upcoming release (which I personally won't be affected, but old
scripts by others need to be audited and adjusted, which I won't
volunteer to do myself), we should make such a change consistently,
e.g. "git update-index A --add --remove B" should no longer error
out when it sees A and it is not yet in the index because "--add"
hasn't been given yet, or A is in the index but is missing from the
working tree because "--remove" hasn't been given yet.  Then it may
be more justifiable if "update-index --chmod=-x A --chmod=+x B"
added A as an executable.  With the current form of this patch, it
is not.

Can we do this "fix" without this change?

Re: [RFC/PATCH] ls-files: adding support for submodules

[Junio C Hamano]

Stefan Beller  writes:

> The plan is to hook the ls-files machinery into
> git-grep as the way of obtaining files to grep for a pattern.

That does not make much sense to me for exactly the same reason why
the "grab the list of paths and run 'git add' on them" example in
the message you are responding to does not make sense.  The use of
the thread-pool would still need to honor the submodule boundary so
that one thread may be assigned files in the top-level superproject
while another may be assigned files in lib/ submodule repository,
and the latter would be doing a rough equivalent of "git -C lib
grep" perhaps with a new option "--output-path-prefix=lib/" that
makes any and all paths that are reported from the command prefixed
with the specified string, so the result of its grepping in Makefile
may be reported as findings in lib/Makefile.

For that, it is not sufficient for the enumeration of paths done in
the top-level to just list lib/Makefile and lib/hello.py along with
Makefile and main.py, is it?  You would somehow need to have a way
to tell that 'lib/' and everything in there is inside a separate
repository.  Without knowing that "lib/" is its own repository, you
would not even know which files under "lib/" hierarchy in the
filesystem are actually tracked files, which you would learn only by
reading lib/.git/index, or what textconv filtering needs to be done
on them, which you would learn only by reading lib/.gitattributes
and/or lib/.git/config.

So a "ls-files" that is done internally in the end-user facing "git
grep --recurse-submodules" needs to be run _without_ recursing
itself at least once to learn "lib/" is a submodule.  A flat "here
are everything we have" does not sound like a good building block.

Re: git commit -p with file arguments

[Jacob Keller]

On Sun, Sep 11, 2016 at 2:50 PM, Junio C Hamano  wrote:
> Jakub Narębski  writes:
>
>> I wonder, if git-commit is to acquire such feature, what would be the
>> best interface.  "git commit :0:./"?  "git commit -o -p "
>> (that is, "git commit --only --patch ")?
>
> Just do "git reset && git commit -p ", I would say.
> Anything more elaborate would just confuse the end users.
>

Yes, I'm actually confused by "git commit " *not* usinng what's
in the index already, so I think that isn't intuitive as is.

Thanks,
Jake

Re: [PATCH] git-gui: respect commit.gpgsign again

[Junio C Hamano]

Johannes Schindelin  writes:

> Thanks. There are a couple more git-gui patches waiting for quite a long
> time. So you prefer them still as patches to git-gui.git?

I prefer not to have to worry about them myself ;-)  That means that
even if Pat steps down, the next maintainer for git-gui project
would not be me, so I wouldn't be making a unilateral decision to
re-root git-gui.git project one-level down.

> Also, I just noticed poor wording. Would you mind fixing it up by
>
>   s/committing/& with GPG signature/

Ouch, I didn't notice.  It's not in 'next' yet, so let me see if I
can futz with the history.

Thanks.

Re: git commit -p with file arguments

[Junio C Hamano]

Jakub Narębski  writes:

> I wonder, if git-commit is to acquire such feature, what would be the
> best interface.  "git commit :0:./"?  "git commit -o -p "
> (that is, "git commit --only --patch ")?

Just do "git reset && git commit -p ", I would say.
Anything more elaborate would just confuse the end users.

Re: [PATCH v3 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Johannes Schindelin  writes:

>> In other words, instead of trying to be consistent by erroring out
>> in non-regular blob case, I think the attached change on top would
>> make more sense, by consistently passing the object contents as-is
>> for all "not filtered" cases, whether it is run from the batch mode
>> or from the command line.
>>  ...
>> +if ((type == OBJ_BLOB) && S_ISREG(mode)) {
>>  struct strbuf strbuf = STRBUF_INIT;
>>  if (convert_to_working_tree(path, *buf, *size, )) {
>>  free(*buf);
>
> Yes, that makes most sense to me, too.

Alright; will squash it in then before merging it down to 'next'.

Re: [PATCH v7 06/10] pkt-line: add functions to read/write flush terminated packet streams

[Junio C Hamano]

Stefan Beller  writes:

> So as a caller the strbuf is in a different state in case of error
> depending whether
> the strbuf already had some data in it. I think it would be better if
> we only did
> `strbuf_setlen(sb_out, oldlen);` here, such that the caller can
> strbuf_release it
> unconditionally.

If the caller _knows_ that the strbuf is no longer needed, it can
unconditionally call strbuf_release() on it, whether its buffer was
already released or only its length was set to 0, no?

The callee is merely trying to be nice by resetting the strbuf to a
state close to the original in the error return codepath, I would
think.  It may be debatable if such a niceness is needed, but it is
a different matter that does not relate to the burden imposed on the
caller.

[PATCH] Do not record unstaged deleted file upon recursive merge if file was moved outside of working tree with enabled sparse-checkout.

[Mikhail Filippov]

---
 merge-recursive.c|  9 +---
 t/t6042-merge-rename-corner-cases.sh | 42 
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index e349126..25dc701 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1724,9 +1724,12 @@ static int merge_content(struct merge_options *o,
 */
path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
if (!path_renamed_outside_HEAD) {
-   add_cacheinfo(o, mfi.mode, , path,
- 0, (!o->call_depth), 0);
-   return mfi.clean;
+   struct stat st;
+   if (lstat(path, ) == 0) {
+   add_cacheinfo(o, mfi.mode, , path,
+ 0, (!o->call_depth), 0);
+   return mfi.clean;
+   }
}
} else
output(o, 2, _("Auto-merging %s"), path);
diff --git a/t/t6042-merge-rename-corner-cases.sh 
b/t/t6042-merge-rename-corner-cases.sh
index 411550d..2073e49 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -575,4 +575,46 @@ test_expect_success 'rename/rename/add-dest merge still 
knows about conflicting
test ! -f c
 '
 
+test_expect_success 'move file/sparse-checkout/merge should not delete moved 
file' '
+   git rm -rf . &&
+   git clean -fdqx &&
+   rm -rf .git &&
+   git init &&
+
+   echo output >.gitignore &&
+   echo .gitignore >>.gitignore &&
+
+   echo b1 >b1 &&
+   git add b1 &&
+   git commit -m b1 &&
+
+   mkdir excluded &&
+   echo problem >excluded/to-be-moved.txt &&
+   git add excluded/to-be-moved.txt &&
+   git commit -m to-be-moved &&
+   git tag split_point &&
+
+   echo b2 >b2 &&
+   git add b2 &&
+   git commit -m b2 &&
+   git tag b2 &&
+
+   git reset --hard split_point &&
+
+   git mv excluded/to-be-moved.txt excluded/moved.txt &&
+   git commit -m move &&
+   git tag b1 &&
+
+   git config core.sparsecheckout true &&
+   echo "/*" >.git/info/sparse-checkout &&
+   echo "!excluded/" >>.git/info/sparse-checkout &&
+   git read-tree -mu HEAD &&
+
+   git merge -m merge b2 &&
+
+   git status >output &&
+   cp output /tmp/a &&
+   test_i18ngrep "nothing to commit" output
+'
+
 test_done
-- 
2.7.4 (Apple Git-66)

Re: Bug: git-add .* errors out

[Thomas Gummerer]

Hi,

On 09/12, Pranit Bauva wrote:
> Hey everyone,
> 
> One of my friend was trying to add files using the command `git add
> .*` and got an error that "fatal: ..: '..' is outside repository"
> which did seem a little obvious to me. But then I tried to reproduce
> it in my machine with `git add ".*"` and it didn't error out. I am
> currently using git 2.9.3 on Ubuntu 15.04 while he is using git 1.9.1
> on Ubuntu 16.04. What might have gone wrong?

The difference seems to be that you quoted the .*, which leaves the .*
in place for gits internal pathspec machinery, which then only
considers paths inside of the repository.

The non quoted version your friend used meanwhile is expanded by the
shell itself, which seems to be expanding it to ., the current
directory, and .., the parent directory.  This behaviour also depends
on the shell used, for me .* in bash includes the current as well as
the parent directory, while .* in zsh doesn't include either of these.

> Regards,
> Pranit Bauva

Hope this helps,
Thomas

Re: Bug: git-add .* errors out

[Andreas Schwab]

On Sep 12 2016, Pranit Bauva  wrote:

> One of my friend was trying to add files using the command `git add
> .*` and got an error that "fatal: ..: '..' is outside repository"
> which did seem a little obvious to me. But then I tried to reproduce
> it in my machine with `git add ".*"` and it didn't error out.

Probably you were running it in a subdirectory.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Bug: git-add .* errors out

[Pranit Bauva]

Hey everyone,

One of my friend was trying to add files using the command `git add
.*` and got an error that "fatal: ..: '..' is outside repository"
which did seem a little obvious to me. But then I tried to reproduce
it in my machine with `git add ".*"` and it didn't error out. I am
currently using git 2.9.3 on Ubuntu 15.04 while he is using git 1.9.1
on Ubuntu 16.04. What might have gone wrong?

Regards,
Pranit Bauva

Re: Missing RPM spec file in tarball

[Stefan Beller]

On Sat, Sep 10, 2016 at 1:15 AM, Johannes Schindelin
 wrote:
> Hi Stefan,
>
> On Fri, 9 Sep 2016, Stefan Beller wrote:
>
>> On Fri, Sep 9, 2016 at 9:19 AM, Sergio Martín Turiel
>>  wrote:
>>
>> > Can you tell me what I'm doing wrong?
>>
>> Not crying out loud when that commit was discussed on the
>> mailing list. ;)
>
> Umm, I think it would be more: "Not stepping up to maintain the RPM
> specs"...
>
> ;-)

You are right of course.

Thanks,
Stefan

>
> Ciao,
> Dscho

Re: [PATCH v7 06/10] pkt-line: add functions to read/write flush terminated packet streams

[Stefan Beller]

On Sun, Sep 11, 2016 at 5:33 AM, Lars Schneider
 wrote:

> Does this convince you to keep the proposed error handling? If yes, then
> I would add a comment to the function to document that behavior explicitly!

oops. I should read the docs more carefully.

Thanks for pointing out.
Then I'd be happy with the patch as is.

Thanks,
Stefan

Re: [PATCH v7 03/10] pkt-line: add packet_write_fmt_gently()

[Stefan Beller]

On Sun, Sep 11, 2016 at 4:36 AM, Lars Schneider
 wrote:

>>
>>call check_pipe from write_or_die here instead of
>>reproducing that function?
>
> Yes, might be better. I wasn't sure because the check_pipe is
> not public.
>
> Where would you declare check_pipe? In cache.h?

IIRC, once upon a time the community decided to not
clutter cache.h any more as it is like a dirty kitchen sink,
piling up all unrelated things, but on the other hand that
would be handy.

> Maybe it would be more suitable to move check_pipe to
> run-command.h/c?

That's certainly possible.
I don't have a strong opinion, where the code actually
resides, but I do have a strong-ish opinion on code
duplication. ;)

[GIT PULL] l10n updates for 2.10.0 maint branch

[Jiang Xin]

Hi Junio,

There are some l10n updates for Git 2.10.0, please merge them to the
maint branch.

The following changes since commit e8e349249c86550d3505c4abfac28caf3d13df46:

  Merge branch 'master' of https://github.com/vnwildman/git
(2016-09-02 21:29:48 +0800)

are available in the git repository at:

  git://github.com/git-l10n/git-po l10n-2.10.0-rnd2.3

for you to fetch changes up to 9a4b694c539fead26833c2104c1a93d3a2b4c50a:

  l10n: zh_CN: review for git v2.10.0 l10n (2016-09-11 21:34:23 +0800)


l10n-2.10.0-rnd2.3


Jiang Xin (1):
  l10n: zh_CN: fixed some typos for git 2.10.0

Ray Chen (1):
  l10n: zh_CN: review for git v2.10.0 l10n

Vasco Almeida (2):
  l10n: pt_PT: update Portuguese translation
  l10n: pt_PT: update Portuguese repository info

 po/TEAMS|   5 +-
 po/pt_PT.po | 739 
 po/zh_CN.po | 104 -
 3 files changed, 402 insertions(+), 446 deletions(-)

--
Jiang Xin

Re: [PATCH v7 08/10] convert: modernize tests

[Lars Schneider]

> On 09 Sep 2016, at 00:05, Stefan Beller  wrote:
> 
> On Thu, Sep 8, 2016 at 11:21 AM,   wrote:
>> From: Lars Schneider 
>> 
>> Use `test_config` to set the config, check that files are empty with
>> `test_must_be_empty`, compare files with `test_cmp`, and remove spaces
>> after ">" and "<".
>> 
>> Please note that the "rot13" filter configured in "setup" keeps using
>> `git config` instead of `test_config` because subsequent tests might
>> depend on it.
>> 
>> Signed-off-by: Lars Schneider 
>> ---
> 
> Makes sense & Reviewed-by "Stefan Beller "

Thank you,
Lars

Re: [PATCH v7 06/10] pkt-line: add functions to read/write flush terminated packet streams

[Lars Schneider]

> On 08 Sep 2016, at 23:49, Stefan Beller  wrote:
> 
> On Thu, Sep 8, 2016 at 11:21 AM,   wrote:
>> From: Lars Schneider 
>> 
>> write_packetized_from_fd() and write_packetized_from_buf() write a
>> stream of packets. All content packets use the maximal packet size
>> except for the last one. After the last content packet a `flush` control
>> packet is written.
> 
> I presume we need both write_* things in a later patch; can you clarify why
> we need both of them?

Since 9035d7 Git streams from fd to required filters and from buf to
non-required filters. The Git filter protocol v2 makes use of all that,
too.

https://github.com/git/git/commit/9035d75a2be9d80d82676504d69553245017f6d4


>> +   if (paket_len < 0) {
>> +   if (oldalloc == 0)
>> +   strbuf_release(sb_out);
> 
> So if old alloc is 0, we release it, which is documented as
> /**
> * Release a string buffer and the memory it used. You should not use the
> * string buffer after using this function, unless you initialize it again.
> */
> 
>> +   else
>> +   strbuf_setlen(sb_out, oldlen);
> 
> Otherwise we just set the length back, such that it looks like before.
> 
> So as a caller the strbuf is in a different state in case of error
> depending whether
> the strbuf already had some data in it. I think it would be better if
> we only did
> `strbuf_setlen(sb_out, oldlen);` here, such that the caller can
> strbuf_release it
> unconditionally.

I tried to mimic the behavior of strbuf_read() [1]. The error handling
was introduced in 2fc647 [2] to ease error handling:

"This allows for easier error handling, as callers only need to call
strbuf_release() if A) the command succeeded or B) if they would have had
to do so anyway because they added something to the strbuf themselves."

Does this convince you to keep the proposed error handling? If yes, then
I would add a comment to the function to document that behavior explicitly!

[1] 
https://github.com/git/git/blob/cda1bbd474805e653dda8a71d4ea3790e2a66cbb/strbuf.c#L377-L383
[2] https://github.com/git/git/commit/2fc647004ac7016128372a85db8245581e493812


> Or to make things more confusing, you could use strbuf_reset in case of 0,
> as that is a strbuf_setlen internally. ;)


Thanks,
Lars

Re: [PATCH v7 05/10] pkt-line: add packet_write_gently()

[Lars Schneider]

> On 08 Sep 2016, at 23:24, Stefan Beller  wrote:
> 
> On Thu, Sep 8, 2016 at 11:21 AM,   wrote:
> 
>> 
>> Add packet_write_gently() which writes arbitrary data and returns `0`
>> for success and `-1` for an error.
> 
> I think documenting the return code is better done in either the header file
> or in a commend preceding the implementation instead of the commit message?
> 
> Maybe just a generic comment for *_gently is good enough, maybe even no
> comment. So the commit is fine, too. I dunno.

I agree that this is too verbose as this function follows the standard
Git return value conventions (AFAIK). I'll remove this from all commit
messages.


>> This function is used by other
>> pkt-line functions in a subsequent patch.
> 
> That's what I figured. Do we also need to mention that in the preceding patch
> for packet_flush_gently ?

I'll add this note to all commit messages that introduce new functions which
are used later.


Thanks,
Lars

Re: [PATCH v7 03/10] pkt-line: add packet_write_fmt_gently()

[Lars Schneider]

> On 08 Sep 2016, at 23:18, Stefan Beller  wrote:
> 
> On Thu, Sep 8, 2016 at 11:21 AM,   wrote:
> 
>> +static int packet_write_fmt_1(int fd, int gently,
>> +  const char *fmt, va_list args)
>> +{
>> +   struct strbuf buf = STRBUF_INIT;
>> +   size_t count;
>> +
>> +   format_packet(, fmt, args);
>> +   count = write_in_full(fd, buf.buf, buf.len);
>> +   if (count == buf.len)
>> +   return 0;
>> +
>> +   if (!gently) {
> 
>call check_pipe from write_or_die here instead of
>reproducing that function?

Yes, might be better. I wasn't sure because the check_pipe is
not public.

Where would you declare check_pipe? In cache.h?
Maybe it would be more suitable to move check_pipe to 
run-command.h/c?


>> +   if (errno == EPIPE) {
>> +   if (in_async())
>> +   async_exit(141);
>> +
>> +   signal(SIGPIPE, SIG_DFL);
>> +   raise(SIGPIPE);
>> +   /* Should never happen, but just in case... */
>> +   exit(141);
>> +   }
>> +   die_errno("packet write error");
>> +   }
>> +   error("packet write failed");
>> +   return -1;
> 
> I think the more idiomatic way is to
> 
>return error(...);
> 
> as error always return -1.

Of course!!


Thank you,
Lars

[PATCH v2 21/25] sequencer: refactor write_message()

[Johannes Schindelin]

The write_message() function safely writes an strbuf to a file.
Sometimes it is inconvenient to require an strbuf, though: the text to
be written may not be stored in a strbuf, or the strbuf should not be
released after writing.

Let's refactor "safely writing string to a file" into
write_with_lock_file(), and make write_message() use it. The new
function makes it easy to create new convenience function
write_file_gently(); as some of the upcoming callers of this new
function would want to append a newline character, add a flag for it in
write_file_gently(), and thus in write_with_lock_file().

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 5e5d113..aa949d4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -242,22 +242,37 @@ static void print_advice(int show_hint, struct 
replay_opts *opts)
}
 }
 
-static int write_message(struct strbuf *msgbuf, const char *filename)
+static int write_with_lock_file(const char *filename,
+   const void *buf, size_t len, int append_eol)
 {
static struct lock_file msg_file;
 
int msg_fd = hold_lock_file_for_update(_file, filename, 0);
if (msg_fd < 0)
return error_errno(_("Could not lock '%s'"), filename);
-   if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
-   return error_errno(_("Could not write to %s"), filename);
-   strbuf_release(msgbuf);
+   if (write_in_full(msg_fd, buf, len) < 0)
+   return error_errno(_("Could not write to '%s'"), filename);
+   if (append_eol && write(msg_fd, "\n", 1) < 0)
+   return error_errno(_("Could not write eol to '%s"), filename);
if (commit_lock_file(_file) < 0)
return error(_("Error wrapping up %s."), filename);
 
return 0;
 }
 
+static int write_message(struct strbuf *msgbuf, const char *filename)
+{
+   int res = write_with_lock_file(filename, msgbuf->buf, msgbuf->len, 0);
+   strbuf_release(msgbuf);
+   return res;
+}
+
+static int write_file_gently(const char *filename,
+const char *text, int append_eol)
+{
+   return write_with_lock_file(filename, text, strlen(text), append_eol);
+}
+
 /*
  * Reads a file that was presumably written by a shell script, i.e.
  * with an end-of-line marker that needs to be stripped.
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 24/25] sequencer: quote filenames in error messages

[Johannes Schindelin]

This makes the code consistent by fixing quite a couple of error messages.

Suggested by Jakub Narębski.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 1e7f29e..465e018 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -255,7 +255,7 @@ static int write_with_lock_file(const char *filename,
if (append_eol && write(msg_fd, "\n", 1) < 0)
return error_errno(_("Could not write eol to '%s"), filename);
if (commit_lock_file(_file) < 0)
-   return error(_("Error wrapping up %s."), filename);
+   return error(_("Error wrapping up '%s'."), filename);
 
return 0;
 }
@@ -955,16 +955,16 @@ static int read_populate_todo(struct todo_list *todo_list,
strbuf_reset(_list->buf);
fd = open(todo_file, O_RDONLY);
if (fd < 0)
-   return error_errno(_("Could not open %s"), todo_file);
+   return error_errno(_("Could not open '%s'"), todo_file);
if (strbuf_read(_list->buf, fd, 0) < 0) {
close(fd);
-   return error(_("Could not read %s."), todo_file);
+   return error(_("Could not read '%s'."), todo_file);
}
close(fd);
 
res = parse_insn_buffer(todo_list->buf.buf, todo_list);
if (res)
-   return error(_("Unusable instruction sheet: %s"), todo_file);
+   return error(_("Unusable instruction sheet: '%s'"), todo_file);
 
if (!is_rebase_i(opts)) {
enum todo_command valid =
@@ -1050,7 +1050,7 @@ static int read_populate_opts(struct replay_opts *opts)
 * are pretty certain that it is syntactically correct.
 */
if (git_config_from_file(populate_opts_cb, git_path_opts_file(), opts) 
< 0)
-   return error(_("Malformed options sheet: %s"),
+   return error(_("Malformed options sheet: '%s'"),
git_path_opts_file());
return 0;
 }
@@ -1093,7 +1093,7 @@ static int create_seq_dir(void)
return -1;
}
else if (mkdir(git_path_seq_dir(), 0777) < 0)
-   return error_errno(_("Could not create sequencer directory %s"),
+   return error_errno(_("Could not create sequencer directory 
'%s'"),
   git_path_seq_dir());
return 0;
 }
@@ -1112,12 +1112,12 @@ static int save_head(const char *head)
strbuf_addf(, "%s\n", head);
if (write_in_full(fd, buf.buf, buf.len) < 0) {
rollback_lock_file(_lock);
-   return error_errno(_("Could not write to %s"),
+   return error_errno(_("Could not write to '%s'"),
   git_path_head_file());
}
if (commit_lock_file(_lock) < 0) {
rollback_lock_file(_lock);
-   return error(_("Error wrapping up %s."), git_path_head_file());
+   return error(_("Error wrapping up '%s'."), 
git_path_head_file());
}
return 0;
 }
@@ -1162,9 +1162,9 @@ int sequencer_rollback(struct replay_opts *opts)
return rollback_single_pick();
}
if (!f)
-   return error_errno(_("cannot open %s"), git_path_head_file());
+   return error_errno(_("cannot open '%s'"), git_path_head_file());
if (strbuf_getline_lf(, f)) {
-   error(_("cannot read %s: %s"), git_path_head_file(),
+   error(_("cannot read '%s': %s"), git_path_head_file(),
  ferror(f) ?  strerror(errno) : _("unexpected end of 
file"));
fclose(f);
goto fail;
@@ -1203,7 +1203,7 @@ static int save_todo(struct todo_list *todo_list, struct 
replay_opts *opts)
todo_list->buf.len - offset) < 0)
return error_errno(_("Could not write to '%s'"), todo_path);
if (commit_lock_file(_lock) < 0)
-   return error(_("Error wrapping up %s."), todo_path);
+   return error(_("Error wrapping up '%s'."), todo_path);
return 0;
 }
 
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 25/25] sequencer: remove bogus hint for translators

[Johannes Schindelin]

When translating error messages, we need to be careful *not* to translate
the todo commands such as "pick", "reword", etc because they are commands,
and Git would not understand translated versions of those commands.

Therefore, translating the commands in the error messages would simply be
misleading.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 465e018..cdff0f1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -697,8 +697,6 @@ static int do_pick_commit(enum todo_command command, struct 
commit *commit,
return fast_forward_to(commit->object.oid.hash, head, unborn, 
opts);
 
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_to_string(command),
oid_to_hex(>object.oid));
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 23/25] sequencer: mark action_name() for translation

[Johannes Schindelin]

The definition of this function goes back all the way to 043a449
(sequencer: factor code out of revert builtin, 2012-01-11), long before a
serious effort was made to translate all the error messages.

It is slightly out of the context of the current patch series (whose
purpose it is to re-implement the performance critical parts of the
interactive rebase in C) to make the error messages in the sequencer
translatable, but what the heck. We'll just do it while we're looking at
this part of the code.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 5144245..1e7f29e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -176,7 +176,7 @@ int sequencer_remove_state(struct replay_opts *opts)
 
 static const char *action_name(const struct replay_opts *opts)
 {
-   return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
+   return opts->action == REPLAY_REVERT ? N_("revert") : N_("cherry-pick");
 }
 
 struct commit_message {
@@ -312,10 +312,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(_(action_name(opts)));
 
error(_("Your local changes would be overwritten by %s."),
-   action_name(opts));
+   _(action_name(opts)));
 
if (advice_commit_before_merge)
advise(_("Commit your changes or stash them to proceed."));
@@ -333,7 +333,7 @@ static int fast_forward_to(const unsigned char *to, const 
unsigned char *from,
if (checkout_fast_forward(from, to, 1))
return -1; /* the callee should have complained already */
 
-   strbuf_addf(, _("%s: fast-forward"), action_name(opts));
+   strbuf_addf(, _("%s: fast-forward"), _(action_name(opts)));
 
transaction = ref_transaction_begin();
if (!transaction ||
@@ -409,7 +409,7 @@ static int do_recursive_merge(struct commit *base, struct 
commit *next,
write_locked_index(_index, _lock, COMMIT_LOCK))
/* TRANSLATORS: %s will be "revert" or "cherry-pick" */
return error(_("%s: Unable to write new index file"),
-   action_name(opts));
+   _(action_name(opts)));
rollback_lock_file(_lock);
 
if (opts->signoff)
@@ -840,14 +840,14 @@ static int read_and_refresh_cache(struct replay_opts 
*opts)
if (read_index_preload(_index, NULL) < 0) {
rollback_lock_file(_lock);
return error(_("git %s: failed to read the index"),
-   action_name(opts));
+   _(action_name(opts)));
}
refresh_index(_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, 
NULL);
if (the_index.cache_changed && index_fd >= 0) {
if (write_locked_index(_index, _lock, COMMIT_LOCK)) {
rollback_lock_file(_lock);
return error(_("git %s: failed to refresh the index"),
-   action_name(opts));
+   _(action_name(opts)));
}
}
rollback_lock_file(_lock);
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 22/25] sequencer: remove overzealous assumption in rebase -i mode

[Johannes Schindelin]

The sequencer was introduced to make the cherry-pick and revert
functionality available as library function, with the original idea
being to extend the sequencer to also implement the rebase -i
functionality.

The test to ensure that all of the commands in the script are identical
to the overall operation does not mesh well with that.

Therefore let's disable the test in rebase -i mode.

While at it, error out early if the "instruction sheet" (i.e. the todo
script) could not be parsed.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index aa949d4..5144245 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -963,7 +963,10 @@ static int read_populate_todo(struct todo_list *todo_list,
close(fd);
 
res = parse_insn_buffer(todo_list->buf.buf, todo_list);
-   if (!res) {
+   if (res)
+   return error(_("Unusable instruction sheet: %s"), todo_file);
+
+   if (!is_rebase_i(opts)) {
enum todo_command valid =
opts->action == REPLAY_PICK ? TODO_PICK : TODO_REVERT;
int i;
@@ -977,8 +980,6 @@ static int read_populate_todo(struct todo_list *todo_list,
return error(_("Cannot revert during a 
cherry-pick."));
}
 
-   if (res)
-   return error(_("Unusable instruction sheet: %s"), todo_file);
return 0;
 }
 
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 19/25] sequencer: remember do_recursive_merge()'s return value

[Johannes Schindelin]

The return value of do_recursive_merge() may be positive (indicating merge
conflicts), so let's OR later error conditions so as not to overwrite them
with 0.

This is not yet a problem, but preparing for the patches to come: we will
teach the sequencer to do rebase -i's job.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 75772b8..7953a05 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -630,7 +630,7 @@ static int do_pick_commit(enum todo_command command, struct 
commit *commit,
const char *base_label, *next_label;
struct commit_message msg = { NULL, NULL, NULL, NULL };
struct strbuf msgbuf = STRBUF_INIT;
-   int res, unborn = 0, allow;
+   int res = 0, unborn = 0, allow;
 
if (opts->no_commit) {
/*
@@ -741,7 +741,7 @@ static int do_pick_commit(enum todo_command command, struct 
commit *commit,
}
 
if (!opts->strategy || !strcmp(opts->strategy, "recursive") || command 
== TODO_REVERT) {
-   res = do_recursive_merge(base, next, base_label, next_label,
+   res |= do_recursive_merge(base, next, base_label, next_label,
 head, , opts);
if (res < 0)
return res;
@@ -750,7 +750,7 @@ static int do_pick_commit(enum todo_command command, struct 
commit *commit,
struct commit_list *common = NULL;
struct commit_list *remotes = NULL;
 
-   res = write_message(, git_path_merge_msg());
+   res |= write_message(, git_path_merge_msg());
 
commit_list_insert(base, );
commit_list_insert(next, );
@@ -787,11 +787,12 @@ static int do_pick_commit(enum todo_command command, 
struct commit *commit,
 
allow = allow_empty(opts, commit);
if (allow < 0) {
-   res = allow;
+   res |= allow;
goto leave;
}
if (!opts->no_commit)
-   res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(),
+   res |= sequencer_commit(opts->edit ?
+   NULL : git_path_merge_msg(),
opts, allow, opts->edit, 0, 0);
 
 leave:
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 20/25] sequencer: left-trim lines read from the script

[Johannes Schindelin]

Interactive rebase's scripts may be indented; we need to handle this
case, too, now that we prepare the sequencer to process interactive
rebases.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 7953a05..5e5d113 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -875,6 +875,9 @@ static int parse_insn_line(struct todo_item *item, const 
char *bol, char *eol)
char *end_of_object_name;
int i, saved, status, padding;
 
+   /* left-trim */
+   bol += strspn(bol, " \t");
+
for (i = 0; i < ARRAY_SIZE(todo_command_strings); i++)
if (skip_prefix(bol, todo_command_strings[i], )) {
item->command = i;
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 18/25] sequencer: support cleaning up commit messages

[Johannes Schindelin]

The sequencer_commit() function already knows how to amend commits, and
with this new option, it can also clean up commit messages (i.e. strip
out commented lines). This is needed to implement rebase -i's 'fixup'
and 'squash' commands as sequencer commands.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 10 +++---
 sequencer.h |  3 ++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 60b522e..75772b8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -485,7 +485,8 @@ static char **read_author_script(void)
  * author metadata.
  */
 int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty, int edit, int amend)
+ int allow_empty, int edit, int amend,
+ int cleanup_commit_message)
 {
char **env = NULL;
struct argv_array array;
@@ -522,9 +523,12 @@ int sequencer_commit(const char *defmsg, struct 
replay_opts *opts,
argv_array_push(, "-s");
if (defmsg)
argv_array_pushl(, "-F", defmsg, NULL);
+   if (cleanup_commit_message)
+   argv_array_push(, "--cleanup=strip");
if (edit)
argv_array_push(, "-e");
-   else if (!opts->signoff && !opts->record_origin &&
+   else if (!cleanup_commit_message &&
+!opts->signoff && !opts->record_origin &&
 git_config_get_value("commit.cleanup", ))
argv_array_push(, "--cleanup=verbatim");
 
@@ -788,7 +792,7 @@ static int do_pick_commit(enum todo_command command, struct 
commit *commit,
}
if (!opts->no_commit)
res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(),
-   opts, allow, opts->edit, 0);
+   opts, allow, opts->edit, 0, 0);
 
 leave:
free_message(commit, );
diff --git a/sequencer.h b/sequencer.h
index c45f5c4..688fff1 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -54,7 +54,8 @@ int sequencer_rollback(struct replay_opts *opts);
 int sequencer_remove_state(struct replay_opts *opts);
 
 int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty, int edit, int amend);
+ int allow_empty, int edit, int amend,
+ int cleanup_commit_message);
 
 extern const char sign_off_header[];
 
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 17/25] sequencer: support amending commits

[Johannes Schindelin]

This teaches the sequencer_commit() function to take an argument that
will allow us to implement "todo" commands that need to amend the commit
messages ("fixup", "squash" and "reword").

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 6 --
 sequencer.h | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 6e9732c..60b522e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -485,7 +485,7 @@ static char **read_author_script(void)
  * author metadata.
  */
 int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty, int edit)
+ int allow_empty, int edit, int amend)
 {
char **env = NULL;
struct argv_array array;
@@ -514,6 +514,8 @@ int sequencer_commit(const char *defmsg, struct replay_opts 
*opts,
argv_array_push(, "commit");
argv_array_push(, "-n");
 
+   if (amend)
+   argv_array_push(, "--amend");
if (opts->gpg_sign)
argv_array_pushf(, "-S%s", opts->gpg_sign);
if (opts->signoff)
@@ -786,7 +788,7 @@ static int do_pick_commit(enum todo_command command, struct 
commit *commit,
}
if (!opts->no_commit)
res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(),
-   opts, allow, opts->edit);
+   opts, allow, opts->edit, 0);
 
 leave:
free_message(commit, );
diff --git a/sequencer.h b/sequencer.h
index 7f5222f..c45f5c4 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -54,7 +54,7 @@ int sequencer_rollback(struct replay_opts *opts);
 int sequencer_remove_state(struct replay_opts *opts);
 
 int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty, int edit);
+ int allow_empty, int edit, int amend);
 
 extern const char sign_off_header[];
 
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 16/25] sequencer: allow editing the commit message on a case-by-case basis

[Johannes Schindelin]

In the upcoming commits, we will implement more and more of rebase
-i's functionality. One particular feature of the commands to come is
that some of them allow editing the commit message while others don't,
i.e. we cannot define in the replay_opts whether the commit message
should be edited or not.

Let's add a new parameter to the sequencer_commit() function. Previously,
it was the duty of the caller to ensure that the opts->edit setting
indicates whether to let the user edit the commit message or not,
indicating that it is an "all or nothing" setting, i.e. that the sequencer
wants to let the user edit *all* commit message, or none at all. In the
upcoming rebase -i mode, it will depend on the particular command that is
currently executed, though.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 6 +++---
 sequencer.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index bf02565..6e9732c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -485,7 +485,7 @@ static char **read_author_script(void)
  * author metadata.
  */
 int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty)
+ int allow_empty, int edit)
 {
char **env = NULL;
struct argv_array array;
@@ -520,7 +520,7 @@ int sequencer_commit(const char *defmsg, struct replay_opts 
*opts,
argv_array_push(, "-s");
if (defmsg)
argv_array_pushl(, "-F", defmsg, NULL);
-   if (opts->edit)
+   if (edit)
argv_array_push(, "-e");
else if (!opts->signoff && !opts->record_origin &&
 git_config_get_value("commit.cleanup", ))
@@ -786,7 +786,7 @@ static int do_pick_commit(enum todo_command command, struct 
commit *commit,
}
if (!opts->no_commit)
res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(),
-   opts, allow);
+   opts, allow, opts->edit);
 
 leave:
free_message(commit, );
diff --git a/sequencer.h b/sequencer.h
index 16deb6c..7f5222f 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -54,7 +54,7 @@ int sequencer_rollback(struct replay_opts *opts);
 int sequencer_remove_state(struct replay_opts *opts);
 
 int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty);
+ int allow_empty, int edit);
 
 extern const char sign_off_header[];
 
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 13/25] sequencer: prepare for rebase -i's commit functionality

[Johannes Schindelin]

In interactive rebases, we commit a little bit differently than the
sequencer did so far: we heed the "author-script", the "message" and
the "amend" files in the .git/rebase-merge/ subdirectory.

Likewise, we may want to edit the commit message *even* when providing
a file containing the suggested commit message. Therefore we change the
code to not even provide a default message when we do not want any, and
to call the editor explicitly.

As interactive rebase's GPG settings are configured differently from
how cherry-pick (and therefore sequencer) handles them, we will leave
support for that to the next commit.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 97 ++---
 sequencer.h |  3 ++
 2 files changed, 89 insertions(+), 11 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index ca1961c..6c35fe8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -27,6 +27,19 @@ static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
 static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
 static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
 
+/*
+ * A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
+ * GIT_AUTHOR_DATE that will be used for the commit that is currently
+ * being rebased.
+ */
+static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
+
+/* We will introduce the 'interactive rebase' mode later */
+static inline int is_rebase_i(const struct replay_opts *opts)
+{
+   return 0;
+}
+
 static const char *get_dir(const struct replay_opts *opts)
 {
return git_path_seq_dir();
@@ -377,20 +390,76 @@ static int is_index_unchanged(void)
return !hashcmp(active_cache_tree->sha1, 
head_commit->tree->object.oid.hash);
 }
 
+static char **read_author_script(void)
+{
+   struct strbuf script = STRBUF_INIT;
+   int i, count = 0;
+   char *p, *p2, **env;
+   size_t env_size;
+
+   if (strbuf_read_file(, rebase_path_author_script(), 256) <= 0)
+   return NULL;
+
+   for (p = script.buf; *p; p++)
+   if (skip_prefix(p, "'''", (const char **)))
+   strbuf_splice(, p - script.buf, p2 - p, "'", 1);
+   else if (*p == '\'')
+   strbuf_splice(, p-- - script.buf, 1, "", 0);
+   else if (*p == '\n') {
+   *p = '\0';
+   count++;
+   }
+
+   env_size = (count + 1) * sizeof(*env);
+   strbuf_grow(, env_size);
+   memmove(script.buf + env_size, script.buf, script.len);
+   p = script.buf + env_size;
+   env = (char **)strbuf_detach(, NULL);
+
+   for (i = 0; i < count; i++) {
+   env[i] = p;
+   p += strlen(p) + 1;
+   }
+   env[count] = NULL;
+
+   return env;
+}
+
 /*
  * 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.
+ *
+ * An exception is when sequencer_commit() is called during an
+ * interactive rebase: in that case, we will want to retain the
+ * author metadata.
  */
-static int run_git_commit(const char *defmsg, struct replay_opts *opts,
+int sequencer_commit(const char *defmsg, struct replay_opts *opts,
  int allow_empty)
 {
+   char **env = NULL;
struct argv_array array;
int rc;
const char *value;
 
+   if (is_rebase_i(opts)) {
+   env = read_author_script();
+   if (!env)
+   return error("You have staged changes in your working "
+   "tree. If these changes are meant to be\n"
+   "squashed into the previous commit, run:\n\n"
+   "  git commit --amend $gpg_sign_opt_quoted\n\n"
+   "If they are meant to go into a new commit, "
+   "run:\n\n"
+   "  git commit $gpg_sign_opt_quoted\n\n"
+   "In both cases, once you're done, continue "
+   "with:\n\n"
+   "  git rebase --continue\n");
+   }
+
argv_array_init();
argv_array_push(, "commit");
argv_array_push(, "-n");
@@ -399,14 +468,13 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
argv_array_pushf(, "-S%s", opts->gpg_sign);
if (opts->signoff)
argv_array_push(, "-s");
-   if (!opts->edit) {
-   argv_array_push(, "-F");
-   argv_array_push(, defmsg);
-   if (!opts->signoff &&
-   !opts->record_origin &&
-   git_config_get_value("commit.cleanup", ))
-   

[PATCH v2 15/25] sequencer: prepare for rebase -i's GPG settings

[Johannes Schindelin]

The rebase command sports a `--gpg-sign` option that is heeded by the
interactive rebase.

This patch teaches the sequencer that trick, as part of the bigger
effort to make the sequencer the work horse of the interactive rebase.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 42 +-
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 086cd0b..bf02565 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -15,6 +15,7 @@
 #include "merge-recursive.h"
 #include "refs.h"
 #include "argv-array.h"
+#include "quote.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -33,6 +34,11 @@ static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
  * being rebased.
  */
 static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
+/*
+ * The following files are written by git-rebase just after parsing the
+ * command-line (and are only consumed, not modified, by the sequencer).
+ */
+static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt")
 
 /* We will introduce the 'interactive rebase' mode later */
 static inline int is_rebase_i(const struct replay_opts *opts)
@@ -132,6 +138,16 @@ static int has_conforming_footer(struct strbuf *sb, struct 
strbuf *sob,
return 1;
 }
 
+static const char *gpg_sign_opt_quoted(struct replay_opts *opts)
+{
+   static struct strbuf buf = STRBUF_INIT;
+
+   strbuf_reset();
+   if (opts->gpg_sign)
+   sq_quotef(, "-S%s", opts->gpg_sign);
+   return buf.buf;
+}
+
 void *sequencer_entrust(struct replay_opts *opts, void *to_free)
 {
ALLOC_GROW(opts->owned, opts->owned_nr + 1, opts->owned_alloc);
@@ -478,17 +494,20 @@ int sequencer_commit(const char *defmsg, struct 
replay_opts *opts,
 
if (is_rebase_i(opts)) {
env = read_author_script();
-   if (!env)
+   if (!env) {
+   const char *gpg_opt = gpg_sign_opt_quoted(opts);
+
return error("You have staged changes in your working "
"tree. If these changes are meant to be\n"
"squashed into the previous commit, run:\n\n"
-   "  git commit --amend $gpg_sign_opt_quoted\n\n"
+   "  git commit --amend %s\n\n"
"If they are meant to go into a new commit, "
"run:\n\n"
-   "  git commit $gpg_sign_opt_quoted\n\n"
+   "  git commit %s\n\n"
"In both cases, once you're done, continue "
"with:\n\n"
-   "  git rebase --continue\n");
+   "  git rebase --continue\n", gpg_opt, gpg_opt);
+   }
}
 
argv_array_init();
@@ -980,8 +999,21 @@ static int populate_opts_cb(const char *key, const char 
*value, void *data)
 
 static int read_populate_opts(struct replay_opts *opts)
 {
-   if (is_rebase_i(opts))
+   if (is_rebase_i(opts)) {
+   struct strbuf buf = STRBUF_INIT;
+
+   if (read_oneliner(, rebase_path_gpg_sign_opt(), 1)) {
+   if (!starts_with(buf.buf, "-S"))
+   strbuf_reset();
+   else {
+   opts->gpg_sign = buf.buf + 2;
+   sequencer_entrust(opts,
+   strbuf_detach(, NULL));
+   }
+   }
+
return 0;
+   }
 
if (!file_exists(git_path_opts_file()))
return 0;
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 12/25] sequencer: remember the onelines when parsing the todo file

[Johannes Schindelin]

The `git-rebase-todo` file contains a list of commands. Most of those
commands have the form

  

The  is displayed primarily for the user's convenience, as
rebase -i really interprets only the   part. However, there
are *some* places in interactive rebase where the  is used to
display messages, e.g. for reporting at which commit we stopped.

So let's just remember it when parsing the todo file; we keep a copy of
the entire todo file anyway (to write out the new `done` and
`git-rebase-todo` file just before processing each command), so all we
need to do is remember the begin offsets and lengths.

As we will have to parse and remember the command-line for `exec` commands
later, we do not call the field "oneline" but rather "arg" (and will reuse
that for exec's command-line).

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 0c8dec4..ca1961c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -713,6 +713,8 @@ static int read_and_refresh_cache(struct replay_opts *opts)
 struct todo_item {
enum todo_command command;
struct commit *commit;
+   const char *arg;
+   int arg_len;
size_t offset_in_buf;
 };
 
@@ -764,6 +766,9 @@ static int parse_insn_line(struct todo_item *item, const 
char *bol, char *eol)
status = get_sha1(bol, commit_sha1);
*end_of_object_name = saved;
 
+   item->arg = end_of_object_name + strspn(end_of_object_name, " \t");
+   item->arg_len = (int)(eol - item->arg);
+
if (status < 0)
return -1;
 
@@ -905,6 +910,8 @@ static int walk_revs_populate_todo(struct todo_list 
*todo_list,
 
item->command = command;
item->commit = commit;
+   item->arg = NULL;
+   item->arg_len = 0;
item->offset_in_buf = todo_list->buf.len;
subject_len = find_commit_subject(commit_buffer, );
strbuf_addf(_list->buf, "%s %s %.*s\n", command_string,
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 09/25] sequencer: avoid completely different messages for different actions

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index b971ad0..7ba932b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -232,11 +232,8 @@ static int error_dirty_index(struct replay_opts *opts)
if (read_cache_unmerged())
return error_resolve_conflict(action_name(opts));
 
-   /* Different translation strings for cherry-pick and revert */
-   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."));
+   error(_("Your local changes would be overwritten by %s."),
+   action_name(opts));
 
if (advice_commit_before_merge)
advise(_("Commit your changes or stash them to proceed."));
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 14/25] sequencer: introduce a helper to read files written by scripts

[Johannes Schindelin]

As we are slowly teaching the sequencer to perform the hard work for
the interactive rebase, we need to read files that were written by
shell scripts.

These files typically contain a single line and are invariably ended
by a line feed (and possibly a carriage return before that). Let's use
a helper to read such files and to remove the line ending.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 6c35fe8..086cd0b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -242,6 +242,37 @@ static int write_message(struct strbuf *msgbuf, const char 
*filename)
return 0;
 }
 
+/*
+ * Reads a file that was presumably written by a shell script, i.e.
+ * with an end-of-line marker that needs to be stripped.
+ *
+ * Returns 1 if the file was read, 0 if it could not be read or does not exist.
+ */
+static int read_oneliner(struct strbuf *buf,
+   const char *path, int skip_if_empty)
+{
+   int orig_len = buf->len;
+
+   if (!file_exists(path))
+   return 0;
+
+   if (strbuf_read_file(buf, path, 0) < 0) {
+   warning_errno("could not read '%s'", path);
+   return 0;
+   }
+
+   if (buf->len > orig_len && buf->buf[buf->len - 1] == '\n') {
+   if (--buf->len > orig_len && buf->buf[buf->len - 1] == '\r')
+   --buf->len;
+   buf->buf[buf->len] = '\0';
+   }
+
+   if (skip_if_empty && buf->len == orig_len)
+   return 0;
+
+   return 1;
+}
+
 static struct tree *empty_tree(void)
 {
return lookup_tree(EMPTY_TREE_SHA1_BIN);
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 10/25] sequencer: get rid of the subcommand field

[Johannes Schindelin]

The subcommands are used exactly once, at the very beginning of
sequencer_pick_revisions(), to determine what to do. This is an
unnecessary level of indirection: we can simply call the correct
function to begin with. So let's do that.

While at it, ensure that the subcommands return an error code so that
they do not have to die() all over the place (bad practice for library
functions...).

Signed-off-by: Johannes Schindelin 
---
 builtin/revert.c | 36 
 sequencer.c  | 35 +++
 sequencer.h  | 13 -
 3 files changed, 31 insertions(+), 53 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 7365559..c9ae4dc 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -71,7 +71,7 @@ static void verify_opt_compatible(const char *me, const char 
*base_opt, ...)
die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
 }
 
-static void parse_args(int argc, const char **argv, struct replay_opts *opts)
+static int run_sequencer(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);
@@ -115,25 +115,15 @@ static void parse_args(int argc, const char **argv, 
struct replay_opts *opts)
if (opts->keep_redundant_commits)
opts->allow_empty = 1;
 
-   /* Set the subcommand */
-   if (cmd == 'q')
-   opts->subcommand = REPLAY_REMOVE_STATE;
-   else if (cmd == 'c')
-   opts->subcommand = REPLAY_CONTINUE;
-   else if (cmd == 'a')
-   opts->subcommand = REPLAY_ROLLBACK;
-   else
-   opts->subcommand = REPLAY_NONE;
-
/* Check for incompatible command line arguments */
-   if (opts->subcommand != REPLAY_NONE) {
+   if (cmd) {
char *this_operation;
-   if (opts->subcommand == REPLAY_REMOVE_STATE)
+   if (cmd == 'q')
this_operation = "--quit";
-   else if (opts->subcommand == REPLAY_CONTINUE)
+   else if (cmd == 'c')
this_operation = "--continue";
else {
-   assert(opts->subcommand == REPLAY_ROLLBACK);
+   assert(cmd == 'a');
this_operation = "--abort";
}
 
@@ -156,7 +146,7 @@ static void parse_args(int argc, const char **argv, struct 
replay_opts *opts)
"--edit", opts->edit,
NULL);
 
-   if (opts->subcommand != REPLAY_NONE) {
+   if (cmd) {
opts->revs = NULL;
} else {
struct setup_revision_opt s_r_opt;
@@ -174,6 +164,14 @@ static void parse_args(int argc, const char **argv, struct 
replay_opts *opts)
 
if (argc > 1)
usage_with_options(usage_str, options);
+
+   if (cmd == 'q')
+   return sequencer_remove_state(opts);
+   if (cmd == 'c')
+   return sequencer_continue(opts);
+   if (cmd == 'a')
+   return sequencer_rollback(opts);
+   return sequencer_pick_revisions(opts);
 }
 
 int cmd_revert(int argc, const char **argv, const char *prefix)
@@ -185,8 +183,7 @@ int cmd_revert(int argc, const char **argv, const char 
*prefix)
opts.edit = 1;
opts.action = REPLAY_REVERT;
git_config(git_default_config, NULL);
-   parse_args(argc, argv, );
-   res = sequencer_pick_revisions();
+   res = run_sequencer(argc, argv, );
if (res < 0)
die(_("revert failed"));
return res;
@@ -199,8 +196,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char 
*prefix)
 
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
-   parse_args(argc, argv, );
-   res = sequencer_pick_revisions();
+   res = run_sequencer(argc, argv, );
if (res < 0)
die(_("cherry-pick failed"));
return res;
diff --git a/sequencer.c b/sequencer.c
index 7ba932b..053e78c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -127,7 +127,7 @@ void *sequencer_entrust(struct replay_opts *opts, void 
*to_free)
return to_free;
 }
 
-static void remove_sequencer_state(const struct replay_opts *opts)
+int sequencer_remove_state(struct replay_opts *opts)
 {
struct strbuf dir = STRBUF_INIT;
int i;
@@ -141,6 +141,8 @@ static void remove_sequencer_state(const struct replay_opts 
*opts)
strbuf_addf(, "%s", get_dir(opts));
remove_dir_recursively(, 0);
strbuf_release();
+
+   return 0;
 }
 
 static const char *action_name(const struct replay_opts *opts)
@@ -971,7 +973,7 @@ static int rollback_single_pick(void)
return reset_for_rollback(head_sha1);
 }
 
-static int sequencer_rollback(struct replay_opts *opts)
+int sequencer_rollback(struct 

[PATCH v2 11/25] sequencer: refactor the code to obtain a short commit name

[Johannes Schindelin]

Not only does this DRY up the code (providing a better documentation what
the code is about, as well as allowing to change the behavior in a single
place), it also makes it substantially shorter to use the same
functionality in functions to be introduced when we teach the sequencer to
process interactive-rebase's git-rebase-todo file.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 053e78c..0c8dec4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -157,13 +157,18 @@ struct commit_message {
const char *message;
 };
 
+static const char *short_commit_name(struct commit *commit)
+{
+   return find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV);
+}
+
 static int get_message(struct commit *commit, struct commit_message *out)
 {
const char *abbrev, *subject;
int subject_len;
 
out->message = logmsg_reencode(commit, NULL, 
get_commit_output_encoding());
-   abbrev = find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV);
+   abbrev = short_commit_name(commit);
 
subject_len = find_commit_subject(out->message, );
 
@@ -647,8 +652,7 @@ static int do_pick_commit(enum todo_command command, struct 
commit *commit,
error(command == TODO_REVERT
  ? _("could not revert %s... %s")
  : _("could not apply %s... %s"),
- find_unique_abbrev(commit->object.oid.hash, 
DEFAULT_ABBREV),
- msg.subject);
+ short_commit_name(commit), msg.subject);
print_advice(res == 1, opts);
rerere(opts->allow_rerere_auto);
goto leave;
@@ -904,9 +908,7 @@ static int walk_revs_populate_todo(struct todo_list 
*todo_list,
item->offset_in_buf = todo_list->buf.len;
subject_len = find_commit_subject(commit_buffer, );
strbuf_addf(_list->buf, "%s %s %.*s\n", command_string,
-   find_unique_abbrev(commit->object.oid.hash,
-   DEFAULT_ABBREV),
-   subject_len, subject);
+   short_commit_name(commit), subject_len, subject);
unuse_commit_buffer(commit, commit_buffer);
}
return 0;
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 07/25] sequencer: future-proof read_populate_todo()

[Johannes Schindelin]

Over the next commits, we will work on improving the sequencer to the
point where it can process the edit script of an interactive rebase. To
that end, we will need to teach the sequencer to read interactive
rebase's todo file. In preparation, we consolidate all places where
that todo file is needed to call a function that we will later extend.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 3ca231f..da6de22 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -32,6 +32,11 @@ static const char *get_dir(const struct replay_opts *opts)
return git_path_seq_dir();
 }
 
+static const char *get_todo_path(const struct replay_opts *opts)
+{
+   return git_path_todo_file();
+}
+
 static int is_rfc2822_line(const char *buf, int len)
 {
int i;
@@ -776,25 +781,24 @@ static int parse_insn_buffer(char *buf, struct 
commit_list **todo_list,
 static int read_populate_todo(struct commit_list **todo_list,
struct replay_opts *opts)
 {
+   const char *todo_file = get_todo_path(opts);
struct strbuf buf = STRBUF_INIT;
int fd, res;
 
-   fd = open(git_path_todo_file(), O_RDONLY);
+   fd = open(todo_file, O_RDONLY);
if (fd < 0)
-   return error_errno(_("Could not open %s"),
-  git_path_todo_file());
+   return error_errno(_("Could not open %s"), todo_file);
if (strbuf_read(, fd, 0) < 0) {
close(fd);
strbuf_release();
-   return error(_("Could not read %s."), git_path_todo_file());
+   return error(_("Could not read %s."), todo_file);
}
close(fd);
 
res = parse_insn_buffer(buf.buf, todo_list, opts);
strbuf_release();
if (res)
-   return error(_("Unusable instruction sheet: %s"),
-   git_path_todo_file());
+   return error(_("Unusable instruction sheet: %s"), todo_file);
return 0;
 }
 
@@ -1077,7 +1081,7 @@ static int sequencer_continue(struct replay_opts *opts)
 {
struct commit_list *todo_list = NULL;
 
-   if (!file_exists(git_path_todo_file()))
+   if (!file_exists(get_todo_path(opts)))
return continue_single_pick();
if (read_populate_opts(opts) ||
read_populate_todo(_list, opts))
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 08/25] sequencer: completely revamp the "todo" script parsing

[Johannes Schindelin]

When we came up with the "sequencer" idea, we really wanted to have
kind of a plumbing equivalent of the interactive rebase. Hence the
choice of words: the "todo" script, a "pick", etc.

However, when it came time to implement the entire shebang, somehow this
idea got lost and the sequencer was used as working horse for
cherry-pick and revert instead. So as not to interfere with the
interactive rebase, it even uses a separate directory to store its
state.

Furthermore, it also is stupidly strict about the "todo" script it
accepts: while it parses commands in a way that was *designed* to be
similar to the interactive rebase, it then goes on to *error out* if the
commands disagree with the overall action (cherry-pick or revert).

Finally, the sequencer code chose to deviate from the interactive rebase
code insofar that it *reformats* the "todo" script instead of just
writing the part of the parsed script that were not yet processed. This
is not only unnecessary churn, but might well lose information that is
valuable to the user (i.e. comments after the commands).

Let's just bite the bullet and rewrite the entire parser; the code now
becomes not only more elegant: it allows us to go on and teach the
sequencer how to parse *true* "todo" scripts as used by the interactive
rebase itself. In a way, the sequencer is about to grow up to do its
older brother's job. Better.

In particular, we choose to maintain the list of commands in an array
instead of a linked list: this is flexible enough to allow us later on to
even implement rebase -i's reordering of fixup!/squash! commits very
easily (and with a very nice speed bonus, at least on Windows).

While at it, do not stop at the first problem, but list *all* of the
problems. This will help the user when the sequencer will do `rebase
-i`'s work by allowing to address all issues in one go rather than going
back and forth until the todo list is valid.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 275 +++-
 1 file changed, 159 insertions(+), 116 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index da6de22..b971ad0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -473,7 +473,26 @@ static int allow_empty(struct replay_opts *opts, struct 
commit *commit)
return 1;
 }
 
-static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
+enum todo_command {
+   TODO_PICK = 0,
+   TODO_REVERT
+};
+
+static const char *todo_command_strings[] = {
+   "pick",
+   "revert"
+};
+
+static const char *command_to_string(const enum todo_command command)
+{
+   if (command < ARRAY_SIZE(todo_command_strings))
+   return todo_command_strings[command];
+   die("Unknown command: %d", command);
+}
+
+
+static int do_pick_commit(enum todo_command command, struct commit *commit,
+   struct replay_opts *opts)
 {
unsigned char head[20];
struct commit *base, *next, *parent;
@@ -535,7 +554,8 @@ 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), oid_to_hex(>object.oid));
+   command_to_string(command),
+   oid_to_hex(>object.oid));
 
if (get_message(commit, ) != 0)
return error(_("Cannot get commit message for %s"),
@@ -548,7 +568,7 @@ static int do_pick_commit(struct commit *commit, struct 
replay_opts *opts)
 * reverse of it if we are revert.
 */
 
-   if (opts->action == REPLAY_REVERT) {
+   if (command == TODO_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -589,7 +609,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") || command 
== TODO_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
 head, , opts);
if (res < 0)
@@ -615,17 +635,17 @@ 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 (command == TODO_PICK && !opts->no_commit && (res == 0 || res == 1) 
&&
update_ref(NULL, "CHERRY_PICK_HEAD", commit->object.oid.hash, NULL,
   REF_NODEREF, UPDATE_REFS_MSG_ON_ERR))
res = -1;
-   if (opts->action 

[PATCH v2 03/25] sequencer: avoid unnecessary indirection

[Johannes Schindelin]

We really do not need the *pointer to a* pointer to the options in
the read_populate_opts() function.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index cb16cbd..c2fbf6f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -813,7 +813,7 @@ static int populate_opts_cb(const char *key, const char 
*value, void *data)
return 0;
 }
 
-static int read_populate_opts(struct replay_opts **opts)
+static int read_populate_opts(struct replay_opts *opts)
 {
if (!file_exists(git_path_opts_file()))
return 0;
@@ -823,7 +823,7 @@ static int read_populate_opts(struct replay_opts **opts)
 * about this case, though, because we wrote that file ourselves, so we
 * are pretty certain that it is syntactically correct.
 */
-   if (git_config_from_file(populate_opts_cb, git_path_opts_file(), *opts) 
< 0)
+   if (git_config_from_file(populate_opts_cb, git_path_opts_file(), opts) 
< 0)
return error(_("Malformed options sheet: %s"),
git_path_opts_file());
return 0;
@@ -1054,7 +1054,7 @@ static int sequencer_continue(struct replay_opts *opts)
 
if (!file_exists(git_path_todo_file()))
return continue_single_pick();
-   if (read_populate_opts() ||
+   if (read_populate_opts(opts) ||
read_populate_todo(_list, opts))
return -1;
 
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 04/25] sequencer: future-proof remove_sequencer_state()

[Johannes Schindelin]

In a couple of commits, we will teach the sequencer to handle the
nitty gritty of the interactive rebase, which keeps its state in a
different directory.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index c2fbf6f..8d272fb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -27,6 +27,11 @@ static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
 static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
 static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
 
+static const char *get_dir(const struct replay_opts *opts)
+{
+   return git_path_seq_dir();
+}
+
 static int is_rfc2822_line(const char *buf, int len)
 {
int i;
@@ -109,13 +114,13 @@ static int has_conforming_footer(struct strbuf *sb, 
struct strbuf *sob,
return 1;
 }
 
-static void remove_sequencer_state(void)
+static void remove_sequencer_state(const struct replay_opts *opts)
 {
-   struct strbuf seq_dir = STRBUF_INIT;
+   struct strbuf dir = STRBUF_INIT;
 
-   strbuf_addstr(_dir, git_path_seq_dir());
-   remove_dir_recursively(_dir, 0);
-   strbuf_release(_dir);
+   strbuf_addf(, "%s", get_dir(opts));
+   remove_dir_recursively(, 0);
+   strbuf_release();
 }
 
 static const char *action_name(const struct replay_opts *opts)
@@ -940,7 +945,7 @@ static int sequencer_rollback(struct replay_opts *opts)
}
if (reset_for_rollback(sha1))
goto fail;
-   remove_sequencer_state();
+   remove_sequencer_state(opts);
strbuf_release();
return 0;
 fail:
@@ -1034,7 +1039,7 @@ static int pick_commits(struct commit_list *todo_list, 
struct replay_opts *opts)
 * Sequence of picks finished successfully; cleanup by
 * removing the .git/sequencer directory
 */
-   remove_sequencer_state();
+   remove_sequencer_state(opts);
return 0;
 }
 
@@ -1095,7 +1100,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 * one that is being continued
 */
if (opts->subcommand == REPLAY_REMOVE_STATE) {
-   remove_sequencer_state();
+   remove_sequencer_state(opts);
return 0;
}
if (opts->subcommand == REPLAY_ROLLBACK)
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 05/25] sequencer: allow the sequencer to take custody of malloc()ed data

[Johannes Schindelin]

The sequencer is our attempt to lib-ify cherry-pick. Yet it behaves
like a one-shot command when it reads its configuration: memory is
allocated and released only when the command exits.

This is kind of okay for git-cherry-pick, which *is* a one-shot
command. All the work to make the sequencer its work horse was
done to allow using the functionality as a library function, though,
including proper clean-up after use.

This patch introduces an API to pass the responsibility of releasing
certain memory to the sequencer. Example:

const char *label =
sequencer_entrust(opts, xstrfmt("From: %s", email));

The entrusted memory will remain valid until sequencer_remove_state() is
called, or the program exits, whichever comes first.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 13 +
 sequencer.h | 10 ++
 2 files changed, 23 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 8d272fb..8d56a05 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -114,9 +114,22 @@ static int has_conforming_footer(struct strbuf *sb, struct 
strbuf *sob,
return 1;
 }
 
+void *sequencer_entrust(struct replay_opts *opts, void *to_free)
+{
+   ALLOC_GROW(opts->owned, opts->owned_nr + 1, opts->owned_alloc);
+   opts->owned[opts->owned_nr++] = to_free;
+
+   return to_free;
+}
+
 static void remove_sequencer_state(const struct replay_opts *opts)
 {
struct strbuf dir = STRBUF_INIT;
+   int i;
+
+   for (i = 0; i < opts->owned_nr; i++)
+   free(opts->owned[i]);
+   free(opts->owned);
 
strbuf_addf(, "%s", get_dir(opts));
remove_dir_recursively(, 0);
diff --git a/sequencer.h b/sequencer.h
index dd4d33a..04892a9 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -43,9 +43,19 @@ struct replay_opts {
 
/* Only used by REPLAY_NONE */
struct rev_info *revs;
+
+   /* malloc()ed data entrusted to the sequencer */
+   void **owned;
+   int owned_nr, owned_alloc;
 };
 #define REPLAY_OPTS_INIT { -1, -1 }
 
+/*
+ * Make it the duty of sequencer_remove_state() to release the memory;
+ * For ease of use, return the same pointer.
+ */
+void *sequencer_entrust(struct replay_opts *opts, void *to_free);
+
 int sequencer_pick_revisions(struct replay_opts *opts);
 
 extern const char sign_off_header[];
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 01/25] sequencer: use static initializers for replay_opts

[Johannes Schindelin]

This change is not completely faithful: instead of initializing all fields
to 0, we choose to initialize command and subcommand to -1 (instead of
defaulting to REPLAY_REVERT and REPLAY_NONE, respectively). Practically,
it makes no difference at all, but future-proofs the code to require
explicit assignments for both fields.

Signed-off-by: Johannes Schindelin 
---
 builtin/revert.c | 6 ++
 sequencer.h  | 1 +
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 4e69380..7365559 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -178,10 +178,9 @@ static void parse_args(int argc, const char **argv, struct 
replay_opts *opts)
 
 int cmd_revert(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
opts.action = REPLAY_REVERT;
@@ -195,10 +194,9 @@ int cmd_revert(int argc, const char **argv, const char 
*prefix)
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
-   struct replay_opts opts;
+   struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
 
-   memset(, 0, sizeof(opts));
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, );
diff --git a/sequencer.h b/sequencer.h
index 5ed5cb1..db425ad 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -47,6 +47,7 @@ struct replay_opts {
/* Only used by REPLAY_NONE */
struct rev_info *revs;
 };
+#define REPLAY_OPTS_INIT { -1, -1 }
 
 int sequencer_pick_revisions(struct replay_opts *opts);
 
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 02/25] sequencer: use memoized sequencer directory path

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin 
---
 builtin/commit.c |  2 +-
 sequencer.c  | 11 ++-
 sequencer.h  |  5 +
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index bb9f79b..e79af9d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -183,7 +183,7 @@ static void determine_whence(struct wt_status *s)
whence = FROM_MERGE;
else if (file_exists(git_path_cherry_pick_head())) {
whence = FROM_CHERRY_PICK;
-   if (file_exists(git_path(SEQ_DIR)))
+   if (file_exists(git_path_seq_dir()))
sequencer_in_use = 1;
}
else
diff --git a/sequencer.c b/sequencer.c
index eec8a60..cb16cbd 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -21,10 +21,11 @@
 const char sign_off_header[] = "Signed-off-by: ";
 static const char cherry_picked_prefix[] = "(cherry picked from commit ";
 
-static GIT_PATH_FUNC(git_path_todo_file, SEQ_TODO_FILE)
-static GIT_PATH_FUNC(git_path_opts_file, SEQ_OPTS_FILE)
-static GIT_PATH_FUNC(git_path_seq_dir, SEQ_DIR)
-static GIT_PATH_FUNC(git_path_head_file, SEQ_HEAD_FILE)
+GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
+
+static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
+static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
+static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
 
 static int is_rfc2822_line(const char *buf, int len)
 {
@@ -112,7 +113,7 @@ static void remove_sequencer_state(void)
 {
struct strbuf seq_dir = STRBUF_INIT;
 
-   strbuf_addstr(_dir, git_path(SEQ_DIR));
+   strbuf_addstr(_dir, git_path_seq_dir());
remove_dir_recursively(_dir, 0);
strbuf_release(_dir);
 }
diff --git a/sequencer.h b/sequencer.h
index db425ad..dd4d33a 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -1,10 +1,7 @@
 #ifndef SEQUENCER_H
 #define SEQUENCER_H
 
-#define SEQ_DIR"sequencer"
-#define SEQ_HEAD_FILE  "sequencer/head"
-#define SEQ_TODO_FILE  "sequencer/todo"
-#define SEQ_OPTS_FILE  "sequencer/opts"
+const char *git_path_seq_dir(void);
 
 #define APPEND_SIGNOFF_DEDUP (1u << 0)
 
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 06/25] sequencer: release memory that was allocated when reading options

[Johannes Schindelin]

The sequencer reads options from disk and stores them in its struct
for use during sequencer's operations.

With this patch, the memory is released afterwards, plugging a
memory leak.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 8d56a05..3ca231f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -131,6 +131,8 @@ static void remove_sequencer_state(const struct replay_opts 
*opts)
free(opts->owned[i]);
free(opts->owned);
 
+   free(opts->xopts);
+
strbuf_addf(, "%s", get_dir(opts));
remove_dir_recursively(, 0);
strbuf_release();
@@ -815,13 +817,18 @@ static int populate_opts_cb(const char *key, const char 
*value, void *data)
opts->allow_ff = git_config_bool_or_int(key, value, 
_flag);
else if (!strcmp(key, "options.mainline"))
opts->mainline = git_config_int(key, value);
-   else if (!strcmp(key, "options.strategy"))
+   else if (!strcmp(key, "options.strategy")) {
git_config_string(>strategy, key, value);
-   else if (!strcmp(key, "options.gpg-sign"))
+   sequencer_entrust(opts, (char *) opts->strategy);
+   }
+   else if (!strcmp(key, "options.gpg-sign")) {
git_config_string(>gpg_sign, key, value);
+   sequencer_entrust(opts, (char *) opts->gpg_sign);
+   }
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);
+   opts->xopts[opts->xopts_nr++] =
+   sequencer_entrust(opts, xstrdup(value));
} else
return error(_("Invalid key: %s"), key);
 
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 00/25] Prepare the sequencer for the upcoming rebase -i patches

[Johannes Schindelin]

This patch series marks the  '4' in the countdown to speed up rebase -i
by implementing large parts in C. It is based on the `libify-sequencer`
patch series that I submitted last week.

The patches in this series merely prepare the sequencer code for the
next patch series that actually teaches the sequencer to run an
interactive rebase.

The reason to split these two patch series is simple: to keep them at a
sensible size.

The two patch series after that are much smaller: a two-patch "series"
that switches rebase -i to use the sequencer (except with --root or
--preserve-merges), and a couple of patches to move several pretty
expensive script processing steps to C (think: autosquash).

The end game of this patch series is a git-rebase--helper that makes
rebase -i 5x faster on Windows (according to t/perf/p3404). Travis says
that even MacOSX and Linux benefit (4x and 3x, respectively).

I have been working on this since early February, whenever time allowed,
and it is time to put it into the users' hands. To that end, I will most
likely submit the remaining three patch series in the next two days, and
integrate the whole shebang into Git for Windows 2.10.0.

Therefore I would be most grateful for every in-depth review.

Changes vs v1:

- clarified why the code refactoring into the short_commit_name()
  function is desirable.

- fixed a typo in a commit message (s/se/so/).

- removed a bogus call to read_and_refresh_cache(opts) that was added
  to sequencer_rollback() by mistake.

- clarified in the commit message why the TODO_LIST_INIT macro does not
  assign all-zeroes.

- converted IS_REBASE_I() into an inline function.

- clarified the comment about retaining author metadata when calling
  sequencer_commit() in rebase -i mode.

- simplified TODO_LIST_INIT and REPLAY_OPTS_INIT.

- added an example how to use sequencer_entrust() to the commit message
  introducing it.

- the gpg_sign option's value is now also entrusted to the sequencer
  (i.e. it is released in sequencer_remove_state()).

- renamed the "set_me_free_after_use" to "to_free". Since that is still
  not a self-explanatory name, add a comment to the function
  declaration.

- clarified in the "completely revamped todo parsing" commit message
  that the main benefit of the early parsing is for rebase -i.

- start the `enum todo_command` with TODO_PICK that is explicitly
  assigned the value 0.

- an error was marked as translatable.

- converted one forgotten git_path_todo_file() to use
  get_todo_path(opts) instead.

- fixed numbering of "malformed instruction sheet"s after removing one.

- reintroduced the overzealous assumption that todo scripts can only
  perform revert commands during `git revert` and pick commands during
  `git cherry-pick`. This overzealous assumption is *still* disabled in
  rebase -i mode, of course.

- clarified in the commit message why we call the field "arg", not
  "oneline", and fixed the description that claimed that we store the
  end offset (we store the length instead).

- clarified in the commit message of "allow editing the commit message
  on a case-by-case basis" that we are talking about the
  sequencer_commit() function, and how things were done previously.

- some grammar touch-ups.

- marked a couple of error messages for translation.

- explicitly assign the first todo_command the value 0, so that we can
  be certain that the array of command strings matches up.

- removed code to skip CR/LF at the end of line after reading with
  read_oneliner(): That function already ensures that.

- ensured that the todo_list is released even when reading/parsing fails.

- replaced an error(..., strerror()) call with an error_errno(...) one.

- clarified in the commit message why the new todo_list maintains the
  script as an array instead of a linked list.

- renamed the append_todo() function into append_new_todo(), to explain
  better what the function does and why it does not take the values as
  parameters that should populate the new todo_item.

- marked action_name() for translation.

- surrounded all file names in error messages in sequencer.c with single
  quotes, for consistency.

- removed a bogus hint for translators that asked them to translate
  commands of the git-rebase-todo file (or cherry-pick's equivalent).

- turned capitals after semicolons to lower-case.


Johannes Schindelin (25):
  sequencer: use static initializers for replay_opts
  sequencer: use memoized sequencer directory path
  sequencer: avoid unnecessary indirection
  sequencer: future-proof remove_sequencer_state()
  sequencer: allow the sequencer to take custody of malloc()ed data
  sequencer: release memory that was allocated when reading options
  sequencer: future-proof read_populate_todo()
  sequencer: completely revamp the "todo" script parsing
  sequencer: avoid completely different messages for different actions
  sequencer: get rid of the subcommand field
  sequencer: refactor the code to obtain a short commit name
  

[PATCH v2 3/4] read-cache: introduce chmod_index_entry

[Thomas Gummerer]

As there are chmod options for both add and update-index, introduce a
new chmod_index_entry function to do the work.  Use it in update-index,
while it will be used in add in the next patch.

Signed-off-by: Thomas Gummerer 
---
 builtin/update-index.c |  8 +---
 cache.h|  2 ++
 read-cache.c   | 19 +++
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 85a57db..1569c81 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -423,20 +423,14 @@ static void chmod_path(int force_mode, const char *path)
 {
int pos;
struct cache_entry *ce;
-   unsigned int mode;
char flip = force_mode == 0777 ? '+' : '-';
 
pos = cache_name_pos(path, strlen(path));
if (pos < 0)
goto fail;
ce = active_cache[pos];
-   mode = ce->ce_mode;
-   if (!S_ISREG(mode))
+   if (chmod_cache_entry(ce, force_mode) < 0)
goto fail;
-   ce->ce_mode = create_ce_mode(force_mode);
-   cache_tree_invalidate_path(_index, path);
-   ce->ce_flags |= CE_UPDATE_IN_BASE;
-   active_cache_changed |= CE_ENTRY_CHANGED;
 
report("chmod %cx '%s'", flip, path);
return;
diff --git a/cache.h b/cache.h
index b780a91..44a4f76 100644
--- a/cache.h
+++ b/cache.h
@@ -369,6 +369,7 @@ extern void free_name_hash(struct index_state *istate);
 #define remove_file_from_cache(path) remove_file_from_index(_index, (path))
 #define add_to_cache(path, st, flags) add_to_index(_index, (path), (st), 
(flags), 0)
 #define add_file_to_cache(path, flags) add_file_to_index(_index, (path), 
(flags), 0)
+#define chmod_cache_entry(ce, force_mode) chmod_index_entry(_index, (ce), 
(force_mode))
 #define refresh_cache(flags) refresh_index(_index, (flags), NULL, NULL, 
NULL)
 #define ce_match_stat(ce, st, options) ie_match_stat(_index, (ce), (st), 
(options))
 #define ce_modified(ce, st, options) ie_modified(_index, (ce), (st), 
(options))
@@ -584,6 +585,7 @@ extern int remove_file_from_index(struct index_state *, 
const char *path);
 extern int add_to_index(struct index_state *, const char *path, struct stat *, 
int flags, int force_mode);
 extern int add_file_to_index(struct index_state *, const char *path, int 
flags, int force_mode);
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned 
char *sha1, const char *path, int stage, unsigned int refresh_options);
+extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, int 
force_mode);
 extern int ce_same_name(const struct cache_entry *a, const struct cache_entry 
*b);
 extern void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);
 extern int index_name_is_other(const struct index_state *, const char *, int);
diff --git a/read-cache.c b/read-cache.c
index 491e52d..367be57 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -756,6 +756,25 @@ struct cache_entry *make_cache_entry(unsigned int mode,
return ret;
 }
 
+/*
+ * Change the mode of an index entry to force_mode, where force_mode can
+ * either be 0777 or 0666.
+ * Returns -1 if the chmod for the particular cache entry failed (if it's
+ * not a regular file), 0 otherwise.
+ */
+int chmod_index_entry(struct index_state *istate, struct cache_entry *ce,
+  int force_mode)
+{
+   if (!S_ISREG(ce->ce_mode))
+   return -1;
+   ce->ce_mode = create_ce_mode(force_mode);
+   cache_tree_invalidate_path(istate, ce->name);
+   ce->ce_flags |= CE_UPDATE_IN_BASE;
+   istate->cache_changed |= CE_ENTRY_CHANGED;
+
+   return 0;
+}
+
 int ce_same_name(const struct cache_entry *a, const struct cache_entry *b)
 {
int len = ce_namelen(a);
-- 
2.10.0.304.gf2ff484

[PATCH v2 4/4] add: modify already added files when --chmod is given

[Thomas Gummerer]

When the chmod option was added to git add, it was hooked up to the diff
machinery, meaning that it only works when the version in the index
differs from the version on disk.

As the option was supposed to mirror the chmod option in update-index,
which always changes the mode in the index, regardless of the status of
the file, make sure the option behaves the same way in git add.

Signed-off-by: Thomas Gummerer 
---
 builtin/add.c  | 36 +---
 builtin/checkout.c |  2 +-
 builtin/commit.c   |  2 +-
 cache.h| 10 +-
 read-cache.c   | 14 ++
 t/t3700-add.sh | 21 +
 6 files changed, 59 insertions(+), 26 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index b1dddb4..892198a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -26,10 +26,25 @@ static int patch_interactive, add_interactive, 
edit_interactive;
 static int take_worktree_changes;
 
 struct update_callback_data {
-   int flags, force_mode;
+   int flags;
int add_errors;
 };
 
+static void chmod_pathspec(struct pathspec *pathspec, int force_mode)
+{
+   int i;
+   
+   for (i = 0; i < active_nr; i++) {
+   struct cache_entry *ce = active_cache[i];
+
+   if (pathspec && !ce_path_match(ce, pathspec, NULL))
+   continue;
+
+   if (chmod_cache_entry(ce, force_mode) < 0)
+   fprintf(stderr, "cannot chmod '%s'", ce->name);
+   }
+}
+
 static int fix_unmerged_status(struct diff_filepair *p,
   struct update_callback_data *data)
 {
@@ -65,8 +80,7 @@ static void update_callback(struct diff_queue_struct *q,
die(_("unexpected diff status %c"), p->status);
case DIFF_STATUS_MODIFIED:
case DIFF_STATUS_TYPE_CHANGED:
-   if (add_file_to_index(_index, path,
-   data->flags, data->force_mode)) {
+   if (add_file_to_index(_index, path, data->flags)) {
if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
die(_("updating files failed"));
data->add_errors++;
@@ -84,15 +98,14 @@ static void update_callback(struct diff_queue_struct *q,
}
 }
 
-int add_files_to_cache(const char *prefix, const struct pathspec *pathspec,
-   int flags, int force_mode)
+int add_files_to_cache(const char *prefix,
+  const struct pathspec *pathspec, int flags)
 {
struct update_callback_data data;
struct rev_info rev;
 
memset(, 0, sizeof(data));
data.flags = flags;
-   data.force_mode = force_mode;
 
init_revisions(, prefix);
setup_revisions(0, NULL, , NULL);
@@ -281,7 +294,7 @@ static int add_config(const char *var, const char *value, 
void *cb)
return git_default_config(var, value, cb);
 }
 
-static int add_files(struct dir_struct *dir, int flags, int force_mode)
+static int add_files(struct dir_struct *dir, int flags)
 {
int i, exit_status = 0;
 
@@ -294,8 +307,7 @@ static int add_files(struct dir_struct *dir, int flags, int 
force_mode)
}
 
for (i = 0; i < dir->nr; i++)
-   if (add_file_to_index(_index, dir->entries[i]->name,
-   flags, force_mode)) {
+   if (add_file_to_index(_index, dir->entries[i]->name, 
flags)) {
if (!ignore_add_errors)
die(_("adding files failed"));
exit_status = 1;
@@ -441,11 +453,13 @@ int cmd_add(int argc, const char **argv, const char 
*prefix)
 
plug_bulk_checkin();
 
-   exit_status |= add_files_to_cache(prefix, , flags, force_mode);
+   exit_status |= add_files_to_cache(prefix, , flags);
 
if (add_new_files)
-   exit_status |= add_files(, flags, force_mode);
+   exit_status |= add_files(, flags);
 
+   if (force_mode)
+   chmod_pathspec(, force_mode);
unplug_bulk_checkin();
 
 finish:
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8672d07..a83c78f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -548,7 +548,7 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
 * entries in the index.
 */
 
-   add_files_to_cache(NULL, NULL, 0, 0);
+   add_files_to_cache(NULL, NULL, 0);
/*
 * NEEDSWORK: carrying over local changes
 * when branches have different end-of-line
diff --git a/builtin/commit.c b/builtin/commit.c
index 77e3dc8..7a1ade0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -387,7 +387,7 @@ static const char *prepare_index(int argc, const char 
**argv, const char 

[PATCH v2 2/4] update-index: use the same structure for chmod as add

[Thomas Gummerer]

While the chmod options for update-index and the add have the same
functionality, they are using different ways to parse and handle the
option internally.  Unify these modes in order to make further
refactoring simpler.

Signed-off-by: Thomas Gummerer 
---
 builtin/update-index.c | 49 +
 1 file changed, 21 insertions(+), 28 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index ba04b19..85a57db 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -419,11 +419,12 @@ static int add_cacheinfo(unsigned int mode, const 
unsigned char *sha1,
return 0;
 }
 
-static void chmod_path(int flip, const char *path)
+static void chmod_path(int force_mode, const char *path)
 {
int pos;
struct cache_entry *ce;
unsigned int mode;
+   char flip = force_mode == 0777 ? '+' : '-';
 
pos = cache_name_pos(path, strlen(path));
if (pos < 0)
@@ -432,17 +433,11 @@ static void chmod_path(int flip, const char *path)
mode = ce->ce_mode;
if (!S_ISREG(mode))
goto fail;
-   switch (flip) {
-   case '+':
-   ce->ce_mode |= 0111; break;
-   case '-':
-   ce->ce_mode &= ~0111; break;
-   default:
-   goto fail;
-   }
+   ce->ce_mode = create_ce_mode(force_mode);
cache_tree_invalidate_path(_index, path);
ce->ce_flags |= CE_UPDATE_IN_BASE;
active_cache_changed |= CE_ENTRY_CHANGED;
+
report("chmod %cx '%s'", flip, path);
return;
  fail:
@@ -788,16 +783,6 @@ static int really_refresh_callback(const struct option 
*opt,
return refresh(opt->value, REFRESH_REALLY);
 }
 
-static int chmod_callback(const struct option *opt,
-   const char *arg, int unset)
-{
-   char *flip = opt->value;
-   if ((arg[0] != '-' && arg[0] != '+') || arg[1] != 'x' || arg[2])
-   return error("option 'chmod' expects \"+x\" or \"-x\"");
-   *flip = arg[0];
-   return 0;
-}
-
 static int resolve_undo_clear_callback(const struct option *opt,
const char *arg, int unset)
 {
@@ -917,7 +902,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
int read_from_stdin = 0;
int prefix_length = prefix ? strlen(prefix) : 0;
int preferred_index_format = 0;
-   char set_executable_bit = 0;
+   char *chmod_arg = 0;
+   int force_mode = 0;
struct refresh_params refresh_args = {0, _errors};
int lock_error = 0;
int split_index = -1;
@@ -955,10 +941,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
PARSE_OPT_NOARG | /* disallow --cacheinfo= form */
PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
(parse_opt_cb *) cacheinfo_callback},
-   {OPTION_CALLBACK, 0, "chmod", _executable_bit, N_("(+/-)x"),
-   N_("override the executable bit of the listed files"),
-   PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
-   chmod_callback},
+   OPT_STRING( 0, "chmod", _arg, N_("(+/-)x"),
+   N_("override the executable bit of the listed files")),
{OPTION_SET_INT, 0, "assume-unchanged", _valid_only, NULL,
N_("mark files as \"not changing\""),
PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, MARK_FLAG},
@@ -1018,6 +1002,15 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(update_index_usage, options);
 
+   if (!chmod_arg)
+   force_mode = 0;
+   else if (!strcmp(chmod_arg, "-x"))
+   force_mode = 0666;
+   else if (!strcmp(chmod_arg, "+x"))
+   force_mode = 0777;
+   else
+   die(_("option 'chmod' expects \"+x\" or \"-x\""));
+
git_config(git_default_config, NULL);
 
/* We can't free this memory, it becomes part of a linked list parsed 
atexit() */
@@ -1055,8 +1048,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
setup_work_tree();
p = prefix_path(prefix, prefix_length, path);
update_one(p);
-   if (set_executable_bit)
-   chmod_path(set_executable_bit, p);
+   if (force_mode)
+   chmod_path(force_mode, p);
free(p);
ctx.argc--;
ctx.argv++;
@@ -1100,8 +1093,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
}
p = prefix_path(prefix, prefix_length, buf.buf);

[PATCH v2 0/4] git add --chmod: always change the file

[Thomas Gummerer]

Changes since v1:
Added missing x in the documentation.

The changes since v1 are only minor, but as it's been a week, this is
as much a ping as a fixup of my error :)

Thomas Gummerer (4):
  add: document the chmod option
  update-index: use the same structure for chmod as add
  read-cache: introduce chmod_index_entry
  add: modify already added files when --chmod is given

 Documentation/git-add.txt |  7 +-
 builtin/add.c | 36 +--
 builtin/checkout.c|  2 +-
 builtin/commit.c  |  2 +-
 builtin/update-index.c| 55 ++-
 cache.h   | 12 ++-
 read-cache.c  | 33 +---
 t/t3700-add.sh| 21 ++
 8 files changed, 107 insertions(+), 61 deletions(-)

-- 
2.10.0.304.gf2ff484

[PATCH v2 1/4] add: document the chmod option

[Thomas Gummerer]

The git add --chmod option was introduced in 4e55ed3 ("add: add
--chmod=+x / --chmod=-x options", 2016-05-31), but was never
documented.  Document the feature.

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-add.txt | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 6a96a66..7ed63dc 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | 
-i] [--patch | -p]
  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
  [--intent-to-add | -N] [--refresh] [--ignore-errors] 
[--ignore-missing]
- [--] [...]
+ [--chmod=(+|-)x] [--] [...]
 
 DESCRIPTION
 ---
@@ -165,6 +165,11 @@ for "git add --no-all ...", i.e. ignored removed 
files.
be ignored, no matter if they are already present in the work
tree or not.
 
+--chmod=(+|-)x::
+   Override the executable bit of the added files.  The executable
+   bit is only changed in the index, the files on disk are left
+   unchanged.
+
 \--::
This option can be used to separate command-line options from
the list of files, (useful when filenames might be mistaken
-- 
2.10.0.304.gf2ff484

Draft of Git Rev News edition 19

[Christian Couder]

Hi,

A draft of a new Git Rev News edition is available here:

  https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-19.md

Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:

  https://github.com/git/git.github.io/issues/179

You can also reply to this email.

I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.

Thomas and myself plan to publish this edition on Wednesday
September 14.

Thanks,
Christian.

Git garden shears, was Re: [PATCH 13/22] sequencer: remember the onelines when parsing the todo file

[Johannes Schindelin]

Hi Kuba,

On Fri, 9 Sep 2016, Jakub Narębski wrote:

> Hello Johannes,
> 
> W dniu 09.09.2016 o 17:12, Johannes Schindelin napisał:
> > On Thu, 1 Sep 2016, Junio C Hamano wrote: 
> >> Johannes Schindelin  writes:
> 
> >> I was sort of expecting that, when you do the preserve-merges mode
> >> of "rebase -i", you would need to jump around, doing "we have
> >> reconstructed the side branch on a new 'onto', let's give the result
> >> this temporary name ':1', and then switch to the trunk (which would
> >> call for 'reset ' instruction) and merge that thing (which
> >> would be 'merge :1' or perhaps called 'pick :1')", and at that point
> >> you no longer validate the object references upfront.
> > 
> > Except that is not how --preserve-merges works: it *still* uses the SHA-1s
> > as identifiers, even when the SHA-1 may have changed in the meantime.
> > 
> > That is part of why it was a bad design.
> 
> When preserving merges, there are (as far as I understand it), two
> problems:
>  - what it means to preserve changes (which change to pick,
>that is what is the mainline changes rebase is re-applying)
>  - what are parents of the merge commit (at least one parent
>would be usually rewritten)
> 
> Maybe the internal (and perhaps also user-visible) representation
> of merge in instruction sheet could use the notation of filter-branch,
> that is 'map()'... it could also imply the mainline.
> 
> That is the instruction in the internal instruction sheet could
> look like this:
> 
>   merge -m 1 map(2fd4e1c67a2d28fced849ee1bb76e7391b93eb12) 
> da39a3ee5e6b4b0d3255bfef95601890afd80709 \t Merge 'foo' into master  
> 
> 
> Note that it has nothing to do with this series!

Right. But I did solve that already. In the Git garden shears [*1*]
(essentially my New And Improved attempt at recreating branch structures
while rebasing), I generate and process scripts like this:

mark onto

# Branch: super-cool-feature
rewind onto
pick 1 feature
pick 2 documentation
mark super-cool-feature

# Branch: typo-fix
rewind onto
pick a fix a tyop

rewind onto
merge -C cafebabe super-cool-feature
merge -C babecafe typo-fix

cleanup super-cool-feature typo-fix

Of course this will change a little, still, once I get around to implement
this on top of the rebase--helper.

For example, I am not so hot about the "merge -C ..." syntax. I'll
probably split that into a "remerge  " and a new "merge
" command (the latter asking interactively for the merge commit
message).

And also: the cleanup stage should not be necessary, as the "mark"
commands can accumulate the known marks into a file in the state
directory.

But you get the idea.

No :1 or some such. That's machine readable. But it's utter nonsense for
user-facing UIs.

Ciao,
Dscho

Footnote *1*:
https://github.com/git-for-windows/build-extra/blob/master/shears.sh

Re: [PATCH 22/22] sequencer: refactor write_message()

[Johannes Schindelin]

Hi Kuba,

On Fri, 9 Sep 2016, Jakub Narębski wrote:

> W dniu 09.09.2016 o 16:40, Johannes Schindelin napisał:
> > On Fri, 2 Sep 2016, Jakub Narębski wrote:
> >> W dniu 01.09.2016 o 16:20, Johannes Schindelin pisze:
> >>> On Thu, 1 Sep 2016, Jakub Narębski wrote: 
>  W dniu 29.08.2016 o 10:06, Johannes Schindelin pisze:
> >>
> > if (commit_lock_file(_file) < 0)
> > return error(_("Error wrapping up %s."), filename);
> 
>  Another "while at it"... though the one that can be safely postponed
>  (well, the make message easier to understand part, not the quote
>  filename part):
> 
>   return error(_("Error wrapping up writing to '%s'."), 
>  filename);
> >>>
> >>> As I inherited this message, I'll keep it.
> >>
> >> Well, please then add quotes while at it, at least, for consistency
> >>
> >>return error(_("Error wrapping up '%s'."), filename);
> > 
> > I may do that as a final patch, once all the other concerns are addressed.
> > I really do not want to change the error message during the conversion.
> 
> Is not wanting to change error messages during conversion because of
> your use of Scientist tool to catch errors in conversion process?

It is more out of an aversion to mix unrelated purposes in the same patch.
You will see that I inserted an extra patch with the purpose of fixing the
style, that touches all the relevant error messages in sequencer.c.

> BTW. could you tell us what were those three regression caught by the
> cross-validation?

Sure!

The first one was that my original version of the rebase-i-extra patches
did not reorder patches correctly when there was more than one space after
the fixup!. I fixed it, and added this test:

cbcd2cb (rebase -i: we allow extra spaces after fixup!/squash!, 2016-07-07)

The second one was that `git commit --fixup` unwraps the commit subject
into one long line and rebase -i *still* manages to find the correct
commit to fix up. The test is part of the rebase-i-extra patches (and
therefore you will find this commit only in my fork):

9fc25ce (t3415: test fixup with wrapped oneline, 2016-07-24)

The third one was an obscure one: when I marked a commit as 'edit' and
there was a merge conflict cherry-picking that particular commit, rebase
--continue would squash the resolved changes *into the previous* commit,
but with the cherry-picked commit's message. It was a simple, stupid
oversight to write the "amend" file in the "edit" code path even if the
cherry-pick failed.

All fixed, of course.

Ciao,
Dscho

Re: [PATCH v3 16/17] lib'ify checkout_fast_forward_to()

[Johannes Schindelin]

Hi Junio,

On Fri, 9 Sep 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > Instead of dying there, let the caller high up in the callchain
> > notice the error and handle it (by dying, still).
> >
> > The only callers of checkout_fast_forward_to(), cmd_merge(),
> > pull_into_void(), cmd_pull() and sequencer's fast_forward_to(),
> > already check the return value and handle it appropriately. With this
> > step, we make it notice an error return from this function.
> >
> > So this is a safe conversion to make checkout_fast_forward_to()
> > callable from new callers that want it not to die, without changing
> > the external behaviour of anything existing.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> 
> I'll retitle this to
> 
> sequencer: lib'ify chckout_fast_forward()
> 
> and checkout_fast_forward_to() in the second paragraph to match the
> reality.  Other than that, the above analysis matches what I see in
> the code and the libification done here looks correct.

Good catch. Please also fix it in the third paragraph. (I changed it also
locally, in case a v4 is required.)

Ciao,
Dscho

[PATCH v2 5/5] wt-status: teach has_{unstaged,uncommitted}_changes() about submodules

[Johannes Schindelin]

Sometimes we are *actually* interested in those changes... For
example when an interactive rebase wants to continue with a staged
submodule update.

Signed-off-by: Johannes Schindelin 
---
 builtin/pull.c |  2 +-
 wt-status.c| 16 +---
 wt-status.h|  7 ---
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 14ef8b5..c639167 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -810,7 +810,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
 
if (!autostash)
require_clean_work_tree(N_("pull with rebase"),
-   "Please commit or stash them.", 0);
+   "Please commit or stash them.", 1, 0);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
diff --git a/wt-status.c b/wt-status.c
index f1120e6..086ae79 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2214,13 +2214,14 @@ void wt_status_print(struct wt_status *s)
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
-int has_unstaged_changes(void)
+int has_unstaged_changes(int ignore_submodules)
 {
struct rev_info rev_info;
int result;
 
init_revisions(_info, NULL);
-   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
+   if (ignore_submodules)
+   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(_info.diffopt, QUICK);
diff_setup_done(_info.diffopt);
result = run_diff_files(_info, 0);
@@ -2230,7 +2231,7 @@ int has_unstaged_changes(void)
 /**
  * Returns 1 if there are uncommitted changes, 0 otherwise.
  */
-int has_uncommitted_changes(void)
+int has_uncommitted_changes(int ignore_submodules)
 {
struct rev_info rev_info;
int result;
@@ -2239,7 +2240,8 @@ int has_uncommitted_changes(void)
return 0;
 
init_revisions(_info, NULL);
-   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
+   if (ignore_submodules)
+   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(_info.diffopt, QUICK);
add_head_to_pending(_info);
diff_setup_done(_info.diffopt);
@@ -2251,7 +2253,7 @@ int has_uncommitted_changes(void)
  * If the work tree has unstaged or uncommitted changes, dies with the
  * appropriate message.
  */
-int require_clean_work_tree(const char *action, const char *hint, int gently)
+int require_clean_work_tree(const char *action, const char *hint, int 
ignore_submodules, int gently)
 {
struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
int err = 0;
@@ -2261,12 +2263,12 @@ int require_clean_work_tree(const char *action, const 
char *hint, int gently)
update_index_if_able(_index, lock_file);
rollback_lock_file(lock_file);
 
-   if (has_unstaged_changes()) {
+   if (has_unstaged_changes(ignore_submodules)) {
error(_("Cannot %s: You have unstaged changes."), _(action));
err = 1;
}
 
-   if (has_uncommitted_changes()) {
+   if (has_uncommitted_changes(ignore_submodules)) {
if (err)
error(_("Additionally, your index contains uncommitted 
changes."));
else
diff --git a/wt-status.h b/wt-status.h
index 68e367a..54fec77 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -129,8 +129,9 @@ __attribute__((format (printf, 3, 4)))
 void status_printf(struct wt_status *s, const char *color, const char *fmt, 
...);
 
 /* The following functions expect that the caller took care of reading the 
index. */
-int has_unstaged_changes(void);
-int has_uncommitted_changes(void);
-int require_clean_work_tree(const char *action, const char *hint, int gently);
+int has_unstaged_changes(int ignore_submodules);
+int has_uncommitted_changes(int ignore_submodules);
+int require_clean_work_tree(const char *action, const char *hint,
+   int ignore_submodules, int gently);
 
 #endif /* STATUS_H */
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 3/5] Make the require_clean_work_tree() function truly reusable

[Johannes Schindelin]

It is remarkable that libgit.a did not sport this function yet... Let's
move it into a more prominent (and into an actually reusable) spot:
wt-status.[ch].

Signed-off-by: Johannes Schindelin 
---
 builtin/pull.c | 76 +-
 wt-status.c| 75 +
 wt-status.h|  3 +++
 3 files changed, 79 insertions(+), 75 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index a3ed054..14ef8b5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -17,6 +17,7 @@
 #include "revision.h"
 #include "tempfile.h"
 #include "lockfile.h"
+#include "wt-status.h"
 
 enum rebase_type {
REBASE_INVALID = -1,
@@ -326,81 +327,6 @@ static int git_pull_config(const char *var, const char 
*value, void *cb)
 }
 
 /**
- * Returns 1 if there are unstaged changes, 0 otherwise.
- */
-static int has_unstaged_changes(void)
-{
-   struct rev_info rev_info;
-   int result;
-
-   init_revisions(_info, NULL);
-   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
-   DIFF_OPT_SET(_info.diffopt, QUICK);
-   diff_setup_done(_info.diffopt);
-   result = run_diff_files(_info, 0);
-   return diff_result_code(_info.diffopt, result);
-}
-
-/**
- * Returns 1 if there are uncommitted changes, 0 otherwise.
- */
-static int has_uncommitted_changes(void)
-{
-   struct rev_info rev_info;
-   int result;
-
-   if (is_cache_unborn())
-   return 0;
-
-   init_revisions(_info, NULL);
-   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
-   DIFF_OPT_SET(_info.diffopt, QUICK);
-   add_head_to_pending(_info);
-   diff_setup_done(_info.diffopt);
-   result = run_diff_index(_info, 1);
-   return diff_result_code(_info.diffopt, result);
-}
-
-/**
- * If the work tree has unstaged or uncommitted changes, dies with the
- * appropriate message.
- */
-static int require_clean_work_tree(const char *action, const char *hint,
-   int gently)
-{
-   struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
-   int err = 0;
-
-   hold_locked_index(lock_file, 0);
-   refresh_cache(REFRESH_QUIET);
-   update_index_if_able(_index, lock_file);
-   rollback_lock_file(lock_file);
-
-   if (has_unstaged_changes()) {
-   error(_("Cannot %s: You have unstaged changes."), _(action));
-   err = 1;
-   }
-
-   if (has_uncommitted_changes()) {
-   if (err)
-   error(_("Additionally, your index contains uncommitted 
changes."));
-   else
-   error(_("Cannot %s: Your index contains uncommitted 
changes."),
- _(action));
-   err = 1;
-   }
-
-   if (err) {
-   if (hint)
-   error("%s", hint);
-   if (!gently)
-   exit(err);
-   }
-
-   return err;
-}
-
-/**
  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
  * into merge_heads.
  */
diff --git a/wt-status.c b/wt-status.c
index 539aac1..9ab9adc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -16,6 +16,7 @@
 #include "strbuf.h"
 #include "utf8.h"
 #include "worktree.h"
+#include "lockfile.h"
 
 static const char cut_line[] =
 " >8 \n";
@@ -2209,3 +2210,77 @@ void wt_status_print(struct wt_status *s)
break;
}
 }
+
+/**
+ * Returns 1 if there are unstaged changes, 0 otherwise.
+ */
+static int has_unstaged_changes(void)
+{
+   struct rev_info rev_info;
+   int result;
+
+   init_revisions(_info, NULL);
+   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
+   DIFF_OPT_SET(_info.diffopt, QUICK);
+   diff_setup_done(_info.diffopt);
+   result = run_diff_files(_info, 0);
+   return diff_result_code(_info.diffopt, result);
+}
+
+/**
+ * Returns 1 if there are uncommitted changes, 0 otherwise.
+ */
+static int has_uncommitted_changes(void)
+{
+   struct rev_info rev_info;
+   int result;
+
+   if (is_cache_unborn())
+   return 0;
+
+   init_revisions(_info, NULL);
+   DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
+   DIFF_OPT_SET(_info.diffopt, QUICK);
+   add_head_to_pending(_info);
+   diff_setup_done(_info.diffopt);
+   result = run_diff_index(_info, 1);
+   return diff_result_code(_info.diffopt, result);
+}
+
+/**
+ * If the work tree has unstaged or uncommitted changes, dies with the
+ * appropriate message.
+ */
+int require_clean_work_tree(const char *action, const char *hint, int gently)
+{
+   struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
+   int err = 0;
+
+   hold_locked_index(lock_file, 0);
+   refresh_cache(REFRESH_QUIET);
+   update_index_if_able(_index, lock_file);
+   rollback_lock_file(lock_file);
+
+   if (has_unstaged_changes()) 

[PATCH v2 4/5] Export also the has_un{staged,committed}_changed() functions

[Johannes Schindelin]

They will be used in the upcoming rebase helper.

Signed-off-by: Johannes Schindelin 
---
 wt-status.c | 4 ++--
 wt-status.h | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 9ab9adc..f1120e6 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2214,7 +2214,7 @@ void wt_status_print(struct wt_status *s)
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
-static int has_unstaged_changes(void)
+int has_unstaged_changes(void)
 {
struct rev_info rev_info;
int result;
@@ -2230,7 +2230,7 @@ static int has_unstaged_changes(void)
 /**
  * Returns 1 if there are uncommitted changes, 0 otherwise.
  */
-static int has_uncommitted_changes(void)
+int has_uncommitted_changes(void)
 {
struct rev_info rev_info;
int result;
diff --git a/wt-status.h b/wt-status.h
index 03ecf53..68e367a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -128,7 +128,9 @@ void status_printf_ln(struct wt_status *s, const char 
*color, const char *fmt, .
 __attribute__((format (printf, 3, 4)))
 void status_printf(struct wt_status *s, const char *color, const char *fmt, 
...);
 
-/* The following function expect that the caller took care of reading the 
index. */
+/* The following functions expect that the caller took care of reading the 
index. */
+int has_unstaged_changes(void);
+int has_uncommitted_changes(void);
 int require_clean_work_tree(const char *action, const char *hint, int gently);
 
 #endif /* STATUS_H */
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 2/5] pull: make code more similar to the shell script again

[Johannes Schindelin]

When converting the pull command to a builtin, the
require_clean_work_tree() function was renamed and the pull-specific
parts hard-coded.

This makes it impossible to reuse the code, so let's modify the code to
make it more similar to the original shell script again.

Signed-off-by: Johannes Schindelin 
---
 builtin/pull.c | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index d4bd635..a3ed054 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -365,10 +365,11 @@ static int has_uncommitted_changes(void)
  * If the work tree has unstaged or uncommitted changes, dies with the
  * appropriate message.
  */
-static void die_on_unclean_work_tree(void)
+static int require_clean_work_tree(const char *action, const char *hint,
+   int gently)
 {
struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
-   int do_die = 0;
+   int err = 0;
 
hold_locked_index(lock_file, 0);
refresh_cache(REFRESH_QUIET);
@@ -376,20 +377,27 @@ static void die_on_unclean_work_tree(void)
rollback_lock_file(lock_file);
 
if (has_unstaged_changes()) {
-   error(_("Cannot pull with rebase: You have unstaged changes."));
-   do_die = 1;
+   error(_("Cannot %s: You have unstaged changes."), _(action));
+   err = 1;
}
 
if (has_uncommitted_changes()) {
-   if (do_die)
+   if (err)
error(_("Additionally, your index contains uncommitted 
changes."));
else
-   error(_("Cannot pull with rebase: Your index contains 
uncommitted changes."));
-   do_die = 1;
+   error(_("Cannot %s: Your index contains uncommitted 
changes."),
+ _(action));
+   err = 1;
}
 
-   if (do_die)
-   exit(1);
+   if (err) {
+   if (hint)
+   error("%s", hint);
+   if (!gently)
+   exit(err);
+   }
+
+   return err;
 }
 
 /**
@@ -875,7 +883,8 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
die(_("Updating an unborn branch with changes added to 
the index."));
 
if (!autostash)
-   die_on_unclean_work_tree();
+   require_clean_work_tree(N_("pull with rebase"),
+   "Please commit or stash them.", 0);
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
-- 
2.10.0.windows.1.10.g803177d

[PATCH v2 0/5] Pull out require_clean_work_tree() functionality from builtin/pull.c

[Johannes Schindelin]

This is the 5th last patch series of my work to accelerate interactive
rebases in particular on Windows.

Basically, all it does is to make reusable some functions that were
ported over from git-pull.sh but made private to builtin/pull.c.

Changes since v1:

- skipped patch that tries to make require_clean_work_tree() smart
  enough to read the index if it was not read yet.

- added a code-comment clarifying that it is the duty of
  require_clean_work_tree()'s caller to ensure that the index has been
  read.

- made the action in require_clean_work_tree() translateable. This
  amazingly easy without complexifying the code, simply by using N_()
  and _() as indicated by Jakub.


Johannes Schindelin (5):
  pull: drop confusing prefix parameter of die_on_unclean_work_tree()
  pull: make code more similar to the shell script again
  Make the require_clean_work_tree() function truly reusable
  Export also the has_un{staged,committed}_changed() functions
  wt-status: teach has_{unstaged,uncommitted}_changes() about submodules

 builtin/pull.c | 71 +++--
 wt-status.c| 77 ++
 wt-status.h|  6 +
 3 files changed, 86 insertions(+), 68 deletions(-)

Published-As: 
https://github.com/dscho/git/releases/tag/require-clean-work-tree-v2
Fetch-It-Via: git fetch https://github.com/dscho/git require-clean-work-tree-v2

Interdiff vs v1:

 diff --git a/builtin/pull.c b/builtin/pull.c
 index 843ff19..c639167 100644
 --- a/builtin/pull.c
 +++ b/builtin/pull.c
 @@ -809,7 +809,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
die(_("Updating an unborn branch with changes added to 
the index."));
  
if (!autostash)
 -  require_clean_work_tree("pull with rebase",
 +  require_clean_work_tree(N_("pull with rebase"),
"Please commit or stash them.", 1, 0);
  
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
 diff --git a/wt-status.c b/wt-status.c
 index 129b054..086ae79 100644
 --- a/wt-status.c
 +++ b/wt-status.c
 @@ -2258,20 +2258,13 @@ int require_clean_work_tree(const char *action, const 
char *hint, int ignore_sub
struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
int err = 0;
  
 -  if (read_cache() < 0) {
 -  error(_("Could not read index"));
 -  if (gently)
 -  return -1;
 -  exit(1);
 -  }
 -
hold_locked_index(lock_file, 0);
refresh_cache(REFRESH_QUIET);
update_index_if_able(_index, lock_file);
rollback_lock_file(lock_file);
  
if (has_unstaged_changes(ignore_submodules)) {
 -  error(_("Cannot %s: You have unstaged changes."), action);
 +  error(_("Cannot %s: You have unstaged changes."), _(action));
err = 1;
}
  
 @@ -2279,7 +2272,8 @@ int require_clean_work_tree(const char *action, const 
char *hint, int ignore_sub
if (err)
error(_("Additionally, your index contains uncommitted 
changes."));
else
 -  error(_("Cannot %s: Your index contains uncommitted 
changes."), action);
 +  error(_("Cannot %s: Your index contains uncommitted 
changes."),
 +_(action));
err = 1;
}
  
 diff --git a/wt-status.h b/wt-status.h
 index f0e66c4..54fec77 100644
 --- a/wt-status.h
 +++ b/wt-status.h
 @@ -128,6 +128,7 @@ void status_printf_ln(struct wt_status *s, const char 
*color, const char *fmt, .
  __attribute__((format (printf, 3, 4)))
  void status_printf(struct wt_status *s, const char *color, const char *fmt, 
...);
  
 +/* The following functions expect that the caller took care of reading the 
index. */
  int has_unstaged_changes(int ignore_submodules);
  int has_uncommitted_changes(int ignore_submodules);
  int require_clean_work_tree(const char *action, const char *hint,

-- 
2.10.0.windows.1.10.g803177d

base-commit: cda1bbd474805e653dda8a71d4ea3790e2a66cbb

[PATCH v2 1/5] pull: drop confusing prefix parameter of die_on_unclean_work_tree()

[Johannes Schindelin]

In cmd_pull(), when verifying that there are no changes preventing a
rebasing pull, we diligently pass the prefix parameter to the
die_on_unclean_work_tree() function which in turn diligently passes it
to the has_unstaged_changes() and has_uncommitted_changes() functions.

The casual reader might now be curious (as this developer was) whether
that means that calling `git pull --rebase` in a subdirectory will
ignore unstaged changes in other parts of the working directory. And be
puzzled that `git pull --rebase` (correctly) complains about those
changes outside of the current directory.

The puzzle is easily resolved: while we take pains to pass around the
prefix and even pass it to init_revisions(), the fact that no paths are
passed to init_revisions() ensures that the prefix is simply ignored.

That, combined with the fact that we will *always* want a *full* working
directory check before running a rebasing pull, is reason enough to
simply do away with the actual prefix parameter and to pass NULL
instead, as if we were running this from the top-level working directory
anyway.

Signed-off-by: Johannes Schindelin 
---
 builtin/pull.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 398aae1..d4bd635 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -328,12 +328,12 @@ static int git_pull_config(const char *var, const char 
*value, void *cb)
 /**
  * Returns 1 if there are unstaged changes, 0 otherwise.
  */
-static int has_unstaged_changes(const char *prefix)
+static int has_unstaged_changes(void)
 {
struct rev_info rev_info;
int result;
 
-   init_revisions(_info, prefix);
+   init_revisions(_info, NULL);
DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(_info.diffopt, QUICK);
diff_setup_done(_info.diffopt);
@@ -344,7 +344,7 @@ static int has_unstaged_changes(const char *prefix)
 /**
  * Returns 1 if there are uncommitted changes, 0 otherwise.
  */
-static int has_uncommitted_changes(const char *prefix)
+static int has_uncommitted_changes(void)
 {
struct rev_info rev_info;
int result;
@@ -352,7 +352,7 @@ static int has_uncommitted_changes(const char *prefix)
if (is_cache_unborn())
return 0;
 
-   init_revisions(_info, prefix);
+   init_revisions(_info, NULL);
DIFF_OPT_SET(_info.diffopt, IGNORE_SUBMODULES);
DIFF_OPT_SET(_info.diffopt, QUICK);
add_head_to_pending(_info);
@@ -365,7 +365,7 @@ static int has_uncommitted_changes(const char *prefix)
  * If the work tree has unstaged or uncommitted changes, dies with the
  * appropriate message.
  */
-static void die_on_unclean_work_tree(const char *prefix)
+static void die_on_unclean_work_tree(void)
 {
struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file));
int do_die = 0;
@@ -375,12 +375,12 @@ static void die_on_unclean_work_tree(const char *prefix)
update_index_if_able(_index, lock_file);
rollback_lock_file(lock_file);
 
-   if (has_unstaged_changes(prefix)) {
+   if (has_unstaged_changes()) {
error(_("Cannot pull with rebase: You have unstaged changes."));
do_die = 1;
}
 
-   if (has_uncommitted_changes(prefix)) {
+   if (has_uncommitted_changes()) {
if (do_die)
error(_("Additionally, your index contains uncommitted 
changes."));
else
@@ -875,7 +875,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
die(_("Updating an unborn branch with changes added to 
the index."));
 
if (!autostash)
-   die_on_unclean_work_tree(prefix);
+   die_on_unclean_work_tree();
 
if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
hashclr(rebase_fork_point);
-- 
2.10.0.windows.1.10.g803177d