Re: [PATCH 24/25] worktree move: accept destination as directory

[Johannes Sixt]

Am 11.05.2016 um 23:34 schrieb Junio C Hamano:

Johannes Sixt  writes:


As this path is read from a file git itself creates, and if we know
that it will always contain forward slashes, then I agree that it
could be potentially confusing to later readers to see
git_find_last_dir_sep(). So, keeping it as-is seems correct.


Please allow me to disagree. There should not be any assumption that a
path uses forward slashes as directory separator, except when the path
is

- a pathspec
- a ref
- a path found in the object database including the index


I think standardising on one way for what we write out would give
less hassle to users.  The human end users should not be opening
these files in their editors and futzing with their contents, but
there are third-party tools and reimplementations of Git.  Forcing
them to be prepared for input with slashes and backslashes does not
make much sense to me.


It is the opposite: We would force other tools to write slashes even 
though on Windows both types of slashes are allowed as directory separators.



Is there an upside for us to accept both slashes in this file?


Obviously, yes: We can accept what third-party tools write.

BTW, we also have to accept absolute paths in the file, no? Then we 
cannot assume that the path begins with a slash on Windows; absolute 
paths would come in drive letter style on Windows.


-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] submodule groups

[Junio C Hamano]

Stefan Beller  writes:

> If you have lots of submodules, you probably don't need all of them at once,
> but you have functional units. Some submodules are absolutely required,
> some are optional and only for very specific purposes.
>
> This patch series adds labels to submodules in the .gitmodules file.

I hate to bring this up in this thread, primarily because I cannot
see how to make it mesh well with the "submodule spec lets you
specify a group of submodule with labels", but for completeness of
discussion, I'll mention it anyway.

Instead of specifying "all files written in Perl" to work on by
giving a pathspec with three elements, e.g.

git cmd -- \*.perl \*.pl \*.pm

I've often wondered if it would be a good idea to let attributes
file to specify "these paths form the group Perl" with something
like:

*.pmgroup=perl
*.plgroup=perl
*.perl  group=perl
*.h group=c
*.c group=c

and say

git cmd -- ':(group=perl)'

instead.

The reason why I suspect that this may not work well with submodule
labels is because submodule labels (or any attribute we give via
.gitmodules to a submodule) are keyed by a submodule name, which is
the primary unchanging key (so that people can "mv" a submodule in
the context of the toplevel superproject without losing track of
submodule identity), not by paths to submodules, while the "group"
thing I want is merely a short-hand for pathspec elements and wants
to be keyed by paths.

But there may be somebody more clever than I who can come up with a
way to unify these two similar concepts without confusing end users.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v6 3/3] bisect--helper: `write_terms` shell function in C

[Pranit Bauva]

Reimplement the `write_terms` shell function in C and add a `write-terms`
subcommand to `git bisect--helper` to call it from git-bisect.sh . Also
remove the subcommand `--check-term-format` as it can now be called from
inside the function write_terms() C implementation.

Using `--write-terms` subcommand is a temporary measure to port shell
function to C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired and will be called by some
other method.

Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 37 ++---
 git-bisect.sh| 22 +++---
 2 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 3c748d1..2b21c02 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -6,7 +6,7 @@
 
 static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --next-all [--no-checkout]"),
-   N_("git bisect--helper --check-term-format  "),
+   N_("git bisect--helper --write-terms  "),
NULL
 };
 
@@ -56,18 +56,41 @@ static int check_term_format(const char *term, const char 
*orig_term)
return 0;
 }
 
+int write_terms(const char *bad, const char *good)
+{
+   struct strbuf content = STRBUF_INIT;
+   FILE *fp;
+   int res;
+
+   if (!strcmp(bad, good))
+   return error(_("please use two different terms"));
+
+   if (check_term_format(bad, "bad") || check_term_format(good, "good"))
+   return -1;
+
+   strbuf_addf(, "%s\n%s\n", bad, good);
+   fp = fopen(".git/BISECT_TERMS", "w");
+   if (!fp){
+   strbuf_release();
+   die_errno(_("could not open the file to read terms"));
+   }
+   res = strbuf_write(, fp);
+   fclose(fp);
+   strbuf_release();
+   return (res < 0) ? -1 : 0;
+}
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
enum {
NEXT_ALL = 1,
-   CHECK_TERM_FMT
+   WRITE_TERMS
} cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", ,
 N_("perform 'git bisect next'"), NEXT_ALL),
-   OPT_CMDMODE(0, "check-term-format", ,
-N_("check format of the term"), CHECK_TERM_FMT),
+   OPT_CMDMODE(0, "write-terms", ,
+N_("write the terms to .git/BISECT_TERMS"), 
WRITE_TERMS),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -82,10 +105,10 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
switch (cmdmode) {
case NEXT_ALL:
return bisect_next_all(prefix, no_checkout);
-   case CHECK_TERM_FMT:
+   case WRITE_TERMS:
if (argc != 2)
-   die(_("--check-term-format requires two arguments"));
-   return check_term_format(argv[0], argv[1]);
+   die(_("--write-terms requires two arguments"));
+   return write_terms(argv[0], argv[1]);
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 7d7965d..2dd7ec5 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -210,7 +210,7 @@ bisect_start() {
eval "$eval true" &&
if test $must_write_terms -eq 1
then
-   write_terms "$TERM_BAD" "$TERM_GOOD"
+   git bisect--helper --write-terms "$TERM_BAD" "$TERM_GOOD"
fi &&
echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
#
@@ -557,18 +557,6 @@ get_terms () {
fi
 }
 
-write_terms () {
-   TERM_BAD=$1
-   TERM_GOOD=$2
-   if test "$TERM_BAD" = "$TERM_GOOD"
-   then
-   die "$(gettext "please use two different terms")"
-   fi
-   git bisect--helper --check-term-format "$TERM_BAD" bad || exit
-   git bisect--helper --check-term-format "$TERM_GOOD" good || exit
-   printf '%s\n%s\n' "$TERM_BAD" "$TERM_GOOD" >"$GIT_DIR/BISECT_TERMS"
-}
-
 check_and_set_terms () {
cmd="$1"
case "$cmd" in
@@ -582,13 +570,17 @@ check_and_set_terms () {
bad|good)
if ! test -s "$GIT_DIR/BISECT_TERMS"
then
-   write_terms bad good
+   TERM_BAD=bad
+   TERM_GOOD=good
+   git bisect--helper --write-terms "$TERM_BAD" 
"$TERM_GOOD"
fi
;;
new|old)
 

[PATCH v6 1/3] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

[Pranit Bauva]

`--next-all` is meant to be used as a subcommand to support multiple
"operation mode" though the current implementation does not contain any
other subcommand along side with `--next-all` but further commits will
include some more subcommands.

Helped-by: Johannes Schindelin 
Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 3324229..8111c91 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -10,11 +10,11 @@ static const char * const git_bisect_helper_usage[] = {
 
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
-   int next_all = 0;
+   enum { NEXT_ALL = 1 } cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
-   OPT_BOOL(0, "next-all", _all,
-N_("perform 'git bisect next'")),
+   OPT_CMDMODE(0, "next-all", ,
+N_("perform 'git bisect next'"), NEXT_ALL),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -23,9 +23,14 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
argc = parse_options(argc, argv, prefix, options,
 git_bisect_helper_usage, 0);
 
-   if (!next_all)
+   if (!cmdmode)
usage_with_options(git_bisect_helper_usage, options);
 
-   /* next-all */
-   return bisect_next_all(prefix, no_checkout);
+   switch (cmdmode) {
+   case NEXT_ALL:
+   return bisect_next_all(prefix, no_checkout);
+   default:
+   die("BUG: unknown subcommand '%d'", cmdmode);
+   }
+   return 0;
 }
-- 
2.8.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v6 2/3] bisect: rewrite `check_term_format` shell function in C

[Pranit Bauva]

Reimplement the `check_term_format` shell function in C and add
a `--check-term-format` subcommand to `git bisect--helper` to call it
from git-bisect.sh

Using `--check-term-format` subcommand is a temporary measure to port
shell function to C so as to use the existing test suite. As more
functions are ported, this subcommand will be retired and will
be called by some other method/subcommand. For eg. In conversion of
write_terms() of git-bisect.sh, the subcommand will be removed and
instead check_term_format() will be called in its C implementation while
a new subcommand will be introduced for write_terms().

Helped-by: Johannes Schindelein 
Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 59 +++-
 git-bisect.sh| 31 ++---
 2 files changed, 60 insertions(+), 30 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 8111c91..3c748d1 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -2,19 +2,72 @@
 #include "cache.h"
 #include "parse-options.h"
 #include "bisect.h"
+#include "refs.h"
 
 static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --next-all [--no-checkout]"),
+   N_("git bisect--helper --check-term-format  "),
NULL
 };
 
+/*
+ * Check whether the string `term` belongs to the set of strings
+ * included in the variable arguments.
+ */
+static int one_of(const char *term, ...)
+{
+   int res = 0;
+   va_list matches;
+   const char *match;
+
+   va_start(matches, term);
+   while (!res && (match = va_arg(matches, const char *)))
+   res = !strcmp(term, match);
+   va_end(matches);
+
+   return res;
+}
+
+static int check_term_format(const char *term, const char *orig_term)
+{
+   struct strbuf new_term = STRBUF_INIT;
+   strbuf_addf(_term, "refs/bisect/%s", term);
+
+   if (check_refname_format(new_term.buf, 0)) {
+   strbuf_release(_term);
+   return error(_("'%s' is not a valid term"), term);
+   }
+   strbuf_release(_term);
+
+   if (one_of(term, "help", "start", "skip", "next", "reset",
+   "visualize", "replay", "log", "run", NULL))
+   return error(_("can't use the builtin command '%s' as a term"), 
term);
+
+   /*
+* In theory, nothing prevents swapping completely good and bad,
+* but this situation could be confusing and hasn't been tested
+* enough. Forbid it for now.
+*/
+
+   if ((strcmp(orig_term, "bad") && one_of(term, "bad", "new", NULL)) ||
+(strcmp(orig_term, "good") && one_of(term, "good", "old", 
NULL)))
+   return error(_("can't change the meaning of the term '%s'"), 
term);
+
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
-   enum { NEXT_ALL = 1 } cmdmode = 0;
+   enum {
+   NEXT_ALL = 1,
+   CHECK_TERM_FMT
+   } cmdmode = 0;
int no_checkout = 0;
struct option options[] = {
OPT_CMDMODE(0, "next-all", ,
 N_("perform 'git bisect next'"), NEXT_ALL),
+   OPT_CMDMODE(0, "check-term-format", ,
+N_("check format of the term"), CHECK_TERM_FMT),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -29,6 +82,10 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
switch (cmdmode) {
case NEXT_ALL:
return bisect_next_all(prefix, no_checkout);
+   case CHECK_TERM_FMT:
+   if (argc != 2)
+   die(_("--check-term-format requires two arguments"));
+   return check_term_format(argv[0], argv[1]);
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 5d1cb00..7d7965d 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -564,38 +564,11 @@ write_terms () {
then
die "$(gettext "please use two different terms")"
fi
-   check_term_format "$TERM_BAD" bad
-   check_term_format "$TERM_GOOD" good
+   git bisect--helper --check-term-format "$TERM_BAD" bad || exit
+   git bisect--helper --check-term-format "$TERM_GOOD" good || exit
printf '%s\n%s\n' "$TERM_BAD" "$TERM_GOOD" >"$GIT_DIR/BISECT_TERMS"
 }
 
-check_term_format () {
-   term=$1
-   git check-ref-format refs/bisect/"$term" ||
-   die "$(eval_gettext "'\$term' is not a valid term")"
-   case "$term" in
-   help|start|terms|skip|next|reset|visualize|replay|log|run)
-  

[PATCH v6 0/3] bisect--helper: check_term_format() & write_terms()

[Pranit Bauva]

To show how I am going to approach conversion of shell function to its C
implementation by using the subcommand/cmdmode approach, I have first converted
check_terms_format() to a subcommand then I have converted write_terms() to
a subcommand and then remove the subcommand for check_terms_format() and
instead call that function from write_terms().

Also I investigated about the test coverage which I mentioned in my previous
version and there is nothing that could be done to improve it.

Minor changes wrt v5:
 * Use the term cmdmode instead of subcommand as pointed out by Junio. (I
   didn't understand his response in v4 then Christian and Johannes pointed
   it out to me what he meant).
 * Move the enum cmdmode (for subcommand) to function scope as suggested by
   Johannes Schindelin.
 * A few minor nits by Eric Sunshine.

Link for v5: http://thread.gmane.org/gmane.comp.version-control.git/293785

Pranit Bauva (3):
  bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
  bisect: rewrite `check_term_format` shell function in C
  bisect--helper: `write_terms` shell function in C

 builtin/bisect--helper.c | 97 +---
 git-bisect.sh| 49 
 2 files changed, 98 insertions(+), 48 deletions(-)

-- 
2.8.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] am: plug FILE * leak in split_mail_conv()

[Jeff King]

On Thu, May 12, 2016 at 07:23:02AM +0200, Mikael Magnusson wrote:

> >> - if (!out)
> >> + if (!out) {
> >> + fclose(in);
> >>   return error(_("could not open '%s' for writing: 
> >> %s"),
> >>   mail, strerror(errno));
> >> + }
> >
> > Presumably `fclose` doesn't ever overwrite errno in practice, but I
> > guess it could in theory.
> 
> It probably does pretty often in general, but not when the file is
> opened for input only.

Right, I should have said "this fclose".

I think EBADF is the only likely error when closing input, and that's
presumably impossible here.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] am: plug FILE * leak in split_mail_conv()

[Mikael Magnusson]

On Thu, May 12, 2016 at 6:47 AM, Jeff King  wrote:
> On Wed, May 11, 2016 at 04:35:46PM -0700, Junio C Hamano wrote:
>
>> Signed-off-by: Junio C Hamano 
>> ---
>>  builtin/am.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/builtin/am.c b/builtin/am.c
>> index f1a84c6..a373928 100644
>> --- a/builtin/am.c
>> +++ b/builtin/am.c
>> @@ -761,9 +761,11 @@ static int split_mail_conv(mail_conv_fn fn, struct 
>> am_state *state,
>>   mail = mkpath("%s/%0*d", state->dir, state->prec, i + 1);
>>
>>   out = fopen(mail, "w");
>> - if (!out)
>> + if (!out) {
>> + fclose(in);
>>   return error(_("could not open '%s' for writing: %s"),
>>   mail, strerror(errno));
>> + }
>
> Presumably `fclose` doesn't ever overwrite errno in practice, but I
> guess it could in theory.

It probably does pretty often in general, but not when the file is
opened for input only.

-- 
Mikael Magnusson
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] am: plug FILE * leak in split_mail_conv()

[Jeff King]

On Wed, May 11, 2016 at 04:35:46PM -0700, Junio C Hamano wrote:

> Signed-off-by: Junio C Hamano 
> ---
>  builtin/am.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/am.c b/builtin/am.c
> index f1a84c6..a373928 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -761,9 +761,11 @@ static int split_mail_conv(mail_conv_fn fn, struct 
> am_state *state,
>   mail = mkpath("%s/%0*d", state->dir, state->prec, i + 1);
>  
>   out = fopen(mail, "w");
> - if (!out)
> + if (!out) {
> + fclose(in);
>   return error(_("could not open '%s' for writing: %s"),
>   mail, strerror(errno));
> + }

Presumably `fclose` doesn't ever overwrite errno in practice, but I
guess it could in theory.

I also found it weird that we might fclose(stdin) via this line, but
that matches what happens in the non-error path, so I guess it's OK?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] submodule groups

[Junio C Hamano]

Junio C Hamano  writes:

> I'd suggest not to over-engineer this.  Go back and imagine how
> "/bin/ls" would work is a good sanity check to gauge what complexity
> levels ordinary users would feel comfortable to handle.
>
> "ls a b" would give union of what "ls a" and "ls b" would output,
> there is no negation, and the users won't die from having to say "ls
> [A-Za-qs-z]*" to exclude files whose names begin with 'r'.

Having said all that, there is one thing we may want to consider.

For negative pathspecs, we deliberately defined its semantics to
"see if the path matches with any positive pathspec element (and it
is a non-match if no positive one matches), and then exclude if it
matches with any negative pathspec element".

That is, when you are matching 'x.o' against three-element pathspec 

'*.c' ':(exclude)*.o' 'x.?'

you do not say "x.o does not match *.c, but it matches *.o so it is
to be excluded, ah, wait, x.? matches to revive it so it is a
match".  Instead "*.c does not and x.? does match, but *.o matches
so it is excluded".  IOW, the order of the pathspec elements given
on the command line does not matter.

Now we are adding two different criteria to specify inclusion based
on labels and names.

As implemented in the patch series under discussion, we are saying
that a submodule is included if

- its path matches the pathspec (using the "matches one or more
  positive pathspec elements, and does not match any negaative
  pathspec element" as the definition of "matches"), OR

- it is included in the specified set of *labels, OR

- its name is specified by :name

There is no reason not to consider future enhancements to specify
exclusion base on these two new criretia.  A naive and easier to
implement enhancement would be to rewrite the above to (the latter
two item changes):


- its path matches the pathspec (using the "matches one or more
  positive pathspec elements, and does not match any negaative
  pathspec element" as the definition of "matches"), OR

- it is included in the specified set of *labels, and does not
  have any excluded *!labels, OR

- its name is specified by :name, and is not among any excluded
  :!name.

but it does not have to be that way.  I suspect that it may make it
easier to understand and use if we OR'ed together only the positive
ones from three classes of criteria together and require at least
one of them to match, and then requiring none of the negative ones
to match.  That is, a module-spec with three elements:

 'sub*' '*label0' './:(exclude)*0'

with the implemented algorithm would choose submodules whose name
begins with 'sub' except the ones whose name ends with '0', OR those
with label0, but if we redefine the behaviour to "positive ones
together, and then exclude negative ones", then we would choose ones
whose name begins with 'sub' or ones with label0, and among these,
exclude those whose name ends with '0' (which is what your "test"
wanted to express).

The implementation of match_pathspec() does the "first positive ones
only and then negative ones" two-step process already, so I think
you could update its "int is_dir" argument to "unsigned flags",
introduce a "negative only" flag, and then do something like:

for each path
if (!(match_pathspec(ps, name, ..., 0) ||
  has a "positive" label specified ||
  has its name specified as a "postiive")
/* does not match any positive criteria */
continue;
if (match_pathspec(ps, name, ..., NEGATIVE_ONLY) ||
has a "negative" label specified ||
has its name specified as a "negative")
/* explicitly excluded */
continue;
/* included! */

That would open door to something like

 'sub*' '*label0' './:(exclude)*0' '*!label1'

to allow you to say "(those with label0 or whose path begins with
sub) minus (those with label1 or whose path ends with 0)".

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: t5551 hangs ?

[Jeff King]

On Wed, May 11, 2016 at 10:03:45PM +0200, Torsten Bögershausen wrote:

> > If you are, can you confirm that it's actually hanging, and not just
> > slow? On my system, test 26 takes about a minute to run (which is why we
> > don't do it by default).
> Nearly sure. After 10 minutes, the test was still running.
> 
> Yesterday another machine was running even longer.
> 
> Any tips, how to debug, are welcome.

Try running with "-x" to see what the test is doing. It will probably be
in:

   + git -C too-many-refs fetch -q --tags

after a while. Check "ps" to see if you have a fetch-pack sub-process
running. It should be writing "have" lines and reading lots of ACK
lines, which you can check via strace.

If it's blocked on read() or write(), then it's probably some kind of
I/O deadlock.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] submodule groups

[Junio C Hamano]

Stefan Beller  writes:

> git submodule--helper matches-submodulespec sub0 ./.
> ./:(exclude)*0 *label-sub0
>
> which should test if the first argument (sub0) matches the submodulespec
> which follows.

This, according to that "OR'ed together" definition, asks to find a
submodule

- whose path matches pathspec ./. ./:(exclude)*0; or
- is labeled with label-sub0.

So I'd say it is natural sub0 matches if its path is at sub0 and has
a label label-sub0.

You could instead choose to use "AND'ed together" semantics, but
that would break expectation by those who expect "This OR that"
behaviour.  Unless you are willing to support --and/--or/(/) like
"git grep" does to express a way to combine hits from individual
terms, that is an inherent limitation.

I'd suggest not to over-engineer this.  Go back and imagine how
"/bin/ls" would work is a good sanity check to gauge what complexity
levels ordinary users would feel comfortable to handle.

"ls a b" would give union of what "ls a" and "ls b" would output,
there is no negation, and the users won't die from having to say "ls
[A-Za-qs-z]*" to exclude files whose names begin with 'r'.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] submodule groups

[Stefan Beller]

On Wed, May 11, 2016 at 4:48 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>>>   git ls-files . :(file-size:>1024k)
>>
>> I somehow do not think this is a way normal people (read: end users)
>> would want to interact with Git.  Pathspec is about "paths" and
>> various ways to match them.  It is not about contents that happens
>> to be currently named by that path.  Don't tie types or sizes to it.
>
> To clarify, think what that non-pathspec means when used like this:
>
> $ git diff :(size>1M)
> $ git log --follow :(size>1M)
>
> Which side of comparison does the "size" thing apply?  Either, both,
> randomly?  More importantly, what use case of users do these
> commands serve?
>
> That is why I said that pathspec should never consider anything but
> the pathname string you see.

I see. That is bad indeed. So I'll go back and finish the submodulespec
to present.

Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] submodule groups

[Junio C Hamano]

Junio C Hamano  writes:

>>   git ls-files . :(file-size:>1024k)
>
> I somehow do not think this is a way normal people (read: end users)
> would want to interact with Git.  Pathspec is about "paths" and
> various ways to match them.  It is not about contents that happens
> to be currently named by that path.  Don't tie types or sizes to it.

To clarify, think what that non-pathspec means when used like this:

$ git diff :(size>1M)
$ git log --follow :(size>1M)

Which side of comparison does the "size" thing apply?  Either, both,
randomly?  More importantly, what use case of users do these
commands serve?

That is why I said that pathspec should never consider anything but
the pathname string you see.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] submodule groups

[Junio C Hamano]

Stefan Beller  writes:

> So I wonder if we rather want to extend the pathspec magic to
> include properties of blobs (i.e. submodules):
>
> git  . :(sub-label:label-sub0) :(exclude)*0
>
> would look much more powerful too me. Properties of blobs
> may also be interesting for otherwise. Imagine looking for huge files
> (in a bare repo, so you have to use Git and not your shell tools):
>
>   git ls-files . :(file-size:>1024k)

I somehow do not think this is a way normal people (read: end users)
would want to interact with Git.  Pathspec is about "paths" and
various ways to match them.  It is not about contents that happens
to be currently named by that path.  Don't tie types or sizes to it.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] am: plug FILE * leak in split_mail_conv()

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/am.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index f1a84c6..a373928 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -761,9 +761,11 @@ static int split_mail_conv(mail_conv_fn fn, struct 
am_state *state,
mail = mkpath("%s/%0*d", state->dir, state->prec, i + 1);
 
out = fopen(mail, "w");
-   if (!out)
+   if (!out) {
+   fclose(in);
return error(_("could not open '%s' for writing: %s"),
mail, strerror(errno));
+   }
 
ret = fn(out, in, keep_cr);
 
-- 
2.8.2-679-g91c6421

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] am: plug small memory leak when split_mail_stgit_series() fails

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 builtin/am.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index ec75906..f1a84c6 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -842,9 +842,11 @@ static int split_mail_stgit_series(struct am_state *state, 
const char **paths,
series_dir = dirname(series_dir_buf);
 
fp = fopen(*paths, "r");
-   if (!fp)
+   if (!fp) {
+   free(series_dir_buf);
return error(_("could not open '%s' for reading: %s"), *paths,
strerror(errno));
+   }
 
while (!strbuf_getline(, fp, '\n')) {
if (*sb.buf == '#')
-- 
2.8.2-679-g91c6421

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] rerere: plug memory leaks upon "rerere forget" failure

[Junio C Hamano]

Signed-off-by: Junio C Hamano 
---
 rerere.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/rerere.c b/rerere.c
index 1693866..a804171 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1052,8 +1052,8 @@ static int rerere_forget_one_path(const char *path, 
struct string_list *rr)
handle_cache(path, sha1, rerere_path(id, "thisimage"));
if (read_mmfile(, rerere_path(id, "thisimage"))) {
free(cur.ptr);
-   return error("Failed to update conflicted state in 
'%s'",
-path);
+   error("Failed to update conflicted state in '%s'", 
path);
+   goto fail_exit;
}
cleanly_resolved = !try_merge(id, path, , );
free(result.ptr);
@@ -1062,14 +1062,19 @@ static int rerere_forget_one_path(const char *path, 
struct string_list *rr)
break;
}
 
-   if (id->collection->status_nr <= id->variant)
-   return error("no remembered resolution for '%s'", path);
+   if (id->collection->status_nr <= id->variant) {
+   error("no remembered resolution for '%s'", path);
+   goto fail_exit;
+   }
 
filename = rerere_path(id, "postimage");
-   if (unlink(filename))
-   return (errno == ENOENT
-   ? error("no remembered resolution for %s", path)
-   : error("cannot unlink %s: %s", filename, 
strerror(errno)));
+   if (unlink(filename)) {
+   if (errno == ENOENT)
+   error("no remembered resolution for %s", path);
+   else
+   error("cannot unlink %s: %s", filename, 
strerror(errno));
+   goto fail_exit;
+   };
 
/*
 * Update the preimage so that the user can resolve the
@@ -1088,6 +1093,10 @@ static int rerere_forget_one_path(const char *path, 
struct string_list *rr)
item->util = id;
fprintf(stderr, "Forgot resolution for %s\n", path);
return 0;
+
+fail_exit:
+   free(id);
+   return -1;
 }
 
 int rerere_forget(struct pathspec *pathspec)
-- 
2.8.2-679-g91c6421

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] submodule groups

[Stefan Beller]

On Tue, May 10, 2016 at 7:08 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> I started from scratch as I think there were some sharp edges in the design.
>> My thinking shifted from "submodule groups" towards "actually it's just an
>> enhanced pathspec, called submodulespec".
>
> Except for minor things I mentioned separately, overall, this seems
> quite cleanly done.

I disagree (now).

I started documenting the  as an extension of
the pathspecs. While I thought the logical OR was the right way to go,
I think it is wrong now. So there is stuff in tests like:

git submodule--helper matches-submodulespec sub0 ./.
./:(exclude)*0 *label-sub0

which should test if the first argument (sub0) matches the submodulespec
which follows. And it matches sub0 by matching the label, although
we told it to ignore anything ending in 0

So I wonder if we rather want to extend the pathspec magic to
include properties of blobs (i.e. submodules):

git  . :(sub-label:label-sub0) :(exclude)*0

would look much more powerful too me. Properties of blobs
may also be interesting for otherwise. Imagine looking for huge files
(in a bare repo, so you have to use Git and not your shell tools):

  git ls-files . :(file-size:>1024k)

>
> Looks very promising.
>

Thanks for the encouragement!

Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

What's cooking in git.git (May 2016, #04; Wed, 11)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

The 'master' branch now has the eleventh batch of topics of this
cycle.  On the 'maint' front, 2.8.2 is out and fixes that have been
in 'master' accumulates on it for 2.8.3.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* ew/doc-split-pack-disables-bitmap (2016-04-28) 1 commit
  (merged to 'next' on 2016-05-06 at 6343d1e)
 + pack-objects: warn on split packs disabling bitmaps

 Doc update.


* ew/normal-to-e (2016-05-02) 1 commit
  (merged to 'next' on 2016-05-06 at 65a2c52)
 + .mailmap: update to my shorter email address


* js/close-packs-before-gc (2016-05-02) 1 commit
  (merged to 'next' on 2016-05-06 at bfd39bf)
 + t5510: run auto-gc in the foreground


* ls/p4-lfs (2016-04-28) 3 commits
  (merged to 'next' on 2016-05-06 at 3e1354d)
 + git-p4: fix Git LFS pointer parsing
 + travis-ci: express Linux/OS X dependency versions more clearly
 + travis-ci: update Git-LFS and P4 to the latest version

 Recent update to Git LFS broke "git p4" by changing the output from
 its "lfs pointer" subcommand.


* ls/travis-submitting-patches (2016-05-02) 1 commit
  (merged to 'next' on 2016-05-06 at 467930e)
 + Documentation: add setup instructions for Travis CI


* rn/glossary-typofix (2016-05-02) 1 commit
  (merged to 'next' on 2016-05-06 at 1e73e76)
 + Documentation: fix typo 'In such these cases'


* sb/clean-test-fix (2016-05-03) 1 commit
  (merged to 'next' on 2016-05-06 at d80c9c6)
 + t7300: mark test with SANITY


* sb/misc-cleanups (2016-04-28) 2 commits
  (merged to 'next' on 2016-05-06 at 87bc8a5)
 + submodule-config: don't shadow `cache`
 + config.c: drop local variable


* sk/gitweb-highlight-encoding (2016-05-03) 1 commit
  (merged to 'next' on 2016-05-06 at 441302c)
 + gitweb: apply fallback encoding before highlight

 Some multi-byte encoding can have a backslash byte as a later part
 of one letter, which would confuse "highlight" filter used in
 gitweb.

--
[New Topics]

* es/t1500-modernize (2016-05-10) 7 commits
 - t1500: finish preparation upfront
 - t1500: be considerate to future potential tests
 - t1500: avoid setting environment variables outside of tests
 - t1500: avoid setting configuration options outside of tests
 - t1500: avoid changing working directory outside of tests
 - t1500: reduce dependence upon global state
 - t1500: test_rev_parse: facilitate future test enhancements

 test updates to make it more readable and maintainable.

 Will be rerolled.


* ls/travis-build-doc (2016-05-10) 1 commit
  (merged to 'next' on 2016-05-10 at 7f63497)
 + travis-ci: build documentation

 CI test was taught to build documentation pages.

 Will merge to 'master'.


* js/t3404-typofix (2016-05-10) 1 commit
  (merged to 'next' on 2016-05-10 at cbeabc0)
 + t3404: fix typo

 Will merge to 'master'.


* jk/rebase-interative-eval-fix (2016-05-10) 1 commit
  (merged to 'next' on 2016-05-11 at 4fdf387)
 + rebase--interactive: avoid empty list in shell for-loop

 Portability enhancement for "rebase -i" to help platforms whose
 shell does not like "for i in " (which is not POSIX-kosher).

 Will merge to 'master'.


* jk/test-send-sh-x-trace-elsewhere (2016-05-11) 1 commit
  (merged to 'next' on 2016-05-11 at 273a137)
 + test-lib: set BASH_XTRACEFD automatically

 Running tests with '-x' option to trace the individual command
 executions is a useful way to debug test scripts, but some tests
 that capture the standard error stream and check what the command
 said can be broken with the trace output mixed in.  When running
 our tests under "bash", however, we can redirect the trace output
 to another file descriptor to keep the standard error of programs
 being tested intact.

 Will merge to 'master'.


* js/perf-rebase-i (2016-05-11) 3 commits
 - Add a perf test for rebase -i
 - perf: make the tests work in worktrees
 - perf: let's disable symlinks when they are not available

 Add perf test for "rebase -i"


* nd/worktree-cleanup-post-head-protection (2016-05-10) 7 commits
 - worktree: simplify prefixing paths
 - worktree: avoid 0{40}, too many zeroes, hard to read
 - worktree.c: add clear_worktree()
 - worktree.c: use is_dot_or_dotdot()
 - git-worktree.txt: keep subcommand listing in alphabetical order
 - worktree.c: rewrite mark_current_worktree() to avoid strbuf
 - completion: support git-worktree
 (this branch uses nd/worktree-various-heads.)


* va/mailinfo-doc-typofix (2016-05-11) 1 commit
  (merged to 'next' on 2016-05-11 at 7180176)
 + Documentation/git-mailinfo: fix typo

 Typofix.

 Will merge to 

Re: [PATCH] diff: run arguments through precompose_argv

[Junio C Hamano]

Alexander Rinass  writes:

>> On 05 Apr 2016, at 21:15, Johannes Sixt  wrote:
>> 
>> Am 05.04.2016 um 19:09 schrieb Junio C Hamano:
 Thanks-to: Torsten Bögershausen 
>> 
>> I sense NFD disease: The combining diaresis should combine with the o, not 
>> the g. Here is a correct line to copy-and-paste if you like:
>> 
>> Thanks-to: Torsten Bögershausen 
>> 
>> -- Hannes
>
> Thanks for reviewing and catching the NFD encoding error.
>
> I will send in a patch v2 with the correct NFC encoding.
>
> Would you also like me to alter the commit message as mentioned by Junio?
>
> I could rewrite the sentence:
>
> “As a result, no diff is displayed when feeding such a file path to the
> diff command.”
>
> into simply saying:
>
> “As a result, no diff is displayed.”
>
> However, I don't read the original message as it would imply that only
> file paths are affected by the precompose_argv call. 
>
> Are there other suggestions on improving the commit message?

I think after this message there were a few suggestions, and then we
heard nothing.  Should we still be waiting for a response from you?

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] submodule-config: keep labels around

[Stefan Beller]

+Heiko

On Wed, May 11, 2016 at 2:28 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> On Tue, May 10, 2016 at 6:15 PM, Junio C Hamano  wrote:
>>> Stefan Beller  writes:
>>>
 @@ -199,6 +203,7 @@ static struct submodule 
 *lookup_or_create_by_name(struct submodule_cache *cache,
   submodule->update_strategy.command = NULL;
   submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
   submodule->ignore = NULL;
 + submodule->labels = NULL;
>>>
>>> Hmph, is there a reason to do this, instead of embedding an instance
>>> of "struct string_list" inside submodule structure?
>>>
>>> I am not yet claiming that embedding is better.  Just wondering if
>>> it makes it easier to handle initialization as seen in the hunk
>>> below, and also _clear() procedure.
>>
>> Thanks for pointing out that alternative.  That looks so much
>> better in this patch. Let's see how the follow up patches develop.
>> As we'd not check != NULL first, but check against the count of the
>> string list. (I expect no problems down that road though).
>
> I also wonder if we can say the same for the .ignore field, by the
> way, if we agree that it is a better direction to go.

I don't think this is a good idea, as it's just a string, like .{url,
name, path}

Instead of storing the string we could store an enum {UNTRACKED,
DIRTY, ALL, NONE, NOT_INIT} though. That would be better I guess.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Pre-Process Files for Commits and Pulls

[Junio C Hamano]

Brandon Teska  writes:

> 1. Person A works on (binary) file locally
> 2. Person A commits and pushes to the repo
> 3. Before the push, a script deconstructs the binary file into several
> text files
> 4. Those text files are pushed

A smudge/clean filter pair is how this is done, but you need to drop
"several text files" from the requirement.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation/git-mailinfo: fix typo

[Junio C Hamano]

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 24/25] worktree move: accept destination as directory

[Junio C Hamano]

Johannes Sixt  writes:

>> As this path is read from a file git itself creates, and if we know
>> that it will always contain forward slashes, then I agree that it
>> could be potentially confusing to later readers to see
>> git_find_last_dir_sep(). So, keeping it as-is seems correct.
>
> Please allow me to disagree. There should not be any assumption that a
> path uses forward slashes as directory separator, except when the path
> is
>
> - a pathspec
> - a ref
> - a path found in the object database including the index

I think standardising on one way for what we write out would give
less hassle to users.  The human end users should not be opening
these files in their editors and futzing with their contents, but
there are third-party tools and reimplementations of Git.  Forcing
them to be prepared for input with slashes and backslashes does not
make much sense to me.

Is there an upside for us to accept both slashes in this file?



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] submodule-config: keep labels around

[Junio C Hamano]

Stefan Beller  writes:

> On Tue, May 10, 2016 at 6:15 PM, Junio C Hamano  wrote:
>> Stefan Beller  writes:
>>
>>> @@ -199,6 +203,7 @@ static struct submodule 
>>> *lookup_or_create_by_name(struct submodule_cache *cache,
>>>   submodule->update_strategy.command = NULL;
>>>   submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
>>>   submodule->ignore = NULL;
>>> + submodule->labels = NULL;
>>
>> Hmph, is there a reason to do this, instead of embedding an instance
>> of "struct string_list" inside submodule structure?
>>
>> I am not yet claiming that embedding is better.  Just wondering if
>> it makes it easier to handle initialization as seen in the hunk
>> below, and also _clear() procedure.
>
> Thanks for pointing out that alternative.  That looks so much
> better in this patch. Let's see how the follow up patches develop.
> As we'd not check != NULL first, but check against the count of the
> string list. (I expect no problems down that road though).

I also wonder if we can say the same for the .ignore field, by the
way, if we agree that it is a better direction to go.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: wishlist; unify behavior while cloning non-bare repos over http to be in line with ssh/local

[Junio C Hamano]

Yaroslav Halchenko  writes:

> On Tue, 10 May 2016, Junio C Hamano wrote:
>> >> The necessary update to the client might be as simple as using
>> >> $GIVEN_URL/.git/ and attempting the request again after seeing the
>> >> probe for $GIVEN_URL/info/refs fails.
>
>> > Sure -- workarounds are possible,...
>
>> Just so that there is no misunderstanding, the above was not a
>> workaround but is an outline of an implementation of updated http
>> client shipped with Git.
>
> ah, sorry, I have indeed might have misread it.  So we are on the same
> page -- that is I saw also as the tentative implementation ;)

Good.  Now somebody who is interested in seeing that happen (when I
said "shipped with Git" above, I meant "shipped with possible future
Git") needs to find (or be) somebody to code that change ;-)

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] Add a perf test for rebase -i

[Junio C Hamano]

Johannes Schindelin  writes:

> diff --git a/t/perf/p3404-rebase-interactive.sh 
> b/t/perf/p3404-rebase-interactive.sh
> new file mode 100755
> index 000..382163c
> --- /dev/null
> +++ b/t/perf/p3404-rebase-interactive.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +
> +test_description='Tests rebase -i performance'
> +. ./perf-lib.sh
> +
> +test_perf_default_repo
> +
> +# This commit merges a sufficiently long topic branch for reasonable
> +# performance testing
> +branch_merge=ba5312d
> +export branch_merge

t/perf/README mentions the possibility to use your own repository as
a test data via GIT_PERF_REPO, but doing so would obviously break
this test.

I wonder if there is a way to say "running this perf script with
custom GIT_PERF_REPO is not supported" and error out.  That may
help other existing tests that (incorrectly) assume that their test
data is this project (if there is any).

> +
> +write_script swap-first-two.sh <<\EOF
> +case "$1" in
> +*/COMMIT_EDITMSG)
> + mv "$1" "$1".bak &&
> + sed -e '1{h;d}' -e 2G <"$1".bak >"$1"
> + ;;
> +esac
> +EOF
> +
> +test_expect_success 'setup' '
> + git config core.editor "\"$PWD"/swap-first-two.sh\" &&
> + git checkout -f $branch_merge^2
> +'
> +
> +test_perf 'rebase -i' '
> + git rebase -i $branch_merge^
> +'
> +
> +test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Pre-Process Files for Commits and Pulls

[Brandon Teska]

Hi everyone,

I have an unusual question. I'm curious if git can pre-process files
before pushing them to a remote repo and then reprocess them on pulls.
Basically, I'm trying to work collaboratively with a few colleagues on
a project using another software program. I've decoded the file we've
been working on, so that we have the "source" that would be well
managed by git. However, I need this to be accessible to laypeople so
I need my workflow to look like this:

1. Person A works on (binary) file locally
2. Person A commits and pushes to the repo
3. Before the push, a script deconstructs the binary file into several
text files
4. Those text files are pushed

Similarly, when Person B pulls from the repo, this is what I need to happen:

1. Person B pulls
2. Before sending the pull, git calls a script that repackages the
text files into a "binary" files that the software can use.
3. Person B can now update the file as he wishes

So, basically I am curious if git can store a different "form" of the
file(s) that what are actually worked on. Is this possible? (I'd like
to avoid running client side scripts if at all possible, but would be
willing if that's a possibility.)

Thanks!

Brandon
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Documentation/git-mailinfo: fix typo

[Vasco Almeida]

Signed-off-by: Vasco Almeida 
---
 Documentation/git-mailinfo.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-mailinfo.txt b/Documentation/git-mailinfo.txt
index 0947084..3bbc731 100644
--- a/Documentation/git-mailinfo.txt
+++ b/Documentation/git-mailinfo.txt
@@ -85,7 +85,7 @@ with comments and suggestions on the message you are 
responding to, and to
 conclude it with a patch submission, separating the discussion and the
 beginning of the proposed commit log message with a scissors line.
 +
-This can enabled by default with the configuration option mailinfo.scissors.
+This can be enabled by default with the configuration option mailinfo.scissors.
 
 --no-scissors::
Ignore scissors lines. Useful for overriding mailinfo.scissors settings.
-- 
2.7.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: t5551 hangs ?

[Torsten Bögershausen]

On 11.05.16 19:31, Jeff King wrote:
> On Wed, May 11, 2016 at 07:13:56PM +0200, Torsten Bögershausen wrote:
> 
>> On 10.05.16 09:08, Johannes Schindelin wrote:
>> - I'm not sure, if this is the right thread to report on -
>>
>> It seems as if t5551 is hanging ?
>> This is the last line from the log:
>> ok 25 - large fetch-pack requests can be split across POSTs
> 
> Are you running the tests with "--long" or GIT_TEST_LONG in the
> environment? The next line should show it skipping test 26 unless one of
> those is set.
Yes
> 
> If you are, can you confirm that it's actually hanging, and not just
> slow? On my system, test 26 takes about a minute to run (which is why we
> don't do it by default).
Nearly sure. After 10 minutes, the test was still running.

Yesterday another machine was running even longer.

Any tips, how to debug, are welcome.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] gitk: Add Portuguese translation

[Vasco Almeida]

Signed-off-by: Vasco Almeida 
---

 Oops, fix typo.
  #: gitk:3081
  #, tcl-format
  msgid "<%s-Down>\tScroll commit list down one line"
  msgstr [-"<%s-Baixo>\tDescolar-]{+"<%s-Baixo>\tDeslocar+} a lista de commits 
uma linha para baixo"

 po/pt_pt.po | 1376 +++
 1 file changed, 1376 insertions(+)
 create mode 100644 po/pt_pt.po

diff --git a/po/pt_pt.po b/po/pt_pt.po
new file mode 100644
index 000..a018ef9
--- /dev/null
+++ b/po/pt_pt.po
@@ -0,0 +1,1376 @@
+# Portuguese translations for gitk package.
+# Copyright (C) 2016 Paul Mackerras
+# This file is distributed under the same license as the gitk package.
+# Vasco Almeida , 2016.
+msgid ""
+msgstr ""
+"Project-Id-Version: gitk\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2016-04-15 16:52+\n"
+"PO-Revision-Date: 2016-05-06 15:35+\n"
+"Last-Translator: Vasco Almeida \n"
+"Language-Team: Portuguese\n"
+"Language: pt\n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+"Plural-Forms: nplurals=2; plural=(n != 1);\n"
+"X-Generator: Virtaal 0.7.1\n"
+
+#: gitk:140
+msgid "Couldn't get list of unmerged files:"
+msgstr "Não foi possível obter lista de ficheiros não integrados:"
+
+#: gitk:212 gitk:2399
+msgid "Color words"
+msgstr "Colorir palavras"
+
+#: gitk:217 gitk:2399 gitk:8239 gitk:8272
+msgid "Markup words"
+msgstr "Marcar palavras"
+
+#: gitk:324
+msgid "Error parsing revisions:"
+msgstr "Erro ao analisar revisões:"
+
+#: gitk:380
+msgid "Error executing --argscmd command:"
+msgstr "Erro ao executar o comando de --argscmd:"
+
+#: gitk:393
+msgid "No files selected: --merge specified but no files are unmerged."
+msgstr ""
+"Nenhum ficheiro selecionado: --merge especificado mas não há ficheiros por "
+"integrar."
+
+#: gitk:396
+msgid ""
+"No files selected: --merge specified but no unmerged files are within file "
+"limit."
+msgstr ""
+"Nenhum ficheiro selecionado: --merge especificado mas não há ficheiros por "
+"integrar ao nível de ficheiro."
+
+#: gitk:418 gitk:566
+msgid "Error executing git log:"
+msgstr "Erro ao executar git log:"
+
+#: gitk:436 gitk:582
+msgid "Reading"
+msgstr "A ler"
+
+#: gitk:496 gitk:4544
+msgid "Reading commits..."
+msgstr "A ler commits..."
+
+#: gitk:499 gitk:1637 gitk:4547
+msgid "No commits selected"
+msgstr "Nenhum commit selecionado"
+
+#: gitk:1445 gitk:4064 gitk:12469
+msgid "Command line"
+msgstr "Linha de comandos"
+
+#: gitk:1511
+msgid "Can't parse git log output:"
+msgstr "Não é possível analisar a saída de git log:"
+
+#: gitk:1740
+msgid "No commit information available"
+msgstr "Não há informação disponível sobre o commit"
+
+#: gitk:1903 gitk:1932 gitk:4334 gitk:9702 gitk:11274 gitk:11554
+msgid "OK"
+msgstr "OK"
+
+#: gitk:1934 gitk:4336 gitk:9215 gitk:9294 gitk:9424 gitk:9473 gitk:9704
+#: gitk:11275 gitk:11555
+msgid "Cancel"
+msgstr "Cancelar"
+
+#: gitk:2083
+msgid ""
+msgstr "At"
+
+#: gitk:2084
+msgid ""
+msgstr ""
+
+#: gitk:2085
+msgid "Reread re"
+msgstr "Reler reências"
+
+#: gitk:2086
+msgid " references"
+msgstr " referências"
+
+#: gitk:2088
+msgid "Start git "
+msgstr "Iniciar git "
+
+#: gitk:2090
+msgid ""
+msgstr ""
+
+#: gitk:2082
+msgid ""
+msgstr ""
+
+#: gitk:2094
+msgid ""
+msgstr "ências"
+
+#: gitk:2093
+msgid ""
+msgstr ""
+
+#: gitk:2098
+msgid " view..."
+msgstr " vista..."
+
+#: gitk:2099
+msgid " view..."
+msgstr " vista..."
+
+#: gitk:2100
+msgid " view"
+msgstr "Elimina vista"
+
+#: gitk:2102
+msgid " files"
+msgstr " os ficheiros"
+
+#: gitk:2097
+msgid ""
+msgstr ""
+
+#: gitk:2107 gitk:2117
+msgid " gitk"
+msgstr " gitk"
+
+#: gitk:2108 gitk:2122
+msgid " bindings"
+msgstr ""
+
+#: gitk:2106 gitk:2121
+msgid ""
+msgstr ""
+
+#: gitk:2199 gitk:8671
+msgid "SHA1 ID:"
+msgstr "ID SHA1:"
+
+#: gitk:2243
+msgid "Row"
+msgstr "Linha"
+
+#: gitk:2281
+msgid "Find"
+msgstr "Procurar"
+
+#: gitk:2309
+msgid "commit"
+msgstr "commit"
+
+#: gitk:2313 gitk:2315 gitk:4706 gitk:4729 gitk:4753 gitk:6774 gitk:6846
+#: gitk:6931
+msgid "containing:"
+msgstr "contendo:"
+
+#: gitk:2316 gitk:3545 gitk:3550 gitk:4782
+msgid "touching paths:"
+msgstr "altera os caminhos:"
+
+#: gitk:2317 gitk:4796
+msgid "adding/removing string:"
+msgstr "adiciona/remove a cadeia:"
+
+#: gitk:2318 gitk:4798
+msgid "changing lines matching:"
+msgstr "altera linhas com:"
+
+#: gitk:2327 gitk:2329 gitk:4785
+msgid "Exact"
+msgstr "Exato"
+
+#: gitk:2329 gitk:4873 gitk:6742
+msgid "IgnCase"
+msgstr "IgnMaiúsculas"
+
+#: gitk:2329 gitk:4755 gitk:4871 gitk:6738
+msgid "Regexp"
+msgstr "Expr. regular"
+
+#: gitk:2331 gitk:2332 gitk:4893 gitk:4923 gitk:4930 gitk:6867 gitk:6935
+msgid "All fields"
+msgstr "Todos os campos"
+
+#: gitk:2332 gitk:4890 gitk:4923 gitk:6805
+msgid "Headline"
+msgstr "Cabeçalho"
+
+#: gitk:2333 gitk:4890 gitk:6805 gitk:6935 gitk:7408
+msgid "Comments"
+msgstr "Comentários"
+
+#: gitk:2333 

Re: [PATCH 24/25] worktree move: accept destination as directory

[Johannes Sixt]

Am 11.05.2016 um 19:32 schrieb Eric Sunshine:

On Wed, May 11, 2016 at 9:34 AM, Duy Nguyen  wrote:

On Wed, May 11, 2016 at 11:43 AM, Eric Sunshine  wrote:

On Wed, Apr 13, 2016 at 9:15 AM, Nguyễn Thái Ngọc Duy  wrote:

+   if (is_directory(dst.buf)) {
+   const char *sep = strrchr(wt->path, '/');


Does this need to take Windows into account?


wt->path comes from $GIT_DIR/worktrees/xxx/gitdir, which normally uses
forward slashes, so we should be safe. We already rely on forward
slashes in get_linked_worktree()


Perhaps git_find_last_dir_sep()?


But this is probably a good thing to do anyway, to be more robust in
future. But it could confuse the reader later on why it's necessary
when backward slashes can't exist in wt->path. I don't know. Maybe
just have a comment that backward slashes can't never appear here?


As this path is read from a file git itself creates, and if we know
that it will always contain forward slashes, then I agree that it
could be potentially confusing to later readers to see
git_find_last_dir_sep(). So, keeping it as-is seems correct.


Please allow me to disagree. There should not be any assumption that a 
path uses forward slashes as directory separator, except when the path is


- a pathspec
- a ref
- a path found in the object database including the index

In particular, everything concerning paths in the file system (including 
paths pointing to Git's own files) must not assume forward slashes.


We do convert backslashes to forward slashes in a number of places, but 
this is only for cosmetic reasons, not to maintain an invariant.



If we look at fspathcmp() as a function which performs whatever magic
is needed to make comparisons work on a platform/filesystem, then it
might indeed be reasonable to enhance it to recognize '/' and '\' as
equivalent (with possible caveats for Windows corner cases).


That sounds reasonable.

-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] perf: make the tests work in worktrees

[Eric Sunshine]

On Wed, May 11, 2016 at 4:42 AM, Johannes Schindelin
 wrote:
> This patch makes perf-lib.sh more robust so that it can run correctly
> even inside a worktree. For example, it assumed that $GIT_DIR/objects is
> the objects directory (which is not the case for worktrees) and it used
> the commondir file verbatim, even if it contained a relative path.
>
> Furthermore, the setup code expected `git rev-parse --git-dir` to spit
> out a relative path, which is also not true for worktrees. Let's just
> change the code to accept both relative and absolute paths, by avoiding
> the `cd` into the copied working directory.
>
> Signed-off-by: Johannes Schindelin 
> ---
> diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
> @@ -80,22 +80,22 @@ test_perf_create_repo_from () {
> -   source_git=$source/$(cd "$source" && git rev-parse --git-dir)
> +   source_git="$(cd "$source" && git rev-parse --git-dir)"
> +   objects_dir="$(cd "$source" && git rev-parse --git-path objects)"

Would it be out of the scope of this patch to simplify these by using -C?

source_git=$(git -C "$source" rev-parse --git-dir)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] submodule-config: keep labels around

[Stefan Beller]

On Tue, May 10, 2016 at 6:15 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct 
>> submodule_cache *cache,
>>   submodule->update_strategy.command = NULL;
>>   submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
>>   submodule->ignore = NULL;
>> + submodule->labels = NULL;
>
> Hmph, is there a reason to do this, instead of embedding an instance
> of "struct string_list" inside submodule structure?
>
> I am not yet claiming that embedding is better.  Just wondering if
> it makes it easier to handle initialization as seen in the hunk
> below, and also _clear() procedure.

Thanks for pointing out that alternative.  That looks so much
better in this patch. Let's see how the follow up patches develop.
As we'd not check != NULL first, but check against the count of the
string list. (I expect no problems down that road though).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 24/25] worktree move: accept destination as directory

[Eric Sunshine]

On Wed, May 11, 2016 at 9:34 AM, Duy Nguyen  wrote:
> On Wed, May 11, 2016 at 11:43 AM, Eric Sunshine  
> wrote:
>> On Wed, Apr 13, 2016 at 9:15 AM, Nguyễn Thái Ngọc Duy  
>> wrote:
>>> +   if (is_directory(dst.buf)) {
>>> +   const char *sep = strrchr(wt->path, '/');
>>
>> Does this need to take Windows into account?
>
> wt->path comes from $GIT_DIR/worktrees/xxx/gitdir, which normally uses
> forward slashes, so we should be safe. We already rely on forward
> slashes in get_linked_worktree()
>
>> Perhaps git_find_last_dir_sep()?
>
> But this is probably a good thing to do anyway, to be more robust in
> future. But it could confuse the reader later on why it's necessary
> when backward slashes can't exist in wt->path. I don't know. Maybe
> just have a comment that backward slashes can't never appear here?

As this path is read from a file git itself creates, and if we know
that it will always contain forward slashes, then I agree that it
could be potentially confusing to later readers to see
git_find_last_dir_sep(). So, keeping it as-is seems correct.

Not sure if it needs a comment. I reviewed this rather quickly since
(I think) you plan on re-rolling it and I'm far behind on my reviews.
Consequently, I didn't check the existing code, and reviewed only
within the context of the patch itself. If the end result is that it's
clear from reading the code that it will always contain forward
slashes, then a comment would be redundant. You could perhaps mention
in the commit message that the slash will always be forward, which
should satisfy future reviewers and readers of the code once its in
the tree.

> There is also a potential problem with find_worktree_by_path(). I was
> counting on real_path() to normalize paths and could simply do
> strcmp_icase (or its new name, fspathcmp). But real_path() does not
> seem to convert unify slashes. I will need to have a closer look at
> this. Hopefully prefix_filename() already makes sure everything uses
> forward slashes. Or maybe we could improve fspathcmp to see '/' and
> '\' the same thing on Windows.

If we look at fspathcmp() as a function which performs whatever magic
is needed to make comparisons work on a platform/filesystem, then it
might indeed be reasonable to enhance it to recognize '/' and '\' as
equivalent (with possible caveats for Windows corner cases).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: t5551 hangs ?

[Jeff King]

On Wed, May 11, 2016 at 07:13:56PM +0200, Torsten Bögershausen wrote:

> On 10.05.16 09:08, Johannes Schindelin wrote:
> - I'm not sure, if this is the right thread to report on -
> 
> It seems as if t5551 is hanging ?
> This is the last line from the log:
> ok 25 - large fetch-pack requests can be split across POSTs

Are you running the tests with "--long" or GIT_TEST_LONG in the
environment? The next line should show it skipping test 26 unless one of
those is set.

If you are, can you confirm that it's actually hanging, and not just
slow? On my system, test 26 takes about a minute to run (which is why we
don't do it by default).

> I have 7 such processes running:
> /trash directory.t5551-http-fetch-smart/httpd -f
> /Users/tb/projects/git/git.pu/t/lib-httpd/apache.conf -DDarwin -c Listen
> 127.0.0.1:5551 -k start

That's normal while the test is running; apache pre-forks a bunch of
worker threads.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] submodule add: label submodules if asked to

[Stefan Beller]

>> +cat >expect <<-EOF
>> +labelA
>> +labelB
>> +EOF
>> +
>> +test_expect_success 'submodule add records multiple labels' '
>
> The existing tests in this file may be littered with this bad
> construct, but please do not add more example of running things
> outside of test_expect_{success,failure} block unless there is a
> good reason to do so.

I thought that was the standard for tests as I have seen
them quite a few times. Looking for those "cat >expect ..." constructs
more closely they are often found inside tests as well. Makes sense
if the expectation is used for only one test.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

t5551 hangs ?

[Torsten Bögershausen]

On 10.05.16 09:08, Johannes Schindelin wrote:
- I'm not sure, if this is the right thread to report on -

It seems as if t5551 is hanging ?
This is the last line from the log:
ok 25 - large fetch-pack requests can be split across POSTs

I have 7 such processes running:
/trash directory.t5551-http-fetch-smart/httpd -f
/Users/tb/projects/git/git.pu/t/lib-httpd/apache.conf -DDarwin -c Listen
127.0.0.1:5551 -k start

This happens both under Mac OS X and Debian.

Does anybody have the same hanging ?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Re: [RFD/PATCH] submodule doc: describe where we can configure them

[Heiko Voigt]

On Mon, May 09, 2016 at 10:32:50AM -0700, Stefan Beller wrote:
> >> Here is what I imagine
> >> When B mirrors from A, B sets up this special ref for its repository,
> >> e.g. refs/meta/submodule-B and have a symbolic ref pointing at that.
> >> (e.g. SUBMODULE_CONFIG pointing at refs/meta/submodule-B,
> >> which has a worktree which contains a .gitmodules files which
> >> sets up
> >>   "submodule.baz.url = http://B/baz;
> >>   "submodule.relativeBase = http://A;
> >>
> >> That way anyone cloning from B would get
> >> the superproject and the submodule baz from B while the
> >> rest of the submodules are found at A.
> >
> > This sounds sensible. But my imagination of a conflict was in a
> > different way. E.g. project A has a submodule B. And now A has a remote
> > 1 where you publish and maybe another remote 2 where someone else (a
> > colleague?) publishes. Which configuration do you use? Here the two
> > remotes are independent instead of subsequent forks. In this case my
> > solution would be to use the configuration branch from 1 for B when
> > interacting with 1. I do not have completely checked whether we always
> > have a remote at hand for such a resolution.
> 
> I think it is the responsibility of the pusher to make sure the
> configuration is sane.
> So if I were to push to remote 2 and you push to remote 1, we'd both configure
> the special branch of our superprojects for these remotes for that submodule.
> 
> If the superproject has relative urls for the submodule, all we had to do was
> unset (or overwrite) the submodule.baseConfig.

What if (because we work together) you and me have both remotes in our
local repository. We only push to our private remotes but fetch from
both. Since we work together we also forked the same submodule B and
have different URL configurations for it. I push to B1 and you to B2.
Now we both have two special branches (one from B1 and one from B2) in
our local repositories, since on either of our private remotes there is
one special branch.

Which values are valid now? I see you are advocating for a symbolic ref
SUBMODULE_CONFIG that points to a single special branch in charge, but
maybe we can avoid that. In this case there actually is no real
conflict, since we can just add both remotes B1, B2 to the submodule B.
Which one is used is a choice of the user during push.

For submodule.relativeBase we could try a similar solution and just add
all remotes that can be constructed with the different configurations.
Probably under the same name as in the superproject.

So if we limit ourselves to only allow URL'ish (actually remote'ish is
probably a better term) we can actually avoid conflict resolution and
just add/use them all. If we limit ourselves to the fork use case and my
hypothesis that we only need to allow remote'ish values in these special
branches for it is true, we can actually keep it quite simple and have
no conflict resolution at all I think (and realize now).

What do you think?

> >> When C mirrors from A, they add another branch  refs/meta/submodule-C,
> >> which can either be a fork of refs/meta/submodule-B with some changes on
> >> top of it or it can add a reference to refs/meta/submodule-B, i.e. the
> >> configuration
> >> would be:
> >>
> >>   "submodule.baseConfig = refs/meta/submodule-B"
> >>   "submodule.foo.url = ssh://C/foo"
> >>
> >> and SUBMODULE_CONFIG would point at refs/meta/submodule-C.
> >>
> >> When cloning from C, the user would get
> >>
> >>  * the superproject from C
> >>  * submodule foo from C
> >>  * submodule baz from B
> >>  * all other submodules from A
> >>
> >> By the inheriting property of the branch of B there are no conflicting 
> >> values.
> >> C could just overwrite submodule.baseConfig for example.
> >
> > So that means in the default case we create a chain of all previous
> > forks embedded in repository database.
> 
> Not necessarily. I was just pointing out that this was possible. The
> intermediate
> party could decide that their upstream is too unreliable and not point
> to their upstream.
> 
> This would incur the cost of having to clone all submodules and
> overwriting the absolute
> urls. For the relative URLs this would just work as of now.
> 
> All I wanted with that example is to offer the flexibility to not have
> to clone all the
> submodule, but I can fork a mega-project with 100s of submodules and maybe
> just fiddle with one of them and then publish that.

Do you mean 'not having to fork all the submodules' here? Since 'without
cloning' is already possible, no?

I am assuming you meant fork. So submodule.relativeBase is meant to
solve that right? You set it and all relative submodule URLs that are
not configured otherwise relate to it.

My point was about the chaining with submodule.baseConfig. That is not
necessary to support partial forks of just a few submodules.

Actually while thinking about submodule.relativeBase now, I found it
might be nice to extend it a little. Imagine someone wants fork a 

Re: [RFD/PATCH] submodule doc: describe where we can configure them

[Heiko Voigt]

On Mon, May 09, 2016 at 09:19:44AM -0700, Junio C Hamano wrote:
> Heiko Voigt  writes:
> 
> >> > - When upstream adds a new submodule, I have to do the same manual
> >> >   work to change the options for that new submodule.
> >> 
> >> Because a new module is not automatically "init"ed by default?
> >> 
> >> Isn't "config only" vs "config with gitmodules fallback" orthogonal
> >> to that issue?
> >
> > What do you mean with "orthogonal to that issue"? AFAICS a gitmodule
> > fallback does not have that issue.
> >
> > Actually I would see it more like:
> > .gitmodule is the default and .git/config a possibility to override.
> 
> The way I read Jonathan's "I have to do the same manual..." above is:
> 
>   Back when I cloned, the upstream had one submodule A.  I didn't like
>   some aspect of the configuration for that submodule so I did a
>   customization in [submodule "A"] section of .git/config for it.
> 
>   Now the upstream added another submodule B.  I want a tweak similar
>   to what I did to A applied to this one, but that would mean I need
>   to edit the entry in .git/config copied by "init" from .gitmodules.
> 
> I do not see how difference between ".git/config is the only source
> of truth" or ".git/config overrides what is in .gitmodules" would
> matter to the above scenario.

I see with that explanation your comment makes sense to me. So what we
are here talking about is the wish to configure some general user set
settings that are applied to a group of/all submodules.

Thinking about it: Maybe sticking configurations to the submodule
groups, which Stefan Beller introduced in a different topic, could be a
direction we can go for such needs.

Cheers Heiko
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[ANNOUNCE] Git Rev News edition 15

[Christian Couder]

Hi everyone,

I'm happy announce that the 15th edition of Git Rev News is now published:

http://git.github.io/rev_news/2016/05/11/edition-15/

Thanks a lot to all the contributors and helpers, especially David Turner!

Enjoy,
Christian, Thomas and Nicola.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] Windows: only add a no-op pthread_sigmask() when needed

[Johannes Schindelin]

In f924b52 (Windows: add pthread_sigmask() that does nothing,
2016-05-01), we introduced a no-op for Windows. However, this breaks
building Git in Git for Windows' SDK because pthread_sigmask() is
already a no-op there, #define'd in the pthread_signal.h header in
/mingw64/x86_64-w64-mingw32/include/.

Let's wrap the definition of pthread_sigmask() in a guard that skips
it when compiling with MinGW-w64' headers.

Signed-off-by: Johannes Schindelin 
---
Published-As: https://github.com/dscho/git/releases/tag/mingw-sigmask-v2
 compat/win32/pthread.h | 2 ++
 1 file changed, 2 insertions(+)
Interdiff vs v1:

 diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
 index 8df702c..1c16408 100644
 --- a/compat/win32/pthread.h
 +++ b/compat/win32/pthread.h
 @@ -104,7 +104,7 @@ static inline void *pthread_getspecific(pthread_key_t key)
return TlsGetValue(key);
  }
  
 -#ifndef pthread_sigmask
 +#ifndef __MINGW64_VERSION_MAJOR
  static inline int pthread_sigmask(int how, const sigset_t *set, sigset_t 
*oset)
  {
return 0;


diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index d336451..1c16408 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -104,9 +104,11 @@ static inline void *pthread_getspecific(pthread_key_t key)
return TlsGetValue(key);
 }
 
+#ifndef __MINGW64_VERSION_MAJOR
 static inline int pthread_sigmask(int how, const sigset_t *set, sigset_t *oset)
 {
return 0;
 }
+#endif
 
 #endif /* PTHREAD_H */
-- 
2.8.2.465.gb077790
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Windows: only add a no-op pthread_sigmask() when needed

[Johannes Schindelin]

Hi Junio,

On Tue, 10 May 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > In f924b52 (Windows: add pthread_sigmask() that does nothing,
> > 2016-05-01), we introduced a no-op for Windows. However, this breaks
> > building Git in Git for Windows' SDK because pthread_sigmask() is
> > already a no-op there, #define'd in the pthread_signal.h header in
> > /mingw64/x86_64-w64-mingw32/include/.
> >
> > Let's guard the definition of pthread_sigmask() in #ifndef...#endif to
> > make the code compile both with modern MinGW-w64 as well as with the
> > previously common MinGW headers.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> >
> > This patch is obviously based on 'next' (because 'master' does not
> > have the referenced commit yet).
> 
> One thing that makes me wonder is what would happen when
> /mingw64/x86_64-w64-mingw32/include/pthread_signal.h changes its
> mind and uses "static inline" instead of "#define".  How much
> control does Git-for-Windows folks have over that header file?

We have no control over this, it is defined by one of the MSYS2 packages.
(I think those headers come directly from the MinGW-w64 GCC project.)

I can think of two different ways to resolve this, would you please
indicate your preference?

1. fix the non-POSIX-ness:

#ifdef pthread_sigmask
#undef pthread_sigmask
#endif

   or even shorter:

#undef pthread_sigmask

2. just go with whatever MSYS2 provides:

#ifndef __MINGW64_VERSION_MAJOR
[... define the no-op ...]
#endif

> Also, could you explain "However" part a bit?  Obviously in _some_
> environment other than "Git for Windows' SDK", the previous patch
> helped you compile.

Yes, Hannes uses his own MSys environment. (Which is different from
everything *I* have, I think, it's not even msysGit.)

> What I am trying to get at is:
> 
>  (1) If the answer is "we have total control", then I am perfectly
>  fine with using "#ifdef pthread_sigmask" here.
> 
>  (2) If not, i.e. "they can change the implementation to 'static
>  inline' themselves", then I do not think it is prudent to use
>  "#ifndef pthread_sigmask" as the conditional here--using a
>  symbol that lets you check for that "other" environment and
>  doing "#ifdef THAT_OTHER_ONE_THAT_LACKS_SIGMASK" would be
>  safer.

So I guess that you're preferring my 2. above. Going on that assumption, I
will send out another iteration.

> Also is
> https://lists.gnu.org/archive/html/bug-gnulib/2015-04/msg00068.html
> relevant?  Does /mingw64/x86_64-w64-mingw32/include/ implement "macro
> only without function"?

Yes, it has that problem. Do we care, really?

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Bug: `git-filter-branch −−subdirectory−filter` fails on Darwin, others?

[Geoff Nixon]

  On Wed, 11 May 2016 07:08:02 -0700 Jeff King  wrote  
 > On Wed, May 11, 2016 at 06:47:20AM -0700, Geoff Nixon wrote: 
 >  
 > >the last line before it fails appears to be 
 > > `git rev-parse --no-flags --revs-only --symbolic-full-name 
 > > --default HEAD 
 \210\222 \210\222subdirectory \210\222filter` 
 > > (including the octal sequences and bad-unicode character, those 
 > > are not email artifacts) 
 >  
 > Are you sure that you are invoking filter-branch with regular ascii 
 > dashes, and not Unicode "minus-sign" (U+2212)? 
 >  
 > I seem to recall this coming up once before related to OS X, but I can't 
 > seem to find it in the archive. And I don't recall if it was related to 
 > the terminal, a keyboard setting, or something else. 

That was it. I'm an idiot.

I sometimes use `man -t` to generate postscript for lengthy man pages so I can 
"page through" outside my terminal.
It appears that for reasons unknown this converts all "-"s to "−"s. I must have 
copy-pasted the example.
Then I kept using my shell history completion, never typing it out again.

Sorry for wasting your time, thanks for helping me figure this out.

-Geoff

 >  
 > -Peff 
 >

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Bug: `git-filter-branch −−subdirectory−filter` fails on Darwin, others?

[Jeff King]

On Wed, May 11, 2016 at 06:47:20AM -0700, Geoff Nixon wrote:

> I believe I have found a bug in git. On Mac OS X (at least 10.9
> through 10.11), and versions of git from the current HEAD down through
> at least 1.8.x, `git filter-branch −−subdirectory−filter ...` fails.
> Using, e.g., the following example (from the docs for
> git-filter-branch), `git filter-branch --subdirectory-filter foodir --
> --all`, and using the git repository as the example repository, `git
> filter-branch --subdirectory-filter Documentation -- --all`, the
> "error message" one receives is "fatal: bad revision
> '−−subdirectory−filter'".

I just double-checked, and it works fine for me in a simple test:

  git init
  mkdir subdir
  for i in 1 2 3 4 5; do
echo $i >base-$i
echo $i >subdir/sub-$i
git add .
git commit -m $i
  done
  git filter-branch --subdirectory-filter subdir -- --all

That's on:

  $ sw_vers
  ProductName:Mac OS X
  ProductVersion: 10.9.5
  BuildVersion:   13F34

However, I notice the error message you show has non-ascii dashes when
it prints "--subdirectory-filter". That matches what you said below:

> - Exporting PS4 to 'WTF: $LINENO ' and setting `-x` is practically
> of no use, except that the last line before it fails appears to be
> `git rev-parse --no-flags --revs-only --symbolic-full-name
> --default HEAD $'�\210\222�\210\222subdirectory�\210\222filter`
> (including the octal sequences and bad-unicode character, those
> are not email artifacts)

Are you sure that you are invoking filter-branch with regular ascii
dashes, and not Unicode "minus-sign" (U+2212)?

I seem to recall this coming up once before related to OS X, but I can't
seem to find it in the archive. And I don't recall if it was related to
the terminal, a keyboard setting, or something else.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Bug: `git-filter-branch −−subdirectory−filter` fails on Darwin, others?

[Geoff Nixon]

May 11, 2016
Geoff Nixon
geoff@geoff.codes

To Whom It May Concern:

I believe I have found a bug in git. On Mac OS X (at least 10.9 through 
10.11), and versions of git from the current HEAD down through at least 1.8.x, 
`git filter-branch −−subdirectory−filter ...` fails. Using, e.g., the following 
example (from the docs for git-filter-branch), `git filter-branch 
--subdirectory-filter foodir -- --all`, and using the git repository as the 
example repository, `git filter-branch --subdirectory-filter Documentation -- 
--all`, the "error message" one receives is "fatal: bad revision 
'−−subdirectory−filter'".

I have tried to find and eliminate the bug myself, but despite my efforts 
it has proved elusive. Here is what I can tell you:

- It is apparently Darwin specific, or at least, I cannot reproduce on Linux
- It applies across a wide swath of versions of git and Mac OS X.
- Debugging is a challenge, because the code is pretty wack sauce,
- I.e., I don't understand how it `s
 Doesn't matter which version of the OS or which version of git, at least 
going back to 10.9 and 1.8, I believe.
- There is some extremely strange magic going on here, i.e.,
  - I don't understand how it sources `git-sh-setup` on line 90, while 
still in $PWD
  - It begins with a heredoc of fuctions which are then immediately,
  - `eval`'d, for no apparent reason.
  - There's way to many uses of `eval` to follow, many of them needless, 
and it resets its own positional arguments to the result of the expansion of 
command substitution in places, withought saving the original parameters 
  - It possibly seems to be expanding '-' to an octal sequence at some 
point?
- Exporting PS4 to 'WTF: $LINENO ' and setting `-x` is practically of no 
use, except that the last line before it fails appears to be `git rev-parse 
--no-flags --revs-only --symbolic-full-name --default HEAD 
$'�\210\222�\210\222subdirectory�\210\222filter` (including the octal sequences 
and bad-unicode character, those are not email artifacts)

Thank you for you time and consideration.

Yours,

Geoff Nixon


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] test-lib: set BASH_XTRACEFD automatically

[Jeff King]

On Tue, May 10, 2016 at 03:06:59PM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > The patch itself is a trivial-looking one-liner, but there
> > are a few subtleties worth mentioning:
> >
> >   - the variable is _not_ exported; the "set -x" is local to
> > our process, and so the tracefd should match
> >
> >   - this line has to come after we do the redirection for fd
> > 4, as bash will otherwise complain during the variable
> > assignment
> >
> >   - likewise, we cannot ever unset this variable, as it
> > would close descriptor 4
> >
> >   - setting it once here is sufficient to cover both the
> > regular "-x" case (which implies "--verbose"), as well
> > as "--verbose-only=1". This works because the latter
> > flips "set -x" off and on for particular tests (if it
> > didn't, we would get tracing for all tests, as going to
> > descriptor 4 effectively circumvents the verbose flag).
> 
> Thanks.  Some of them may deserve to be next to the one-liner to
> prevent people from making changes that tickle them?

Good idea. Here's a v2 that moves most of that text into a comment.

-- >8 --
Subject: test-lib: set BASH_XTRACEFD automatically

Passing "-x" to a test script enables the shell's "set -x"
tracing, which can help with tracking down the command that
is causing a failure. Unfortunately, it can also _cause_
failures in some tests that redirect the stderr of a shell
function.  Inside the function the shell continues to
respect "set -x", and the trace output is collected along
with whatever stderr is generated normally by the function.

You can see an example of this by running:

  ./t0040-parse-options.sh -x -i

which will fail immediately in the first test, as it
expects:

  test_must_fail some-cmd 2>output.err

to leave output.err empty (but with "-x" it has our trace
output).

Unfortunately there isn't a portable or scalable solution to
this. We could teach test_must_fail to disable "set -x", but
that doesn't help any of the other functions or subshells.

However, we can work around it by pointing the "set -x"
output to our descriptor 4, which always points to the
original stderr of the test script. Unfortunately this only
works for bash, but it's better than nothing (and other
shells will just ignore the BASH_XTRACEFD variable).

The patch itself is a simple one-liner, but note the caveats
in the accompanying comments.

Automatic tests for our "-x" option may be a bit too meta
(and a pain, because they are bash-specific), but I did
confirm that it works correctly both with regular "-x" and
with "--verbose-only=1". This works because the latter flips
"set -x" off and on for particular tests (if it didn't, we
would get tracing for all tests, as going to descriptor 4
effectively circumvents the verbose flag).

Signed-off-by: Jeff King 
---
 t/README  |  6 +++---
 t/test-lib.sh | 13 +
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/t/README b/t/README
index 1dc908e..76a0daa 100644
--- a/t/README
+++ b/t/README
@@ -84,9 +84,9 @@ appropriately before running "make".
 
 -x::
Turn on shell tracing (i.e., `set -x`) during the tests
-   themselves. Implies `--verbose`. Note that this can cause
-   failures in some tests which redirect and test the
-   output of shell functions. Use with caution.
+   themselves. Implies `--verbose`. Note that in non-bash shells,
+   this can cause failures in some tests which redirect and test
+   the output of shell functions. Use with caution.
 
 -d::
 --debug::
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 286c5f3..0055ebb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -322,6 +322,19 @@ else
exec 4>/dev/null 3>/dev/null
 fi
 
+# Send any "-x" output directly to stderr to avoid polluting tests
+# which capture stderr. We can do this unconditionally since it
+# has no effect if tracing isn't turned on.
+#
+# Note that this sets up the trace fd as soon as we assign the variable, so it
+# must come after the creation of descriptor 4 above. Likewise, we must never
+# unset this, as it has the side effect of closing descriptor 4, which we
+# use to show verbose tests to the user.
+#
+# Note also that we don't need or want to export it. The tracing is local to
+# this shell, and we would not want to influence any shells we exec.
+BASH_XTRACEFD=4
+
 test_failure=0
 test_count=0
 test_fixed=0
-- 
2.8.2.812.gd91b08f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 24/25] worktree move: accept destination as directory

[Duy Nguyen]

On Wed, May 11, 2016 at 11:43 AM, Eric Sunshine  wrote:
> On Wed, Apr 13, 2016 at 9:15 AM, Nguyễn Thái Ngọc Duy  
> wrote:
>> Similar to "mv a b/", which is actually "mv a b/a", we extract basename
>> of source worktree and create a directory of the same name at
>> destination if dst path is a directory.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> @@ -538,7 +538,13 @@ static int move_worktree(int ac, const char **av, const 
>> char *prefix)
>> -   if (file_exists(dst.buf))
>> +   if (is_directory(dst.buf))
>> +   /*
>> +* keep going, dst will be appended after we get the
>> +* source's absolute path
>> +*/
>> +   ;
>> +   else if (file_exists(dst.buf))
>> die(_("target '%s' already exists"), av[1]);
>> @@ -558,6 +564,17 @@ static int move_worktree(int ac, const char **av, const 
>> char *prefix)
>> +   if (is_directory(dst.buf)) {
>> +   const char *sep = strrchr(wt->path, '/');
>
> Does this need to take Windows into account?

wt->path comes from $GIT_DIR/worktrees/xxx/gitdir, which normally uses
forward slashes, so we should be safe. We already rely on forward
slashes in get_linked_worktree()

> Perhaps git_find_last_dir_sep()?

But this is probably a good thing to do anyway, to be more robust in
future. But it could confuse the reader later on why it's necessary
when backward slashes can't exist in wt->path. I don't know. Maybe
just have a comment that backward slashes can't never appear here?

There is also a potential problem with find_worktree_by_path(). I was
counting on real_path() to normalize paths and could simply do
strcmp_icase (or its new name, fspathcmp). But real_path() does not
seem to convert unify slashes. I will need to have a closer look at
this. Hopefully prefix_filename() already makes sure everything uses
forward slashes. Or maybe we could improve fspathcmp to see '/' and
'\' the same thing on Windows.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: syntax error in git-rebase while running t34* tests

[Jeff King]

On Wed, May 11, 2016 at 03:28:35PM +0200, Johannes Schindelin wrote:

> Looks obviously correct to me.

Thanks.

> I had a look at our other shell scripts and it looks as if there is only
> one more candidate for this issue: git-bisect.sh has a couple of 'for arg
> in "$@"' constructs. But from a cursory look, it appears that none of
> these "$@" can be empty lists because at least one parameter is passed to
> those functions (check_expected_revs() is only called from bisect_state()
> with 1 or 2 parameters, bisect_skip() makes no sense without parameters,
> and bisect_state() has another for loop if it got 2 parameters).
> 
> So I think we're fine.

I'm not even sure that:

  for arg in "$@"

is a problem if "$@" is empty. The issue here is the eval, in which we
generate syntactically funny code and expect the shell to interpret it.
It's possible a shell could get the more mundane case wrong, but I'd
expect most to get it right.

I did some brief grepping around myself, but didn't find any other
interesting cases. That doesn't mean much, though; tracking down what
content actually makes it into some of our "eval" calls would be pretty
time-consuming. So I'd rely on people like Armin to report failures in
the test suite.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 22/94] builtin/apply: move 'threeway' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'threeway' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 6216723..3650922 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -40,6 +40,7 @@ struct apply_state {
int numstat;
 
int summary;
+   int threeway;
 
/*
 *  --check turns on checking that the working tree matches the
@@ -63,7 +64,6 @@ static int state_p_value = 1;
 static int p_value_known;
 static int apply = 1;
 static int no_add;
-static int threeway;
 static int unsafe_paths;
 static const char *fake_ancestor;
 static int line_termination = '\n';
@@ -3501,7 +3501,7 @@ static int apply_data(struct apply_state *state, struct 
patch *patch,
if (patch->direct_to_threeway ||
apply_fragments(state, , patch) < 0) {
/* Note: with --reject, apply_fragments() returns 0 */
-   if (!threeway || try_threeway(state, , patch, st, ce) < 0)
+   if (!state->threeway || try_threeway(state, , patch, st, 
ce) < 0)
return -1;
}
patch->result = image.buf;
@@ -3796,7 +3796,7 @@ static int check_patch(struct apply_state *state, struct 
patch *patch)
((0 < patch->is_new) || patch->is_rename || patch->is_copy)) {
int err = check_to_create(state, new_name, ok_if_exists);
 
-   if (err && threeway) {
+   if (err && state->threeway) {
patch->direct_to_threeway = 1;
} else switch (err) {
case 0:
@@ -4625,7 +4625,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
N_("accept a patch that touches outside the working 
area")),
OPT_BOOL(0, "apply", _apply,
N_("also apply the patch (use with 
--stat/--summary/--check)")),
-   OPT_BOOL('3', "3way", ,
+   OPT_BOOL('3', "3way", ,
 N_( "attempt three-way merge if a patch does not 
apply")),
OPT_FILENAME(0, "build-fake-ancestor", _ancestor,
N_("build a temporary index based on embedded index 
information")),
@@ -4669,11 +4669,11 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
apply_usage, 0);
 
-   if (state.apply_with_reject && threeway)
+   if (state.apply_with_reject && state.threeway)
die("--reject and --3way cannot be used together.");
-   if (state.cached && threeway)
+   if (state.cached && state.threeway)
die("--cached and --3way cannot be used together.");
-   if (threeway) {
+   if (state.threeway) {
if (is_not_gitdir)
die(_("--3way outside a repository"));
state.check_index = 1;
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: syntax error in git-rebase while running t34* tests

[Johannes Schindelin]

Hi,

On Tue, 10 May 2016, Jeff King wrote:

> On Tue, May 10, 2016 at 01:53:56PM -0700, Junio C Hamano wrote:
> 
> > Jeff King  writes:
> > 
> > > I think it is clear why it works. If $strategy_opts is empty, then the
> > > code we generate looks like:
> > >
> > >   for strategy_opt in
> > >   do
> > >   ...
> > >   done
> > 
> > Ah, of course.  Thanks.
> 
> Here it is as a patch and commit message.
> 
> -- >8 --
> Subject: [PATCH] rebase--interactive: avoid empty list in shell for-loop
> 
> The $strategy_opts variable contains a space-separated list
> of strategy options, each individually shell-quoted. To loop
> over each, we "unwrap" them by doing an eval like:
> 
>   eval '
> for opt in '"$strategy_opts"'
> do
>...
> done
>   '
> 
> Note the quoting that means we expand $strategy_opts inline
> in the code to be evaluated (which is the right thing
> because we want the IFS-split and de-quoting). If the
> variable is empty, however, we ask the shell to eval the
> following code:
> 
>   for opt in
>   do
>  ...
>   done
> 
> without anything between "in" and "do".  Most modern shells
> are happy to treat that like a noop, but reportedly ksh88 on
> AIX considers it a syntax error. So let's catch the case
> that the variable is empty and skip the eval altogether
> (since we know the loop would be a noop anyway).
> 
> Reported-by: Armin Kunaschik 
> Signed-off-by: Jeff King 
> ---
>  git-rebase--interactive.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 9ea3075..1c6dfb6 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -82,6 +82,7 @@ rewritten_pending="$state_dir"/rewritten-pending
>  cr=$(printf "\015")
>  
>  strategy_args=${strategy:+--strategy=$strategy}
> +test -n "$strategy_opts" &&
>  eval '
>   for strategy_opt in '"$strategy_opts"'
>   do

Looks obviously correct to me.

I had a look at our other shell scripts and it looks as if there is only
one more candidate for this issue: git-bisect.sh has a couple of 'for arg
in "$@"' constructs. But from a cursory look, it appears that none of
these "$@" can be empty lists because at least one parameter is passed to
those functions (check_expected_revs() is only called from bisect_state()
with 1 or 2 parameters, bisect_skip() makes no sense without parameters,
and bisect_state() has another for loop if it got 2 parameters).

So I think we're fine.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 19/94] builtin/apply: move 'diffstat' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'diffstat' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 09af5dc..43c7198 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -33,6 +33,9 @@ struct apply_state {
/* --cached updates only the cache without ever touching the working 
tree. */
int cached;
 
+   /* --stat does just a diffstat, and doesn't actually apply */
+   int diffstat;
+
/*
 *  --check turns on checking that the working tree matches the
 *files that are being modified, but doesn't apply the patch
@@ -47,7 +50,6 @@ struct apply_state {
 };
 
 /*
- *  --stat does just a diffstat, and doesn't actually apply
  *  --numstat does numeric diffstat, and doesn't actually apply
  *  --index-info shows the old and new index info for paths if available.
  */
@@ -55,7 +57,6 @@ static int newfd = -1;
 
 static int state_p_value = 1;
 static int p_value_known;
-static int diffstat;
 static int numstat;
 static int summary;
 static int apply = 1;
@@ -4493,7 +4494,7 @@ static int apply_patch(struct apply_state *state,
if (fake_ancestor)
build_fake_ancestor(list, fake_ancestor);
 
-   if (diffstat)
+   if (state->diffstat)
stat_patch_list(list);
 
if (numstat)
@@ -4604,7 +4605,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
0, option_parse_p },
OPT_BOOL(0, "no-add", _add,
N_("ignore additions made by the patch")),
-   OPT_BOOL(0, "stat", ,
+   OPT_BOOL(0, "stat", ,
N_("instead of applying the patch, output diffstat for 
the input")),
OPT_NOOP_NOARG(0, "allow-binary-replacement"),
OPT_NOOP_NOARG(0, "binary"),
@@ -4677,7 +4678,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
}
if (state.apply_with_reject)
apply = state.apply_verbosely = 1;
-   if (!force_apply && (diffstat || numstat || summary || state.check || 
fake_ancestor))
+   if (!force_apply && (state.diffstat || numstat || summary || 
state.check || fake_ancestor))
apply = 0;
if (state.check_index && is_not_gitdir)
die(_("--index outside a repository"));
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 16/94] builtin/apply: move 'update_index' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'update_index' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 45 ++---
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 97af6ea..635a9ff 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -39,6 +39,7 @@ struct apply_state {
int check_index;
 
int unidiff_zero;
+   int update_index;
 };
 
 /*
@@ -51,7 +52,6 @@ static int newfd = -1;
 
 static int state_p_value = 1;
 static int p_value_known;
-static int update_index;
 static int cached;
 static int diffstat;
 static int numstat;
@@ -4095,9 +4095,9 @@ static void patch_stats(struct patch *patch)
}
 }
 
-static void remove_file(struct patch *patch, int rmdir_empty)
+static void remove_file(struct apply_state *state, struct patch *patch, int 
rmdir_empty)
 {
-   if (update_index) {
+   if (state->update_index) {
if (remove_file_from_cache(patch->old_name) < 0)
die(_("unable to remove %s from index"), 
patch->old_name);
}
@@ -4108,14 +4108,18 @@ static void remove_file(struct patch *patch, int 
rmdir_empty)
}
 }
 
-static void add_index_file(const char *path, unsigned mode, void *buf, 
unsigned long size)
+static void add_index_file(struct apply_state *state,
+  const char *path,
+  unsigned mode,
+  void *buf,
+  unsigned long size)
 {
struct stat st;
struct cache_entry *ce;
int namelen = strlen(path);
unsigned ce_size = cache_entry_size(namelen);
 
-   if (!update_index)
+   if (!state->update_index)
return;
 
ce = xcalloc(1, ce_size);
@@ -4225,13 +4229,14 @@ static void create_one_file(char *path, unsigned mode, 
const char *buf, unsigned
die_errno(_("unable to write file '%s' mode %o"), path, mode);
 }
 
-static void add_conflicted_stages_file(struct patch *patch)
+static void add_conflicted_stages_file(struct apply_state *state,
+  struct patch *patch)
 {
int stage, namelen;
unsigned ce_size, mode;
struct cache_entry *ce;
 
-   if (!update_index)
+   if (!state->update_index)
return;
namelen = strlen(patch->new_name);
ce_size = cache_entry_size(namelen);
@@ -4252,7 +4257,7 @@ static void add_conflicted_stages_file(struct patch 
*patch)
}
 }
 
-static void create_file(struct patch *patch)
+static void create_file(struct apply_state *state, struct patch *patch)
 {
char *path = patch->new_name;
unsigned mode = patch->new_mode;
@@ -4264,22 +4269,24 @@ static void create_file(struct patch *patch)
create_one_file(path, mode, buf, size);
 
if (patch->conflicted_threeway)
-   add_conflicted_stages_file(patch);
+   add_conflicted_stages_file(state, patch);
else
-   add_index_file(path, mode, buf, size);
+   add_index_file(state, path, mode, buf, size);
 }
 
 /* phase zero is to remove, phase one is to create */
-static void write_out_one_result(struct patch *patch, int phase)
+static void write_out_one_result(struct apply_state *state,
+struct patch *patch,
+int phase)
 {
if (patch->is_delete > 0) {
if (phase == 0)
-   remove_file(patch, 1);
+   remove_file(state, patch, 1);
return;
}
if (patch->is_new > 0 || patch->is_copy) {
if (phase == 1)
-   create_file(patch);
+   create_file(state, patch);
return;
}
/*
@@ -4287,9 +4294,9 @@ static void write_out_one_result(struct patch *patch, int 
phase)
 * thing: remove the old, write the new
 */
if (phase == 0)
-   remove_file(patch, patch->is_rename);
+   remove_file(state, patch, patch->is_rename);
if (phase == 1)
-   create_file(patch);
+   create_file(state, patch);
 }
 
 static int write_out_one_reject(struct apply_state *state, struct patch *patch)
@@ -4376,7 +4383,7 @@ static int write_out_results(struct apply_state *state, 
struct patch *list)
if (l->rejected)
errs = 1;
else {
-   write_out_one_result(l, phase);
+   write_out_one_result(state, l, phase);
if (phase == 1) {
if (write_out_one_reject(state, l))
 

[PATCH v2 20/94] builtin/apply: move 'numstat' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'numstat' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 43c7198..887c5d0 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -36,6 +36,9 @@ struct apply_state {
/* --stat does just a diffstat, and doesn't actually apply */
int diffstat;
 
+   /* --numstat does numeric diffstat, and doesn't actually apply */
+   int numstat;
+
/*
 *  --check turns on checking that the working tree matches the
 *files that are being modified, but doesn't apply the patch
@@ -50,14 +53,12 @@ struct apply_state {
 };
 
 /*
- *  --numstat does numeric diffstat, and doesn't actually apply
  *  --index-info shows the old and new index info for paths if available.
  */
 static int newfd = -1;
 
 static int state_p_value = 1;
 static int p_value_known;
-static int numstat;
 static int summary;
 static int apply = 1;
 static int no_add;
@@ -4497,7 +4498,7 @@ static int apply_patch(struct apply_state *state,
if (state->diffstat)
stat_patch_list(list);
 
-   if (numstat)
+   if (state->numstat)
numstat_patch_list(list);
 
if (summary)
@@ -4609,7 +4610,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
N_("instead of applying the patch, output diffstat for 
the input")),
OPT_NOOP_NOARG(0, "allow-binary-replacement"),
OPT_NOOP_NOARG(0, "binary"),
-   OPT_BOOL(0, "numstat", ,
+   OPT_BOOL(0, "numstat", ,
N_("show number of added and deleted lines in decimal 
notation")),
OPT_BOOL(0, "summary", ,
N_("instead of applying the patch, output a summary for 
the input")),
@@ -4678,7 +4679,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
}
if (state.apply_with_reject)
apply = state.apply_verbosely = 1;
-   if (!force_apply && (state.diffstat || numstat || summary || 
state.check || fake_ancestor))
+   if (!force_apply && (state.diffstat || state.numstat || summary || 
state.check || fake_ancestor))
apply = 0;
if (state.check_index && is_not_gitdir)
die(_("--index outside a repository"));
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 28/94] builtin/apply: move 'apply' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'apply' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 31 +--
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 2ba2b21..a3db284 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -25,6 +25,7 @@ struct apply_state {
const char *prefix;
int prefix_length;
 
+   int apply;
int allow_overlap;
int apply_in_reverse;
int apply_with_reject;
@@ -65,7 +66,7 @@ static int newfd = -1;
 
 static int state_p_value = 1;
 static int p_value_known;
-static int apply = 1;
+
 static const char * const apply_usage[] = {
N_("git apply [] [...]"),
NULL
@@ -135,10 +136,11 @@ static void parse_ignorewhitespace_option(const char 
*option)
die(_("unrecognized whitespace ignore option '%s'"), option);
 }
 
-static void set_default_whitespace_mode(const char *whitespace_option)
+static void set_default_whitespace_mode(struct apply_state *state,
+   const char *whitespace_option)
 {
if (!whitespace_option && !apply_default_whitespace)
-   ws_error_action = (apply ? warn_on_ws_error : nowarn_ws_error);
+   ws_error_action = (state->apply ? warn_on_ws_error : 
nowarn_ws_error);
 }
 
 /*
@@ -2067,7 +2069,7 @@ static int parse_chunk(struct apply_state *state, char 
*buffer, unsigned long si
 * without metadata change.  A binary patch appears
 * empty to us here.
 */
-   if ((apply || state->check) &&
+   if ((state->apply || state->check) &&
(!patch->is_binary && !metadata_changes(patch)))
die(_("patch with only garbage at line %d"), 
state_linenr);
}
@@ -2925,7 +2927,7 @@ static int apply_one_fragment(struct apply_state *state,
 * apply_data->apply_fragments->apply_one_fragment
 */
if (ws_error_action == die_on_ws_error)
-   apply = 0;
+   state->apply = 0;
}
 
if (state->apply_verbosely && applied_pos != pos) {
@@ -4469,9 +4471,9 @@ static int apply_patch(struct apply_state *state,
die(_("unrecognized input"));
 
if (whitespace_error && (ws_error_action == die_on_ws_error))
-   apply = 0;
+   state->apply = 0;
 
-   state->update_index = state->check_index && apply;
+   state->update_index = state->check_index && state->apply;
if (state->update_index && newfd < 0)
newfd = hold_locked_index(_file, 1);
 
@@ -4480,12 +4482,12 @@ static int apply_patch(struct apply_state *state,
die(_("unable to read index file"));
}
 
-   if ((state->check || apply) &&
+   if ((state->check || state->apply) &&
check_patch_list(state, list) < 0 &&
!state->apply_with_reject)
exit(1);
 
-   if (apply && write_out_results(state, list)) {
+   if (state->apply && write_out_results(state, list)) {
if (state->apply_with_reject)
exit(1);
/* with --3way, we still need to write the index out */
@@ -4574,6 +4576,7 @@ static void init_apply_state(struct apply_state *state, 
const char *prefix)
memset(state, 0, sizeof(*state));
state->prefix = prefix;
state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
+   state->apply = 1;
state->line_termination = '\n';
state->p_context = UINT_MAX;
 
@@ -4680,9 +4683,9 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
state.check_index = 1;
}
if (state.apply_with_reject)
-   apply = state.apply_verbosely = 1;
+   state.apply = state.apply_verbosely = 1;
if (!force_apply && (state.diffstat || state.numstat || state.summary 
|| state.check || state.fake_ancestor))
-   apply = 0;
+   state.apply = 0;
if (state.check_index && is_not_gitdir)
die(_("--index outside a repository"));
if (state.cached) {
@@ -4710,11 +4713,11 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
if (fd < 0)
die_errno(_("can't open patch '%s'"), arg);
read_stdin = 0;
-   set_default_whitespace_mode(whitespace_option);
+   set_default_whitespace_mode(, whitespace_option);
errs |= apply_patch(, fd, arg, options);
close(fd);
}
-   set_default_whitespace_mode(whitespace_option);
+   

[PATCH v2 14/94] builtin/apply: move 'apply_with_reject' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'apply_with_reject' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 73cef9b..53cc280 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -26,6 +26,7 @@ struct apply_state {
int prefix_length;
 
int apply_in_reverse;
+   int apply_with_reject;
 
/*
 *  --check turns on checking that the working tree matches the
@@ -55,7 +56,6 @@ static int diffstat;
 static int numstat;
 static int summary;
 static int apply = 1;
-static int apply_with_reject;
 static int apply_verbosely;
 static int allow_overlap;
 static int no_add;
@@ -3101,7 +3101,7 @@ static int apply_fragments(struct apply_state *state, 
struct image *img, struct
nth++;
if (apply_one_fragment(state, img, frag, inaccurate_eof, 
ws_rule, nth)) {
error(_("patch failed: %s:%ld"), name, frag->oldpos);
-   if (!apply_with_reject)
+   if (!state->apply_with_reject)
return -1;
frag->rejected = 1;
}
@@ -4467,11 +4467,11 @@ static int apply_patch(struct apply_state *state,
 
if ((state->check || apply) &&
check_patch_list(state, list) < 0 &&
-   !apply_with_reject)
+   !state->apply_with_reject)
exit(1);
 
if (apply && write_out_results(list)) {
-   if (apply_with_reject)
+   if (state->apply_with_reject)
exit(1);
/* with --3way, we still need to write the index out */
return 1;
@@ -4631,7 +4631,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
N_("apply the patch in reverse")),
OPT_BOOL(0, "unidiff-zero", _zero,
N_("don't expect at least one line of context")),
-   OPT_BOOL(0, "reject", _with_reject,
+   OPT_BOOL(0, "reject", _with_reject,
N_("leave the rejected hunks in corresponding *.rej 
files")),
OPT_BOOL(0, "allow-overlap", _overlap,
N_("allow overlapping hunks")),
@@ -4653,7 +4653,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
apply_usage, 0);
 
-   if (apply_with_reject && threeway)
+   if (state.apply_with_reject && threeway)
die("--reject and --3way cannot be used together.");
if (cached && threeway)
die("--cached and --3way cannot be used together.");
@@ -4662,7 +4662,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
die(_("--3way outside a repository"));
state.check_index = 1;
}
-   if (apply_with_reject)
+   if (state.apply_with_reject)
apply = apply_verbosely = 1;
if (!force_apply && (diffstat || numstat || summary || state.check || 
fake_ancestor))
apply = 0;
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 27/94] builtin/apply: move 'p_context' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'p_context' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 6f9a090..2ba2b21 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -57,6 +57,8 @@ struct apply_state {
int update_index;
int unsafe_paths;
int line_termination;
+
+   unsigned int p_context;
 };
 
 static int newfd = -1;
@@ -64,7 +66,6 @@ static int newfd = -1;
 static int state_p_value = 1;
 static int p_value_known;
 static int apply = 1;
-static unsigned int p_context = UINT_MAX;
 static const char * const apply_usage[] = {
N_("git apply [] [...]"),
NULL
@@ -2880,7 +2881,7 @@ static int apply_one_fragment(struct apply_state *state,
break;
 
/* Am I at my context limits? */
-   if ((leading <= p_context) && (trailing <= p_context))
+   if ((leading <= state->p_context) && (trailing <= 
state->p_context))
break;
if (match_beginning || match_end) {
match_beginning = match_end = 0;
@@ -4574,6 +4575,7 @@ static void init_apply_state(struct apply_state *state, 
const char *prefix)
state->prefix = prefix;
state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
state->line_termination = '\n';
+   state->p_context = UINT_MAX;
 
git_apply_config();
if (apply_default_whitespace)
@@ -4631,7 +4633,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
/* Think twice before adding "--nul" synonym to this */
OPT_SET_INT('z', NULL, _termination,
N_("paths are separated with NUL character"), '\0'),
-   OPT_INTEGER('C', NULL, _context,
+   OPT_INTEGER('C', NULL, _context,
N_("ensure at least  lines of context 
match")),
{ OPTION_CALLBACK, 0, "whitespace", _option, 
N_("action"),
N_("detect new or modified lines that have whitespace 
errors"),
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 11/94] builtin/apply: move 'check' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'check' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 44ae95d..6bf103a 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -25,12 +25,15 @@ struct apply_state {
const char *prefix;
int prefix_length;
 
+   /*
+*  --check turns on checking that the working tree matches the
+*files that are being modified, but doesn't apply the patch
+*/
+   int check;
int unidiff_zero;
 };
 
 /*
- *  --check turns on checking that the working tree matches the
- *files that are being modified, but doesn't apply the patch
  *  --stat does just a diffstat, and doesn't actually apply
  *  --numstat does numeric diffstat, and doesn't actually apply
  *  --index-info shows the old and new index info for paths if available.
@@ -47,7 +50,6 @@ static int cached;
 static int diffstat;
 static int numstat;
 static int summary;
-static int check;
 static int apply = 1;
 static int apply_in_reverse;
 static int apply_with_reject;
@@ -2052,7 +2054,7 @@ static int parse_chunk(struct apply_state *state, char 
*buffer, unsigned long si
 * without metadata change.  A binary patch appears
 * empty to us here.
 */
-   if ((apply || check) &&
+   if ((apply || state->check) &&
(!patch->is_binary && !metadata_changes(patch)))
die(_("patch with only garbage at line %d"), 
state_linenr);
}
@@ -4439,7 +4441,7 @@ static int apply_patch(struct apply_state *state,
die(_("unable to read index file"));
}
 
-   if ((check || apply) &&
+   if ((state->check || apply) &&
check_patch_list(state, list) < 0 &&
!apply_with_reject)
exit(1);
@@ -4573,7 +4575,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
N_("show number of added and deleted lines in decimal 
notation")),
OPT_BOOL(0, "summary", ,
N_("instead of applying the patch, output a summary for 
the input")),
-   OPT_BOOL(0, "check", ,
+   OPT_BOOL(0, "check", ,
N_("instead of applying the patch, see if the patch is 
applicable")),
OPT_BOOL(0, "index", _index,
N_("make sure the patch is applicable to the current 
index")),
@@ -4638,7 +4640,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
}
if (apply_with_reject)
apply = apply_verbosely = 1;
-   if (!force_apply && (diffstat || numstat || summary || check || 
fake_ancestor))
+   if (!force_apply && (diffstat || numstat || summary || state.check || 
fake_ancestor))
apply = 0;
if (check_index && is_not_gitdir)
die(_("--index outside a repository"));
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 21/94] builtin/apply: move 'summary' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'summary' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 887c5d0..6216723 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -39,6 +39,8 @@ struct apply_state {
/* --numstat does numeric diffstat, and doesn't actually apply */
int numstat;
 
+   int summary;
+
/*
 *  --check turns on checking that the working tree matches the
 *files that are being modified, but doesn't apply the patch
@@ -59,7 +61,6 @@ static int newfd = -1;
 
 static int state_p_value = 1;
 static int p_value_known;
-static int summary;
 static int apply = 1;
 static int no_add;
 static int threeway;
@@ -4501,7 +4502,7 @@ static int apply_patch(struct apply_state *state,
if (state->numstat)
numstat_patch_list(list);
 
-   if (summary)
+   if (state->summary)
summary_patch_list(list);
 
free_patch_list(list);
@@ -4612,7 +4613,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
OPT_NOOP_NOARG(0, "binary"),
OPT_BOOL(0, "numstat", ,
N_("show number of added and deleted lines in decimal 
notation")),
-   OPT_BOOL(0, "summary", ,
+   OPT_BOOL(0, "summary", ,
N_("instead of applying the patch, output a summary for 
the input")),
OPT_BOOL(0, "check", ,
N_("instead of applying the patch, see if the patch is 
applicable")),
@@ -4679,7 +4680,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
}
if (state.apply_with_reject)
apply = state.apply_verbosely = 1;
-   if (!force_apply && (state.diffstat || state.numstat || summary || 
state.check || fake_ancestor))
+   if (!force_apply && (state.diffstat || state.numstat || state.summary 
|| state.check || fake_ancestor))
apply = 0;
if (state.check_index && is_not_gitdir)
die(_("--index outside a repository"));
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 51/94] builtin/apply: make apply_patch() return -1 instead of die()ing

[Christian Couder]

To libify `git apply` functionality we have to signal errors
to the caller instead of die()ing.

As a first step in this direction, let's make apply_patch() return
-1 in case of errors instead of dying. For now its only caller
apply_all_patches() will exit(1) when apply_patch() return -1.

In a later patch, apply_all_patches() will return -1 too instead of
exiting.

Helped-by: Eric Sunshine 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 54 +++---
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index ec55768..d95630c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4512,6 +4512,14 @@ static int write_out_results(struct apply_state *state, 
struct patch *list)
 #define INACCURATE_EOF (1<<0)
 #define RECOUNT(1<<1)
 
+/*
+ * Try to apply a patch.
+ *
+ * Returns:
+ *  -1 if an error happened
+ *   0 if the patch applied
+ *   1 if the patch did not apply
+ */
 static int apply_patch(struct apply_state *state,
   int fd,
   const char *filename,
@@ -4521,6 +4529,7 @@ static int apply_patch(struct apply_state *state,
struct strbuf buf = STRBUF_INIT; /* owns the patch text */
struct patch *list = NULL, **listp = 
int skipped_patch = 0;
+   int res = 0;
 
state->patch_input_file = filename;
read_patch_file(, fd);
@@ -4553,8 +4562,10 @@ static int apply_patch(struct apply_state *state,
offset += nr;
}
 
-   if (!list && !skipped_patch)
-   die(_("unrecognized input"));
+   if (!list && !skipped_patch) {
+   res = error(_("unrecognized input"));
+   goto end;
+   }
 
if (state->whitespace_error && (state->ws_error_action == 
die_on_ws_error))
state->apply = 0;
@@ -4563,21 +4574,22 @@ static int apply_patch(struct apply_state *state,
if (state->update_index && state->newfd < 0)
state->newfd = hold_locked_index(state->lock_file, 1);
 
-   if (state->check_index) {
-   if (read_cache() < 0)
-   die(_("unable to read index file"));
+   if (state->check_index && read_cache() < 0) {
+   res = error(_("unable to read index file"));
+   goto end;
}
 
if ((state->check || state->apply) &&
check_patch_list(state, list) < 0 &&
-   !state->apply_with_reject)
-   exit(1);
+   !state->apply_with_reject) {
+   res = -1;
+   goto end;
+   }
 
if (state->apply && write_out_results(state, list)) {
-   if (state->apply_with_reject)
-   exit(1);
/* with --3way, we still need to write the index out */
-   return 1;
+   res = state->apply_with_reject ? -1 : 1;
+   goto end;
}
 
if (state->fake_ancestor)
@@ -4592,10 +4604,11 @@ static int apply_patch(struct apply_state *state,
if (state->summary)
summary_patch_list(list);
 
+end:
free_patch_list(list);
strbuf_release();
string_list_clear(>fn_table, 0);
-   return 0;
+   return res;
 }
 
 static void git_apply_config(void)
@@ -4722,6 +4735,7 @@ static int apply_all_patches(struct apply_state *state,
 int options)
 {
int i;
+   int res;
int errs = 0;
int read_stdin = 1;
 
@@ -4730,7 +4744,10 @@ static int apply_all_patches(struct apply_state *state,
int fd;
 
if (!strcmp(arg, "-")) {
-   errs |= apply_patch(state, 0, "", options);
+   res = apply_patch(state, 0, "", options);
+   if (res < 0)
+   exit(1);
+   errs |= res;
read_stdin = 0;
continue;
} else if (0 < state->prefix_length)
@@ -4743,12 +4760,19 @@ static int apply_all_patches(struct apply_state *state,
die_errno(_("can't open patch '%s'"), arg);
read_stdin = 0;
set_default_whitespace_mode(state);
-   errs |= apply_patch(state, fd, arg, options);
+   res = apply_patch(state, fd, arg, options);
+   if (res < 0)
+   exit(1);
+   errs |= res;
close(fd);
}
set_default_whitespace_mode(state);
-   if (read_stdin)
-   errs |= apply_patch(state, 0, "", options);
+   if (read_stdin) {
+   res = apply_patch(state, 0, "", options);
+   if (res < 0)
+   exit(1);
+   errs |= res;
+   }
 
if (state->whitespace_error) {
if 

[PATCH v2 40/94] builtin/apply: move 'ws_error_action' into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'ws_error_action' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Signed-off-by: Christian Couder 
---
 builtin/apply.c | 62 +++--
 1 file changed, 34 insertions(+), 28 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index e68fd2c..10d45c7 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -21,6 +21,13 @@
 #include "ll-merge.h"
 #include "rerere.h"
 
+enum ws_error_action {
+   nowarn_ws_error,
+   warn_on_ws_error,
+   die_on_ws_error,
+   correct_ws_error
+};
+
 struct apply_state {
const char *prefix;
int prefix_length;
@@ -71,6 +78,8 @@ struct apply_state {
int whitespace_error;
int squelch_whitespace_errors;
int applied_after_fixing_ws;
+
+   enum ws_error_action ws_error_action;
 };
 
 static int newfd = -1;
@@ -80,12 +89,6 @@ static const char * const apply_usage[] = {
NULL
 };
 
-static enum ws_error_action {
-   nowarn_ws_error,
-   warn_on_ws_error,
-   die_on_ws_error,
-   correct_ws_error
-} ws_error_action = warn_on_ws_error;
 
 static enum ws_ignore {
ignore_ws_none,
@@ -96,28 +99,28 @@ static enum ws_ignore {
 static void parse_whitespace_option(struct apply_state *state, const char 
*option)
 {
if (!option) {
-   ws_error_action = warn_on_ws_error;
+   state->ws_error_action = warn_on_ws_error;
return;
}
if (!strcmp(option, "warn")) {
-   ws_error_action = warn_on_ws_error;
+   state->ws_error_action = warn_on_ws_error;
return;
}
if (!strcmp(option, "nowarn")) {
-   ws_error_action = nowarn_ws_error;
+   state->ws_error_action = nowarn_ws_error;
return;
}
if (!strcmp(option, "error")) {
-   ws_error_action = die_on_ws_error;
+   state->ws_error_action = die_on_ws_error;
return;
}
if (!strcmp(option, "error-all")) {
-   ws_error_action = die_on_ws_error;
+   state->ws_error_action = die_on_ws_error;
state->squelch_whitespace_errors = 0;
return;
}
if (!strcmp(option, "strip") || !strcmp(option, "fix")) {
-   ws_error_action = correct_ws_error;
+   state->ws_error_action = correct_ws_error;
return;
}
die(_("unrecognized whitespace option '%s'"), option);
@@ -141,7 +144,7 @@ static void parse_ignorewhitespace_option(const char 
*option)
 static void set_default_whitespace_mode(struct apply_state *state)
 {
if (!state->whitespace_option && !apply_default_whitespace)
-   ws_error_action = (state->apply ? warn_on_ws_error : 
nowarn_ws_error);
+   state->ws_error_action = (state->apply ? warn_on_ws_error : 
nowarn_ws_error);
 }
 
 /*
@@ -1676,12 +1679,12 @@ static int parse_fragment(struct apply_state *state,
leading++;
trailing++;
if (!state->apply_in_reverse &&
-   ws_error_action == correct_ws_error)
+   state->ws_error_action == correct_ws_error)
check_whitespace(state, line, len, 
patch->ws_rule);
break;
case '-':
if (state->apply_in_reverse &&
-   ws_error_action != nowarn_ws_error)
+   state->ws_error_action != nowarn_ws_error)
check_whitespace(state, line, len, 
patch->ws_rule);
deleted++;
oldlines--;
@@ -1689,7 +1692,7 @@ static int parse_fragment(struct apply_state *state,
break;
case '+':
if (!state->apply_in_reverse &&
-   ws_error_action != nowarn_ws_error)
+   state->ws_error_action != nowarn_ws_error)
check_whitespace(state, line, len, 
patch->ws_rule);
added++;
newlines--;
@@ -2402,7 +2405,8 @@ static int line_by_line_fuzzy_match(struct image *img,
return 1;
 }
 
-static int match_fragment(struct image *img,
+static int match_fragment(struct apply_state *state,
+ struct image *img,
  struct image *preimage,
  struct image *postimage,
  unsigned long try,
@@ -2423,7 +2427,7 @@ static int match_fragment(struct image *img,
preimage_limit = preimage->nr;
if (match_end && (preimage->nr + try_lno != img->nr))
return 0;
-   } else if 

[PATCH v2 52/94] builtin/apply: read_patch_file() return -1 instead of die()ing

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing. Let's do that by using error() instead
of die()ing in read_patch_file().

Signed-off-by: Christian Couder 
---
 builtin/apply.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index d95630c..a166d70 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -445,10 +445,10 @@ static void say_patch_name(FILE *output, const char *fmt, 
struct patch *patch)
 
 #define SLOP (16)
 
-static void read_patch_file(struct strbuf *sb, int fd)
+static int read_patch_file(struct strbuf *sb, int fd)
 {
if (strbuf_read(sb, fd, 0) < 0)
-   die_errno("git apply: failed to read");
+   return error("git apply: failed to read: %s", strerror(errno));
 
/*
 * Make sure that we have some slop in the buffer
@@ -457,6 +457,7 @@ static void read_patch_file(struct strbuf *sb, int fd)
 */
strbuf_grow(sb, SLOP);
memset(sb->buf + sb->len, 0, SLOP);
+   return 0;
 }
 
 static unsigned long linelen(const char *buffer, unsigned long size)
@@ -4532,7 +4533,8 @@ static int apply_patch(struct apply_state *state,
int res = 0;
 
state->patch_input_file = filename;
-   read_patch_file(, fd);
+   if (read_patch_file(, fd))
+   return -1;
offset = 0;
while (offset < buf.len) {
struct patch *patch;
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 43/94] builtin/apply: move 'state_linenr' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'state_linenr' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 75 ++---
 1 file changed, 40 insertions(+), 35 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index deba14c..5c003a1 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -84,6 +84,13 @@ struct apply_state {
int max_change;
int max_len;
 
+   /*
+* Various "current state", notably line numbers and what
+* file (and how) we're patching right now.. The "is_"
+* things are flags, where -1 means "don't know yet".
+*/
+   int linenr;
+
int p_value;
int p_value_known;
unsigned int p_context;
@@ -156,13 +163,6 @@ static void set_default_whitespace_mode(struct apply_state 
*state)
state->ws_error_action = (state->apply ? warn_on_ws_error : 
nowarn_ws_error);
 }
 
-/*
- * Various "current state", notably line numbers and what
- * file (and how) we're patching right now.. The "is_"
- * things are flags, where -1 means "don't know yet".
- */
-static int state_linenr = 1;
-
 /*
  * This represents one "hunk" from a patch, starting with
  * "@@ -oldpos,oldlines +newpos,newlines @@" marker.  The
@@ -939,7 +939,7 @@ static void parse_traditional_patch(struct apply_state 
*state,
}
}
if (!name)
-   die(_("unable to find filename in patch at line %d"), 
state_linenr);
+   die(_("unable to find filename in patch at line %d"), 
state->linenr);
 }
 
 static int gitdiff_hdrend(struct apply_state *state,
@@ -977,17 +977,17 @@ static void gitdiff_verify_name(struct apply_state *state,
char *another;
if (isnull)
die(_("git apply: bad git-diff - expected /dev/null, 
got %s on line %d"),
-   *name, state_linenr);
+   *name, state->linenr);
another = find_name(state, line, NULL, state->p_value, 
TERM_TAB);
if (!another || memcmp(another, *name, len + 1))
die((side == DIFF_NEW_NAME) ?
_("git apply: bad git-diff - inconsistent new 
filename on line %d") :
-   _("git apply: bad git-diff - inconsistent old 
filename on line %d"), state_linenr);
+   _("git apply: bad git-diff - inconsistent old 
filename on line %d"), state->linenr);
free(another);
} else {
/* expect "/dev/null" */
if (memcmp("/dev/null", line, 9) || line[9] != '\n')
-   die(_("git apply: bad git-diff - expected /dev/null on 
line %d"), state_linenr);
+   die(_("git apply: bad git-diff - expected /dev/null on 
line %d"), state->linenr);
}
 }
 
@@ -1350,8 +1350,8 @@ static int parse_git_header(struct apply_state *state,
 
line += len;
size -= len;
-   state_linenr++;
-   for (offset = len ; size > 0 ; offset += len, size -= len, line += len, 
state_linenr++) {
+   state->linenr++;
+   for (offset = len ; size > 0 ; offset += len, size -= len, line += len, 
state->linenr++) {
static const struct opentry {
const char *str;
int (*fn)(struct apply_state *, const char *, struct 
patch *);
@@ -1522,7 +1522,7 @@ static int find_header(struct apply_state *state,
patch->is_new = patch->is_delete = -1;
patch->old_mode = patch->new_mode = 0;
patch->old_name = patch->new_name = NULL;
-   for (offset = 0; size > 0; offset += len, size -= len, line += len, 
state_linenr++) {
+   for (offset = 0; size > 0; offset += len, size -= len, line += len, 
state->linenr++) {
unsigned long nextlen;
 
len = linelen(line, size);
@@ -1543,7 +1543,7 @@ static int find_header(struct apply_state *state,
if (parse_fragment_header(line, len, ) < 0)
continue;
die(_("patch fragment without header at line %d: %.*s"),
-   state_linenr, (int)len-1, line);
+   state->linenr, (int)len-1, line);
}
 
if (size < len + 6)
@@ -1564,13 +1564,13 @@ static int find_header(struct apply_state *state,
   "git diff header lacks filename 
information when removing "
   "%d leading pathname components 
(line %d)",
   state->p_value),
-   state->p_value, state_linenr);
+   

[PATCH v2 29/94] builtin/apply: move 'patch_input_file' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'patch_input_file' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index a3db284..e43da9c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -44,6 +44,7 @@ struct apply_state {
int threeway;
int no_add;
const char *fake_ancestor;
+   const char *patch_input_file;
 
/*
 *  --check turns on checking that the working tree matches the
@@ -88,7 +89,6 @@ static enum ws_ignore {
 } ws_ignore_action = ignore_ws_none;
 
 
-static const char *patch_input_file;
 static struct strbuf root = STRBUF_INIT;
 
 static void parse_whitespace_option(const char *option)
@@ -1534,7 +1534,11 @@ static int find_header(struct apply_state *state,
return -1;
 }
 
-static void record_ws_error(unsigned result, const char *line, int len, int 
linenr)
+static void record_ws_error(struct apply_state *state,
+   unsigned result,
+   const char *line,
+   int len,
+   int linenr)
 {
char *err;
 
@@ -1548,15 +1552,18 @@ static void record_ws_error(unsigned result, const char 
*line, int len, int line
 
err = whitespace_error_string(result);
fprintf(stderr, "%s:%d: %s.\n%.*s\n",
-   patch_input_file, linenr, err, len, line);
+   state->patch_input_file, linenr, err, len, line);
free(err);
 }
 
-static void check_whitespace(const char *line, int len, unsigned ws_rule)
+static void check_whitespace(struct apply_state *state,
+const char *line,
+int len,
+unsigned ws_rule)
 {
unsigned result = ws_check(line + 1, len - 1, ws_rule);
 
-   record_ws_error(result, line + 1, len - 2, state_linenr);
+   record_ws_error(state, result, line + 1, len - 2, state_linenr);
 }
 
 /*
@@ -1611,12 +1618,12 @@ static int parse_fragment(struct apply_state *state,
trailing++;
if (!state->apply_in_reverse &&
ws_error_action == correct_ws_error)
-   check_whitespace(line, len, patch->ws_rule);
+   check_whitespace(state, line, len, 
patch->ws_rule);
break;
case '-':
if (state->apply_in_reverse &&
ws_error_action != nowarn_ws_error)
-   check_whitespace(line, len, patch->ws_rule);
+   check_whitespace(state, line, len, 
patch->ws_rule);
deleted++;
oldlines--;
trailing = 0;
@@ -1624,7 +1631,7 @@ static int parse_fragment(struct apply_state *state,
case '+':
if (!state->apply_in_reverse &&
ws_error_action != nowarn_ws_error)
-   check_whitespace(line, len, patch->ws_rule);
+   check_whitespace(state, line, len, 
patch->ws_rule);
added++;
newlines--;
trailing = 0;
@@ -2913,7 +2920,7 @@ static int apply_one_fragment(struct apply_state *state,
preimage.nr + applied_pos >= img->nr &&
(ws_rule & WS_BLANK_AT_EOF) &&
ws_error_action != nowarn_ws_error) {
-   record_ws_error(WS_BLANK_AT_EOF, "+", 1,
+   record_ws_error(state, WS_BLANK_AT_EOF, "+", 1,
found_new_blank_lines_at_end);
if (ws_error_action == correct_ws_error) {
while (new_blank_lines_at_end--)
@@ -4436,7 +4443,7 @@ static int apply_patch(struct apply_state *state,
struct patch *list = NULL, **listp = 
int skipped_patch = 0;
 
-   patch_input_file = filename;
+   state->patch_input_file = filename;
read_patch_file(, fd);
offset = 0;
while (offset < buf.len) {
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 88/94] apply: don't print on stdout when be_silent is set

[Christian Couder]

Signed-off-by: Christian Couder 
---
 apply.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/apply.c b/apply.c
index 5459ee1..e0fdd1d 100644
--- a/apply.c
+++ b/apply.c
@@ -4669,13 +4669,13 @@ static int apply_patch(struct apply_state *state,
goto end;
}
 
-   if (state->diffstat)
+   if (state->diffstat && !state->be_silent)
stat_patch_list(state, list);
 
-   if (state->numstat)
+   if (state->numstat && !state->be_silent)
numstat_patch_list(state, list);
 
-   if (state->summary)
+   if (state->summary && !state->be_silent)
summary_patch_list(list);
 
 end:
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 68/94] builtin/apply: make build_fake_ancestor() return -1 on error

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.

To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", build_fake_ancestor() should return -1 using
error() instead of calling die().

Helped-by: Eric Sunshine 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 41 ++---
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 06c1c16..a2cc099 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3890,11 +3890,12 @@ static int preimage_sha1_in_gitlink_patch(struct patch 
*p, unsigned char sha1[20
 }
 
 /* Build an index that contains the just the files needed for a 3way merge */
-static void build_fake_ancestor(struct patch *list, const char *filename)
+static int build_fake_ancestor(struct patch *list, const char *filename)
 {
struct patch *patch;
struct index_state result = { NULL };
static struct lock_file lock;
+   int res;
 
/* Once we start supporting the reverse patch, it may be
 * worth showing the new sha1 prefix, but until then...
@@ -3912,31 +3913,38 @@ static void build_fake_ancestor(struct patch *list, 
const char *filename)
if (!preimage_sha1_in_gitlink_patch(patch, sha1))
; /* ok, the textual part looks sane */
else
-   die("sha1 information is lacking or useless for 
submodule %s",
-   name);
+   return error("sha1 information is lacking or "
+"useless for submodule %s", name);
} else if (!get_sha1_blob(patch->old_sha1_prefix, sha1)) {
; /* ok */
} else if (!patch->lines_added && !patch->lines_deleted) {
/* mode-only change: update the current */
if (get_current_sha1(patch->old_name, sha1))
-   die("mode change for %s, which is not "
-   "in current HEAD", name);
+   return error("mode change for %s, which is not "
+"in current HEAD", name);
} else
-   die("sha1 information is lacking or useless "
-   "(%s).", name);
+   return error("sha1 information is lacking or useless "
+"(%s).", name);
 
ce = make_cache_entry(patch->old_mode, sha1, name, 0, 0);
if (!ce)
-   die(_("make_cache_entry failed for path '%s'"), name);
-   if (add_index_entry(, ce, ADD_CACHE_OK_TO_ADD))
-   die ("Could not add %s to temporary index", name);
+   return error(_("make_cache_entry failed for path '%s'"),
+name);
+   if (add_index_entry(, ce, ADD_CACHE_OK_TO_ADD)) {
+   free(ce);
+   return error("Could not add %s to temporary index",
+name);
+   }
}
 
hold_lock_file_for_update(, filename, LOCK_DIE_ON_ERROR);
-   if (write_locked_index(, , COMMIT_LOCK))
-   die ("Could not write temporary index to %s", filename);
-
+   res = write_locked_index(, , COMMIT_LOCK);
discard_index();
+
+   if (res)
+   return error("Could not write temporary index to %s", filename);
+
+   return 0;
 }
 
 static void stat_patch_list(struct apply_state *state, struct patch *patch)
@@ -4475,8 +4483,11 @@ static int apply_patch(struct apply_state *state,
goto end;
}
 
-   if (state->fake_ancestor)
-   build_fake_ancestor(list, state->fake_ancestor);
+   if (state->fake_ancestor &&
+   build_fake_ancestor(list, state->fake_ancestor)) {
+   res = -1;
+   goto end;
+   }
 
if (state->diffstat)
stat_patch_list(state, list);
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 86/94] apply: add 'be_silent' variable to 'struct apply_state'

[Christian Couder]

This variable should prevent anything to be printed on both stderr
and stdout.

Let's not take care of stdout and apply_verbosely for now though,
as that will be taken care of in following patches.

Signed-off-by: Christian Couder 
---
 apply.c | 43 +--
 apply.h |  1 +
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/apply.c b/apply.c
index 7480ae8..f69a61a 100644
--- a/apply.c
+++ b/apply.c
@@ -1600,8 +1600,9 @@ static void record_ws_error(struct apply_state *state,
return;
 
err = whitespace_error_string(result);
-   fprintf(stderr, "%s:%d: %s.\n%.*s\n",
-   state->patch_input_file, linenr, err, len, line);
+   if (!state->be_silent)
+   fprintf(stderr, "%s:%d: %s.\n%.*s\n",
+   state->patch_input_file, linenr, err, len, line);
free(err);
 }
 
@@ -1796,7 +1797,7 @@ static int parse_single_patch(struct apply_state *state,
return error(_("new file %s depends on old contents"), 
patch->new_name);
if (0 < patch->is_delete && newlines)
return error(_("deleted file %s still has contents"), 
patch->old_name);
-   if (!patch->is_delete && !newlines && context)
+   if (!patch->is_delete && !newlines && context && !state->be_silent)
fprintf_ln(stderr,
   _("** warning: "
 "file %s becomes empty but is not deleted"),
@@ -3020,8 +3021,8 @@ static int apply_one_fragment(struct apply_state *state,
 * Warn if it was necessary to reduce the number
 * of context lines.
 */
-   if ((leading != frag->leading) ||
-   (trailing != frag->trailing))
+   if ((leading != frag->leading ||
+trailing != frag->trailing) && !state->be_silent)
fprintf_ln(stderr, _("Context reduced to (%ld/%ld)"
 " to apply fragment at %d"),
   leading, trailing, applied_pos+1);
@@ -3518,7 +3519,8 @@ static int try_threeway(struct apply_state *state,
 read_blob_object(, pre_sha1, patch->old_mode))
return error("repository lacks the necessary blob to fall back 
on 3-way merge.");
 
-   fprintf(stderr, "Falling back to three-way merge...\n");
+   if (!state->be_silent)
+   fprintf(stderr, "Falling back to three-way merge...\n");
 
img = strbuf_detach(, );
prepare_image(_image, img, len, 1);
@@ -3548,7 +3550,9 @@ static int try_threeway(struct apply_state *state,
status = three_way_merge(image, patch->new_name,
 pre_sha1, our_sha1, post_sha1);
if (status < 0) {
-   fprintf(stderr, "Failed to fall back on three-way merge...\n");
+   if (!state->be_silent)
+   fprintf(stderr,
+   "Failed to fall back on three-way merge...\n");
return status;
}
 
@@ -3560,9 +3564,15 @@ static int try_threeway(struct apply_state *state,
hashcpy(patch->threeway_stage[0].hash, pre_sha1);
hashcpy(patch->threeway_stage[1].hash, our_sha1);
hashcpy(patch->threeway_stage[2].hash, post_sha1);
-   fprintf(stderr, "Applied patch to '%s' with conflicts.\n", 
patch->new_name);
+   if (!state->be_silent)
+   fprintf(stderr,
+   "Applied patch to '%s' with conflicts.\n",
+   patch->new_name);
} else {
-   fprintf(stderr, "Applied patch to '%s' cleanly.\n", 
patch->new_name);
+   if (!state->be_silent)
+   fprintf(stderr,
+   "Applied patch to '%s' cleanly.\n",
+   patch->new_name);
}
return 0;
 }
@@ -4461,7 +4471,8 @@ static int write_out_one_reject(struct apply_state 
*state, struct patch *patch)
"Applying patch %%s with %d rejects...",
cnt),
cnt);
-   say_patch_name(stderr, sb.buf, patch);
+   if (!state->be_silent)
+   say_patch_name(stderr, sb.buf, patch);
strbuf_release();
 
cnt = strlen(patch->new_name);
@@ -4488,10 +4499,12 @@ static int write_out_one_reject(struct apply_state 
*state, struct patch *patch)
 frag;
 cnt++, frag = frag->next) {
if (!frag->rejected) {
-   fprintf_ln(stderr, _("Hunk #%d applied cleanly."), cnt);
+   if (!state->be_silent)
+   fprintf_ln(stderr, _("Hunk #%d applied 
cleanly."), cnt);
continue;
}
-   fprintf_ln(stderr, 

[PATCH v2 85/94] write_or_die: use warning() instead of fprintf(stderr, ...)

[Christian Couder]

Signed-off-by: Christian Couder 
---
 write_or_die.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/write_or_die.c b/write_or_die.c
index 49e80aa..c29f677 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -87,8 +87,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t 
count, const char *msg)
 {
if (write_in_full(fd, buf, count) < 0) {
check_pipe(errno);
-   fprintf(stderr, "%s: write error (%s)\n",
-   msg, strerror(errno));
+   warning("%s: write error (%s)\n", msg, strerror(errno));
return 0;
}
 
@@ -98,8 +97,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t 
count, const char *msg)
 int write_or_whine(int fd, const void *buf, size_t count, const char *msg)
 {
if (write_in_full(fd, buf, count) < 0) {
-   fprintf(stderr, "%s: write error (%s)\n",
-   msg, strerror(errno));
+   warning("%s: write error (%s)\n", msg, strerror(errno));
return 0;
}
 
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 92/94] am: use be_silent in 'struct apply_state' to shut up applying patches

[Christian Couder]

Signed-off-by: Christian Couder 
---
 builtin/am.c | 29 -
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index cc66a48..c158c4d 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1526,7 +1526,6 @@ static int run_apply(const struct am_state *state, const 
char *index_file)
struct argv_array apply_paths = ARGV_ARRAY_INIT;
struct argv_array apply_opts = ARGV_ARRAY_INIT;
struct apply_state apply_state;
-   int save_stdout_fd, save_stderr_fd;
int res, opts_left;
char *save_index_file;
static struct lock_file lock_file;
@@ -1560,18 +1559,6 @@ static int run_apply(const struct am_state *state, const 
char *index_file)
OPT_END()
};
 
-   /*
-* If we are allowed to fall back on 3-way merge, don't give false
-* errors during the initial attempt.
-*/
-
-   if (state->threeway && !index_file) {
-   save_stdout_fd = dup(1);
-   dup_devnull(1);
-   save_stderr_fd = dup(2);
-   dup_devnull(2);
-   }
-
if (index_file) {
save_index_file = get_index_file();
set_index_file((char *)index_file);
@@ -1594,6 +1581,14 @@ static int run_apply(const struct am_state *state, const 
char *index_file)
else
apply_state.check_index = 1;
 
+   /*
+* If we are allowed to fall back on 3-way merge, don't give false
+* errors during the initial attempt.
+*/
+
+   if (state->threeway && !index_file)
+   apply_state.be_silent = 1;
+
if (check_apply_state(_state, 0))
die("check_apply_state() failed");
 
@@ -1601,14 +1596,6 @@ static int run_apply(const struct am_state *state, const 
char *index_file)
 
res = apply_all_patches(_state, apply_paths.argc, 
apply_paths.argv, 0);
 
-   /* Restore stdout and stderr */
-   if (state->threeway && !index_file) {
-   dup2(save_stdout_fd, 1);
-   close(save_stdout_fd);
-   dup2(save_stderr_fd, 2);
-   close(save_stderr_fd);
-   }
-
if (index_file)
set_index_file(save_index_file);
 
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 67/94] builtin/apply: change die_on_unsafe_path() to check_unsafe_path()

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.

To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", die_on_unsafe_path() should return -1 using
error() instead of calling die(), so while doing that let's change
its name to check_unsafe_path().

Signed-off-by: Christian Couder 
---
 builtin/apply.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 42b0a24..06c1c16 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3698,7 +3698,7 @@ static int path_is_beyond_symlink(struct apply_state 
*state, const char *name_)
return ret;
 }
 
-static void die_on_unsafe_path(struct patch *patch)
+static int check_unsafe_path(struct patch *patch)
 {
const char *old_name = NULL;
const char *new_name = NULL;
@@ -3710,9 +3710,10 @@ static void die_on_unsafe_path(struct patch *patch)
new_name = patch->new_name;
 
if (old_name && !verify_path(old_name))
-   die(_("invalid path '%s'"), old_name);
+   return error(_("invalid path '%s'"), old_name);
if (new_name && !verify_path(new_name))
-   die(_("invalid path '%s'"), new_name);
+   return error(_("invalid path '%s'"), new_name);
+   return 0;
 }
 
 /*
@@ -3802,8 +3803,8 @@ static int check_patch(struct apply_state *state, struct 
patch *patch)
}
}
 
-   if (!state->unsafe_paths)
-   die_on_unsafe_path(patch);
+   if (!state->unsafe_paths && check_unsafe_path(patch))
+   return -1;
 
/*
 * An attempt to read from or delete a path that is beyond a
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 70/94] builtin/apply: make add_conflicted_stages_file() return -1 on error

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.

To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", add_conflicted_stages_file() should return -1
using error() instead of calling die().

Helped-by: Eric Sunshine 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 52f36c2..ca3502f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4214,7 +4214,7 @@ static void create_one_file(struct apply_state *state,
die_errno(_("unable to write file '%s' mode %o"), path, mode);
 }
 
-static void add_conflicted_stages_file(struct apply_state *state,
+static int add_conflicted_stages_file(struct apply_state *state,
   struct patch *patch)
 {
int stage, namelen;
@@ -4222,7 +4222,7 @@ static void add_conflicted_stages_file(struct apply_state 
*state,
struct cache_entry *ce;
 
if (!state->update_index)
-   return;
+   return 0;
namelen = strlen(patch->new_name);
ce_size = cache_entry_size(namelen);
mode = patch->new_mode ? patch->new_mode : (S_IFREG | 0644);
@@ -4237,9 +4237,14 @@ static void add_conflicted_stages_file(struct 
apply_state *state,
ce->ce_flags = create_ce_flags(stage);
ce->ce_namelen = namelen;
hashcpy(ce->sha1, patch->threeway_stage[stage - 1].hash);
-   if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)
-   die(_("unable to add cache entry for %s"), 
patch->new_name);
+   if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
+   free(ce);
+   return error(_("unable to add cache entry for %s"),
+patch->new_name);
+   }
}
+
+   return 0;
 }
 
 static void create_file(struct apply_state *state, struct patch *patch)
@@ -4253,9 +4258,10 @@ static void create_file(struct apply_state *state, 
struct patch *patch)
mode = S_IFREG | 0644;
create_one_file(state, path, mode, buf, size);
 
-   if (patch->conflicted_threeway)
-   add_conflicted_stages_file(state, patch);
-   else
+   if (patch->conflicted_threeway) {
+   if (add_conflicted_stages_file(state, patch))
+   exit(1);
+   } else
add_index_file(state, path, mode, buf, size);
 }
 
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 91/94] apply: change error_routine when be_silent is set

[Christian Couder]

Signed-off-by: Christian Couder 
---
 apply.c | 29 +
 apply.h |  3 +++
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/apply.c b/apply.c
index e0fdd1d..1dafc82 100644
--- a/apply.c
+++ b/apply.c
@@ -100,6 +100,11 @@ int init_apply_state(struct apply_state *state,
return 0;
 }
 
+static void mute_routine(const char *bla, va_list params)
+{
+   /* do nothing */
+}
+
 int check_apply_state(struct apply_state *state, int force_apply)
 {
int is_not_gitdir = !startup_info->have_repository;
@@ -132,6 +137,13 @@ int check_apply_state(struct apply_state *state, int 
force_apply)
if (state->be_silent && state->apply_verbosely)
return error(_("incompatible internal 'be_silent' and 
'apply_verbosely' flags"));
 
+   if (state->be_silent) {
+   state->saved_error_routine = get_error_routine();
+   state->saved_warn_routine = get_warn_routine();
+   set_error_routine(mute_routine);
+   set_warn_routine(mute_routine);
+   }
+
return 0;
 }
 
@@ -4750,6 +4762,7 @@ int apply_all_patches(struct apply_state *state,
 {
int i;
int res;
+   int retval = -1;
int errs = 0;
int read_stdin = 1;
 
@@ -4822,17 +4835,25 @@ int apply_all_patches(struct apply_state *state,
if (state->update_index) {
res = write_locked_index(_index, state->lock_file, 
COMMIT_LOCK);
state->newfd = -1;
-   if (res)
-   return error(_("Unable to write new index file"));
+   if (res) {
+   error(_("Unable to write new index file"));
+   goto rollback_end;
+   }
}
 
-   return !!errs;
+   retval = !!errs;
 
 rollback_end:
if (state->newfd >= 0) {
rollback_lock_file(state->lock_file);
state->newfd = -1;
}
-   return -1;
+
+   if (state->be_silent) {
+   set_error_routine(state->saved_error_routine);
+   set_warn_routine(state->saved_warn_routine);
+   }
+
+   return retval;
 }
 
diff --git a/apply.h b/apply.h
index 2dd3706..029b79f 100644
--- a/apply.h
+++ b/apply.h
@@ -46,6 +46,9 @@ struct apply_state {
int apply_verbosely;
int be_silent;
 
+   void (*saved_error_routine)(const char *err, va_list params);
+   void (*saved_warn_routine)(const char *warn, va_list params);
+
/* --cached updates only the cache without ever touching the working 
tree. */
int cached;
 
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 89/94] usage: add set_warn_routine()

[Christian Couder]

There are already set_die_routine() and set_error_routine(),
so let's add set_warn_routine() as this will be needed in a
following commit.

Signed-off-by: Christian Couder 
---
 git-compat-util.h | 1 +
 usage.c   | 5 +
 2 files changed, 6 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 1f8b5f3..987eb99 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -438,6 +438,7 @@ static inline int const_error(void)
 
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, 
va_list params));
 extern void set_error_routine(void (*routine)(const char *err, va_list 
params));
+extern void set_warn_routine(void (*routine)(const char *warn, va_list 
params));
 extern void set_die_is_recursing_routine(int (*routine)(void));
 extern void set_error_handle(FILE *);
 
diff --git a/usage.c b/usage.c
index 82ff131..8fbedb3 100644
--- a/usage.c
+++ b/usage.c
@@ -70,6 +70,11 @@ void set_error_routine(void (*routine)(const char *err, 
va_list params))
error_routine = routine;
 }
 
+void set_warn_routine(void (*routine)(const char *warn, va_list params))
+{
+   warn_routine = routine;
+}
+
 void set_die_is_recursing_routine(int (*routine)(void))
 {
die_is_recursing = routine;
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 77/94] builtin/apply: rename option parsing functions

[Christian Couder]

As these functions are going to be part of the libified
apply api, let's give them a name that is more specific
to the apply api.

Signed-off-by: Christian Couder 
---
 builtin/apply.c | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 8c31617..f05dc96 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4565,16 +4565,16 @@ end:
return res;
 }
 
-static int option_parse_exclude(const struct option *opt,
-   const char *arg, int unset)
+static int apply_option_parse_exclude(const struct option *opt,
+ const char *arg, int unset)
 {
struct apply_state *state = opt->value;
add_name_limit(state, arg, 1);
return 0;
 }
 
-static int option_parse_include(const struct option *opt,
-   const char *arg, int unset)
+static int apply_option_parse_include(const struct option *opt,
+ const char *arg, int unset)
 {
struct apply_state *state = opt->value;
add_name_limit(state, arg, 0);
@@ -4582,9 +4582,9 @@ static int option_parse_include(const struct option *opt,
return 0;
 }
 
-static int option_parse_p(const struct option *opt,
- const char *arg,
- int unset)
+static int apply_option_parse_p(const struct option *opt,
+   const char *arg,
+   int unset)
 {
struct apply_state *state = opt->value;
state->p_value = atoi(arg);
@@ -4592,8 +4592,8 @@ static int option_parse_p(const struct option *opt,
return 0;
 }
 
-static int option_parse_space_change(const struct option *opt,
-const char *arg, int unset)
+static int apply_option_parse_space_change(const struct option *opt,
+  const char *arg, int unset)
 {
struct apply_state *state = opt->value;
if (unset)
@@ -4603,8 +4603,8 @@ static int option_parse_space_change(const struct option 
*opt,
return 0;
 }
 
-static int option_parse_whitespace(const struct option *opt,
-  const char *arg, int unset)
+static int apply_option_parse_whitespace(const struct option *opt,
+const char *arg, int unset)
 {
struct apply_state *state = opt->value;
state->whitespace_option = arg;
@@ -4613,8 +4613,8 @@ static int option_parse_whitespace(const struct option 
*opt,
return 0;
 }
 
-static int option_parse_directory(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_directory(const struct option *opt,
+   const char *arg, int unset)
 {
struct apply_state *state = opt->value;
strbuf_reset(>root);
@@ -4714,13 +4714,13 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix)
struct option builtin_apply_options[] = {
{ OPTION_CALLBACK, 0, "exclude", , N_("path"),
N_("don't apply changes matching the given path"),
-   0, option_parse_exclude },
+   0, apply_option_parse_exclude },
{ OPTION_CALLBACK, 0, "include", , N_("path"),
N_("apply changes matching the given path"),
-   0, option_parse_include },
+   0, apply_option_parse_include },
{ OPTION_CALLBACK, 'p', NULL, , N_("num"),
N_("remove  leading slashes from traditional diff 
paths"),
-   0, option_parse_p },
+   0, apply_option_parse_p },
OPT_BOOL(0, "no-add", _add,
N_("ignore additions made by the patch")),
OPT_BOOL(0, "stat", ,
@@ -4752,13 +4752,13 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix)
N_("ensure at least  lines of context 
match")),
{ OPTION_CALLBACK, 0, "whitespace", , N_("action"),
N_("detect new or modified lines that have whitespace 
errors"),
-   0, option_parse_whitespace },
+   0, apply_option_parse_whitespace },
{ OPTION_CALLBACK, 0, "ignore-space-change", , NULL,
N_("ignore changes in whitespace when finding context"),
-   PARSE_OPT_NOARG, option_parse_space_change },
+   PARSE_OPT_NOARG, apply_option_parse_space_change },
{ OPTION_CALLBACK, 0, "ignore-whitespace", , NULL,
N_("ignore changes in whitespace when finding context"),
-   PARSE_OPT_NOARG, option_parse_space_change },
+   

[PATCH v2 94/94] builtin/apply: add a cli option for be_silent

[Christian Couder]

Let's make it possible to request a silent operation on the
command line.

Signed-off-by: Christian Couder 
---
 builtin/apply.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/apply.c b/builtin/apply.c
index ce12769..397ef26 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -70,6 +70,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix)
OPT_BOOL(0, "allow-overlap", _overlap,
N_("allow overlapping hunks")),
OPT__VERBOSE(_verbosely, N_("be verbose")),
+   OPT_BOOL(0, "silent", _silent,
+   N_("do not print any output")),
OPT_BIT(0, "inaccurate-eof", ,
N_("tolerate incorrectly detected missing new-line at 
the end of file"),
APPLY_OPT_INACCURATE_EOF),
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 73/94] builtin/apply: make write_out_one_result() return -1 on error

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.

To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", write_out_one_result() should just return what
remove_file() and create_file() are returning instead of calling
exit().

Signed-off-by: Christian Couder 
---
 builtin/apply.c | 38 --
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 2b562db..f06bf16 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4277,36 +4277,29 @@ static int create_file(struct apply_state *state, 
struct patch *patch)
 }
 
 /* phase zero is to remove, phase one is to create */
-static void write_out_one_result(struct apply_state *state,
-struct patch *patch,
-int phase)
+static int write_out_one_result(struct apply_state *state,
+   struct patch *patch,
+   int phase)
 {
if (patch->is_delete > 0) {
-   if (phase == 0) {
-   if (remove_file(state, patch, 1))
-   exit(1);
-   }
-   return;
+   if (phase == 0)
+   return remove_file(state, patch, 1);
+   return 0;
}
if (patch->is_new > 0 || patch->is_copy) {
-   if (phase == 1) {
-   if (create_file(state, patch))
-   exit(1);
-   }
-   return;
+   if (phase == 1)
+   return create_file(state, patch);
+   return 0;
}
/*
 * Rename or modification boils down to the same
 * thing: remove the old, write the new
 */
-   if (phase == 0) {
-   if (remove_file(state, patch, patch->is_rename))
-   exit(1);
-   }
-   if (phase == 1) {
-   if (create_file(state, patch))
-   exit(1);
-   }
+   if (phase == 0)
+   return remove_file(state, patch, patch->is_rename);
+   if (phase == 1)
+   return create_file(state, patch);
+   return 0;
 }
 
 static int write_out_one_reject(struct apply_state *state, struct patch *patch)
@@ -4393,7 +4386,8 @@ static int write_out_results(struct apply_state *state, 
struct patch *list)
if (l->rejected)
errs = 1;
else {
-   write_out_one_result(state, l, phase);
+   if (write_out_one_result(state, l, phase))
+   exit(1);
if (phase == 1) {
if (write_out_one_reject(state, l))
errs = 1;
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 53/94] builtin/apply: make find_header() return -1 instead of die()ing

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.

To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, find_header() should return -1 using error()
instead of calling die().

Unfortunately find_header() already returns -1 when no header is found,
so let's make it return -2 instead in this case.

Signed-off-by: Christian Couder 
---
 builtin/apply.c   | 33 ++---
 t/t4254-am-corrupt.sh |  2 +-
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index a166d70..4212705 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1529,6 +1529,14 @@ static int parse_fragment_header(const char *line, int 
len, struct fragment *fra
return offset;
 }
 
+/*
+ * Find file diff header
+ *
+ * Returns:
+ *  -1 in case of error
+ *  -2 if no header was found
+ *   the size of the header in bytes (called "offset") otherwise
+ */
 static int find_header(struct apply_state *state,
   const char *line,
   unsigned long size,
@@ -1562,8 +1570,8 @@ static int find_header(struct apply_state *state,
struct fragment dummy;
if (parse_fragment_header(line, len, ) < 0)
continue;
-   die(_("patch fragment without header at line %d: %.*s"),
-   state->linenr, (int)len-1, line);
+   return error(_("patch fragment without header at line 
%d: %.*s"),
+state->linenr, (int)len-1, line);
}
 
if (size < len + 6)
@@ -1579,18 +1587,18 @@ static int find_header(struct apply_state *state,
continue;
if (!patch->old_name && !patch->new_name) {
if (!patch->def_name)
-   die(Q_("git diff header lacks filename 
information when removing "
-  "%d leading pathname component 
(line %d)",
-  "git diff header lacks filename 
information when removing "
-  "%d leading pathname components 
(line %d)",
-  state->p_value),
-   state->p_value, state->linenr);
+   return error(Q_("git diff header lacks 
filename information when removing "
+   "%d leading pathname 
component (line %d)",
+   "git diff header lacks 
filename information when removing "
+   "%d leading pathname 
components (line %d)",
+   state->p_value),
+state->p_value, 
state->linenr);
patch->old_name = xstrdup(patch->def_name);
patch->new_name = xstrdup(patch->def_name);
}
if (!patch->is_delete && !patch->new_name)
-   die("git diff header lacks filename information 
"
-   "(line %d)", state->linenr);
+   return error("git diff header lacks filename 
information "
+"(line %d)", state->linenr);
patch->is_toplevel_relative = 1;
*hdrsize = git_hdr_len;
return offset;
@@ -1615,7 +1623,7 @@ static int find_header(struct apply_state *state,
state->linenr += 2;
return offset;
}
-   return -1;
+   return -2;
 }
 
 static void record_ws_error(struct apply_state *state,
@@ -2106,6 +2114,9 @@ static int parse_chunk(struct apply_state *state, char 
*buffer, unsigned long si
int hdrsize, patchsize;
int offset = find_header(state, buffer, size, , patch);
 
+   if (offset == -1)
+   exit(1);
+
if (offset < 0)
return offset;
 
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
index 85716dd..9bd7dd2 100755
--- a/t/t4254-am-corrupt.sh
+++ b/t/t4254-am-corrupt.sh
@@ -29,7 +29,7 @@ test_expect_success 'try to apply corrupted patch' '
 '
 
 test_expect_success 'compare diagnostic; ensure file is still here' '
-   echo "fatal: git diff header lacks filename information (line 4)" 
>expected &&
+   echo "error: git diff header lacks filename information (line 4)" 
>expected &&
test_path_is_file f &&
test_cmp expected actual
 '
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe 

[PATCH v2 87/94] apply: make 'be_silent' incomatible with 'apply_verbosely'

[Christian Couder]

It should be an error to have both be_silent and apply_verbosely set,
so let's check that in check_apply_state().

And by the way let's not automatically set apply_verbosely when
be_silent is set.

Signed-off-by: Christian Couder 
---
 apply.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/apply.c b/apply.c
index f69a61a..5459ee1 100644
--- a/apply.c
+++ b/apply.c
@@ -113,8 +113,11 @@ int check_apply_state(struct apply_state *state, int 
force_apply)
return error(_("--3way outside a repository"));
state->check_index = 1;
}
-   if (state->apply_with_reject)
-   state->apply = state->apply_verbosely = 1;
+   if (state->apply_with_reject) {
+   state->apply = 1;
+   if (!state->be_silent)
+   state->apply_verbosely = 1;
+   }
if (!force_apply && (state->diffstat || state->numstat || 
state->summary || state->check || state->fake_ancestor))
state->apply = 0;
if (state->check_index && is_not_gitdir)
@@ -126,6 +129,9 @@ int check_apply_state(struct apply_state *state, int 
force_apply)
}
if (state->check_index)
state->unsafe_paths = 0;
+   if (state->be_silent && state->apply_verbosely)
+   return error(_("incompatible internal 'be_silent' and 
'apply_verbosely' flags"));
+
return 0;
 }
 
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 90/94] usage: add get_error_routine() and get_warn_routine()

[Christian Couder]

Let's make it possible to get the current error_routine and warn_routine,
so that we can store them before using set_error_routine() or
set_warn_routine() to use new ones.

This way we will be able put back the original routines, when we are done
with using new ones.

Signed-off-by: Christian Couder 
---
 git-compat-util.h |  2 ++
 usage.c   | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 987eb99..73446af 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -438,7 +438,9 @@ static inline int const_error(void)
 
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, 
va_list params));
 extern void set_error_routine(void (*routine)(const char *err, va_list 
params));
+extern void (*get_error_routine(void))(const char *err, va_list params);
 extern void set_warn_routine(void (*routine)(const char *warn, va_list 
params));
+extern void (*get_warn_routine(void))(const char *warn, va_list params);
 extern void set_die_is_recursing_routine(int (*routine)(void));
 extern void set_error_handle(FILE *);
 
diff --git a/usage.c b/usage.c
index 8fbedb3..825bd92 100644
--- a/usage.c
+++ b/usage.c
@@ -70,11 +70,21 @@ void set_error_routine(void (*routine)(const char *err, 
va_list params))
error_routine = routine;
 }
 
+void (*get_error_routine(void))(const char *err, va_list params)
+{
+   return error_routine;
+}
+
 void set_warn_routine(void (*routine)(const char *warn, va_list params))
 {
warn_routine = routine;
 }
 
+void (*get_warn_routine(void))(const char *warn, va_list params)
+{
+   return warn_routine;
+}
+
 void set_die_is_recursing_routine(int (*routine)(void))
 {
die_is_recursing = routine;
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 50/94] builtin/apply: move 'newfd' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'newfd' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Signed-off-by: Christian Couder 
---
 builtin/apply.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index cad2c56..ec55768 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -57,6 +57,7 @@ struct apply_state {
 * lock_file structures, it isn't safe to free(lock_file).
 */
struct lock_file *lock_file;
+   int newfd;
 
int apply;
int allow_overlap;
@@ -131,8 +132,6 @@ struct apply_state {
enum ws_ignore ws_ignore_action;
 };
 
-static int newfd = -1;
-
 static const char * const apply_usage[] = {
N_("git apply [] [...]"),
NULL
@@ -4561,8 +4560,8 @@ static int apply_patch(struct apply_state *state,
state->apply = 0;
 
state->update_index = state->check_index && state->apply;
-   if (state->update_index && newfd < 0)
-   newfd = hold_locked_index(state->lock_file, 1);
+   if (state->update_index && state->newfd < 0)
+   state->newfd = hold_locked_index(state->lock_file, 1);
 
if (state->check_index) {
if (read_cache() < 0)
@@ -4671,6 +4670,7 @@ static void init_apply_state(struct apply_state *state,
state->prefix = prefix;
state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
state->lock_file = lock_file ? lock_file : xcalloc(1, 
sizeof(*lock_file));
+   state->newfd = -1;
state->apply = 1;
state->line_termination = '\n';
state->p_value = 1;
@@ -4780,6 +4780,7 @@ static int apply_all_patches(struct apply_state *state,
if (state->update_index) {
if (write_locked_index(_index, state->lock_file, 
COMMIT_LOCK))
die(_("Unable to write new index file"));
+   state->newfd = -1;
}
 
return !!errs;
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 84/94] builtin/am: use apply api in run_apply()

[Christian Couder]

This replaces run_apply() implementation with a new one that
uses the apply api that has been previously prepared in
apply.c and apply.h.

This shoud improve performance a lot in certain cases.

As the previous implementation was creating a new `git apply`
process to apply each patch, it could be slow on systems like
Windows where it is costly to create new processes.

Also the new `git apply` process had to read the index from
disk, and when the process was done the calling process
discarded its own index and read back from disk the new
index that had been created by the `git apply` process.

This could be very inefficient with big repositories that
have big index files, especially when the system decided
that it was a good idea to run the `git apply` processes on
a different processor core.

Also eliminating index reads enables further performance
improvements by using:

`git update-index --split-index`

For example here is a benchmark of a multi hundred commit
rebase on the Linux kernel on a Debian laptop with SSD:

command: git rebase --onto 1993b17 52bef0c 29dde7c

Vanilla "next" without split index:1m54.953s
Vanilla "next" with split index:   1m22.476s
This series on top of "next" without split index:  1m12.034s
This series on top of "next" with split index: 0m15.678s

(using branch "next" from mid April 2016.)

Benchmarked-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Christian Couder 
---
 builtin/am.c | 104 ---
 1 file changed, 86 insertions(+), 18 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index d003939..cc66a48 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -28,6 +28,7 @@
 #include "rerere.h"
 #include "prompt.h"
 #include "mailinfo.h"
+#include "apply.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -1522,39 +1523,106 @@ static int parse_mail_rebase(struct am_state *state, 
const char *mail)
  */
 static int run_apply(const struct am_state *state, const char *index_file)
 {
-   struct child_process cp = CHILD_PROCESS_INIT;
-
-   cp.git_cmd = 1;
-
-   if (index_file)
-   argv_array_pushf(_array, "GIT_INDEX_FILE=%s", 
index_file);
+   struct argv_array apply_paths = ARGV_ARRAY_INIT;
+   struct argv_array apply_opts = ARGV_ARRAY_INIT;
+   struct apply_state apply_state;
+   int save_stdout_fd, save_stderr_fd;
+   int res, opts_left;
+   char *save_index_file;
+   static struct lock_file lock_file;
+
+   struct option am_apply_options[] = {
+   { OPTION_CALLBACK, 0, "whitespace", _state, N_("action"),
+   N_("detect new or modified lines that have whitespace 
errors"),
+   0, apply_option_parse_whitespace },
+   { OPTION_CALLBACK, 0, "ignore-space-change", _state, NULL,
+   N_("ignore changes in whitespace when finding context"),
+   PARSE_OPT_NOARG, apply_option_parse_space_change },
+   { OPTION_CALLBACK, 0, "ignore-whitespace", _state, NULL,
+   N_("ignore changes in whitespace when finding context"),
+   PARSE_OPT_NOARG, apply_option_parse_space_change },
+   { OPTION_CALLBACK, 0, "directory", _state, N_("root"),
+   N_("prepend  to all filenames"),
+   0, apply_option_parse_directory },
+   { OPTION_CALLBACK, 0, "exclude", _state, N_("path"),
+   N_("don't apply changes matching the given path"),
+   0, apply_option_parse_exclude },
+   { OPTION_CALLBACK, 0, "include", _state, N_("path"),
+   N_("apply changes matching the given path"),
+   0, apply_option_parse_include },
+   OPT_INTEGER('C', NULL, _state.p_context,
+   N_("ensure at least  lines of context 
match")),
+   { OPTION_CALLBACK, 'p', NULL, _state, N_("num"),
+   N_("remove  leading slashes from traditional diff 
paths"),
+   0, apply_option_parse_p },
+   OPT_BOOL(0, "reject", _state.apply_with_reject,
+   N_("leave the rejected hunks in corresponding *.rej 
files")),
+   OPT_END()
+   };
 
/*
 * If we are allowed to fall back on 3-way merge, don't give false
 * errors during the initial attempt.
 */
+
if (state->threeway && !index_file) {
-   cp.no_stdout = 1;
-   cp.no_stderr = 1;
+   save_stdout_fd = dup(1);
+   dup_devnull(1);
+   save_stderr_fd = dup(2);
+   dup_devnull(2);
}
 
-   argv_array_push(, "apply");
+   if (index_file) {
+   save_index_file = get_index_file();
+   set_index_file((char 

[PATCH v2 55/94] builtin/apply: make parse_single_patch() return -1 on error

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.

To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, parse_single_patch() should return -1 instead of
calling die().

Let's do that by using error() and let's adjust the related test
cases accordingly.

Signed-off-by: Christian Couder 
---
 builtin/apply.c| 17 +
 t/t4012-diff-binary.sh |  4 ++--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 2380472..58bcfeb 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1776,6 +1776,10 @@ static int parse_fragment(struct apply_state *state,
  *
  * The (fragment->patch, fragment->size) pair points into the memory given
  * by the caller, not a copy, when we return.
+ *
+ * Returns:
+ *   -1 in case of error,
+ *   the number of bytes in the patch otherwise.
  */
 static int parse_single_patch(struct apply_state *state,
  const char *line,
@@ -1793,8 +1797,10 @@ static int parse_single_patch(struct apply_state *state,
fragment = xcalloc(1, sizeof(*fragment));
fragment->linenr = state->linenr;
len = parse_fragment(state, line, size, patch, fragment);
-   if (len <= 0)
-   die(_("corrupt patch at line %d"), state->linenr);
+   if (len <= 0) {
+   free(fragment);
+   return error(_("corrupt patch at line %d"), 
state->linenr);
+   }
fragment->patch = line;
fragment->size = len;
oldlines += fragment->oldlines;
@@ -1830,9 +1836,9 @@ static int parse_single_patch(struct apply_state *state,
patch->is_delete = 0;
 
if (0 < patch->is_new && oldlines)
-   die(_("new file %s depends on old contents"), patch->new_name);
+   return error(_("new file %s depends on old contents"), 
patch->new_name);
if (0 < patch->is_delete && newlines)
-   die(_("deleted file %s still has contents"), patch->old_name);
+   return error(_("deleted file %s still has contents"), 
patch->old_name);
if (!patch->is_delete && !newlines && context)
fprintf_ln(stderr,
   _("** warning: "
@@ -2134,6 +2140,9 @@ static int parse_chunk(struct apply_state *state, char 
*buffer, unsigned long si
   size - offset - hdrsize,
   patch);
 
+   if (patchsize < 0)
+   return -1;
+
if (!patchsize) {
static const char git_binary[] = "GIT binary patch\n";
int hd = hdrsize + offset;
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 643d729..0a8af76 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -68,7 +68,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt 
patch correctly' '
sed -e "s/-CIT/xCIT/" broken &&
test_must_fail git apply --stat --summary broken 2>detected &&
detected=$(cat detected) &&
-   detected=$(expr "$detected" : "fatal.*at line \\([0-9]*\\)\$") &&
+   detected=$(expr "$detected" : "error.*at line \\([0-9]*\\)\$") &&
detected=$(sed -ne "${detected}p" broken) &&
test "$detected" = xCIT
 '
@@ -77,7 +77,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt 
patch correctly' '
git diff --binary | sed -e "s/-CIT/xCIT/" >broken &&
test_must_fail git apply --stat --summary broken 2>detected &&
detected=$(cat detected) &&
-   detected=$(expr "$detected" : "fatal.*at line \\([0-9]*\\)\$") &&
+   detected=$(expr "$detected" : "error.*at line \\([0-9]*\\)\$") &&
detected=$(sed -ne "${detected}p" broken) &&
test "$detected" = xCIT
 '
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 45/94] builtin/apply: move 'symlink_changes' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'symlink_changes' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 49 +++--
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 506e9ec..14286d2 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -34,6 +34,20 @@ enum ws_ignore {
ignore_ws_change
 };
 
+/*
+ * We need to keep track of how symlinks in the preimage are
+ * manipulated by the patches.  A patch to add a/b/c where a/b
+ * is a symlink should not be allowed to affect the directory
+ * the symlink points at, but if the same patch removes a/b,
+ * it is perfectly fine, as the patch removes a/b to make room
+ * to create a directory a/b so that a/b/c can be created.
+ *
+ * See also "struct string_list symlink_changes" in "struct
+ * apply_state".
+ */
+#define SYMLINK_GOES_AWAY 01
+#define SYMLINK_IN_RESULT 02
+
 struct apply_state {
const char *prefix;
int prefix_length;
@@ -61,6 +75,7 @@ struct apply_state {
struct string_list limit_by_name;
int has_include;
struct strbuf root;
+   struct string_list symlink_changes;
 
/*
 *  --check turns on checking that the working tree matches the
@@ -3713,52 +3728,42 @@ static int check_to_create(struct apply_state *state,
return 0;
 }
 
-/*
- * We need to keep track of how symlinks in the preimage are
- * manipulated by the patches.  A patch to add a/b/c where a/b
- * is a symlink should not be allowed to affect the directory
- * the symlink points at, but if the same patch removes a/b,
- * it is perfectly fine, as the patch removes a/b to make room
- * to create a directory a/b so that a/b/c can be created.
- */
-static struct string_list symlink_changes;
-#define SYMLINK_GOES_AWAY 01
-#define SYMLINK_IN_RESULT 02
-
-static uintptr_t register_symlink_changes(const char *path, uintptr_t what)
+static uintptr_t register_symlink_changes(struct apply_state *state,
+ const char *path,
+ uintptr_t what)
 {
struct string_list_item *ent;
 
-   ent = string_list_lookup(_changes, path);
+   ent = string_list_lookup(>symlink_changes, path);
if (!ent) {
-   ent = string_list_insert(_changes, path);
+   ent = string_list_insert(>symlink_changes, path);
ent->util = (void *)0;
}
ent->util = (void *)(what | ((uintptr_t)ent->util));
return (uintptr_t)ent->util;
 }
 
-static uintptr_t check_symlink_changes(const char *path)
+static uintptr_t check_symlink_changes(struct apply_state *state, const char 
*path)
 {
struct string_list_item *ent;
 
-   ent = string_list_lookup(_changes, path);
+   ent = string_list_lookup(>symlink_changes, path);
if (!ent)
return 0;
return (uintptr_t)ent->util;
 }
 
-static void prepare_symlink_changes(struct patch *patch)
+static void prepare_symlink_changes(struct apply_state *state, struct patch 
*patch)
 {
for ( ; patch; patch = patch->next) {
if ((patch->old_name && S_ISLNK(patch->old_mode)) &&
(patch->is_rename || patch->is_delete))
/* the symlink at patch->old_name is removed */
-   register_symlink_changes(patch->old_name, 
SYMLINK_GOES_AWAY);
+   register_symlink_changes(state, patch->old_name, 
SYMLINK_GOES_AWAY);
 
if (patch->new_name && S_ISLNK(patch->new_mode))
/* the symlink at patch->new_name is created or remains 
*/
-   register_symlink_changes(patch->new_name, 
SYMLINK_IN_RESULT);
+   register_symlink_changes(state, patch->new_name, 
SYMLINK_IN_RESULT);
}
 }
 
@@ -3772,7 +3777,7 @@ static int path_is_beyond_symlink_1(struct apply_state 
*state, struct strbuf *na
if (!name->len)
break;
name->buf[name->len] = '\0';
-   change = check_symlink_changes(name->buf);
+   change = check_symlink_changes(state, name->buf);
if (change & SYMLINK_IN_RESULT)
return 1;
if (change & SYMLINK_GOES_AWAY)
@@ -3941,7 +3946,7 @@ static int check_patch_list(struct apply_state *state, 
struct patch *patch)
 {
int err = 0;
 
-   prepare_symlink_changes(patch);
+   prepare_symlink_changes(state, patch);
prepare_fn_table(state, patch);
while (patch) {
if (state->apply_verbosely)
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo 

[PATCH v2 81/94] run-command: make dup_devnull() non static

[Christian Couder]

We will need this function in a later commit to redirect stdout
and stderr to /dev/null.

Helped-by: Johannes Sixt 
Helped-by: Johannes Schindelin 
Signed-off-by: Christian Couder 
---
 run-command.c | 2 +-
 run-command.h | 6 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index e4593cd..5d1cedf 100644
--- a/run-command.c
+++ b/run-command.c
@@ -85,7 +85,7 @@ static inline void close_pair(int fd[2])
 }
 
 #ifndef GIT_WINDOWS_NATIVE
-static inline void dup_devnull(int to)
+void dup_devnull(int to)
 {
int fd = open("/dev/null", O_RDWR);
if (fd < 0)
diff --git a/run-command.h b/run-command.h
index 11f76b0..e05ce7d 100644
--- a/run-command.h
+++ b/run-command.h
@@ -201,4 +201,10 @@ int run_processes_parallel(int n,
   task_finished_fn,
   void *pp_cb);
 
+/**
+ * Misc helper functions
+ */
+
+void dup_devnull(int to);
+
 #endif
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 66/94] builtin/apply: make gitdiff_*() return -1 on error

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.

To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", gitdiff_*() functions should return -1 using
error() instead of calling die().

A previous patch made it possible for gitdiff_*() functions to
return -1 in case of error. Let's take advantage of that to
make gitdiff_verify_name() return -1 on error, and to have
gitdiff_oldname() and gitdiff_newname() directly return
what gitdiff_verify_name() returns.

Helped-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 40 +---
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index b3a9c2e..42b0a24 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -827,54 +827,56 @@ static int gitdiff_hdrend(struct apply_state *state,
 #define DIFF_OLD_NAME 0
 #define DIFF_NEW_NAME 1
 
-static void gitdiff_verify_name(struct apply_state *state,
-   const char *line,
-   int isnull,
-   char **name,
-   int side)
+static int gitdiff_verify_name(struct apply_state *state,
+  const char *line,
+  int isnull,
+  char **name,
+  int side)
 {
if (!*name && !isnull) {
*name = find_name(state, line, NULL, state->p_value, TERM_TAB);
-   return;
+   return 0;
}
 
if (*name) {
int len = strlen(*name);
char *another;
if (isnull)
-   die(_("git apply: bad git-diff - expected /dev/null, 
got %s on line %d"),
-   *name, state->linenr);
+   return error(_("git apply: bad git-diff - expected 
/dev/null, got %s on line %d"),
+*name, state->linenr);
another = find_name(state, line, NULL, state->p_value, 
TERM_TAB);
-   if (!another || memcmp(another, *name, len + 1))
-   die((side == DIFF_NEW_NAME) ?
+   if (!another || memcmp(another, *name, len + 1)) {
+   free(another);
+   return error((side == DIFF_NEW_NAME) ?
_("git apply: bad git-diff - inconsistent new 
filename on line %d") :
_("git apply: bad git-diff - inconsistent old 
filename on line %d"), state->linenr);
+   }
free(another);
} else {
/* expect "/dev/null" */
if (memcmp("/dev/null", line, 9) || line[9] != '\n')
-   die(_("git apply: bad git-diff - expected /dev/null on 
line %d"), state->linenr);
+   return error(_("git apply: bad git-diff - expected 
/dev/null on line %d"), state->linenr);
}
+
+   return 0;
 }
 
 static int gitdiff_oldname(struct apply_state *state,
   const char *line,
   struct patch *patch)
 {
-   gitdiff_verify_name(state, line,
-   patch->is_new, >old_name,
-   DIFF_OLD_NAME);
-   return 0;
+   return gitdiff_verify_name(state, line,
+  patch->is_new, >old_name,
+  DIFF_OLD_NAME);
 }
 
 static int gitdiff_newname(struct apply_state *state,
   const char *line,
   struct patch *patch)
 {
-   gitdiff_verify_name(state, line,
-   patch->is_delete, >new_name,
-   DIFF_NEW_NAME);
-   return 0;
+   return gitdiff_verify_name(state, line,
+  patch->is_delete, >new_name,
+  DIFF_NEW_NAME);
 }
 
 static int gitdiff_oldmode(struct apply_state *state,
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 83/94] environment: add set_index_file()

[Christian Couder]

Introduce set_index_file() to be able to temporarily change the index file.

It should be used like this:

/* Save current index file */
old_index_file = get_index_file();
set_index_file((char *)tmp_index_file);

/* Do stuff that will use tmp_index_file as the index file */
...

/* When finished reset the index file */
set_index_file(old_index_file);

Signed-off-by: Christian Couder 
---
 cache.h   |  1 +
 environment.c | 10 ++
 2 files changed, 11 insertions(+)

diff --git a/cache.h b/cache.h
index 160f8e3..452d0ec 100644
--- a/cache.h
+++ b/cache.h
@@ -461,6 +461,7 @@ extern int is_inside_work_tree(void);
 extern const char *get_git_dir(void);
 extern const char *get_git_common_dir(void);
 extern char *get_object_directory(void);
+extern void set_index_file(char *index_file);
 extern char *get_index_file(void);
 extern char *get_graft_file(void);
 extern int set_git_dir(const char *path);
diff --git a/environment.c b/environment.c
index 57acb2f..9676d2a 100644
--- a/environment.c
+++ b/environment.c
@@ -290,6 +290,16 @@ int odb_pack_keep(char *name, size_t namesz, const 
unsigned char *sha1)
return open(name, O_RDWR|O_CREAT|O_EXCL, 0600);
 }
 
+/*
+ * Temporarily change the index file.
+ * Please save the current index file using get_index_file() before changing
+ * the index file. And when finished, reset it to the saved value.
+ */
+void set_index_file(char *index_file)
+{
+   git_index_file = index_file;
+}
+
 char *get_index_file(void)
 {
if (!git_index_file)
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 74/94] builtin/apply: make write_out_results() return -1 on error

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.

To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", write_out_results() should return -1 instead of
calling exit().

Helped-by: Eric Sunshine 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 27 +--
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index f06bf16..97bc704 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4373,6 +4373,12 @@ static int write_out_one_reject(struct apply_state 
*state, struct patch *patch)
return -1;
 }
 
+/*
+ * Returns:
+ *  -1 if an error happened
+ *   0 if the patch applied cleanly
+ *   1 if the patch did not apply cleanly
+ */
 static int write_out_results(struct apply_state *state, struct patch *list)
 {
int phase;
@@ -4386,8 +4392,10 @@ static int write_out_results(struct apply_state *state, 
struct patch *list)
if (l->rejected)
errs = 1;
else {
-   if (write_out_one_result(state, l, phase))
-   exit(1);
+   if (write_out_one_result(state, l, phase)) {
+   string_list_clear(, 0);
+   return -1;
+   }
if (phase == 1) {
if (write_out_one_reject(state, l))
errs = 1;
@@ -4497,10 +4505,17 @@ static int apply_patch(struct apply_state *state,
goto end;
}
 
-   if (state->apply && write_out_results(state, list)) {
-   /* with --3way, we still need to write the index out */
-   res = state->apply_with_reject ? -1 : 1;
-   goto end;
+   if (state->apply) {
+   int write_res = write_out_results(state, list);
+   if (write_res < 0) {
+   res = -1;
+   goto end;
+   }
+   if (write_res > 0) {
+   /* with --3way, we still need to write the index out */
+   res = state->apply_with_reject ? -1 : 1;
+   goto end;
+   }
}
 
if (state->fake_ancestor &&
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 82/94] apply: roll back index lock file in case of error

[Christian Couder]

According to the lockfile API, when finished with a lockfile, one
should either commit it or roll it back.

This is even more important now that the same lockfile can be passed
to init_apply_state() many times to be reused by series of calls to
the apply lib functions.

Helped-by: Johannes Schindelin 
Signed-off-by: Christian Couder 
---
 apply.c | 31 +--
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/apply.c b/apply.c
index 3285bf7..7480ae8 100644
--- a/apply.c
+++ b/apply.c
@@ -4739,7 +4739,7 @@ int apply_all_patches(struct apply_state *state,
if (!strcmp(arg, "-")) {
res = apply_patch(state, 0, "", options);
if (res < 0)
-   return -1;
+   goto rollback_end;
errs |= res;
read_stdin = 0;
continue;
@@ -4749,21 +4749,23 @@ int apply_all_patches(struct apply_state *state,
  arg);
 
fd = open(arg, O_RDONLY);
-   if (fd < 0)
-   return error(_("can't open patch '%s': %s"), arg, 
strerror(errno));
+   if (fd < 0) {
+   error(_("can't open patch '%s': %s"), arg, 
strerror(errno));
+   goto rollback_end;
+   }
read_stdin = 0;
set_default_whitespace_mode(state);
res = apply_patch(state, fd, arg, options);
close(fd);
if (res < 0)
-   return -1;
+   goto rollback_end;
errs |= res;
}
set_default_whitespace_mode(state);
if (read_stdin) {
res = apply_patch(state, 0, "", options);
if (res < 0)
-   return -1;
+   goto rollback_end;
errs |= res;
}
 
@@ -4777,11 +4779,13 @@ int apply_all_patches(struct apply_state *state,
   squelched),
squelched);
}
-   if (state->ws_error_action == die_on_ws_error)
-   return error(Q_("%d line adds whitespace errors.",
-   "%d lines add whitespace errors.",
-   state->whitespace_error),
-state->whitespace_error);
+   if (state->ws_error_action == die_on_ws_error) {
+   error(Q_("%d line adds whitespace errors.",
+"%d lines add whitespace errors.",
+state->whitespace_error),
+ state->whitespace_error);
+   goto rollback_end;
+   }
if (state->applied_after_fixing_ws && state->apply)
warning("%d line%s applied after"
" fixing whitespace errors.",
@@ -4802,5 +4806,12 @@ int apply_all_patches(struct apply_state *state,
}
 
return !!errs;
+
+rollback_end:
+   if (state->newfd >= 0) {
+   rollback_lock_file(state->lock_file);
+   state->newfd = -1;
+   }
+   return -1;
 }
 
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 71/94] builtin/apply: make add_index_file() return -1 on error

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.

To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", add_index_file() should return -1 using error()
instead of calling die().

Signed-off-by: Christian Couder 
---
 builtin/apply.c | 48 +++-
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index ca3502f..0e20467 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4089,11 +4089,11 @@ static int remove_file(struct apply_state *state, 
struct patch *patch, int rmdir
return 0;
 }
 
-static void add_index_file(struct apply_state *state,
-  const char *path,
-  unsigned mode,
-  void *buf,
-  unsigned long size)
+static int add_index_file(struct apply_state *state,
+ const char *path,
+ unsigned mode,
+ void *buf,
+ unsigned long size)
 {
struct stat st;
struct cache_entry *ce;
@@ -4101,7 +4101,7 @@ static void add_index_file(struct apply_state *state,
unsigned ce_size = cache_entry_size(namelen);
 
if (!state->update_index)
-   return;
+   return 0;
 
ce = xcalloc(1, ce_size);
memcpy(ce->name, path, namelen);
@@ -4112,20 +4112,32 @@ static void add_index_file(struct apply_state *state,
const char *s;
 
if (!skip_prefix(buf, "Subproject commit ", ) ||
-   get_sha1_hex(s, ce->sha1))
-   die(_("corrupt patch for submodule %s"), path);
+   get_sha1_hex(s, ce->sha1)) {
+   free(ce);
+   return error(_("corrupt patch for submodule %s"), path);
+   }
} else {
if (!state->cached) {
-   if (lstat(path, ) < 0)
-   die_errno(_("unable to stat newly created file 
'%s'"),
- path);
+   if (lstat(path, ) < 0) {
+   free(ce);
+   return error(_("unable to stat newly "
+  "created file '%s': %s"),
+path, strerror(errno));
+   }
fill_stat_cache_info(ce, );
}
-   if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0)
-   die(_("unable to create backing store for newly created 
file %s"), path);
+   if (write_sha1_file(buf, size, blob_type, ce->sha1) < 0) {
+   free(ce);
+   return error(_("unable to create backing store "
+  "for newly created file %s"), path);
+   }
}
-   if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)
-   die(_("unable to add cache entry for %s"), path);
+   if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
+   free(ce);
+   return error(_("unable to add cache entry for %s"), path);
+   }
+
+   return 0;
 }
 
 static int try_create_file(const char *path, unsigned int mode, const char 
*buf, unsigned long size)
@@ -4261,8 +4273,10 @@ static void create_file(struct apply_state *state, 
struct patch *patch)
if (patch->conflicted_threeway) {
if (add_conflicted_stages_file(state, patch))
exit(1);
-   } else
-   add_index_file(state, path, mode, buf, size);
+   } else {
+   if (add_index_file(state, path, mode, buf, size))
+   exit(1);
+   }
 }
 
 /* phase zero is to remove, phase one is to create */
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 75/94] builtin/apply: make try_create_file() return -1 on error

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing.

To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", try_create_file() should return -1 in case of
error.

Unfortunately try_create_file() currently returns -1 to signal a
recoverable error. To fix that, let's make it return 1 in case of
a recoverable error and -1 in case of an unrecoverable error.

Helped-by: Eric Sunshine 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 41 ++---
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 97bc704..eaf7b8f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4140,38 +4140,45 @@ static int add_index_file(struct apply_state *state,
return 0;
 }
 
+/*
+ * Returns:
+ *  -1 if an unrecoverable error happened
+ *   0 if everything went well
+ *   1 if a recoverable error happened
+ */
 static int try_create_file(const char *path, unsigned int mode, const char 
*buf, unsigned long size)
 {
-   int fd;
+   int fd, res;
struct strbuf nbuf = STRBUF_INIT;
 
if (S_ISGITLINK(mode)) {
struct stat st;
if (!lstat(path, ) && S_ISDIR(st.st_mode))
return 0;
-   return mkdir(path, 0777);
+   return !!mkdir(path, 0777);
}
 
if (has_symlinks && S_ISLNK(mode))
/* Although buf:size is counted string, it also is NUL
 * terminated.
 */
-   return symlink(buf, path);
+   return !!symlink(buf, path);
 
fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 0100) ? 0777 : 
0666);
if (fd < 0)
-   return -1;
+   return 1;
 
if (convert_to_working_tree(path, buf, size, )) {
size = nbuf.len;
buf  = nbuf.buf;
}
-   write_or_die(fd, buf, size);
+   res = !write_or_whine_pipe(fd, buf, size, path);
strbuf_release();
 
-   if (close(fd) < 0)
-   die_errno(_("closing file '%s'"), path);
-   return 0;
+   if (close(fd) < 0 && !res)
+   return error(_("closing file '%s': %s"), path, strerror(errno));
+
+   return res ? -1 : 0;
 }
 
 /*
@@ -4185,15 +4192,24 @@ static void create_one_file(struct apply_state *state,
const char *buf,
unsigned long size)
 {
+   int res;
+
if (state->cached)
return;
-   if (!try_create_file(path, mode, buf, size))
+
+   res = try_create_file(path, mode, buf, size);
+   if (res < 0)
+   exit(1);
+   if (!res)
return;
 
if (errno == ENOENT) {
if (safe_create_leading_directories(path))
return;
-   if (!try_create_file(path, mode, buf, size))
+   res = try_create_file(path, mode, buf, size);
+   if (res < 0)
+   exit(1);
+   if (!res)
return;
}
 
@@ -4212,7 +4228,10 @@ static void create_one_file(struct apply_state *state,
for (;;) {
char newpath[PATH_MAX];
mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr);
-   if (!try_create_file(newpath, mode, buf, size)) {
+   res = try_create_file(newpath, mode, buf, size);
+   if (res < 0)
+   exit(1);
+   if (!res) {
if (!rename(newpath, path))
return;
unlink_or_warn(newpath);
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 78/94] apply: rename and move opt constants to apply.h

[Christian Couder]

The constants for the "inaccurate-eof" and the "recount" options will
be used in both "apply.c" and "builtin/apply.c", so they need to go
into "apply.h", and therefore they need a name that is more specific
to the API they belong to.

Signed-off-by: Christian Couder 
---
 apply.h |  3 +++
 builtin/apply.c | 11 ---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/apply.h b/apply.h
index 5266612..60e0c2f 100644
--- a/apply.h
+++ b/apply.h
@@ -122,4 +122,7 @@ extern int init_apply_state(struct apply_state *state,
struct lock_file *lock_file);
 extern int check_apply_state(struct apply_state *state, int force_apply);
 
+#define APPLY_OPT_INACCURATE_EOF   (1<<0)
+#define APPLY_OPT_RECOUNT  (1<<1)
+
 #endif
diff --git a/builtin/apply.c b/builtin/apply.c
index f05dc96..9ce177b 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4448,9 +4448,6 @@ static int write_out_results(struct apply_state *state, 
struct patch *list)
return errs;
 }
 
-#define INACCURATE_EOF (1<<0)
-#define RECOUNT(1<<1)
-
 /*
  * Try to apply a patch.
  *
@@ -4479,8 +4476,8 @@ static int apply_patch(struct apply_state *state,
int nr;
 
patch = xcalloc(1, sizeof(*patch));
-   patch->inaccurate_eof = !!(options & INACCURATE_EOF);
-   patch->recount =  !!(options & RECOUNT);
+   patch->inaccurate_eof = !!(options & APPLY_OPT_INACCURATE_EOF);
+   patch->recount =  !!(options & APPLY_OPT_RECOUNT);
nr = parse_chunk(state, buf.buf + offset, buf.len - offset, 
patch);
if (nr < 0) {
free_patch(patch);
@@ -4770,10 +4767,10 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix)
OPT__VERBOSE(_verbosely, N_("be verbose")),
OPT_BIT(0, "inaccurate-eof", ,
N_("tolerate incorrectly detected missing new-line at 
the end of file"),
-   INACCURATE_EOF),
+   APPLY_OPT_INACCURATE_EOF),
OPT_BIT(0, "recount", ,
N_("do not trust the line counts in the hunk headers"),
-   RECOUNT),
+   APPLY_OPT_RECOUNT),
{ OPTION_CALLBACK, 0, "directory", , N_("root"),
N_("prepend  to all filenames"),
0, apply_option_parse_directory },
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 80/94] apply: make some parsing functions static again

[Christian Couder]

Some parsing functions that were used in both "apply.c" and
"builtin/apply.c" are now only used in the former, so they
can be made static to "apply.c".

Signed-off-by: Christian Couder 
---
 apply.c | 6 +++---
 apply.h | 5 -
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/apply.c b/apply.c
index 537221b..3285bf7 100644
--- a/apply.c
+++ b/apply.c
@@ -27,7 +27,7 @@ static void git_apply_config(void)
git_config(git_default_config, NULL);
 }
 
-int parse_whitespace_option(struct apply_state *state, const char *option)
+static int parse_whitespace_option(struct apply_state *state, const char 
*option)
 {
if (!option) {
state->ws_error_action = warn_on_ws_error;
@@ -57,8 +57,8 @@ int parse_whitespace_option(struct apply_state *state, const 
char *option)
return error(_("unrecognized whitespace option '%s'"), option);
 }
 
-int parse_ignorewhitespace_option(struct apply_state *state,
- const char *option)
+static int parse_ignorewhitespace_option(struct apply_state *state,
+const char *option)
 {
if (!option || !strcmp(option, "no") ||
!strcmp(option, "false") || !strcmp(option, "never") ||
diff --git a/apply.h b/apply.h
index c8b79ce..27b26a2 100644
--- a/apply.h
+++ b/apply.h
@@ -112,11 +112,6 @@ struct apply_state {
enum ws_ignore ws_ignore_action;
 };
 
-extern int parse_whitespace_option(struct apply_state *state,
-  const char *option);
-extern int parse_ignorewhitespace_option(struct apply_state *state,
-const char *option);
-
 extern int apply_option_parse_exclude(const struct option *opt,
  const char *arg, int unset);
 extern int apply_option_parse_include(const struct option *opt,
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 76/94] builtin/apply: make create_one_file() return -1 on error

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of exit()ing.

To do that in a compatible manner with the rest of the error handling
in "builtin/apply.c", create_one_file() should return -1 instead of
calling exit().

Signed-off-by: Christian Couder 
---
 builtin/apply.c | 36 +---
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index eaf7b8f..8c31617 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4185,32 +4185,36 @@ static int try_create_file(const char *path, unsigned 
int mode, const char *buf,
  * We optimistically assume that the directories exist,
  * which is true 99% of the time anyway. If they don't,
  * we create them and try again.
+ *
+ * Returns:
+ *   -1 on error
+ *   0 otherwise
  */
-static void create_one_file(struct apply_state *state,
-   char *path,
-   unsigned mode,
-   const char *buf,
-   unsigned long size)
+static int create_one_file(struct apply_state *state,
+  char *path,
+  unsigned mode,
+  const char *buf,
+  unsigned long size)
 {
int res;
 
if (state->cached)
-   return;
+   return 0;
 
res = try_create_file(path, mode, buf, size);
if (res < 0)
-   exit(1);
+   return -1;
if (!res)
-   return;
+   return 0;
 
if (errno == ENOENT) {
if (safe_create_leading_directories(path))
-   return;
+   return 0;
res = try_create_file(path, mode, buf, size);
if (res < 0)
-   exit(1);
+   return -1;
if (!res)
-   return;
+   return 0;
}
 
if (errno == EEXIST || errno == EACCES) {
@@ -4230,10 +4234,10 @@ static void create_one_file(struct apply_state *state,
mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr);
res = try_create_file(newpath, mode, buf, size);
if (res < 0)
-   exit(1);
+   return -1;
if (!res) {
if (!rename(newpath, path))
-   return;
+   return 0;
unlink_or_warn(newpath);
break;
}
@@ -4242,7 +4246,8 @@ static void create_one_file(struct apply_state *state,
++nr;
}
}
-   die_errno(_("unable to write file '%s' mode %o"), path, mode);
+   return error(_("unable to write file '%s' mode %o: %s"),
+path, mode, strerror(errno));
 }
 
 static int add_conflicted_stages_file(struct apply_state *state,
@@ -4287,7 +4292,8 @@ static int create_file(struct apply_state *state, struct 
patch *patch)
 
if (!mode)
mode = S_IFREG | 0644;
-   create_one_file(state, path, mode, buf, size);
+   if (create_one_file(state, path, mode, buf, size))
+   return -1;
 
if (patch->conflicted_threeway)
return add_conflicted_stages_file(state, patch);
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 65/94] builtin/apply: make gitdiff_*() return 1 at end of header

[Christian Couder]

The gitdiff_*() functions that are called as p->fn() in parse_git_header()
should return 1 instead of -1 in case of end of header or unrecognized
input, as these are not real errors. It just instructs the parser to break
out.

This makes it possible for gitdiff_*() functions to return -1 in case of a
real error. This will be done in a following patch.

Helped-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 8e82eea..b3a9c2e 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -812,7 +812,7 @@ static int gitdiff_hdrend(struct apply_state *state,
  const char *line,
  struct patch *patch)
 {
-   return -1;
+   return 1;
 }
 
 /*
@@ -1016,7 +1016,7 @@ static int gitdiff_unrecognized(struct apply_state *state,
const char *line,
struct patch *patch)
 {
-   return -1;
+   return 1;
 }
 
 /*
@@ -1248,9 +1248,13 @@ static int parse_git_header(struct apply_state *state,
for (i = 0; i < ARRAY_SIZE(optable); i++) {
const struct opentry *p = optable + i;
int oplen = strlen(p->str);
+   int res;
if (len < oplen || memcmp(p->str, line, oplen))
continue;
-   if (p->fn(state, line + oplen, patch) < 0)
+   res = p->fn(state, line + oplen, patch);
+   if (res < 0)
+   return -1;
+   if (res > 0)
return offset;
break;
}
@@ -1429,6 +1433,8 @@ static int find_header(struct apply_state *state,
 */
if (!memcmp("diff --git ", line, 11)) {
int git_hdr_len = parse_git_header(state, line, len, 
size, patch);
+   if (git_hdr_len < 0)
+   return -1;
if (git_hdr_len <= len)
continue;
if (!patch->old_name && !patch->new_name) {
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 54/94] builtin/apply: make parse_chunk() return a negative integer on error

[Christian Couder]

To libify `git apply` functionality we have to signal errors to the
caller instead of die()ing or exit()ing.

To do that in a compatible manner with the rest of the error handling
in builtin/apply.c, find_header() should return -1 instead of calling
die() or exit().

As parse_chunk() is called only by apply_patch() which already
returns -1 when an error happened, let's make apply_patch() return -1
when parse_chunk() returns -1.

If find_header() returns -2 because no patch header has been found, it
is ok for parse_chunk() to also return -2. If find_header() returns -1
because an error happened, it is ok for parse_chunk() to do the same.

Helped-by: Eric Sunshine 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 4212705..2380472 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2101,22 +2101,22 @@ static int use_patch(struct apply_state *state, struct 
patch *p)
return !state->has_include;
 }
 
-
 /*
  * Read the patch text in "buffer" that extends for "size" bytes; stop
  * reading after seeing a single patch (i.e. changes to a single file).
  * Create fragments (i.e. patch hunks) and hang them to the given patch.
- * Return the number of bytes consumed, so that the caller can call us
- * again for the next patch.
+ *
+ * Returns:
+ *   -1 on error,
+ *   -2 if no header was found,
+ *   the number of bytes consumed otherwise,
+ * so that the caller can call us again for the next patch.
  */
 static int parse_chunk(struct apply_state *state, char *buffer, unsigned long 
size, struct patch *patch)
 {
int hdrsize, patchsize;
int offset = find_header(state, buffer, size, , patch);
 
-   if (offset == -1)
-   exit(1);
-
if (offset < 0)
return offset;
 
@@ -2176,8 +2176,9 @@ static int parse_chunk(struct apply_state *state, char 
*buffer, unsigned long si
 * empty to us here.
 */
if ((state->apply || state->check) &&
-   (!patch->is_binary && !metadata_changes(patch)))
-   die(_("patch with only garbage at line %d"), 
state->linenr);
+   (!patch->is_binary && !metadata_changes(patch))) {
+   return error(_("patch with only garbage at line %d"), 
state->linenr);
+   }
}
 
return offset + hdrsize + patchsize;
@@ -4557,6 +4558,10 @@ static int apply_patch(struct apply_state *state,
nr = parse_chunk(state, buf.buf + offset, buf.len - offset, 
patch);
if (nr < 0) {
free_patch(patch);
+   if (nr == -1) {
+   res = -1;
+   goto end;
+   }
break;
}
if (state->apply_in_reverse)
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 62/94] builtin/apply: move check_apply_state() to apply.c

[Christian Couder]

To libify `git apply` functionality we must make check_apply_state()
usable outside "builtin/apply.c".

Let's do that by moving it into "apply.c".

Signed-off-by: Christian Couder 
---
 apply.c | 29 +
 apply.h |  1 +
 builtin/apply.c | 29 -
 3 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/apply.c b/apply.c
index 1e2b802..2128594 100644
--- a/apply.c
+++ b/apply.c
@@ -82,3 +82,32 @@ int init_apply_state(struct apply_state *state,
return 0;
 }
 
+int check_apply_state(struct apply_state *state, int force_apply)
+{
+   int is_not_gitdir = !startup_info->have_repository;
+
+   if (state->apply_with_reject && state->threeway)
+   return error("--reject and --3way cannot be used together.");
+   if (state->cached && state->threeway)
+   return error("--cached and --3way cannot be used together.");
+   if (state->threeway) {
+   if (is_not_gitdir)
+   return error(_("--3way outside a repository"));
+   state->check_index = 1;
+   }
+   if (state->apply_with_reject)
+   state->apply = state->apply_verbosely = 1;
+   if (!force_apply && (state->diffstat || state->numstat || 
state->summary || state->check || state->fake_ancestor))
+   state->apply = 0;
+   if (state->check_index && is_not_gitdir)
+   return error(_("--index outside a repository"));
+   if (state->cached) {
+   if (is_not_gitdir)
+   return error(_("--cached outside a repository"));
+   state->check_index = 1;
+   }
+   if (state->check_index)
+   state->unsafe_paths = 0;
+   return 0;
+}
+
diff --git a/apply.h b/apply.h
index f3b2ae4..5266612 100644
--- a/apply.h
+++ b/apply.h
@@ -120,5 +120,6 @@ extern int parse_ignorewhitespace_option(struct apply_state 
*state,
 extern int init_apply_state(struct apply_state *state,
const char *prefix,
struct lock_file *lock_file);
+extern int check_apply_state(struct apply_state *state, int force_apply);
 
 #endif
diff --git a/builtin/apply.c b/builtin/apply.c
index 2b3d10b..ae16f99 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4540,35 +4540,6 @@ static int option_parse_directory(const struct option 
*opt,
return 0;
 }
 
-static int check_apply_state(struct apply_state *state, int force_apply)
-{
-   int is_not_gitdir = !startup_info->have_repository;
-
-   if (state->apply_with_reject && state->threeway)
-   return error("--reject and --3way cannot be used together.");
-   if (state->cached && state->threeway)
-   return error("--cached and --3way cannot be used together.");
-   if (state->threeway) {
-   if (is_not_gitdir)
-   return error(_("--3way outside a repository"));
-   state->check_index = 1;
-   }
-   if (state->apply_with_reject)
-   state->apply = state->apply_verbosely = 1;
-   if (!force_apply && (state->diffstat || state->numstat || 
state->summary || state->check || state->fake_ancestor))
-   state->apply = 0;
-   if (state->check_index && is_not_gitdir)
-   return error(_("--index outside a repository"));
-   if (state->cached) {
-   if (is_not_gitdir)
-   return error(_("--cached outside a repository"));
-   state->check_index = 1;
-   }
-   if (state->check_index)
-   state->unsafe_paths = 0;
-   return 0;
-}
-
 static int apply_all_patches(struct apply_state *state,
 int argc,
 const char **argv,
-- 
2.8.2.490.g3dabe57

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 42/94] builtin/apply: move 'max_change' and 'max_len' into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'max_change' and 'max_len'
variables should not be static and global to the file. Let's move
them into 'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 49 +
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index f8c3677..deba14c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -76,6 +76,14 @@ struct apply_state {
int unsafe_paths;
int line_termination;
 
+   /*
+* For "diff-stat" like behaviour, we keep track of the biggest change
+* we've seen, and the longest filename. That allows us to do simple
+* scaling.
+*/
+   int max_change;
+   int max_len;
+
int p_value;
int p_value_known;
unsigned int p_context;
@@ -148,13 +156,6 @@ static void set_default_whitespace_mode(struct apply_state 
*state)
state->ws_error_action = (state->apply ? warn_on_ws_error : 
nowarn_ws_error);
 }
 
-/*
- * For "diff-stat" like behaviour, we keep track of the biggest change
- * we've seen, and the longest filename. That allows us to do simple
- * scaling.
- */
-static int max_change, max_len;
-
 /*
  * Various "current state", notably line numbers and what
  * file (and how) we're patching right now.. The "is_"
@@ -2179,7 +2180,7 @@ static const char pluses[] =
 static const char minuses[]=
 "--";
 
-static void show_stats(struct patch *patch)
+static void show_stats(struct apply_state *state, struct patch *patch)
 {
struct strbuf qname = STRBUF_INIT;
char *cp = patch->new_name ? patch->new_name : patch->old_name;
@@ -2190,7 +2191,7 @@ static void show_stats(struct patch *patch)
/*
 * "scale" the filename
 */
-   max = max_len;
+   max = state->max_len;
if (max > 50)
max = 50;
 
@@ -2213,13 +2214,13 @@ static void show_stats(struct patch *patch)
/*
 * scale the add/delete
 */
-   max = max + max_change > 70 ? 70 - max : max_change;
+   max = max + state->max_change > 70 ? 70 - max : state->max_change;
add = patch->lines_added;
del = patch->lines_deleted;
 
-   if (max_change > 0) {
-   int total = ((add + del) * max + max_change / 2) / max_change;
-   add = (add * max + max_change / 2) / max_change;
+   if (state->max_change > 0) {
+   int total = ((add + del) * max + state->max_change / 2) / 
state->max_change;
+   add = (add * max + state->max_change / 2) / state->max_change;
del = total - add;
}
printf("%5d %.*s%.*s\n", patch->lines_added + patch->lines_deleted,
@@ -4045,7 +4046,7 @@ static void build_fake_ancestor(struct patch *list, const 
char *filename)
discard_index();
 }
 
-static void stat_patch_list(struct patch *patch)
+static void stat_patch_list(struct apply_state *state, struct patch *patch)
 {
int files, adds, dels;
 
@@ -4053,7 +4054,7 @@ static void stat_patch_list(struct patch *patch)
files++;
adds += patch->lines_added;
dels += patch->lines_deleted;
-   show_stats(patch);
+   show_stats(state, patch);
}
 
print_stat_summary(stdout, files, adds, dels);
@@ -4151,25 +4152,25 @@ static void summary_patch_list(struct patch *patch)
}
 }
 
-static void patch_stats(struct patch *patch)
+static void patch_stats(struct apply_state *state, struct patch *patch)
 {
int lines = patch->lines_added + patch->lines_deleted;
 
-   if (lines > max_change)
-   max_change = lines;
+   if (lines > state->max_change)
+   state->max_change = lines;
if (patch->old_name) {
int len = quote_c_style(patch->old_name, NULL, NULL, 0);
if (!len)
len = strlen(patch->old_name);
-   if (len > max_len)
-   max_len = len;
+   if (len > state->max_len)
+   state->max_len = len;
}
if (patch->new_name) {
int len = quote_c_style(patch->new_name, NULL, NULL, 0);
if (!len)
len = strlen(patch->new_name);
-   if (len > max_len)
-   max_len = len;
+   if (len > state->max_len)
+   state->max_len = len;
}
 }
 
@@ -4526,7 +4527,7 @@ static int apply_patch(struct apply_state *state,
if (state->apply_in_reverse)
reverse_patches(patch);
if (use_patch(state, patch)) {
-   patch_stats(patch);
+   patch_stats(state, patch);
  

[PATCH v2 32/94] builtin/apply: move 'p_value' global into 'struct apply_state'

[Christian Couder]

To libify the apply functionality the 'p_value' variable should
not be static and global to the file. Let's move it into
'struct apply_state'.

Reviewed-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 builtin/apply.c | 151 +---
 1 file changed, 99 insertions(+), 52 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index f2ee8bf..4e476d5 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -62,12 +62,12 @@ struct apply_state {
int unsafe_paths;
int line_termination;
 
+   int p_value;
unsigned int p_context;
 };
 
 static int newfd = -1;
 
-static int state_p_value = 1;
 static int p_value_known;
 
 static const char * const apply_usage[] = {
@@ -888,24 +888,24 @@ static void parse_traditional_patch(struct apply_state 
*state,
q = guess_p_value(state, second);
if (p < 0) p = q;
if (0 <= p && p == q) {
-   state_p_value = p;
+   state->p_value = p;
p_value_known = 1;
}
}
if (is_dev_null(first)) {
patch->is_new = 1;
patch->is_delete = 0;
-   name = find_name_traditional(second, NULL, state_p_value);
+   name = find_name_traditional(second, NULL, state->p_value);
patch->new_name = name;
} else if (is_dev_null(second)) {
patch->is_new = 0;
patch->is_delete = 1;
-   name = find_name_traditional(first, NULL, state_p_value);
+   name = find_name_traditional(first, NULL, state->p_value);
patch->old_name = name;
} else {
char *first_name;
-   first_name = find_name_traditional(first, NULL, state_p_value);
-   name = find_name_traditional(second, first_name, state_p_value);
+   first_name = find_name_traditional(first, NULL, state->p_value);
+   name = find_name_traditional(second, first_name, 
state->p_value);
free(first_name);
if (has_epoch_timestamp(first)) {
patch->is_new = 1;
@@ -924,7 +924,9 @@ static void parse_traditional_patch(struct apply_state 
*state,
die(_("unable to find filename in patch at line %d"), 
state_linenr);
 }
 
-static int gitdiff_hdrend(const char *line, struct patch *patch)
+static int gitdiff_hdrend(struct apply_state *state,
+ const char *line,
+ struct patch *patch)
 {
return -1;
 }
@@ -941,10 +943,14 @@ static int gitdiff_hdrend(const char *line, struct patch 
*patch)
 #define DIFF_OLD_NAME 0
 #define DIFF_NEW_NAME 1
 
-static void gitdiff_verify_name(const char *line, int isnull, char **name, int 
side)
+static void gitdiff_verify_name(struct apply_state *state,
+   const char *line,
+   int isnull,
+   char **name,
+   int side)
 {
if (!*name && !isnull) {
-   *name = find_name(line, NULL, state_p_value, TERM_TAB);
+   *name = find_name(line, NULL, state->p_value, TERM_TAB);
return;
}
 
@@ -954,7 +960,7 @@ static void gitdiff_verify_name(const char *line, int 
isnull, char **name, int s
if (isnull)
die(_("git apply: bad git-diff - expected /dev/null, 
got %s on line %d"),
*name, state_linenr);
-   another = find_name(line, NULL, state_p_value, TERM_TAB);
+   another = find_name(line, NULL, state->p_value, TERM_TAB);
if (!another || memcmp(another, *name, len + 1))
die((side == DIFF_NEW_NAME) ?
_("git apply: bad git-diff - inconsistent new 
filename on line %d") :
@@ -967,81 +973,105 @@ static void gitdiff_verify_name(const char *line, int 
isnull, char **name, int s
}
 }
 
-static int gitdiff_oldname(const char *line, struct patch *patch)
+static int gitdiff_oldname(struct apply_state *state,
+  const char *line,
+  struct patch *patch)
 {
-   gitdiff_verify_name(line, patch->is_new, >old_name,
+   gitdiff_verify_name(state, line,
+   patch->is_new, >old_name,
DIFF_OLD_NAME);
return 0;
 }
 
-static int gitdiff_newname(const char *line, struct patch *patch)
+static int gitdiff_newname(struct apply_state *state,
+  const char *line,
+  struct patch *patch)
 {
-   gitdiff_verify_name(line, patch->is_delete, >new_name,
+   gitdiff_verify_name(state, line,
+   patch->is_delete, >new_name,
DIFF_NEW_NAME);
 

[PATCH v2 59/94] builtin/apply: move init_apply_state() to apply.c

[Christian Couder]

To libify `git apply` functionality we must make init_apply_state()
usable outside "builtin/apply.c".

Let's do that by moving it into a new "apply.c".

Helped-by: Eric Sunshine 
Signed-off-by: Christian Couder 
---
 Makefile|  1 +
 apply.c | 83 +
 apply.h |  9 +++
 builtin/apply.c | 79 --
 4 files changed, 93 insertions(+), 79 deletions(-)
 create mode 100644 apply.c

diff --git a/Makefile b/Makefile
index 3f03366..b2a7b0c 100644
--- a/Makefile
+++ b/Makefile
@@ -683,6 +683,7 @@ LIB_OBJS += abspath.o
 LIB_OBJS += advice.o
 LIB_OBJS += alias.o
 LIB_OBJS += alloc.o
+LIB_OBJS += apply.o
 LIB_OBJS += archive.o
 LIB_OBJS += archive-tar.o
 LIB_OBJS += archive-zip.o
diff --git a/apply.c b/apply.c
new file mode 100644
index 000..508ea64
--- /dev/null
+++ b/apply.c
@@ -0,0 +1,83 @@
+#include "cache.h"
+#include "lockfile.h"
+#include "apply.h"
+
+static void git_apply_config(void)
+{
+   git_config_get_string_const("apply.whitespace", 
_default_whitespace);
+   git_config_get_string_const("apply.ignorewhitespace", 
_default_ignorewhitespace);
+   git_config(git_default_config, NULL);
+}
+
+int parse_whitespace_option(struct apply_state *state, const char *option)
+{
+   if (!option) {
+   state->ws_error_action = warn_on_ws_error;
+   return 0;
+   }
+   if (!strcmp(option, "warn")) {
+   state->ws_error_action = warn_on_ws_error;
+   return 0;
+   }
+   if (!strcmp(option, "nowarn")) {
+   state->ws_error_action = nowarn_ws_error;
+   return 0;
+   }
+   if (!strcmp(option, "error")) {
+   state->ws_error_action = die_on_ws_error;
+   return 0;
+   }
+   if (!strcmp(option, "error-all")) {
+   state->ws_error_action = die_on_ws_error;
+   state->squelch_whitespace_errors = 0;
+   return 0;
+   }
+   if (!strcmp(option, "strip") || !strcmp(option, "fix")) {
+   state->ws_error_action = correct_ws_error;
+   return 0;
+   }
+   return error(_("unrecognized whitespace option '%s'"), option);
+}
+
+int parse_ignorewhitespace_option(struct apply_state *state,
+ const char *option)
+{
+   if (!option || !strcmp(option, "no") ||
+   !strcmp(option, "false") || !strcmp(option, "never") ||
+   !strcmp(option, "none")) {
+   state->ws_ignore_action = ignore_ws_none;
+   return 0;
+   }
+   if (!strcmp(option, "change")) {
+   state->ws_ignore_action = ignore_ws_change;
+   return 0;
+   }
+   return error(_("unrecognized whitespace ignore option '%s'"), option);
+}
+
+void init_apply_state(struct apply_state *state,
+ const char *prefix,
+ struct lock_file *lock_file)
+{
+   memset(state, 0, sizeof(*state));
+   state->prefix = prefix;
+   state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
+   state->lock_file = lock_file ? lock_file : xcalloc(1, 
sizeof(*lock_file));
+   state->newfd = -1;
+   state->apply = 1;
+   state->line_termination = '\n';
+   state->p_value = 1;
+   state->p_context = UINT_MAX;
+   state->squelch_whitespace_errors = 5;
+   state->ws_error_action = warn_on_ws_error;
+   state->ws_ignore_action = ignore_ws_none;
+   state->linenr = 1;
+   strbuf_init(>root, 0);
+
+   git_apply_config();
+   if (apply_default_whitespace && parse_whitespace_option(state, 
apply_default_whitespace))
+   exit(1);
+   if (apply_default_ignorewhitespace && 
parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
+   exit(1);
+}
+
diff --git a/apply.h b/apply.h
index aa11ea6..0f77f4d 100644
--- a/apply.h
+++ b/apply.h
@@ -112,4 +112,13 @@ struct apply_state {
enum ws_ignore ws_ignore_action;
 };
 
+extern int parse_whitespace_option(struct apply_state *state,
+  const char *option);
+extern int parse_ignorewhitespace_option(struct apply_state *state,
+const char *option);
+
+extern void init_apply_state(struct apply_state *state,
+const char *prefix,
+struct lock_file *lock_file);
+
 #endif
diff --git a/builtin/apply.c b/builtin/apply.c
index a7e..805c707 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -27,52 +27,6 @@ static const char * const apply_usage[] = {
NULL
 };
 
-static int parse_whitespace_option(struct apply_state *state, const char 
*option)
-{
-   if (!option) {
-   state->ws_error_action = warn_on_ws_error;
-   return 0;
-   }
-   if