Re: ssh admin git accidentally deleted

2015-07-07 Thread Fredrik Gustafsson
On Wed, Jul 08, 2015 at 12:32:42AM +0700, agnes retnaningsih wrote:
> I use gitolite on linux.
> he2nk is account that I delete on server where gitolite-admin is repository
> to change gitolite configuration. I still can make editing such as add user
> to access gitolite-admin  but when I push it, it error ( file attached), I
> think it because the he2nk has been deleted.
> 
> 
> he2nk is ssh that I have deleted and push it to the server.
> 
> 
> So, if there anyway to revert the change I pushed?? so that I can make a
> change on gitolite admin.
> 


So from what I understand he2nk is a gitolite account and not a linux
account. And you deleted he2nk from your gitolite-admin repository.

Yes you can revert this and you can also add he2nk again, whatever you
like. But as you've seen you can't do this with gitolite. You've to do
this directly on the server since you don't have access to edit the
gitolite-admin repository (if I guess correct).

Please don't forget to CC the git-list.


-- 
Fredrik Gustafsson

phone: +46 733-608274
e-mail: iv...@iveqy.com
website: http://www.iveqy.com
--
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] refs: loosen restrictions on wildcard '*' refspecs

2015-07-07 Thread Jacob Keller
This patch updates the check_refname_component logic in order to allow for
a less strict refspec format in regards to REFNAME_REFSPEC_PATTERN.
Previously the '*' could only replace a single full component, and could
not replace arbitrary text. Now, refs such as `foo/bar*:foo/bar*` will be
accepted. This allows for somewhat more flexibility in references and does
not break any current users. The ref matching code already allows this but
the check_refname_format did not.

This patch also streamlines the code by making this new check part of
check_refname_component instead of checking after we error during
check_refname_format, which makes more sense with how we check other
issues in refname components.

Signed-off-by: Jacob Keller 
Cc: Daniel Barkalow 
Cc: Junio C Hamano 
---
 Documentation/git-check-ref-format.txt |  4 ++--
 refs.c | 39 +++---
 refs.h |  4 ++--
 3 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/Documentation/git-check-ref-format.txt 
b/Documentation/git-check-ref-format.txt
index fc02959..9044dfa 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -94,8 +94,8 @@ OPTIONS
Interpret  as a reference name pattern for a refspec
(as used with remote repositories).  If this option is
enabled,  is allowed to contain a single `*`
-   in place of a one full pathname component (e.g.,
-   `foo/*/bar` but not `foo/bar*`).
+   in the refspec (e.g., `foo/bar*/baz` or `foo/bar*baz/`
+   but not `foo/bar*/baz*`).
 
 --normalize::
Normalize 'refname' by removing any leading slash (`/`)
diff --git a/refs.c b/refs.c
index 7ac05cf..8702644 100644
--- a/refs.c
+++ b/refs.c
@@ -20,11 +20,12 @@ struct ref_lock {
  * 2: ., look for a preceding . to reject .. in refs
  * 3: {, look for a preceding @ to reject @{ in refs
  * 4: A bad character: ASCII control characters, "~", "^", ":" or SP
+ * 5: check for patterns to reject unless REFNAME_REFSPEC_PATTERN is set
  */
 static unsigned char refname_disposition[256] = {
1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
-   4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 2, 1,
+   4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 2, 1,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 4,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 4, 0, 4, 0,
@@ -71,11 +72,13 @@ static unsigned char refname_disposition[256] = {
  * - any path component of it begins with ".", or
  * - it has double dots "..", or
  * - it has ASCII control character, "~", "^", ":" or SP, anywhere, or
- * - it ends with a "/".
- * - it ends with ".lock"
- * - it contains a "\" (backslash)
+ * - it ends with a "/", or
+ * - it ends with ".lock", or
+ * - it contains a "\" (backslash), or
+ * - it contains a "@{" portion, or
+ * - it contains a '*' unless REFNAME_REFSPEC_PATTERN is set
  */
-static int check_refname_component(const char *refname, int flags)
+static int check_refname_component(const char *refname, int *flags)
 {
const char *cp;
char last = '\0';
@@ -96,6 +99,16 @@ static int check_refname_component(const char *refname, int 
flags)
break;
case 4:
return -1;
+   case 5:
+   if (!(*flags & REFNAME_REFSPEC_PATTERN))
+   return -1; /* refspec can't be a pattern */
+
+   /*
+* Unset the pattern flag so that we only accept a 
single glob for
+* the entire refspec.
+*/
+   *flags &= ~ REFNAME_REFSPEC_PATTERN;
+   break;
}
last = ch;
}
@@ -120,18 +133,10 @@ int check_refname_format(const char *refname, int flags)
 
while (1) {
/* We are at the start of a path component. */
-   component_len = check_refname_component(refname, flags);
-   if (component_len <= 0) {
-   if ((flags & REFNAME_REFSPEC_PATTERN) &&
-   refname[0] == '*' &&
-   (refname[1] == '\0' || refname[1] == 
'/')) {
-   /* Accept one wildcard as a full refname 
component. */
-   flags &= ~REFNAME_REFSPEC_PATTERN;
-   component_len = 1;
-   } else {
-   return -1;
-   }
-   }
+   component_len = check_refname_component(refname, &flags);
+   if (component_len <= 0)
+   return -1;
+
component_count++;
if (refname[component_len] == '\0')
   

Re: Git grep does not support multi-byte characters (like UTF-8)

2015-07-07 Thread Junio C Hamano
Duy Nguyen  writes:

> On top of this, pickaxe already supports icase even kws is used. But
> it only works for ascii, so either we fix it and support non-ascii, or
> we remove icase support entirely from diffcore_pickaxe(). I vote the
> former.

I think that is a different issue.  The pickaxe has a single very
narrowly-defined intended use case [*1*] and I do not care too much
how any use that is outside the intended use case behaves.  As long
as its intended use case does not suffer (1) correctness-wise, (2)
performance-wise and (3) code-cleanliness-wise, due to changes to
support such enhancements, I am perfectly fine.

Ascii-only icase match is one example of a feature that is outside
the intended use case, and implementation of it using kws is nearly
free if I am not mistaken, not making the primary use case suffer in
any way.

I however am highly skeptical that the same thing can be done with
non-ascii icase.  As long as it can be added without makinng the
primary use case suffer in any way, I do not mind it very much.

Thanks.


[Footnote]

*1* The requirement is very simple.  You get a string that is unique
in a blob that exists at the revision your traversal begins, and you
want to find the point where the blob at the corresponding path does
not have that exact string with minimal effort.  You do not need to
ensure that the input string is unique (it is a user error and the
behaviour is undefined) and for simplicity you are also allowed to
fire when the blob has more than one copies of the string (even
though the expected use is to find the place where the blob has
zero).

Any other cases, e.g. the string was not unique in the blob, the
user specified "ignore-case" and other irrelevant options, are
allowed to be incorrect or slow or both, as $gmane/217 does not need
such uses to implement 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


Re: [PATCH] git: add optional support for full pattern in fetch refspecs

2015-07-07 Thread Jacob Keller
On Tue, Jul 7, 2015 at 9:24 PM, Junio C Hamano  wrote:
> Jacob Keller  writes:
>
>> +remote..arbitrarypattern::
>> + When set to true, fetching from this remote will allow arbitrary 
>> complex
>> + patterns for the fetch refspecs. For example,
>> + refs/tags/prefix-*:refs/tags/prefix-* will work as expected. Care 
>> should be
>> + taken as you could fetch refs into names that don't exist on the 
>> remote and
>> + may end up pushing them again later by accident.
>
> Bad name and explanation, unless you truly mean "arbitrary", like
> taking something like "refs/ta*/prefix-*-*-foo:refs/*".
>

I couldn't figure out what to use, and the original intent was to add
an option.. but see below,

> More importantly, this is not "pattern"; you are talking about
> refspec, I think.
>

In this case the option was an additional modifier to the
refspec_patterns, and I was talking about how the pattern could be
slightly more arbitrary. I do agree it is a bad name, but i couldn't
actually come up with anything better.

> But that probably does not matter.  I do not think this even needs
> to exist as an option.
>

Yes, I agree especially as I look at it more. I'll work up a patch
version which does this without the option.

> People's existing settings would not have anything other than an
> asterisk as a whole path component on each side (or no asterisk
> anywhere), and if they had an asterisk anywhere else they would have
> gotten an error and wouldn't have made any progress until they fixed
> it.  So if you loosen the current rule sligntly and tell them "If
> your refspec has an asterisk in it, then you must have one asterisk
> on each side of it. That rule hasn't changed. But your asterisks no
> longer have to be a whole path component", such a change would not
> hurt them.  Their existing setting that work would not notice, and
> existing users would not be relying on a refspec with an asterisk as
> part of a path component to error out.
>

Right. We aren't breaking anyones current functionality, just adding
new functionality. We already check for a * in both sides, and my code
will ensure only 1 star total. It will then work for the new somewhat
more expanded patterns and we don't need an option.

I'll work up a v2.

Regards,
Jake
--
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] git: add optional support for full pattern in fetch refspecs

2015-07-07 Thread Junio C Hamano
Jacob Keller  writes:

> +remote..arbitrarypattern::
> + When set to true, fetching from this remote will allow arbitrary complex
> + patterns for the fetch refspecs. For example,
> + refs/tags/prefix-*:refs/tags/prefix-* will work as expected. Care 
> should be
> + taken as you could fetch refs into names that don't exist on the remote 
> and
> + may end up pushing them again later by accident.

Bad name and explanation, unless you truly mean "arbitrary", like
taking something like "refs/ta*/prefix-*-*-foo:refs/*".

More importantly, this is not "pattern"; you are talking about
refspec, I think.

But that probably does not matter.  I do not think this even needs
to exist as an option.

People's existing settings would not have anything other than an
asterisk as a whole path component on each side (or no asterisk
anywhere), and if they had an asterisk anywhere else they would have
gotten an error and wouldn't have made any progress until they fixed
it.  So if you loosen the current rule sligntly and tell them "If
your refspec has an asterisk in it, then you must have one asterisk
on each side of it. That rule hasn't changed. But your asterisks no
longer have to be a whole path component", such a change would not
hurt them.  Their existing setting that work would not notice, and
existing users would not be relying on a refspec with an asterisk as
part of a path component to error out.

--
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] log: add log.follow config option

2015-07-07 Thread Junio C Hamano
David Turner  writes:

>> IOW, I'd like to know why we need more than something like this
>> change to this file, instead of the above?  We didn't muck with
>> revs->diff in the original when FOLLOW_RENAMES was set, but now it
>> does, for example.
>
> We did, but we did it earlier.  But I can just rearrange the code.

Ah, I see.  You don't have to move the existing code for that then.
Just insert the "if prune has one element and DEFAULT_ is set" thing
before the first use of FOLLOW_RENAMES (i.e. "pick, filter and follow
needs diff" piece) and you are done, I think.

Thanks.

>
>> diff --git a/revision.c b/revision.c
>> index 3ff8723..f7bd229 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -2270,6 +2270,10 @@ int setup_revisions(int argc, const char **argv, 
>> struct rev_info *revs, struct s
>>  got_rev_arg = 1;
>>  }
>>  
>> +if (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&
>> +revs->diffopt.pathspec.nr == 1)
>> +DIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);
>> +
>>  if (prune_data.nr) {
>>  /*
>>   * If we need to introduce the magic "a lone ':' means no
>
> revs->diffopt.pathspec isn't set up yet then. But prune_data is, so I
> can use that. 
>
> Will send a v3.
--
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: weaning distributions off tarballs: extended verification of git tags

2015-07-07 Thread Colin Walters


On Sat, Feb 28, 2015, at 10:48 AM, Colin Walters wrote:
> Hi, 
> 
> TL;DR: Let's define a standard for embedding stronger checksums in tags and 
> commit messages:
> https://github.com/cgwalters/homegit/blob/master/bin/git-evtag

[time passes]

I finally had a bit of time to pick this back up again in:

https://github.com/cgwalters/git-evtag

It should address the core concern here about stability of `git archive`.

I prototyped it out with libgit2 because it was easier, and I'd like actually 
to be able to use this with older versions of git.

But I think the next steps here are:

- Validate the core design
  * Tree walking order
  * Submodule recursion
  * Use of SHA512
- Standardize it
  (Would like to see at least a stupid slow shell script implementation to 
cross-validate)
- Add it as an option to `git tag`?

Longer term:
- Support adding `Git-EVTag` as a git note, so I can retroactively add stronger
  checksums to older git repositories
- Anything else?

--
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: Git grep does not support multi-byte characters (like UTF-8)

2015-07-07 Thread Duy Nguyen
On Wed, Jul 8, 2015 at 1:08 AM, Plamen Totev  wrote:
> Junio C Hamano  writes:
>
>> Plamen Totev  writes:
>>
>> > pickaxe search also uses kwsearch so the case insensitive search with
>> > it does not work (e.g. git log -i -S). Maybe this is a less of a
>> > problem here as one is expected to search for exact string (hence
>> > knows the case)
>>
>> You reasoned correctly, I think. Pickaxe, as one of the building
>> blocks to implement Linus's ultimate change tracking tool [*1*],
>> should never pay attention to "-i". It is a step to finding the
>> commit that touches the exact code block given (i.e. "how do you
>> drill down?" part of $gmane/217 message).
>>
>> Thanks.
>>
>> [Footnote]
>> *1* http://article.gmane.org/gmane.comp.version-control.git/217
>
> Now that I read the link again and gave the matter a thought I'm not so sure.
> In some contexts the case of the words does not matter. In SQL for example.
> Let's consider a SQL script file that contains the following line:
>
> select name, address from customers;
>
> At some point we decide to change the coding style to:
>
> SELECT name, address FROM customers;

On top of this, pickaxe already supports icase even kws is used. But
it only works for ascii, so either we fix it and support non-ascii, or
we remove icase support entirely from diffcore_pickaxe(). I vote the
former.
-- 
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


[PATCH v3] log: add log.follow config option

2015-07-07 Thread David Turner
Many users prefer to always use --follow with logs.  Rather than
aliasing the command, an option might be more convenient for some.

Signed-off-by: David Turner 
---
 Documentation/git-log.txt |  6 ++
 builtin/log.c |  7 +++
 diff.c|  5 +++--
 diff.h|  1 +
 revision.c| 17 +++--
 t/t4202-log.sh| 23 +++
 6 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 5692945..79bf4d4 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -184,6 +184,12 @@ log.date::
`--date` option.)  Defaults to "default", which means to write
dates like `Sat May 8 19:35:34 2010 -0500`.
 
+log.follow::
+   If a single file argument is given to git log, it will act as
+   if the `--follow` option was also used.  This has the same
+   limitations as `--follow`, i.e. it cannot be used to follow
+   multiple files and does not work well on non-linear history.
+
 log.showRoot::
If `false`, `git log` and related commands will not treat the
initial commit as a big creation event.  Any root commits in
diff --git a/builtin/log.c b/builtin/log.c
index 8781049..6a068f7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;
 
 static int default_abbrev_commit;
 static int default_show_root = 1;
+static int default_follow;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -102,6 +103,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 {
if (fmt_pretty)
get_commit_format(fmt_pretty, rev);
+   if (default_follow)
+   DIFF_OPT_SET(&rev->diffopt, DEFAULT_FOLLOW_RENAMES);
rev->verbose_header = 1;
DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
rev->diffopt.stat_width = -1; /* use full terminal width */
@@ -390,6 +393,10 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
default_show_root = git_config_bool(var, value);
return 0;
}
+   if (!strcmp(var, "log.follow")) {
+   default_follow = git_config_bool(var, value);
+   return 0;
+   }
if (skip_prefix(var, "color.decorate.", &slot_name))
return parse_decorate_color_config(var, slot_name, value);
if (!strcmp(var, "log.mailmap")) {
diff --git a/diff.c b/diff.c
index 87b16d5..135b222 100644
--- a/diff.c
+++ b/diff.c
@@ -3815,9 +3815,10 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
DIFF_OPT_SET(options, FIND_COPIES_HARDER);
else if (!strcmp(arg, "--follow"))
DIFF_OPT_SET(options, FOLLOW_RENAMES);
-   else if (!strcmp(arg, "--no-follow"))
+   else if (!strcmp(arg, "--no-follow")) {
DIFF_OPT_CLR(options, FOLLOW_RENAMES);
-   else if (!strcmp(arg, "--color"))
+   DIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);
+   } else if (!strcmp(arg, "--color"))
options->use_color = 1;
else if (skip_prefix(arg, "--color=", &arg)) {
int value = git_config_colorbool(NULL, arg);
diff --git a/diff.h b/diff.h
index c7ad42a..f7208ad 100644
--- a/diff.h
+++ b/diff.h
@@ -91,6 +91,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
diff_options *opt, void *data)
 #define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28)
 #define DIFF_OPT_FUNCCONTEXT (1 << 29)
 #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
+#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1 << 31)
 
 #define DIFF_OPT_TST(opts, flag)((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_TOUCHED(opts, flag)((opts)->touched_flags & 
DIFF_OPT_##flag)
diff --git a/revision.c b/revision.c
index 3ff8723..7c1931b 100644
--- a/revision.c
+++ b/revision.c
@@ -2311,15 +2311,13 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
if (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT)
revs->diff = 1;
 
-   /* Pickaxe, diff-filter and rename following need diffs */
-   if (revs->diffopt.pickaxe ||
-   revs->diffopt.filter ||
-   DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
-   revs->diff = 1;
-
if (revs->topo_order)
revs->limited = 1;
 
+   if (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&
+   revs->prune_data.nr == 1)
+   DIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);
+
if (revs->prune_data.nr) {
copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
/* Can't prune commits with rename following: the paths 
change.. */
@@ -2329,6 +2327,13 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
copy_pathspec(&revs->diffopt.pathspec,

Re: [PATCH v2] log: add log.follow config option

2015-07-07 Thread David Turner
On Tue, 2015-07-07 at 15:13 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> > diff --git a/revision.c b/revision.c
> > index 3ff8723..ae6d4c3 100644
> > --- a/revision.c
> > +++ b/revision.c
> > @@ -2322,12 +2322,21 @@ int setup_revisions(int argc, const char **argv, 
> > struct rev_info *revs, struct s
> >  
> > if (revs->prune_data.nr) {
> > copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
> > -   /* Can't prune commits with rename following: the paths 
> > change.. */
> > -   if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
> > -   revs->prune = 1;
> > +
> > if (!revs->full_diff)
> > copy_pathspec(&revs->diffopt.pathspec,
> >   &revs->prune_data);
> > +
> > +   if (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&
> > +   revs->diffopt.pathspec.nr == 1)
> > +   DIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);
> > +
> > +   /* Can't prune commits with rename following: the paths 
> > change.. */
> > +   if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES)) {
> > +   revs->prune = 1;
> > +   } else {
> > +   revs->diff = 1;
> > +   }
> > }
> > if (revs->combine_merges)
> > revs->ignore_merges = 0;
> 
> It is unfortunate that we have to waste one DIFF_OPT bit and touch
> revision.c for something that is "just a convenience".  Because
> setup_revisions() does not have a way to let you flip the settings
> depending on the number of pathspecs specified, I do not think of a
> solution that does not have to touch a low level foundation part of
> the codebase, which I'd really want to avoid.
> 
> But I wonder why your patch needs to touch so much.
> 
> Your changes to other files make sure that diffopt has the
> DEFAULT_FOLLOW_RENAMES when (1) the configuration is set and (2) the
> command line did not override it with --no-follow.  And these look
> very sensible.
> 
> Isn't the only thing left to do in this codepath, after the DEFAULT_
> is set up correctly, to set FOLLOW_RENAMES when (1) DEFAULT_ is set
> and (2) you have only one path?
>
> And once FOLLOW_RENAMES is set or unset according to the end-user
> visible semantics, i.e. "just for a convenience, setting log.follow
> adds --follow to the command line if and only if there is only one
> pathspec", I do not see why existing code needs to be modified in
> any other way.
> 
> IOW, I'd like to know why we need more than something like this
> change to this file, instead of the above?  We didn't muck with
> revs->diff in the original when FOLLOW_RENAMES was set, but now it
> does, for example.

We did, but we did it earlier.  But I can just rearrange the code.

> diff --git a/revision.c b/revision.c
> index 3ff8723..f7bd229 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2270,6 +2270,10 @@ int setup_revisions(int argc, const char **argv, 
> struct rev_info *revs, struct s
>   got_rev_arg = 1;
>   }
>  
> + if (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&
> + revs->diffopt.pathspec.nr == 1)
> + DIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);
> +
>   if (prune_data.nr) {
>   /*
>* If we need to introduce the magic "a lone ':' means no

revs->diffopt.pathspec isn't set up yet then. But prune_data is, so I
can use that. 

Will send a v3.

--
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 v7 7/8] update-ref and tag: add --create-reflog arg

2015-07-07 Thread David Turner
Allow the creation of a ref (e.g. stash) with a reflog already in
place. For most refs (e.g. those under refs/heads), this happens
automatically, but for others, we need this option.

Currently, git does this by pre-creating the reflog, but alternate ref
backends might store reflogs somewhere other than .git/logs.  Code
that now directly manipulates .git/logs should instead use git
plumbing commands.

I also added --create-reflog to git tag, just for completeness.

In a moment, we will use this argument to make git stash work with
alternate ref backends.

Signed-off-by: David Turner 
---
 Documentation/git-tag.txt|  5 -
 Documentation/git-update-ref.txt |  5 -
 builtin/tag.c|  5 +
 builtin/update-ref.c | 17 +++--
 t/t1400-update-ref.sh| 38 ++
 t/t7004-tag.sh   |  9 -
 6 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 034d10d..2312980 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 [ | ]
 'git tag' -d ...
 'git tag' [-n[]] -l [--contains ] [--points-at ]
-   [--column[=] | --no-column] [...]
+   [--column[=] | --no-column] [--create-reflog] [...]
[...]
 'git tag' -v ...
 
@@ -143,6 +143,9 @@ This option is only applicable when listing tags without 
annotation lines.
all, 'whitespace' removes just leading/trailing whitespace lines and
'strip' removes both whitespace and commentary.
 
+--create-reflog::
+   Create a reflog for the tag.
+
 ::
The name of the tag to create, delete, or describe.
The new tag name must pass all checks defined by
diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index c8f5ae5..969bfab 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely
 SYNOPSIS
 
 [verse]
-'git update-ref' [-m ] (-d  [] | [--no-deref]  
 [] | --stdin [-z])
+'git update-ref' [-m ] (-d  [] | [--no-deref] 
[--create-reflog]   [] | --stdin [-z])
 
 DESCRIPTION
 ---
@@ -67,6 +67,9 @@ performs all modifications together.  Specify commands of the 
form:
verify SP  [SP ] LF
option SP  LF
 
+With `--create-reflog`, update-ref will create a reflog for each ref
+even if one would not ordinarily be created.
+
 Quote fields containing whitespace as if they were strings in C source
 code; i.e., surrounded by double-quotes and with backslash escapes.
 Use 40 "0" characters or the empty string to specify a zero value.  To
diff --git a/builtin/tag.c b/builtin/tag.c
index 5f6cdc5..896f434 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -579,6 +579,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct create_tag_options opt;
char *cleanup_arg = NULL;
int annotate = 0, force = 0, lines = -1;
+   int create_reflog = 0;
int cmdmode = 0;
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
@@ -605,6 +606,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_STRING('u', "local-user", &keyid, N_("key-id"),
N_("use another key to sign the tag")),
OPT__FORCE(&force, N_("replace the tag if exists")),
+   OPT_BOOL(0, "create-reflog", &create_reflog, 
N_("create_reflog")),
 
OPT_GROUP(N_("Tag listing options")),
OPT_COLUMN(0, "column", &colopts, N_("show tag list in 
columns")),
@@ -730,6 +732,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (annotate)
create_tag(object, tag, &buf, &opt, prev, object);
 
+   if (create_reflog && safe_create_reflog(ref.buf, &err, 1))
+   die("failed to create reflog for %s: %s", ref.buf, err.buf);
+
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, object, prev,
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 6763cf1..d570e5e 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -14,6 +14,7 @@ static const char * const git_update_ref_usage[] = {
 
 static char line_termination = '\n';
 static int update_flags;
+int create_reflog;
 static const char *msg;
 
 /*
@@ -198,6 +199,9 @@ static const char *parse_cmd_update(struct ref_transaction 
*transaction,
if (*next != line_termination)
die("update %s: extra input: %s", refname, next);
 
+   if (create_reflog && safe_create_reflog(refname, &err, 1))
+   die("failed to create reflog for %s: %s", refname, err.buf);
+
if (ref_transaction_update(transaction, refname,
   new_sha1, have_old ? old_sha1 : NUL

[PATCH v7 4/8] refs: Break out check for reflog autocreation

2015-07-07 Thread David Turner
This is just for clarity.

Signed-off-by: David Turner 
---
 refs.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/refs.c b/refs.c
index e891bed..e694359 100644
--- a/refs.c
+++ b/refs.c
@@ -3118,6 +3118,16 @@ static int copy_msg(char *buf, const char *msg)
return cp - buf;
 }
 
+static int should_autocreate_reflog(const char *refname)
+{
+   if (!log_all_ref_updates)
+   return 0;
+   return starts_with(refname, "refs/heads/") ||
+   starts_with(refname, "refs/remotes/") ||
+   starts_with(refname, "refs/notes/") ||
+   !strcmp(refname, "HEAD");
+}
+
 /* This function will fill in *err and return -1 on failure */
 int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct 
strbuf *err)
 {
@@ -3128,11 +3138,7 @@ int log_ref_setup(const char *refname, struct strbuf 
*sb_logfile, struct strbuf
logfile = sb_logfile->buf;
/* make sure the rest of the function can't change "logfile" */
sb_logfile = NULL;
-   if (log_all_ref_updates &&
-   (starts_with(refname, "refs/heads/") ||
-starts_with(refname, "refs/remotes/") ||
-starts_with(refname, "refs/notes/") ||
-!strcmp(refname, "HEAD"))) {
+   if (should_autocreate_reflog(refname)) {
if (safe_create_leading_directories(logfile) < 0) {
strbuf_addf(err, "unable to create directory for %s. "
"%s", logfile, strerror(errno));
-- 
2.0.5.499.g01f6352.dirty-twtrsrc

--
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 v7 3/8] bisect: treat BISECT_HEAD as a ref

2015-07-07 Thread David Turner
Instead of directly writing to and reading from files in
$GIT_DIR, use ref API to interact with BISECT_HEAD.

Signed-off-by: David Turner 
---
 git-bisect.sh | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index ae3fec2..2fd8ea6 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -35,7 +35,7 @@ _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
 
 bisect_head()
 {
-   if test -f "$GIT_DIR/BISECT_HEAD"
+   if bisect_head_exists
then
echo BISECT_HEAD
else
@@ -209,6 +209,10 @@ check_expected_revs() {
done
 }
 
+bisect_head_exists() {
+   git rev-parse --quiet --verify "BISECT_HEAD" >/dev/null
+}
+
 bisect_skip() {
all=''
for arg in "$@"
@@ -310,7 +314,7 @@ bisect_next() {
bisect_next_check good
 
# Perform all bisection computation, display and checkout
-   git bisect--helper --next-all $(test -f "$GIT_DIR/BISECT_HEAD" && echo 
--no-checkout)
+   git bisect--helper --next-all $(bisect_head_exists && echo 
--no-checkout)
res=$?
 
# Check if we should exit because bisection is finished
@@ -377,7 +381,7 @@ bisect_reset() {
usage ;;
esac
 
-   if ! test -f "$GIT_DIR/BISECT_HEAD" && ! git checkout "$branch" --
+   if ! bisect_head_exists && ! git checkout "$branch" --
then
die "$(eval_gettext "Could not check out original HEAD 
'\$branch'.
 Try 'git bisect reset '.")"
-- 
2.0.5.499.g01f6352.dirty-twtrsrc

--
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 v7 1/8] refs.c: add err arguments to reflog functions

2015-07-07 Thread David Turner
Add an err argument to log_ref_setup that can explain the reason
for a failure. This then eliminates the need to manage errno through
this function since we can just add strerror(errno) to the err string
when meaningful. No callers relied on errno from this function for
anything else than the error message.

Also add err arguments to private functions write_ref_to_lockfile,
log_ref_write_1, commit_ref_update. This again eliminates the need to
manage errno in these functions.

Some error messages change slightly.  For instance, we sometimes lose
"cannot update ref" and instead only show the specific cause of ref
update failure.

Update of a patch by Ronnie Sahlberg.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
---
 builtin/checkout.c |   8 ++--
 refs.c | 130 +
 refs.h |   4 +-
 3 files changed, 79 insertions(+), 63 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index c018ab3..93f63d3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -624,16 +624,18 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
struct strbuf log_file = STRBUF_INIT;
int ret;
const char *ref_name;
+   struct strbuf err = STRBUF_INIT;
 
ref_name = mkpath("refs/heads/%s", 
opts->new_orphan_branch);
temp = log_all_ref_updates;
log_all_ref_updates = 1;
-   ret = log_ref_setup(ref_name, &log_file);
+   ret = log_ref_setup(ref_name, &log_file, &err);
log_all_ref_updates = temp;
strbuf_release(&log_file);
if (ret) {
-   fprintf(stderr, _("Can not do reflog 
for '%s'\n"),
-   opts->new_orphan_branch);
+   fprintf(stderr, _("Can not do reflog 
for '%s'. %s\n"),
+   opts->new_orphan_branch, 
err.buf);
+   strbuf_release(&err);
return;
}
}
diff --git a/refs.c b/refs.c
index fb568d7..e891bed 100644
--- a/refs.c
+++ b/refs.c
@@ -2975,9 +2975,11 @@ static int rename_ref_available(const char *oldname, 
const char *newname)
return ret;
 }
 
-static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char 
*sha1);
+static int write_ref_to_lockfile(struct ref_lock *lock,
+const unsigned char *sha1, struct strbuf *err);
 static int commit_ref_update(struct ref_lock *lock,
-const unsigned char *sha1, const char *logmsg);
+const unsigned char *sha1, const char *logmsg,
+struct strbuf *err);
 
 int rename_ref(const char *oldrefname, const char *newrefname, const char 
*logmsg)
 {
@@ -3038,9 +3040,10 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
}
hashcpy(lock->old_oid.hash, orig_sha1);
 
-   if (write_ref_to_lockfile(lock, orig_sha1) ||
-   commit_ref_update(lock, orig_sha1, logmsg)) {
-   error("unable to write current sha1 into %s", newrefname);
+   if (write_ref_to_lockfile(lock, orig_sha1, &err) ||
+   commit_ref_update(lock, orig_sha1, logmsg, &err)) {
+   error("unable to write current sha1 into %s: %s", newrefname, 
err.buf);
+   strbuf_release(&err);
goto rollback;
}
 
@@ -3056,9 +3059,11 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
 
flag = log_all_ref_updates;
log_all_ref_updates = 0;
-   if (write_ref_to_lockfile(lock, orig_sha1) ||
-   commit_ref_update(lock, orig_sha1, NULL))
-   error("unable to write current sha1 into %s", oldrefname);
+   if (write_ref_to_lockfile(lock, orig_sha1, &err) ||
+   commit_ref_update(lock, orig_sha1, NULL, &err)) {
+   error("unable to write current sha1 into %s: %s", oldrefname, 
err.buf);
+   strbuf_release(&err);
+   }
log_all_ref_updates = flag;
 
  rollbacklog:
@@ -3113,8 +3118,8 @@ static int copy_msg(char *buf, const char *msg)
return cp - buf;
 }
 
-/* This function must set a meaningful errno on failure */
-int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
+/* This function will fill in *err and return -1 on failure */
+int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct 
strbuf *err)
 {
int logfd, oflags = O_APPEND | O_WRONLY;
char *logfile;
@@ -3129,9 +3134,8 @@

[PATCH v7 8/8] git-stash: use update-ref --create-reflog instead of creating files

2015-07-07 Thread David Turner
This is in support of alternate ref backends which don't necessarily
store reflogs as files.

Signed-off-by: David Turner 
---
 git-stash.sh | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 8e9e2cd..1d5ba7a 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -183,9 +183,7 @@ store_stash () {
stash_msg="Created via \"git stash store\"."
fi
 
-   # Make sure the reflog for stash is kept.
-   : >>"$(git rev-parse --git-path logs/$ref_stash)"
-   git update-ref -m "$stash_msg" $ref_stash $w_commit
+   git update-ref --create-reflog -m "$stash_msg" $ref_stash $w_commit
ret=$?
test $ret != 0 && test -z $quiet &&
die "$(eval_gettext "Cannot update \$ref_stash with \$w_commit")"
@@ -262,7 +260,7 @@ save_stash () {
say "$(gettext "No local changes to save")"
exit 0
fi
-   test -f "$(git rev-parse --git-path logs/$ref_stash)" ||
+   git reflog exists $ref_stash ||
clear_stash || die "$(gettext "Cannot initialize stash")"
 
create_stash "$stash_msg" $untracked
-- 
2.0.5.499.g01f6352.dirty-twtrsrc

--
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 v7 2/8] cherry-pick: treat CHERRY_PICK_HEAD and REVERT_HEAD as refs

2015-07-07 Thread David Turner
Instead of directly writing to and reading from files in
$GIT_DIR, use ref API to interact with CHERRY_PICK_HEAD
and REVERT_HEAD.

Signed-off-by: David Turner 
---
 branch.c |  4 ++--
 builtin/commit.c |  6 +++---
 builtin/merge.c  |  2 +-
 contrib/completion/git-prompt.sh |  4 ++--
 git-gui/lib/commit.tcl   |  2 +-
 sequencer.c  | 31 ++-
 t/t7509-commit.sh|  4 ++--
 wt-status.c  |  6 ++
 8 files changed, 23 insertions(+), 36 deletions(-)

diff --git a/branch.c b/branch.c
index b002435..ec598aa 100644
--- a/branch.c
+++ b/branch.c
@@ -302,8 +302,8 @@ void create_branch(const char *head,
 
 void remove_branch_state(void)
 {
-   unlink(git_path("CHERRY_PICK_HEAD"));
-   unlink(git_path("REVERT_HEAD"));
+   delete_ref("CHERRY_PICK_HEAD", NULL, REF_NODEREF);
+   delete_ref("REVERT_HEAD", NULL, REF_NODEREF);
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_RR"));
unlink(git_path("MERGE_MSG"));
diff --git a/builtin/commit.c b/builtin/commit.c
index b5b1158..53c7e90 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -168,7 +168,7 @@ static void determine_whence(struct wt_status *s)
 {
if (file_exists(git_path("MERGE_HEAD")))
whence = FROM_MERGE;
-   else if (file_exists(git_path("CHERRY_PICK_HEAD"))) {
+   else if (ref_exists("CHERRY_PICK_HEAD")) {
whence = FROM_CHERRY_PICK;
if (file_exists(git_path(SEQ_DIR)))
sequencer_in_use = 1;
@@ -1777,8 +1777,8 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
}
ref_transaction_free(transaction);
 
-   unlink(git_path("CHERRY_PICK_HEAD"));
-   unlink(git_path("REVERT_HEAD"));
+   delete_ref("CHERRY_PICK_HEAD", NULL, REF_NODEREF);
+   delete_ref("REVERT_HEAD", NULL, REF_NODEREF);
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_MSG"));
unlink(git_path("MERGE_MODE"));
diff --git a/builtin/merge.c b/builtin/merge.c
index 46aacd6..3e2ae2f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1206,7 +1206,7 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
else
die(_("You have not concluded your merge (MERGE_HEAD 
exists)."));
}
-   if (file_exists(git_path("CHERRY_PICK_HEAD"))) {
+   if (ref_exists("CHERRY_PICK_HEAD")) {
if (advice_resolve_conflict)
die(_("You have not concluded your cherry-pick 
(CHERRY_PICK_HEAD exists).\n"
"Please, commit your changes before you merge."));
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 366f0bc..e2c5583 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -415,9 +415,9 @@ __git_ps1 ()
fi
elif [ -f "$g/MERGE_HEAD" ]; then
r="|MERGING"
-   elif [ -f "$g/CHERRY_PICK_HEAD" ]; then
+   elif git rev-parse --quiet --verify "CHERRY_PICK_HEAD" 
>/dev/null; then
r="|CHERRY-PICKING"
-   elif [ -f "$g/REVERT_HEAD" ]; then
+   elif git rev-parse --quiet --verify "REVERT_HEAD" >/dev/null; 
then
r="|REVERTING"
elif [ -f "$g/BISECT_LOG" ]; then
r="|BISECTING"
diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl
index 864b687..2b08b13 100644
--- a/git-gui/lib/commit.tcl
+++ b/git-gui/lib/commit.tcl
@@ -409,7 +409,7 @@ A rescan will be automatically started now.
catch {file delete [gitdir MERGE_MSG]}
catch {file delete [gitdir SQUASH_MSG]}
catch {file delete [gitdir GITGUI_MSG]}
-   catch {file delete [gitdir CHERRY_PICK_HEAD]}
+   catch {git update-ref -d --no-deref CHERRY_PICK_HEAD}
 
# -- Let rerere do its thing.
#
diff --git a/sequencer.c b/sequencer.c
index f8421a8..90396ba 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -158,21 +158,10 @@ static void free_message(struct commit *commit, struct 
commit_message *msg)
unuse_commit_buffer(commit, msg->message);
 }
 
-static void write_cherry_pick_head(struct commit *commit, const char 
*pseudoref)
+static void write_cherry_pick_head(struct commit *commit, const char *ref)
 {
-   const char *filename;
-   int fd;
-   struct strbuf buf = STRBUF_INIT;
-
-   strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
-
-   filename = git_path("%s", pseudoref);
-   fd = open(filename, O_WRONLY | O_CREAT, 0666);
-   if (fd < 0)
-   die_errno(_("Could not open '%s' for writing"), filename);
-   if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
-   die_errno(_("Could not write to '%s'"), filename);
-   strbuf

[PATCH v7 5/8] refs: new public ref function: safe_create_reflog

2015-07-07 Thread David Turner
The safe_create_reflog function creates a reflog, if it does not
already exist.

The log_ref_setup function becomes private and gains a force_create
parameter to force the creation of a reflog even if log_all_ref_updates
is false or the refname is not one of the special refnames.

The new parameter also reduces the need to store, modify, and restore
the log_all_ref_updates global before reflog creation.

In a moment, we will use this to add reflog creation commands to
git-reflog.

Signed-off-by: David Turner 
---
 builtin/checkout.c | 16 +---
 refs.c | 25 +
 refs.h |  2 +-
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 93f63d3..c840f33 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -620,24 +620,18 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
if (opts->new_branch) {
if (opts->new_orphan_branch) {
if (opts->new_branch_log && !log_all_ref_updates) {
-   int temp;
-   struct strbuf log_file = STRBUF_INIT;
-   int ret;
-   const char *ref_name;
+   char *refname;
struct strbuf err = STRBUF_INIT;
 
-   ref_name = mkpath("refs/heads/%s", 
opts->new_orphan_branch);
-   temp = log_all_ref_updates;
-   log_all_ref_updates = 1;
-   ret = log_ref_setup(ref_name, &log_file, &err);
-   log_all_ref_updates = temp;
-   strbuf_release(&log_file);
-   if (ret) {
+   refname = mkpathdup("refs/heads/%s", 
opts->new_orphan_branch);
+   if (safe_create_reflog(refname, &err, 1)) {
+   free(refname);
fprintf(stderr, _("Can not do reflog 
for '%s'. %s\n"),
opts->new_orphan_branch, 
err.buf);
strbuf_release(&err);
return;
}
+   free(refname);
}
}
else
diff --git a/refs.c b/refs.c
index e694359..01b0af5 100644
--- a/refs.c
+++ b/refs.c
@@ -3128,8 +3128,14 @@ static int should_autocreate_reflog(const char *refname)
!strcmp(refname, "HEAD");
 }
 
-/* This function will fill in *err and return -1 on failure */
-int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct 
strbuf *err)
+/*
+ * Create a reflog for a ref.  If force_create = 0, the reflog will
+ * only be created for certain refs (those for which
+ * should_autocreate_reflog returns non-zero.  Otherwise, it will be
+ * created regardless of the ref name.  This function will fill in
+ * *err and return -1 on failure
+ */
+static int log_ref_setup(const char *refname, struct strbuf *sb_logfile, 
struct strbuf *err, int force_create)
 {
int logfd, oflags = O_APPEND | O_WRONLY;
char *logfile;
@@ -3138,7 +3144,7 @@ int log_ref_setup(const char *refname, struct strbuf 
*sb_logfile, struct strbuf
logfile = sb_logfile->buf;
/* make sure the rest of the function can't change "logfile" */
sb_logfile = NULL;
-   if (should_autocreate_reflog(refname)) {
+   if (force_create || should_autocreate_reflog(refname)) {
if (safe_create_leading_directories(logfile) < 0) {
strbuf_addf(err, "unable to create directory for %s. "
"%s", logfile, strerror(errno));
@@ -3173,6 +3179,17 @@ int log_ref_setup(const char *refname, struct strbuf 
*sb_logfile, struct strbuf
return 0;
 }
 
+
+int safe_create_reflog(const char *refname, struct strbuf *err, int 
force_create)
+{
+   int ret;
+   struct strbuf sb = STRBUF_INIT;
+
+   ret = log_ref_setup(refname, &sb, err, force_create);
+   strbuf_release(&sb);
+   return ret;
+}
+
 static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
const unsigned char *new_sha1,
const char *committer, const char *msg)
@@ -3209,7 +3226,7 @@ static int log_ref_write_1(const char *refname, const 
unsigned char *old_sha1,
if (log_all_ref_updates < 0)
log_all_ref_updates = !is_bare_repository();
 
-   result = log_ref_setup(refname, sb_log_file, err);
+   result = log_ref_setup(refname, sb_log_file, err, 0);
 
if (result)
return result;
diff --git a/refs.h b/refs.h
index debdefc..3b90e16 100644
--- a/refs.h
+++ b/refs.h
@@ -228,7 +228,7 @@

[PATCH v7 6/8] git-reflog: add exists command

2015-07-07 Thread David Turner
Theis are necessary because alternate ref backends might store reflogs
somewhere other than .git/logs.  Code that now directly manipulates
.git/logs should instead go through git-reflog.

Signed-off-by: David Turner 
---
 Documentation/git-reflog.txt |  4 
 builtin/reflog.c | 33 -
 t/t1411-reflog-show.sh   |  5 +
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
index 5e7908e..4b08fc7 100644
--- a/Documentation/git-reflog.txt
+++ b/Documentation/git-reflog.txt
@@ -23,6 +23,7 @@ depending on the subcommand:
[--dry-run] [--verbose] [--all | ...]
 'git reflog delete' [--rewrite] [--updateref]
[--dry-run] [--verbose] ref@\{specifier\}...
+'git reflog exists' 
 
 Reference logs, or "reflogs", record when the tips of branches and
 other references were updated in the local repository. Reflogs are
@@ -52,6 +53,9 @@ argument must be an _exact_ entry (e.g. "`git reflog delete
 master@{2}`"). This subcommand is also typically not used directly by
 end users.
 
+The "exists" subcommand checks whether a ref has a reflog.  It exists
+with zero status if the reflog exists, and non-zero status if it does
+not.
 
 OPTIONS
 ---
diff --git a/builtin/reflog.c b/builtin/reflog.c
index c2eb8ff..7ed0e85 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -13,6 +13,8 @@ static const char reflog_expire_usage[] =
 "git reflog expire [--expire=] [--expire-unreachable=] [--rewrite] 
[--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all] ...";
 static const char reflog_delete_usage[] =
 "git reflog delete [--rewrite] [--updateref] [--dry-run | -n] [--verbose] 
...";
+static const char reflog_exists_usage[] =
+"git reflog exists ";
 
 static unsigned long default_reflog_expire;
 static unsigned long default_reflog_expire_unreachable;
@@ -699,12 +701,38 @@ static int cmd_reflog_delete(int argc, const char **argv, 
const char *prefix)
return status;
 }
 
+static int cmd_reflog_exists(int argc, const char **argv, const char *prefix)
+{
+   int i, start = 0;
+
+   for (i = 1; i < argc; i++) {
+   const char *arg = argv[i];
+   if (!strcmp(arg, "--")) {
+   i++;
+   break;
+   }
+   else if (arg[0] == '-')
+   usage(reflog_exists_usage);
+   else
+   break;
+   }
+
+   start = i;
+
+   if (argc - start != 1)
+   usage(reflog_exists_usage);
+
+   if (check_refname_format(argv[start], REFNAME_ALLOW_ONELEVEL))
+   die("invalid ref format: %s", argv[start]);
+   return !reflog_exists(argv[start]);
+}
+
 /*
  * main "reflog"
  */
 
 static const char reflog_usage[] =
-"git reflog [ show | expire | delete ]";
+"git reflog [ show | expire | delete | exists ]";
 
 int cmd_reflog(int argc, const char **argv, const char *prefix)
 {
@@ -724,5 +752,8 @@ int cmd_reflog(int argc, const char **argv, const char 
*prefix)
if (!strcmp(argv[1], "delete"))
return cmd_reflog_delete(argc - 1, argv + 1, prefix);
 
+   if (!strcmp(argv[1], "exists"))
+   return cmd_reflog_exists(argc - 1, argv + 1, prefix);
+
return cmd_log_reflog(argc, argv, prefix);
 }
diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh
index 6f47c0d..3eb4f10 100755
--- a/t/t1411-reflog-show.sh
+++ b/t/t1411-reflog-show.sh
@@ -166,4 +166,9 @@ test_expect_success 'git log -g -p shows diffs vs. parents' 
'
test_cmp expect actual
 '
 
+test_expect_success 'reflog exists works' '
+   git reflog exists refs/heads/master &&
+   ! git reflog exists refs/heads/nonexistent
+'
+
 test_done
-- 
2.0.5.499.g01f6352.dirty-twtrsrc

--
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 v6 6/7] git-reflog: add create and exists functions

2015-07-07 Thread David Turner
On Mon, 2015-07-06 at 18:51 +0200, Michael Haggerty wrote:

> > +{
> > +   int i, status = 0, start = 0;
> 
> It looks like start is initialized unconditionally after the first loop,
> so the initialization here is a red herring.

Will fix.

> So, I have a philosophical question here with a practical side...
> 
> It appears that this command allows you to create a reflog for a
> reference that doesn't yet exist. At first blush, this seems to make
> sense. Suppose you want the creation of a reference to be logged. Then
> you can first turn on its reflog, then create it.
> 
> But I think this is going to create problems. Reflogs are rather
> intimately connected to their references. For example, writes to a
> reflog are guarded by locking the corresponding reference. And currently
> a reflog file is deleted when the corresponding reference is deleted.
> Also, there are constraints about which references can coexist; for
> example, references "refs/heads/foo" and "refs/heads/foo/bar" cannot
> exist at the same time, because (at least when using the filesystem
> backend), "refs/heads/foo" would have to be both a file and a directory
> at the same time. So if one of these references already exists, it would
> be an error to create a reflog for the other one.
> 
> Similarly, if there is a reflog file for one of these references that
> was created by this command, but there is not yet a corresponding
> reference, then any command that later tries to create the other
> reference will not be able to create the reflog for *that* reference
> because it will conflict with the existing reflog. I doubt that the
> reference creation is smart enough to deal with this situation.
> 
> So all in all, I think it is unwise to allow a reflog to be created
> without its corresponding reference.
> 
> This, in turn, suggests one or both of the following alternatives:
> 
> 1. Allow "git reflog create", but only for references that already exist.

This turns out not to work for git stash, which wants to create a reflog
for stash creation.

> 2. If we want to allow a reflog to be created at the same time as the
> corresponding reference, the reference-creation commands ("git branch",
> "git tag", "git update-ref", and maybe some others) probably need a new
> option like "--create-reflog" (and, for symmetry, probably
> "--no-create-reflog").

git branch should already autocreate reflogs, since the refs it creates
are under refs/heads.

> At the API level, it might make sense for the ref-transaction functions
> to get a new "REF_FORCE_CREATE_REFLOG" flag or something.

Junio was opposed to the converse flag, so I'm going to just add
manually add code to create reflogs.


--
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 v3 23/23] checkout: retire --ignore-other-worktrees in favor of --force

2015-07-07 Thread Mark Levedahl

On 07/06/2015 03:40 PM, Junio C Hamano wrote:
If you are extending the history of some branch, then you would want 
to be on that branch. Why would you want to have another worktree that 
will get into a confusing state once you create that commit on the 
checked out branch in this newly created worktree? Wasn't the whole 
point of making the primary repository aware of the secondary 
worktrees via the "linked checkout" mechanism because that confusion 
was the biggest sore point of the old contrib/workdir implementation? 


The only issue I have with git-new-workdir is that git-gc in one 
worktree is unaware of what is in use in another so can prune things 
away. The linked worktrees here nicely solve that problem.


The main use I have of maintaining multiple checkouts of one branch is 
for testing / analysis (where said tests can take days to weeks to run). 
Disallowing use of git's normal mechanism of tracking what is checked 
out in each such tree forces use of another system to do so, just 
imposing different difficulties for this use case. I note that 1) code 
must be ADDED to git to prevent such duplicate checkouts which otherwise 
cause no difficulty to git itself, and 2) adding those checks requires 
additional work to avoid the fallout. I have yet to hear what the upside 
of such a restriction is, I only see downsides.


Mark
--
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: refspecs with '*' as part of pattern

2015-07-07 Thread Jacob Keller
On Tue, Jul 7, 2015 at 9:28 AM, Junio C Hamano  wrote:
> Daniel Barkalow  writes:
>
>> On Mon, 6 Jul 2015, Junio C Hamano wrote:
>>
>>> I cannot seem to be able to find related discussions around that
>>> patch, so this is only my guess, but I suspect that this is to
>>> discourage people from doing something like:
>>>
>>>  refs/tags/*:refs/tags/foo-*
>>>
>>> which would open can of worms (e.g. imagine you fetch with that
>>> pathspec and then push with refs/tags/*:refs/tags/* back there;
>>> would you now get foo-v1.0.0 and foo-foo-v1.0.0 for their v1.0.0
>>> tag?) we'd prefer not having to worry about.
>>
>> That wouldn't be it, since refs/tags/*:refs/tags/foo/* would have the same
>> problem, assuming you didn't set up the push refspec carefully.
>
> Thanks, I was wondering where you were ;-)  Nice to hear from you
> from time to time.
>
>> I think it was mostly that it would be too easy to accidentally do
>> something you don't want by having some other character instead of a
>> slash, like refs/heads/*:refs/heads-*.
>
> Hmm, interesting thought.
>
> But refs/heads/*:refs/heade/* would not be saved, so I do not think
> that is it, either.

In this case, I'm more in favor of just allowing these refs because
the user already has to manually change the refspecs, which is
something many users will never do. I also think that given the above
comments, we're not really protecting the user from anything extra...
aside from adding more pain because the globs don't work as expected.

I did submit a patch (from my @intel.com address since I can't seem to
get git-for-windows to send from my home computer) but I am willing to
re-work to drop the setting if everyone is ok with that...

Thoughts?

Regards,
Jake
--
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 v6 5/7] refs: new public ref function: safe_create_reflog

2015-07-07 Thread David Turner
On Mon, 2015-07-06 at 18:21 +0200, Michael Haggerty wrote:

 changes applied; will re-roll.

> > +
> > +int safe_create_reflog(const char *refname, struct strbuf *err, int 
> > force_create)
> > +{
> > +   int ret;
> > +   struct strbuf sb = STRBUF_INIT;
> > +
> > +   ret = log_ref_setup(refname, &sb, err, force_create);
> > +   strbuf_release(&sb);
> > +   return ret;
> > +}
> > +
> 
> Is it really necessary to have two functions, safe_create_reflog() and
> log_ref_setup()? I don't see any of the callers doing anything special
> with the sb_logfile argument from the latter, so maybe it could be
> inlined into safe_create_reflog()? Maybe I'm overlooking something.

log_ref_write_1 does use the sb_logfile argument.

--
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 v3 23/23] checkout: retire --ignore-other-worktrees in favor of --force

2015-07-07 Thread Eric Sunshine
On Tue, Jul 7, 2015 at 12:20 PM, Junio C Hamano  wrote:
> Eric Sunshine  writes:
> I would not mind "git worktree add -f" to disable the "no multiple
> checkouts of the same branch" safety, but I do not think it is
> sensible to remove "-i-o-w" and conflate everything into "--force".
> That would force people to disable other safety measures at the same
> time (e.g. protect local changes from differences between the
> current and next branches).

I'm fine with dropping this patch.
--
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: Git force push fails after a rejected push (unpack failed)?

2015-07-07 Thread Eric Sunshine
On Tue, Jul 7, 2015 at 3:49 PM, Jeff King  wrote:
> On Tue, Jul 07, 2015 at 09:31:25PM +0200, X H wrote:
>
>> For the moment, I'm the only one pushing to the remote, always with
>> the same user (second user is planned). I use git-for-windows which is
>> based on MSYS2. I have mounted the network share with noacl option so
>> permissions should be handled by the Windows share. I'm in a group
>> which has read/write access. I have not configured
>> core.sharedrepository, I don't think it is useful with noacl since
>> unix group are not used in this case. The permission for the folder
>> above the file with permission denied is rw, but this file is read
>> only so if git try to modify it it won't work.
>
> Ah, so this is not a push to a server, but to a share mounted on the
> local box?
>
> That is leaving my realm of expertise. I'm not sure if it could be a
> misconfiguration in your share setup, or that git is trying to do
> something that would work on a Unix machine, but not on a Windows share.
> You might want to ask on the msysgit list:
>
>   https://groups.google.com/forum/#!forum/msysgit

Is this possibly another case of Windows virus scanner interference?
That could account for its variable nature.
--
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: Whether Git supports directory level access or not?

2015-07-07 Thread Jacob Keller
On Tue, Jul 7, 2015 at 10:03 AM, Junio C Hamano  wrote:
> Jacob Keller  writes:
>
>> However, in-repo per-directory permissions make no sense, as there
>> would be no way to generate commits.
>
> That may be the case for the current generation of Git, but I do not
> think you have to be so pessimistic.
>
> Suppose that an imaginary future version of Git allowed you to
> "hide" one directory from you.  That is:
>
>  * A commit object records "tree". "git cat-file -t HEAD^{tree}"
>or "git ls-tree HEAD" lets you inspect its contents;
>
>  * The "hidden" directory shows up as one of the subtrees of that
>output.  It may say
>
>  04 tree b4006c408979a0c6261dbfaeaa36639457469ad4   hidden
>
>  * However, your repository lack b4006c40... object.  So if you did
>"git ls-tree HEAD:hidden", you would get "no such tree object".
>
>  * This imaginary future version of Git has a new implementation of
>the index (both on-disk and in-core) that lets you keep just the
>"tree" entry for an unmodified directory, without having to store
>any of the files and subdirectories in it.
>
>  * All the other machinery of this imaginary future version of Git
>are aware of the fact that "hidden" thing is not visible, or even
>available, to your clone of the project repository.  That means
>"fsck" does not complain about missing object b4006c40..., "push"
>knows it should not consider it an error that you cannot enumerate
>and send objects that are reachable from b4006c40..., etc.
>
> With such a Git, you can modify anything outside the parts of the
> project tree that are hidden from you, and make a commit.  The tree
> recorded in a new commit object would record the same
>
>  04 tree b4006c408979a0c6261dbfaeaa36639457469ad4   hidden
>
> for the "hidden" directory, and you can even push it back to update
> the parts for other people to see your work outside the "hidden"
> area.
>
> "All the other machinery" that would need to accomodate such a
> hidden directory would span the entire plumbing layer and
> transports.  The wire protocol would need to be updated, especially
> the part that determines what needs to be sent and received, which
> is currently purely on commit ancestry, needs to become aware of the
> paths.
>
> I am *NOT* saying that this is easy.  I'd imagine if we gather all
> the competent Gits in a room and have them work on it and doing
> nothing else for six months, we would have some system that works.
> It would be a lot of work.
>
> I think it may be worth doing in the longer term, and it will likely
> to have other benefits as side effects.
>
>  - For example, did you notice that my description above does not
>mention "permission" even once?  Yes, that's right.  This does
>not have to be limited to permissions.  The user may have decided
>that the "hidden" part of that directory structure is not
>interesting and said "git clone --exclude=hidden" when she made
>her clone to set it up.
>
>  - Also notice that the "new implementation of the index" that
>lazily expands subtrees does not say anythying about a directory
>that is "hidden"---it just said "an unmodified directory" and
>that was deliberate.  Even when you are not doing a "narrow
>clone", keeping an untouched tree without expanding its subtrees
>and blobs flatted into the index may make it faster when you are
>working on a series of many small commits each of which touches
>only a handful of files.
>
> I might agree with you that "in-repo per-directory permissions make
> no sense", but the reason to say so would not be because "there
> would be no way to generate commits".

Actually as you laid out here, it does make sense I had just assumed
you would need the tree object to actually be able to generate the
commits. It does sound like a lot of work though.

Regards,
Jake
--
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] git: add optional support for full pattern in fetch refspecs

2015-07-07 Thread Jacob Keller
This patch updates the refs.c check_refname_component logic in order to
allow for the possibility of using arbitrary patterns in fetch refspecs.
Specifically, patterns similar to `refs/tags/prefix-*:refs/tags/prefix-*`.

In order to ensure that standard users do not accidentally setup refspecs
which could cause issues, tie this feature to
remote..arbitrary_pattern boolean configuration option. This ensures
that no user will have this enabled on accident.

The primary reason against this is the ability to pull refs incorrectly
and then later push them again. Ie:

refs/tags/some-prefix-*:/refs/tags/other-prefix-*

This ref will essentially re-name the references locally. This is
generally not what you would want but there is no easy way to validate
this doesn't occur. Note this can already occur to normal pattern refspecs
via `refs/tags/*:refs/tags/namespace/*` refspecs, but these are a bit more
difficult to typo.

However, the additional functionality is useful for specifying to pull
certain patterns of refs, and can be useful if the power user decides to
enable it for a given remote.

Signed-off-by: Jacob Keller 
Cc: Daniel Barkalow 
Cc: Junio C Hamano 
---
 Documentation/config.txt   |  7 +
 Documentation/git-check-ref-format.txt | 12 ++---
 builtin/check-ref-format.c |  2 ++
 refs.c | 48 ++
 refs.h | 15 ++-
 remote.c   |  2 ++
 remote.h   |  1 +
 7 files changed, 61 insertions(+), 26 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3e37b93ed2ac..626492de7a7f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2331,6 +2331,13 @@ remote..prune::
remote (as if the `--prune` option was given on the command line).
Overrides `fetch.prune` settings, if any.
 
+remote..arbitrarypattern::
+   When set to true, fetching from this remote will allow arbitrary complex
+   patterns for the fetch refspecs. For example,
+   refs/tags/prefix-*:refs/tags/prefix-* will work as expected. Care 
should be
+   taken as you could fetch refs into names that don't exist on the remote 
and
+   may end up pushing them again later by accident.
+
 remotes.::
The list of remotes which are fetched by "git remote update
".  See linkgit:git-remote[1].
diff --git a/Documentation/git-check-ref-format.txt 
b/Documentation/git-check-ref-format.txt
index fc02959ba4ab..caab0e3037fa 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -43,8 +43,8 @@ Git imposes the following rules on how references are named:
   caret `^`, or colon `:` anywhere.
 
 . They cannot have question-mark `?`, asterisk `*`, or open
-  bracket `[` anywhere.  See the `--refspec-pattern` option below for
-  an exception to this rule.
+  bracket `[` anywhere.  See the `--refspec-pattern` and `--arbitrary-pattern`
+  options below for exceptions to this rule.
 
 . They cannot begin or end with a slash `/` or contain multiple
   consecutive slashes (see the `--normalize` option below for an
@@ -95,7 +95,13 @@ OPTIONS
(as used with remote repositories).  If this option is
enabled,  is allowed to contain a single `*`
in place of a one full pathname component (e.g.,
-   `foo/*/bar` but not `foo/bar*`).
+   `foo/*/bar` but not `foo/bar*`). If '--arbitrary-pattern' is set, then
+   a single `foo/bar*baz` pattern will be accepted.
+
+--arbitrary-pattern::
+   If '--refspec-pattern' is set, allow arbitrary patterns instead of the
+   default simple case. Implies '--refspec-pattern'. Note that only one '*'
+   pattern will be accepted.
 
 --normalize::
Normalize 'refname' by removing any leading slash (`/`)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index fd915d59841e..e0b8d00d5337 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -70,6 +70,8 @@ int cmd_check_ref_format(int argc, const char **argv, const 
char *prefix)
flags &= ~REFNAME_ALLOW_ONELEVEL;
else if (!strcmp(argv[i], "--refspec-pattern"))
flags |= REFNAME_REFSPEC_PATTERN;
+   else if (!strcmp(argv[i], "--arbitrary-pattern"))
+   flags |= REFNAME_ARBITRARY_PATTERN | 
REFNAME_REFSPEC_PATTERN;
else
usage(builtin_check_ref_format_usage);
}
diff --git a/refs.c b/refs.c
index 7ac05cf21a25..e4c24bfc778c 100644
--- a/refs.c
+++ b/refs.c
@@ -20,11 +20,12 @@ struct ref_lock {
  * 2: ., look for a preceding . to reject .. in refs
  * 3: {, look for a preceding @ to reject @{ in refs
  * 4: A bad character: ASCII control characters, "~", "^", ":" or SP
+ * 5: check for patterns to reject unless REFNAME_REFSPEC_PATTERN is set
  */
 static unsigne

Re: [PATCH v6 1/7] refs.c: add err arguments to reflog functions

2015-07-07 Thread David Turner
On Mon, 2015-07-06 at 17:53 +0200, Michael Haggerty wrote:
> On 06/29/2015 10:17 PM, David Turner wrote:
> > Add an err argument to log_ref_setup that can explain the reason
> > for a failure. This then eliminates the need to manage errno through
> > this function since we can just add strerror(errno) to the err string
> > when meaningful. No callers relied on errno from this function for
> > anything else than the error message.
> > 
> > Also add err arguments to private functions write_ref_to_lockfile,
> > log_ref_write_1, commit_ref_update. This again eliminates the need to
> > manage errno in these functions.
> > 
> > Update of a patch by Ronnie Sahlberg.
> 
> First a general comment: we have a convention, not yet very well adhered
> to, that error messages should start with lower-case letters. I know
> that in many cases you are just carrying along pre-existing error
> messages that are capitalized. But at a minimum, please avoid adding new
> error messages that are capitalized. And if you are feeling ambitious,
> feel free to lower-case some existing ones :-)

I'll save lowercasing messages for other patches, but I'll try to take
care with new messages.

...

> Above, the call to close(logfd) could clobber errno. It would be better
> to exchange the calls.

Done, thanks.

> Since you are passing err into log_ref_write(), I assume that it would
> already have an error message written to it by the time you enter into
> this block. Yet in the block you append more text to it.
> 
> It seems to me that you either want to clear err and replace it with
> your own message, or create a new message that looks like
> 
> Cannot update the ref '%s': %s
> 
> where the second "%s" is replaced with the error from log_ref_write().

Done, thanks.

> > @@ -3317,7 +3322,8 @@ static int commit_ref_update(struct ref_lock *lock,
> >   head_sha1, &head_flag);
> > if (head_ref && (head_flag & REF_ISSYMREF) &&
> > !strcmp(head_ref, lock->ref_name))
> > -   log_ref_write("HEAD", lock->old_oid.hash, sha1, logmsg);
> > +   log_ref_write("HEAD", lock->old_oid.hash, sha1, logmsg,
> > + err);
> 
> Here you don't check for errors from log_ref_write(). So it might or
> might not have written something to err. If it has, is that error
> handled correctly?

That was the case before this patch too. But I guess I might as well
check.

> Previously, the error generated here was "cannot update the ref '%s'."
> But now that you are letting write_ref_to_lockfile() fill err, the error
> message will be something from that function. This might be OK or it
> might not. If you think it is, it would be worth a word or two of
> justification in the commit message.

Will adjust commit message.

--
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 (Jul 2015, #02; Tue, 7)

2015-07-07 Thread 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'.

As there is at least one new topic in 2.5-rc that has a real and
severe breakage (I haven't merged the fix for it to 'master' yet),
we may probably need to delay the final by at least a few weeks.

Projects from GSoC students and Ensimag students have also been a
pleasure to work with.  I'd have to say that this year is much
better than some previous years.

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

--
[New Topics]

* es/worktree-add (2015-07-07) 23 commits
 - checkout: retire --ignore-other-worktrees in favor of --force
 - worktree: add: auto-vivify new branch when  is omitted
 - worktree: add: make -b/-B default to HEAD when  is omitted
 - worktree: extract basename computation to new function
 - checkout: require worktree unconditionally
 - checkout: retire --to option
 - tests: worktree: retrofit "checkout --to" tests for "worktree add"
 - worktree: add -b/-B options
 - worktree: add --detach option
 - worktree: add --force option
 - worktree: introduce "add" command
 - checkout: drop 'checkout_opts' dependency from prepare_linked_checkout
 - checkout: make --to unconditionally verbose
 - checkout: prepare_linked_checkout: drop now-unused 'new' argument
 - checkout: relocate --to's "no branch specified" check
 - checkout: fix bug with --to and relative HEAD
 - Documentation/git-worktree: add EXAMPLES section
 - Documentation/git-worktree: add high-level 'lock' overview
 - Documentation/git-worktree: split technical info from general description
 - Documentation/git-worktree: add BUGS section
 - Documentation: move linked worktree description from checkout to worktree
 - Documentation/git-worktree: associate options with commands
 - Documentation/git-checkout: fix incorrect worktree prune command
 (this branch uses nd/multiple-work-trees.)

 Update to the "linked checkout" in 2.5.0-rc1; while I very much
 like what I see in this series, I think it does not work well with
 the date-based release schedule for v2.5, and as we've been
 labelling the feature with "experimental, UI will change" warning,
 I am tempted to cook this (or a reroll of it) in 'next' and shoot
 for a refined version of it in 2.6, rather than delaying 2.5 final.

 An alternative obviously is to slip 2.5 final for a few weeks,
 waiting for this and possibly other hotfix topics to mature.

 Undecided.


* jc/fix-alloc-sortbuf-in-index-pack (2015-07-04) 1 commit
  (merged to 'next' on 2015-07-06 at c05da06)
 + index-pack: fix allocation of sorted_by_pos array

 Another hotfix for what is in 2.5-rc but not in 2.4

 The alternative to slip 2.5 final for a few weeks starting to
 become more and more attractive...


* jc/unexport-git-pager-in-use-in-pager (2015-07-03) 1 commit
 - pager: do not leak "GIT_PAGER_IN_USE" to the pager

 When you say "!" while running say "git log", you'd confuse
 yourself in the resulting shell, that may look as if you took
 control back to the original shell you spawned "git log" from but
 that isn't what is happening.  To that new shell, we leaked
 GIT_PAGER_IN_USE environment variable that was meant as a local
 communication between the original "Git" and subprocesses that was
 spawned by it after we launched the pager, which caused many
 "interesting" things to happen, e.g. "git diff | cat" still paints
 its output in color by default.

 Stop leaking that environment variable to the pager's half of the
 fork; we only need it on "Git" side when we spawn the pager.

 Will merge to 'next'.


* mh/strbuf-read-file-returns-ssize-t (2015-07-03) 1 commit
 - strbuf: strbuf_read_file() should return ssize_t

 Will merge to 'next'.


* mm/branch-doc-updates (2015-07-06) 2 commits
 - Documentation/branch: document -M and -D in terms of --force
 - Documentation/branch: document -d --force and -m --force

 Will merge to 'next'.


* pt/am-tests (2015-07-07) 12 commits
 - t3901: test git-am encoding conversion
 - t3418: non-interactive rebase --continue with rerere enabled
 - t4150: tests for am --[no-]scissors
 - t4150: am with post-applypatch hook
 - t4150: am with pre-applypatch hook
 - t4150: am with applypatch-msg hook
 - t4150: am --resolved fails if index has unmerged entries
 - t4150: am --resolved fails if index has no changes
 - t4150: am refuses patches when paused
 - t4151: am --abort will keep dirty index intact
 - t4150: am fails if index is dirty
 - t4150: am.messageid really adds the message id

 Will merge to 'next'.


* kn/for-each-tag-branch (2015-07-07) 11 commits
 - for-each-ref: add '--contains' option
 - ref-filter: implement '--contains' option
 - parse-options.h: add macros for '--contains' option
 - parse-option: rename parse_opt_with_commit()
 - for-each-ref: add '--merged' and

Re: [msysGit] Re: [PATCH 13/17] engine.pl: provide more debug print statements

2015-07-07 Thread Philip Oakley

From: "Sebastian Schuberth" 

On 25.06.2015 02:03, Philip Oakley wrote:


--- a/contrib/buildsystems/engine.pl
+++ b/contrib/buildsystems/engine.pl
@@ -41,6 +41,7 @@ EOM
  # Parse command-line options
  while (@ARGV) {
  my $arg = shift @ARGV;
+ #print "Arg: $arg \n";
  if ("$arg" eq "-h" || "$arg" eq "--help" || "$arg" eq "-?") {
  showUsage();
  exit(0);
@@ -129,6 +130,7 @@ sub parseMakeOutput
  print "Parsing GNU Make output to figure out build
structure...\n";
  my $line = 0;
  while (my $text = shift @makedry) {
+ #print "Make: $text\n"; # show the makedry line


Please never commit code that's been commented out. Also see

http://dev.solita.fi/2013/07/04/whats-in-a-good-commit.html

;-)


The gif was fun, even if it's a little OTT. It does smack of religious
dogma though ;-)



--


Hi Sebastian,

It's "deactivated code", not dead code [1].

I'd deliberately included this 'code', as per the commit message because
I saw this as a clear part of aiding future maintenance, and it matches
the rest of the code style, i.e. the judicious placement of a comment
that says 'here's the place to pick out the status when debugging' -
perhaps it's my engineering experience that sees the benefits.

These were the key debug points I used - other's I've removed from the 
series.


Philip


[1] In one of the replies to
http://embeddedgurus.com/barr-code/2013/02/dead-code-the-law-and-unintended-consequences/

:>> as per DO178B safety critical software development guideline
document Terms "Dead code" and "Deactivated Code" have distinct 
meanings:


Dead code : Dead code is source code (and it is a part of binary code)
that is not executed in the final system and it will be not having
traceability to any requirements (one can say unintentional code).

Deactivated code: code which is commented out or removed via #ifdef's
(it is not a part of final binary code) and it will be having
traceability to its low level requirements (its a intentional code and
it can be activated in some configurations through hardware traps for
debugging or other purposes.


--
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 v6 3/4] status: give more information during rebase -i

2015-07-07 Thread Junio C Hamano
By the way, does this have any potential interaction with 16cf51c7
(git-rebase--interactive.sh: add config option for custom
instruction format, 2015-06-13)?  I _think_ that the other topic
should only affect the collapsed format, so there hopefully
shouldn't be a problem, but just double checking if you folks
considered the ramifications already.

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] log: add log.follow config option

2015-07-07 Thread Junio C Hamano
David Turner  writes:

> diff --git a/revision.c b/revision.c
> index 3ff8723..ae6d4c3 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2322,12 +2322,21 @@ int setup_revisions(int argc, const char **argv, 
> struct rev_info *revs, struct s
>  
>   if (revs->prune_data.nr) {
>   copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
> - /* Can't prune commits with rename following: the paths 
> change.. */
> - if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
> - revs->prune = 1;
> +
>   if (!revs->full_diff)
>   copy_pathspec(&revs->diffopt.pathspec,
> &revs->prune_data);
> +
> + if (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&
> + revs->diffopt.pathspec.nr == 1)
> + DIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);
> +
> + /* Can't prune commits with rename following: the paths 
> change.. */
> + if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES)) {
> + revs->prune = 1;
> + } else {
> + revs->diff = 1;
> + }
>   }
>   if (revs->combine_merges)
>   revs->ignore_merges = 0;

It is unfortunate that we have to waste one DIFF_OPT bit and touch
revision.c for something that is "just a convenience".  Because
setup_revisions() does not have a way to let you flip the settings
depending on the number of pathspecs specified, I do not think of a
solution that does not have to touch a low level foundation part of
the codebase, which I'd really want to avoid.

But I wonder why your patch needs to touch so much.

Your changes to other files make sure that diffopt has the
DEFAULT_FOLLOW_RENAMES when (1) the configuration is set and (2) the
command line did not override it with --no-follow.  And these look
very sensible.

Isn't the only thing left to do in this codepath, after the DEFAULT_
is set up correctly, to set FOLLOW_RENAMES when (1) DEFAULT_ is set
and (2) you have only one path?

And once FOLLOW_RENAMES is set or unset according to the end-user
visible semantics, i.e. "just for a convenience, setting log.follow
adds --follow to the command line if and only if there is only one
pathspec", I do not see why existing code needs to be modified in
any other way.

IOW, I'd like to know why we need more than something like this
change to this file, instead of the above?  We didn't muck with
revs->diff in the original when FOLLOW_RENAMES was set, but now it
does, for example.

diff --git a/revision.c b/revision.c
index 3ff8723..f7bd229 100644
--- a/revision.c
+++ b/revision.c
@@ -2270,6 +2270,10 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
got_rev_arg = 1;
}
 
+   if (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&
+   revs->diffopt.pathspec.nr == 1)
+   DIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);
+
if (prune_data.nr) {
/*
 * If we need to introduce the magic "a lone ':' means no
--
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 v3] log: add log.follow config option

2015-07-07 Thread David Turner
Many users prefer to always use --follow with logs.  Rather than
aliasing the command, an option might be more convenient for some.

Signed-off-by: David Turner 
---
 Documentation/git-log.txt |  6 ++
 builtin/log.c |  7 +++
 diff.c|  5 +++--
 diff.h|  1 +
 revision.c| 14 +++---
 t/t4202-log.sh| 23 +++
 6 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 5692945..79bf4d4 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -184,6 +184,12 @@ log.date::
`--date` option.)  Defaults to "default", which means to write
dates like `Sat May 8 19:35:34 2010 -0500`.
 
+log.follow::
+   If a single file argument is given to git log, it will act as
+   if the `--follow` option was also used.  This has the same
+   limitations as `--follow`, i.e. it cannot be used to follow
+   multiple files and does not work well on non-linear history.
+
 log.showRoot::
If `false`, `git log` and related commands will not treat the
initial commit as a big creation event.  Any root commits in
diff --git a/builtin/log.c b/builtin/log.c
index 8781049..6a068f7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;
 
 static int default_abbrev_commit;
 static int default_show_root = 1;
+static int default_follow;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -102,6 +103,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 {
if (fmt_pretty)
get_commit_format(fmt_pretty, rev);
+   if (default_follow)
+   DIFF_OPT_SET(&rev->diffopt, DEFAULT_FOLLOW_RENAMES);
rev->verbose_header = 1;
DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
rev->diffopt.stat_width = -1; /* use full terminal width */
@@ -390,6 +393,10 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
default_show_root = git_config_bool(var, value);
return 0;
}
+   if (!strcmp(var, "log.follow")) {
+   default_follow = git_config_bool(var, value);
+   return 0;
+   }
if (skip_prefix(var, "color.decorate.", &slot_name))
return parse_decorate_color_config(var, slot_name, value);
if (!strcmp(var, "log.mailmap")) {
diff --git a/diff.c b/diff.c
index 87b16d5..135b222 100644
--- a/diff.c
+++ b/diff.c
@@ -3815,9 +3815,10 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
DIFF_OPT_SET(options, FIND_COPIES_HARDER);
else if (!strcmp(arg, "--follow"))
DIFF_OPT_SET(options, FOLLOW_RENAMES);
-   else if (!strcmp(arg, "--no-follow"))
+   else if (!strcmp(arg, "--no-follow")) {
DIFF_OPT_CLR(options, FOLLOW_RENAMES);
-   else if (!strcmp(arg, "--color"))
+   DIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);
+   } else if (!strcmp(arg, "--color"))
options->use_color = 1;
else if (skip_prefix(arg, "--color=", &arg)) {
int value = git_config_colorbool(NULL, arg);
diff --git a/diff.h b/diff.h
index c7ad42a..f7208ad 100644
--- a/diff.h
+++ b/diff.h
@@ -91,6 +91,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
diff_options *opt, void *data)
 #define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28)
 #define DIFF_OPT_FUNCCONTEXT (1 << 29)
 #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
+#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1 << 31)
 
 #define DIFF_OPT_TST(opts, flag)((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_TOUCHED(opts, flag)((opts)->touched_flags & 
DIFF_OPT_##flag)
diff --git a/revision.c b/revision.c
index 3ff8723..5b0c2be 100644
--- a/revision.c
+++ b/revision.c
@@ -2322,12 +2322,20 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
 
if (revs->prune_data.nr) {
copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
-   /* Can't prune commits with rename following: the paths 
change.. */
-   if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
-   revs->prune = 1;
+
if (!revs->full_diff)
copy_pathspec(&revs->diffopt.pathspec,
  &revs->prune_data);
+
+   if (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&
+   revs->diffopt.pathspec.nr == 1)
+   DIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);
+
+   /* Can't prune commits with rename following: the paths 
change.. */
+   if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
+   revs->prune = 1;
+   else
+   revs->diff = 1;
}
if 

Re: [PATCH v2] log: add log.follow config option

2015-07-07 Thread David Turner
On Tue, 2015-07-07 at 23:42 +0200, Matthieu Moy wrote:
> Hi,
> 
> Thanks for your patch. Below are some comments. Some of them are just me
> thinking out loudly (don't take it badly if I'm being negative), some
> are more serious, but all are fixable.

Thanks for the feedback!

> David Turner  writes:
> 
> > From: David Turner 
> 
> If you configure your commiter id and your From field to the same value,
> you can avoid this distracting "From:" header.
> 
> You're lacking a Signed-off-by trailer.

Oops.  Cherry-picked over from internal repo.  Will fix.

 (suggestions applied)

> > + git log --name-status --pretty="format:%s"  path1 > current'
> 
> Whitespace: here an elsewhere, you have two spaces before path1, and we
> usually stick the > to the filename like >current.
>
> > --- a/t/t4206-log-follow-harder-copies.sh
> > +++ b/t/t4206-log-follow-harder-copies.sh
> > @@ -53,4 +53,39 @@ test_expect_success \
> >  'validate the output.' \
> >  'compare_diff_patch current expected'
> >  
> > +test_expect_success \
> > +'git config log.follow works like --follow' \
> > +'test_config log.follow true &&
> > + git log --name-status --pretty="format:%s"  path1 > current'
> > +
> > +test_expect_success \
> > +'validate the output.' \
> > +'compare_diff_patch current expected'
> 
> I would squash these two tests. As-is, the first one doesn't really test
> anything (well, just that git log doesn't crash with log.follow).
> 
> I think you meant test_cmp, not compare_diff_patch, as you don't need
> the "similarity index" and "index ..." filtering that compare_diff_patch
> does before test_cmp.
> 
> That said, I see that you are mimicking surrounding code, which is a
> good thing for consistancy. I think the best would be to write tests in
> t4202-log.sh, which already tests the --follow option, and uses a more
> modern coding style which you can mimick.
> t4206-log-follow-harder-copies.sh is really about finding copies in
> --follow. Another option is to start your series with a patch like
> "t4206: modernize style".

I'm going to move over to t4202.  My next re-roll will include fixes for
everything you raised.  

--
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: No one understands diff3 "Temporary merge branch" conflict markers

2015-07-07 Thread Matthieu Moy
Junio C Hamano  writes:

> Matthieu Moy  writes:
>
>> ... I would say: the
>> recursive merge-base was computed internally, but not really meant to be
>> shown to the user.
>
> I wonder if the output becomes easier to read if we unconditionally
> turned off diff3-style for inner merges.

Or replace the whole conflict markers with "Sorry, cannot compute a
merge base" text when doing the recursive merge to build the merge base.

I don't know that area well enough to have a real opinion.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] log: add log.follow config option

2015-07-07 Thread Matthieu Moy
Hi,

Thanks for your patch. Below are some comments. Some of them are just me
thinking out loudly (don't take it badly if I'm being negative), some
are more serious, but all are fixable.

David Turner  writes:

> From: David Turner 

If you configure your commiter id and your From field to the same value,
you can avoid this distracting "From:" header.

You're lacking a Signed-off-by trailer.

> +log.follow::
> + If a single file argument is given to git log, it will act as
> + if the `--follow` option was also used.  This adds no new
> + functionality over what --follow already provides (in other words,
> + it cannot be used to follow multiple files).  It's just a
> + convenience.

Missing `...` around the second --follow.

I would write this as:

This has the same limitations as --follow, i.e. it cannot be
used to follow multiple files and does not work well on
non-linear history.

and drop the "it's just a convenience" part.

> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;
>  
>  static int default_abbrev_commit;
>  static int default_show_root = 1;
> +static int default_follow = 0;

I tend to disagree with this rule, but in Git we usually omit the "= 0"
for static variables, which are already initialized to 0 by default.

> + /* Can't prune commits with rename following: the paths 
> change.. */
> + if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES)) {
> + revs->prune = 1;
> + } else {
> + revs->diff = 1;
> + }

Useless braces.

> + git log --name-status --pretty="format:%s"  path1 > current'

Whitespace: here an elsewhere, you have two spaces before path1, and we
usually stick the > to the filename like >current.

> --- a/t/t4206-log-follow-harder-copies.sh
> +++ b/t/t4206-log-follow-harder-copies.sh
> @@ -53,4 +53,39 @@ test_expect_success \
>  'validate the output.' \
>  'compare_diff_patch current expected'
>  
> +test_expect_success \
> +'git config log.follow works like --follow' \
> +'test_config log.follow true &&
> + git log --name-status --pretty="format:%s"  path1 > current'
> +
> +test_expect_success \
> +'validate the output.' \
> +'compare_diff_patch current expected'

I would squash these two tests. As-is, the first one doesn't really test
anything (well, just that git log doesn't crash with log.follow).

I think you meant test_cmp, not compare_diff_patch, as you don't need
the "similarity index" and "index ..." filtering that compare_diff_patch
does before test_cmp.

That said, I see that you are mimicking surrounding code, which is a
good thing for consistancy. I think the best would be to write tests in
t4202-log.sh, which already tests the --follow option, and uses a more
modern coding style which you can mimick.
t4206-log-follow-harder-copies.sh is really about finding copies in
--follow. Another option is to start your series with a patch like
"t4206: modernize style".

Or you can just accept that the current style in this file is subpar and
continue with it.

> +test_expect_success \
> +'git config log.follow does not die with multiple paths' \
> +'test_config log.follow true &&
> + git log path0 path1 > current &&

You are creating 'current' but not using it.

> + wc -l current'

What is the intent of this "wc -l current"? You're not checking its
output ...

> +test_expect_success \
> +'git config log.follow does not die with no paths' \
> +'test_config log.follow true &&
> + git log -- > current &&

One more creation of current which isn't used ...

> + wc -l current'
> +
> +test_expect_success \
> +'git config log.follow is overridden by --no-follow' \
> +'test_config log.follow true &&
> + git log --no-follow --name-status --pretty="format:%s"  path1 > current'

... because you're overwriting it here.

> +cat >expected <<\EOF
> +Copy path1 from path0
> +Apath1
> +EOF

Put everything in test_expect_..., including creation of expected file.
For more info, read t/README and its Do's, don'ts & things to keep in
mind section.

> +test_expect_success \
> +'validate the output.' \
> +'compare_diff_patch current expected'
> +
>  test_done

Cheers,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: Git installer questions

2015-07-07 Thread McChesney, Adam
I am curious as whether or not the windows installer has silent install flags 
that are configurable for automated installation? I was looking about the 
documentation and haven't been able to find them, if it does exist in the 
documentation could you point me to where they might be?

Thanks,
Adam Mcchesney 

--
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/3] convert "enum date_mode" into a struct

2015-07-07 Thread Junio C Hamano
Jeff King  writes:

> OK. Do you want to leave it be, then, or would you prefer me to do the
> NULL fallback? Or we could bump the enum to start with 1, and then
> explicitly treat "0" as a synonym for DATE_NORMAL (in case it comes in
> through a memset or similar).

I didn't think about the memset() initialization, and keeping the
normal case to be 0 makes tons of sense.

I'd prefer to see the stale code dump core rather than leaving it
stale without anybody noticing.  Hopefully the date-mode change can
hit 'master' fairly soon after the upcoming release, so unless a fix
that involves show_date() need to be written and applied to 2.4
codebase and upwards at the same time, I do not think it is a huge
issue.

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 2/3] convert "enum date_mode" into a struct

2015-07-07 Thread Jeff King
On Tue, Jul 07, 2015 at 02:05:52PM -0700, Junio C Hamano wrote:

> And that is because DATE_NORMAL is defined to be 0; we can claim
> that the compiler is being stupid to take one of the enum
> date_mode_type values that happens to be 0 and misinterpret it as
> the program wanted to pass a NULL pointer to a structure, but that
> is not what happened.

Ah, I didn't think the compiler would coerce an enum into a pointer
constant. That seems kind of insane. But it is indeed what gcc does.

In that case, we can indeed do the NULL-pointer thing I mentioned. Which
is not even _that_ ugly; it follows the standard.

The "cast DATE_RELATIVE to a pointer and uncast it on the other side"
thing _does_ violate the standard. It is not needed for this, but it
would make the DATE_MODE() macro reentrant.

> > +   static const struct fallback_mode = { DATE_NORMAL };
> 
> Yes, that is nasty.  Renumbering the enum to begin with 1 may be a
> much saner solution, unless somebody does

I am worried more about somebody who does memset(0) on a struct, and
expects that to default to DATE_NORMAL.

> In any case, I did another evil merge to fix it.

OK. Do you want to leave it be, then, or would you prefer me to do the
NULL fallback? Or we could bump the enum to start with 1, and then
explicitly treat "0" as a synonym for DATE_NORMAL (in case it comes in
through a memset or similar).

-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/3] convert "enum date_mode" into a struct

2015-07-07 Thread Junio C Hamano
Jeff King  writes:

> My assumption was that using the raw "0" is something we would frowned
> upon in new code. There was a single historical instance that I fixed in
> the series, but I wouldn't expect new ones (and actually, that instance
> was "1", which would be caught by the compiler).

That is not the problem.

The code on the side branch may add a new callsite, something like
this:

show_ident_date(&ident_split, DATE_NORMAL);

based on the current codebase (e.g. 'master' as of today).

The merge goes cleanly, it compiles, even though the new function
signature of show_ident_date(), similar to the updated show_date(),
takes a pointer to a struct where they used to take DATE_$format
constants.

And that is because DATE_NORMAL is defined to be 0; we can claim
that the compiler is being stupid to take one of the enum
date_mode_type values that happens to be 0 and misinterpret it as
the program wanted to pass a NULL pointer to a structure, but that
is not what happened.

> However, if you're concerned, I think we could have show_date massage a
> NULL date, like:
>
> diff --git a/date.c b/date.c
> index 8f91569..a04d089 100644
> --- a/date.c
> +++ b/date.c
> @@ -173,6 +173,10 @@ const char *show_date(unsigned long time, int tz, const 
> struct date_mode *mode)
>  {
>   struct tm *tm;
>   static struct strbuf timebuf = STRBUF_INIT;
> + static const struct fallback_mode = { DATE_NORMAL };

Yes, that is nasty.  Renumbering the enum to begin with 1 may be a
much saner solution, unless somebody does

if (!mode->type)
/* we know DATE_NORMAL is zero, he he */
do the normal thing;

In any case, I did another evil merge to fix 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


Re: [PATCH 2/3] convert "enum date_mode" into a struct

2015-07-07 Thread Jeff King
On Tue, Jul 07, 2015 at 01:37:08PM -0700, Junio C Hamano wrote:

> >   3. Provide a wrapper that generates the correct struct on
> >  the fly. The big downside is that we end up pointing to
> >  a single global, which makes our wrapper non-reentrant.
> >  But show_date is already not reentrant, so it does not
> >  matter.
> >
> > This patch implements 3, along with a minor macro to keep
> > the size of the callers sane.
> 
> Another big downside is that DATE_NORMAL is defined to be "0".
> 
> This makes it very cumbersome to merge a side branch that uses an
> outdated definition of show_date() and its friends and tell them
> to show date normally.  The compiler does not help detecting
> places that need to be adjusted during merge and instead just pass
> a NULL pointer as a pointer to the new struct.

My assumption was that using the raw "0" is something we would frowned
upon in new code. There was a single historical instance that I fixed in
the series, but I wouldn't expect new ones (and actually, that instance
was "1", which would be caught by the compiler).

However, if you're concerned, I think we could have show_date massage a
NULL date, like:

diff --git a/date.c b/date.c
index 8f91569..a04d089 100644
--- a/date.c
+++ b/date.c
@@ -173,6 +173,10 @@ const char *show_date(unsigned long time, int tz, const 
struct date_mode *mode)
 {
struct tm *tm;
static struct strbuf timebuf = STRBUF_INIT;
+   static const struct fallback_mode = { DATE_NORMAL };
+
+   if (!mode)
+   mode = &fallback_mode;
 
if (mode->type == DATE_RAW) {
strbuf_reset(&timebuf);


That would also allow people to explicitly call:

  show_date(t, tz, NULL);

to get the default format, though I personally prefer spelling it out.

I guess we _could_ introduce:

  #define DATE_MODE(x) ((struct date_mode *)(x))

and then take any numeric value, under the assumption that the first
page of memory will never be a valid pointer:

diff --git a/date.c b/date.c
index 8f91569..f388fee 100644
--- a/date.c
+++ b/date.c
@@ -173,6 +173,18 @@ const char *show_date(unsigned long time, int tz, const 
struct date_mode *mode)
 {
struct tm *tm;
static struct strbuf timebuf = STRBUF_INIT;
+   struct date_mode fallback;
+
+   /* hysterical compatibility */
+   if (mode < 1024) {
+   if (mode == DATE_STRFTIME)
+   die("BUG: nice try");
+   fallback.type = mode;
+   mode = &fallback;
+   }
+
+   if (!mode)
+   mode = &fallback_mode;
 
if (mode->type == DATE_RAW) {
strbuf_reset(&timebuf);

That's kind of nasty, but at least it's hidden from the callers.

-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/3] convert "enum date_mode" into a struct

2015-07-07 Thread Junio C Hamano
Jeff King  writes:

> ...  However, the tricky case is where we use the
> enum labels as constants, like:
>
>   show_date(t, tz, DATE_NORMAL);
>
> Ideally we could say:
>
>   show_date(t, tz, &{ DATE_NORMAL });
>
> but of course C does not allow that.
> ...
>   3. Provide a wrapper that generates the correct struct on
>  the fly. The big downside is that we end up pointing to
>  a single global, which makes our wrapper non-reentrant.
>  But show_date is already not reentrant, so it does not
>  matter.
>
> This patch implements 3, along with a minor macro to keep
> the size of the callers sane.

Another big downside is that DATE_NORMAL is defined to be "0".

This makes it very cumbersome to merge a side branch that uses an
outdated definition of show_date() and its friends and tell them
to show date normally.  The compiler does not help detecting
places that need to be adjusted during merge and instead just pass
a NULL pointer as a pointer to the new struct.

--
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 v5 19/44] builtin-am: implement --3way, am.threeWay

2015-07-07 Thread Junio C Hamano
Paul Tan  writes:

> @@ -82,6 +84,8 @@ struct am_state {
>   /* number of digits in patch filename */
>   int prec;
>  
> + int threeway;
> +
>   int quiet;
>  
>   int append_signoff;

These "one line surrounded by blank on both sides" starts to get
irritating, and the structure looksq horrifying after you apply all
these patches.  Perhaps have a clean-up patch at the end to make
them look more like this?

struct am_state {
/* state directory path */
char *dir;

/* current and last patch numbers, 1-indexed */
int cur;
int last;

/* commit metadata and message */
char *author_name;
char *author_email;
char *author_date;
char *msg;
size_t msg_len;

/* when --rebasing, records the original commit the patch came from */
unsigned char orig_commit[GIT_SHA1_RAWSZ];

/* number of digits in patch filename */
int prec;

/* various operating modes and command line options */
int interactive;
int threeway;
int quiet;
int append_signoff;
int utf8;
int committer_date_is_author_date;
int ignore_date;
int allow_rerere_autoupdate;
const char *sign_commit;
int rebasing;

/* one of the enum keep_type values */
int keep;

/* pass -m flag to git-mailinfo */
int message_id;

/* one of the enum scissors_type values */
int scissors;

/* used when spawning "git apply" via run_command() */
struct argv_array git_apply_opts;

/* override error message when patch failure occurs */
const char *resolvemsg;
};
--
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 v5 18/44] cache-tree: introduce write_index_as_tree()

2015-07-07 Thread Junio C Hamano
Paul Tan  writes:

> A caller may wish to write a temporary index as a tree. However,
> write_cache_as_tree() assumes that the index was read from, and will
> write to, the default index file path. Introduce write_index_as_tree()
> which removes this limitation by allowing the caller to specify its own
> index_state and index file path.
>
> Signed-off-by: Paul Tan 
> ---
>  cache-tree.c | 29 +
>  cache-tree.h |  1 +
>  2 files changed, 18 insertions(+), 12 deletions(-)

Makes sense; 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: Git force push fails after a rejected push (unpack failed)?

2015-07-07 Thread Jeff King
On Tue, Jul 07, 2015 at 09:31:25PM +0200, X H wrote:

> For the moment, I'm the only one pushing to the remote, always with
> the same user (second user is planned). I use git-for-windows which is
> based on MSYS2. I have mounted the network share with noacl option so
> permissions should be handled by the Windows share. I'm in a group
> which has read/write access. I have not configured
> core.sharedrepository, I don't think it is useful with noacl since
> unix group are not used in this case. The permission for the folder
> above the file with permission denied is rw, but this file is read
> only so if git try to modify it it won't work.

Ah, so this is not a push to a server, but to a share mounted on the
local box?

That is leaving my realm of expertise. I'm not sure if it could be a
misconfiguration in your share setup, or that git is trying to do
something that would work on a Unix machine, but not on a Windows share.
You might want to ask on the msysgit list:

  https://groups.google.com/forum/#!forum/msysgit

> Why does git try to write a file with the same name? If I amend a
> commit isn't the sha modified?

Yes, but remember that git stores all of the objects for all of the
commits. So for some reason your push is perhaps trying to send an
object that the other side already has. Usually this does not happen
(the receiver says "I already have these commits, do not bother sending
their objects"), but it's possible that you have an object that is not
referenced by any commit, or a similar situation. It's hard to say
without looking at the repository.

-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 v5 00/44] Make git-am a builtin

2015-07-07 Thread Paul Tan
On Wed, Jul 8, 2015 at 2:52 AM, Junio C Hamano  wrote:
> Paul Tan  writes:
>> This patch series rewrites git-am.sh into optimized C builtin/am.c, and is
>> part of my GSoC project to rewrite git-pull and git-am into C builtins[1].
>>
>> [1] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1
>
> Is it really a rewrite into "optimized C", or just "C"?  I suspect
> it is the latter.

Well, "optimized" in comparison to the shell script ;-) We don't
replicate the shell script in its entirety, but optimize the code
where it is obvious and sensible. It's not the "most optimized" (and I
will gladly accept any suggestions where there are obvious
optimizations to be made), but I do consider it an improvement in
terms of efficiency, while keeping the overall structure of the code
close to the shell script for easy review.

I'll remove the word though, because it's true that the main purpose
of this patch series is to "make it work" first.

> This seems to apply cleanly to 'master' but fails to compile, as it
> depends on some new stuff that are not even in 'next' yet, e.g.
> argv_array_pushv() that is from pt/pull-builtin, and it does not
> apply cleanly on top of that branch, either.

I tried applying the series, and yeah it conflicts with 1570856
(config.c: avoid xmmap error messages, 2015-05-28) because the
pt/pull-builtin branch in pu is based on an old version of master.
That's the only conflict though, it applies cleanly otherwise.

> I'll see what's the cleanest way to queue this would be.  Perhaps
> merge pt/builtin-pull on a copy of 'master' and then apply these, or
> something like that.

Thanks.

Regards,
Paul
--
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


[GSOC] Update 4: Unification of tag -l, branch -l and for-each-ref

2015-07-07 Thread Karthik Nayak
Hello All,

As part of GSoC I'm working on the Unification of 'for-each-ref', 'tag -l'
and 'branch -l'. Sorry for the lack of update since Jun 14, was a
little busy with an exam I had. Now thats over, I will be working more
on the project.

Current Progress:

1. Building ref-filter.{c,h} from for-each-ref.
This is the process of creating an initial library for the unification
by moving most of the code from for-each-ref to ref-filter.{c,h}.
Merged into next

2. Add options to ref-filter.
This includes the porting of --points-at, --contains, --merged,
--no-merged options from builtin/branch.c and builtin/tag.c, Also the
implementation of these options into for-each-ref.
The last version (v8) is posted here:
http://thread.gmane.org/gmane.comp.version-control.git/273569
Currently waiting for comments.

3. Port builtin/tag.c to use ref-filter.
Here we port tag.c to use ref-filter and also port the --format,
--sort and --merged and --no-merged options to builtin/tag.c.
The "RFC" version is posted and I'm waiting for comments from the list:
thread.gmane.org/gmane.comp.version-control.git/272654
Will post v2 soon.

Next Plans:
I'm currently working on porting over builtin/branch.c to use
ref-filter.{c,h}, will be pushing intermediate code to my Github repository.
Currently looking at what all branch.c needs in ref-filter and adding respective
options to ref-filter.
https://github.com/KarthikNayak/git

Thanks to everyone who has helped :)

-- 
Regards,
Karthik Nayak
--
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 v5 00/44] Make git-am a builtin

2015-07-07 Thread Junio C Hamano
Paul Tan  writes:

> This patch series depends on pt/pull-builtin.
>
> This is a re-roll of [v4]. Thanks Torsten, Stefan, Junio for the reviews last
> round. Interdiff below.
>
> Previous versions:
>
> [WIP v1] http://thread.gmane.org/gmane.comp.version-control.git/270048
> [WIP v2] http://thread.gmane.org/gmane.comp.version-control.git/271381
> [WIP v3] http://thread.gmane.org/gmane.comp.version-control.git/271967
> [v4] http://thread.gmane.org/gmane.comp.version-control.git/272876
>
> git-am is a commonly used command for applying a series of patches from a
> mailbox to the current branch. Currently, it is implemented by the shell 
> script
> git-am.sh. However, compared to C, shell scripts have certain deficiencies:
> they need to spawn a lot of processes, introduce a lot of dependencies and
> cannot take advantage of git's internal caches.
>
> This patch series rewrites git-am.sh into optimized C builtin/am.c, and is
> part of my GSoC project to rewrite git-pull and git-am into C builtins[1].
>
> [1] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1

Is it really a rewrite into "optimized C", or just "C"?  I suspect
it is the latter.

This seems to apply cleanly to 'master' but fails to compile, as it
depends on some new stuff that are not even in 'next' yet, e.g.
argv_array_pushv() that is from pt/pull-builtin, and it does not
apply cleanly on top of that branch, either.

I'll see what's the cleanest way to queue this would be.  Perhaps
merge pt/builtin-pull on a copy of 'master' and then apply these, or
something like that.

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


[PATCH v2] log: add log.follow config option

2015-07-07 Thread David Turner
From: David Turner 

Many users prefer to always use --follow with logs.  Rather than
aliasing the command, an option might be more convenient for some.
---
 Documentation/git-log.txt   |  7 +++
 builtin/log.c   |  7 +++
 diff.c  |  5 +++--
 diff.h  |  1 +
 revision.c  | 15 ---
 t/t4206-log-follow-harder-copies.sh | 35 +++
 6 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 5692945..5a23085 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -184,6 +184,13 @@ log.date::
`--date` option.)  Defaults to "default", which means to write
dates like `Sat May 8 19:35:34 2010 -0500`.
 
+log.follow::
+   If a single file argument is given to git log, it will act as
+   if the `--follow` option was also used.  This adds no new
+   functionality over what --follow already provides (in other words,
+   it cannot be used to follow multiple files).  It's just a
+   convenience.
+
 log.showRoot::
If `false`, `git log` and related commands will not treat the
initial commit as a big creation event.  Any root commits in
diff --git a/builtin/log.c b/builtin/log.c
index 8781049..9b6abef 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;
 
 static int default_abbrev_commit;
 static int default_show_root = 1;
+static int default_follow = 0;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -102,6 +103,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 {
if (fmt_pretty)
get_commit_format(fmt_pretty, rev);
+   if (default_follow)
+   DIFF_OPT_SET(&rev->diffopt, DEFAULT_FOLLOW_RENAMES);
rev->verbose_header = 1;
DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
rev->diffopt.stat_width = -1; /* use full terminal width */
@@ -390,6 +393,10 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
default_show_root = git_config_bool(var, value);
return 0;
}
+   if (!strcmp(var, "log.follow")) {
+   default_follow = git_config_bool(var, value);
+   return 0;
+   }
if (skip_prefix(var, "color.decorate.", &slot_name))
return parse_decorate_color_config(var, slot_name, value);
if (!strcmp(var, "log.mailmap")) {
diff --git a/diff.c b/diff.c
index 87b16d5..135b222 100644
--- a/diff.c
+++ b/diff.c
@@ -3815,9 +3815,10 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
DIFF_OPT_SET(options, FIND_COPIES_HARDER);
else if (!strcmp(arg, "--follow"))
DIFF_OPT_SET(options, FOLLOW_RENAMES);
-   else if (!strcmp(arg, "--no-follow"))
+   else if (!strcmp(arg, "--no-follow")) {
DIFF_OPT_CLR(options, FOLLOW_RENAMES);
-   else if (!strcmp(arg, "--color"))
+   DIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);
+   } else if (!strcmp(arg, "--color"))
options->use_color = 1;
else if (skip_prefix(arg, "--color=", &arg)) {
int value = git_config_colorbool(NULL, arg);
diff --git a/diff.h b/diff.h
index c7ad42a..f7208ad 100644
--- a/diff.h
+++ b/diff.h
@@ -91,6 +91,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
diff_options *opt, void *data)
 #define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28)
 #define DIFF_OPT_FUNCCONTEXT (1 << 29)
 #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
+#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1 << 31)
 
 #define DIFF_OPT_TST(opts, flag)((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_TOUCHED(opts, flag)((opts)->touched_flags & 
DIFF_OPT_##flag)
diff --git a/revision.c b/revision.c
index 3ff8723..ae6d4c3 100644
--- a/revision.c
+++ b/revision.c
@@ -2322,12 +2322,21 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
 
if (revs->prune_data.nr) {
copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
-   /* Can't prune commits with rename following: the paths 
change.. */
-   if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
-   revs->prune = 1;
+
if (!revs->full_diff)
copy_pathspec(&revs->diffopt.pathspec,
  &revs->prune_data);
+
+   if (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&
+   revs->diffopt.pathspec.nr == 1)
+   DIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);
+
+   /* Can't prune commits with rename following: the paths 
change.. */
+   if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES)) {
+   revs

Re: [PATCH v2 00/12] Improve git-am test coverage

2015-07-07 Thread Johannes Schindelin
Hi Paul

On 2015-07-07 16:08, Paul Tan wrote:
> This is a re-roll of [v1]. Thanks Junio, Johannes, Paolo, Stefan for the
> reviews last round. Interdiff below.

Interdiff looks good to me!

Thanks,
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: Git grep does not support multi-byte characters (like UTF-8)

2015-07-07 Thread Plamen Totev
Junio C Hamano  writes:

> Plamen Totev  writes: 
> 
> > pickaxe search also uses kwsearch so the case insensitive search with 
> > it does not work (e.g. git log -i -S). Maybe this is a less of a 
> > problem here as one is expected to search for exact string (hence 
> > knows the case) 
> 
> You reasoned correctly, I think. Pickaxe, as one of the building 
> blocks to implement Linus's ultimate change tracking tool [*1*], 
> should never pay attention to "-i". It is a step to finding the 
> commit that touches the exact code block given (i.e. "how do you 
> drill down?" part of $gmane/217 message). 
> 
> Thanks. 
> 
> [Footnote] 
> *1* http://article.gmane.org/gmane.comp.version-control.git/217

Now that I read the link again and gave the matter a thought I'm not so sure.
In some contexts the case of the words does not matter. In SQL for example.
Let's consider a SQL script file that contains the following line:

select name, address from customers;

At some point we decide to change the coding style to:

SELECT name, address FROM customers;

What should pickaxe search return - the first commit where the line is 
introduced
or the commit with the refactoring? From this point of view I think the -i 
switch makes sense.
The SQL is not the only case insensitive language - BASIC and Pascal come into 
my mind 
(those two I was using while I was in the high school :)).

Also I think it makes sense (maybe even more?) for natural languages.
For example after editing a text a sentence could be split into two.
Then the first word of the second sentence may change its case.
Of course the natural languages always  complicate the things a bit.
An ultimate tracking tools should be able to handle typo fixes, punctuation 
changes, etc.

But I'm getting a bit off-topic. What I wanted to say is that in some contexts 
it makes sense
(at least to me) to have case insensitive pickaxe search.
Or I'm missing something and there is a better tools to use is such cases?

Regards,
Plamen Totev
--
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: No one understands diff3 "Temporary merge branch" conflict markers

2015-07-07 Thread Junio C Hamano
Matthieu Moy  writes:

> ... I would say: the
> recursive merge-base was computed internally, but not really meant to be
> shown to the user.

I wonder if the output becomes easier to read if we unconditionally
turned off diff3-style for inner merges.
--
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: Whether Git supports directory level access or not?

2015-07-07 Thread Junio C Hamano
Jacob Keller  writes:

> However, in-repo per-directory permissions make no sense, as there
> would be no way to generate commits.

That may be the case for the current generation of Git, but I do not
think you have to be so pessimistic.

Suppose that an imaginary future version of Git allowed you to
"hide" one directory from you.  That is:

 * A commit object records "tree". "git cat-file -t HEAD^{tree}"
   or "git ls-tree HEAD" lets you inspect its contents;

 * The "hidden" directory shows up as one of the subtrees of that
   output.  It may say

 04 tree b4006c408979a0c6261dbfaeaa36639457469ad4   hidden

 * However, your repository lack b4006c40... object.  So if you did
   "git ls-tree HEAD:hidden", you would get "no such tree object".

 * This imaginary future version of Git has a new implementation of
   the index (both on-disk and in-core) that lets you keep just the
   "tree" entry for an unmodified directory, without having to store
   any of the files and subdirectories in it.

 * All the other machinery of this imaginary future version of Git
   are aware of the fact that "hidden" thing is not visible, or even
   available, to your clone of the project repository.  That means
   "fsck" does not complain about missing object b4006c40..., "push"
   knows it should not consider it an error that you cannot enumerate
   and send objects that are reachable from b4006c40..., etc.

With such a Git, you can modify anything outside the parts of the
project tree that are hidden from you, and make a commit.  The tree
recorded in a new commit object would record the same

 04 tree b4006c408979a0c6261dbfaeaa36639457469ad4   hidden

for the "hidden" directory, and you can even push it back to update
the parts for other people to see your work outside the "hidden"
area.

"All the other machinery" that would need to accomodate such a
hidden directory would span the entire plumbing layer and
transports.  The wire protocol would need to be updated, especially
the part that determines what needs to be sent and received, which
is currently purely on commit ancestry, needs to become aware of the
paths.

I am *NOT* saying that this is easy.  I'd imagine if we gather all
the competent Gits in a room and have them work on it and doing
nothing else for six months, we would have some system that works.
It would be a lot of work.

I think it may be worth doing in the longer term, and it will likely
to have other benefits as side effects.

 - For example, did you notice that my description above does not
   mention "permission" even once?  Yes, that's right.  This does
   not have to be limited to permissions.  The user may have decided
   that the "hidden" part of that directory structure is not
   interesting and said "git clone --exclude=hidden" when she made
   her clone to set it up.

 - Also notice that the "new implementation of the index" that
   lazily expands subtrees does not say anythying about a directory
   that is "hidden"---it just said "an unmodified directory" and
   that was deliberate.  Even when you are not doing a "narrow
   clone", keeping an untouched tree without expanding its subtrees
   and blobs flatted into the index may make it faster when you are
   working on a series of many small commits each of which touches
   only a handful of files.

I might agree with you that "in-repo per-directory permissions make
no sense", but the reason to say so would not be because "there
would be no way to generate commits".
--
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: Subject: [PATCH] git am: Transform and skip patches via new hook

2015-07-07 Thread Eric Sunshine
On Tue, Jul 7, 2015 at 3:52 AM, Robert Collins
 wrote:
> From 0428b0a1248fb84c584a5a6c1f110770c6615d5e Mon Sep 17 00:00:00 2001
> From: Robert Collins 
> Date: Tue, 7 Jul 2015 15:43:24 +1200
> Subject: [PATCH] git am: Transform and skip patches via new hook

Drop the "From sha1", "Date:", and "Subject:" headers. "From sha1" is
meaningful only in your repository, thus not useful here, and git-am
will pluck the other information directly from your email, so they are
redundant. The "From:" header, however, should be kept since it
differs from your sending email address.

> A thing I need to do quite a lot of is extracting stuff from
> Python to backported libraries. This involves changing nearly
> every patch but its automatable.
>
> Using a new hook (applypatch-transform) was sufficient to meet all my
> needs and should be acceptable upstream as far as I can tell.

For a commit message, you want to explain the problem you're solving,
in what way the the current implementation is lacking, and justify why
your solution is desirable (possibly citing alternate approaches you
discarded). Unfortunately, the above paragraphs don't really tell us
much about why applypatch-tranforms is needed or how it solves a
problem which can't be solved with some other existing mechanism. You
do mention that it satisfies your "needs", but we don't know
specifically what those are.

The above paragraphs might be perfectly suitable as additional
commentary to supplement the commit messages, however, such commentary
should be placed below the "---" line under your sign-off and above
the diffstat.

> Signed-Off-By: Robert Collins 

This is typically written "Signed-off-by:".

More below.

> ---
>  Documentation/git-am.txt |  6 ++---
>  Documentation/githooks.txt   | 15 
>  git-am.sh| 15 
>  templates/hooks--applypatch-transform.sample | 36 
> 
>  4 files changed, 69 insertions(+), 3 deletions(-)
>  create mode 100755 templates/hooks--applypatch-transform.sample
>
> diff --git a/git-am.sh b/git-am.sh
> index 8733071..796efea 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -869,6 +869,21 @@ To restore the original branch and stop patching
> run \"\$cmdline --abort\"."
>
>   case "$resolved" in
>   '')
> + # Attempt to rewrite the patch.
> + hook="$(git rev-parse --git-path hooks/applypatch-transform)"
> + if test -x "$hook"
> + then
> + "$hook" "$dotest/patch" "$dotest/final-commit"
> + status="$?"
> + if test $status -eq 1
> + then
> + go_next
> + elif test $status -ne 0
> + then
> + stop_here $this
> + fi
> + fi

This indentation looks botched, as if the patch was pasted into your
email client and the client mangled the whitespace. git-send-email may
be of use here.

> diff --git a/templates/hooks--applypatch-transform.sample
> b/templates/hooks--applypatch-transform.sample
> new file mode 100755
> index 000..97cd789
> --- /dev/null
> +++ b/templates/hooks--applypatch-transform.sample
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +#
> +# An example hook script to transform a patch taken from an email
> +# by git am.
> +#
> +# The hook should exit with non-zero status after issuing an
> +# appropriate message if it wants to stop the commit.  The hook is
> +# allowed to edit the patch file.
> +#
> +# To enable this hook, rename this file to "applypatch-transform".
> +#
> +# This example changes the path of Lib/unittest/mock.py to mock.py
> +# Lib/unittest/tests/testmock to tests and Misc/NEWS to NEWS, and
> +# finally skips any patches that did not alter mock.py or its tests.

It's not clear even from this example what applypatch-transform buys
you over simply running your patches through some transformation and
filtering script *before* feeding them to git-am. The answer to that
question is the sort of thing which should be in the commit message to
justify the patch.

> +set -eux
> +
> +patch_path=$1
> +
> +# Pull out mock.py
> +filterdiff --clean --strip 3 --addprefix=a/ -i
> 'a/Lib/unittest/mock.py' $patch_path > $patch_path.mock
> +# And the tests
> +filterdiff --clean --strip 5 --addprefix=a/tests/ -i
> 'a/Lib/unittest/test/testmock/' $patch_path > $patch_path.tests
> +# Lastly we want to pick up any NEWS entries.
> +filterdiff --strip 2 --addprefix=a/ -i a/Misc/NEWS $patch_path >
> $patch_path.NEWS
> +cat $patch_path.mock $patch_path.tests > $patch_path
> +filtered=$(cat $patch_path)
> +if [ -n "${filtered}" ]; then
> +  cat $patch_path.NEWS >> $patch_path
> +  exitcode=0
> +else
> +  exitcode=1
> +fi
> +
> +rm $patch_path.mock $patch_path.tests $patch_path.NEWS
> +exit $exitcode
> --
> 2.1.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: refspecs with '*' as part of pattern

2015-07-07 Thread Junio C Hamano
Daniel Barkalow  writes:

> On Mon, 6 Jul 2015, Junio C Hamano wrote:
>
>> I cannot seem to be able to find related discussions around that
>> patch, so this is only my guess, but I suspect that this is to
>> discourage people from doing something like:
>> 
>>  refs/tags/*:refs/tags/foo-*
>> 
>> which would open can of worms (e.g. imagine you fetch with that
>> pathspec and then push with refs/tags/*:refs/tags/* back there;
>> would you now get foo-v1.0.0 and foo-foo-v1.0.0 for their v1.0.0
>> tag?) we'd prefer not having to worry about.
>
> That wouldn't be it, since refs/tags/*:refs/tags/foo/* would have the same 
> problem, assuming you didn't set up the push refspec carefully.

Thanks, I was wondering where you were ;-)  Nice to hear from you
from time to time.

> I think it was mostly that it would be too easy to accidentally do 
> something you don't want by having some other character instead of a 
> slash, like refs/heads/*:refs/heads-*.

Hmm, interesting thought.

But refs/heads/*:refs/heade/* would not be saved, so I do not think
that is it, either.
--
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 v3 23/23] checkout: retire --ignore-other-worktrees in favor of --force

2015-07-07 Thread Junio C Hamano
Eric Sunshine  writes:

> Is receive.denyCurrentBranch worth mentioning as an argument? Although
> pushing a branch into a non-bare repo where that branch is already
> checked out is normally disallowed, receive.denyCurrentBranch
> overrides the safeguard. Presumably, the user has experience and
> knowledge to know that "git reset --hard" will be required to sync
> things up.

Or the user knows that he does not have a shell access to the box in
the first place.  I do not see much relevance to this discussion.

I would not mind "git worktree add -f" to disable the "no multiple
checkouts of the same branch" safety, but I do not think it is
sensible to remove "-i-o-w" and conflate everything into "--force".
That would force people to disable other safety measures at the same
time (e.g. protect local changes from differences between the
current and next branches).
--
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: No one understands diff3 "Temporary merge branch" conflict markers

2015-07-07 Thread Matthieu Moy
Edward Anderson  writes:

> I have the diff3 conflictstyle enabled and want to be able to
> understand how to understand its output when there are criss-cross
> merges requiring temporary merge branches.  Eg:
>
> <<< HEAD
>   print(A);
> ||| merged common ancestors
> <<< Temporary merge branch 1
>   print(B);
> ===
>   print(C);
> >>> feature

I guess you are seeing the result of the recursive-merge.

>> The details are too advanced for this discussion, but the default
>> "recursive" merge strategy that git uses solves the answer by
>> merging a and b into a temporary commit and using *that* as the
>> merge base.

That is the point. We don't have a good common ancestor, so Git builds
one by merging the common ancestors. Then, two things can happen:

* The merge of the common ancestors is conflict-free. Then, we get a
  "sane" common ancestor.

* The merge has conflicts. In this case, the common ancestor that Git
  built has conflict markers. It is not a big issue, since when merging
  A, B, and ancestor(A, B), the result of the merge is either A or B,
  but never comes from ancestor(A, B). So, you never get to see the
  temporary ancestor(A, B), *except* when you request the common
  ancestor in the merge conflict.

It gets nasty since you get recursive merge conflicts, but you don't see
the recursivity. Let me try to indent your conflict:

 1 <<< HEAD
 2 unless admin
 3   fail Unauthorized.new("Admin only")
 4 end
 5 ||| merged common ancestors
 6 <<< Temporary merge branch 1
 7 unless admin
 8   fail Unauthorized.new("Admin only")
 9 end
10 ||| merged common ancestors
11 unless admin
12   fail Unauthorized.new
13 end
14 ===
15 fail Unauthorized.new unless admin
16 >>> Temporary merge branch 2
17 ===
18 unless admin
19 fail Unauthorized.new("Admin only")
20   fail Unauthorized.new
21 end
22 >>> feature

> It seems lines 6-16 are a conflict that occurred when merging the
> merge-bases.

Yes.

> That conflict could be resolved by merging the change in Temporary
> merge branch 1 (add "Admin only") with Temporary merge branch 2
> (convert multi-line unless to single-line) as this:
>
>fail Unauthorized.new("Admin only") unless admin

That is probably what you would do if you resolved the conflict
manually, but while merging the common ancestors, Git found an ancestor
of an ancestor that was different from both ancestors being merged, and
there was a conflict. Asking you to resolve this conflict would be
essentially a loss of time since Git knows that the result won't appear
in the final merge, but only in the merge base.

 1 <<< HEAD
 2 unless feature.enabled_for_user?(UserIdLookup.new(params).user_id)
 3   fail Unauthorized.new("Requires setting #{label}.")
 4 ||| merged common ancestors
 5 <<< Temporary merge branch 1
 6 unless feature.enabled_for_user?(params[:user_id])
 7   fail Unauthorized.new("Requires setting #{label}.")
 8 ===
 9 unless feature.enabled_for_user?(params[:user_id])
10   fail Unauthorized.new("Requires setting #{label}.")
11 >>> feature

> This is the full conflict, and it doesn't seem to balance.

Right: I guess the merge-base was stg like

<<< Temporary merge branch 1
unless feature.enabled_for_user?(params[:user_id])
  fail Unauthorized.new("Requires setting #{label}.")
|||
blabla 1
===
blabla 2
>>> Temporary merge branch 2

But then, the actual merge happens, using this as merge-base. A conflict
occurs when the commits being merged and the merge-base are all
different. In your case, I guess the commits being merged were identical
on the next different hunks (the line "blablabla 1" probably was in both
commits being merged, which allowed the merge algorithm to move to the
next hunk), and there were no conflict in this hunk, hence you don't see
the merge base.

I hope this helps on the "light and understanding" part of your
question. Now, as of "what to do when I get this?", I would say: the
recursive merge-base was computed internally, but not really meant to be
shown to the user. You should probably ignore it and resolve the merge
by looking only at the 2 sides of the conflict ("ours" and "theirs").
Sorry, this is probably not the answer you expected, but it's the best I
can give ;-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 v3 22/23] worktree: add: auto-vivify new branch when is omitted

2015-07-07 Thread Junio C Hamano
Eric Sunshine  writes:

>> Which may be something we would want to have a test for, though.
>
> Good idea. How about the following as a squash-in?

Sounds sensible.

At this point we do not have "worktree list", but if we gained that,
we may want to add this as one more postcondition after the failed
"worktree add":

   git worktree list >actual &&
   ! grep precious actual

but that should happen in the series that adds "worktree list" ;-)

> --- 8< ---
> From: Eric Sunshine 
> Subject: [PATCH] fixup! worktree: add: auto-vivify new branch when  
> is omitted
>
> ---
>  t/t2025-worktree-add.sh | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> index 8fe242f..ead8aa2 100755
> --- a/t/t2025-worktree-add.sh
> +++ b/t/t2025-worktree-add.sh
> @@ -150,4 +150,13 @@ test_expect_success '"add" with  omitted' '
>   test_cmp_rev HEAD bat
>  '
>  
> +test_expect_success '"add" auto-vivify does not clobber existing branch' '
> + test_commit c1 &&
> + test_commit c2 &&
> + git branch precious HEAD~1 &&
> + test_must_fail git worktree add precious &&
> + test_cmp_rev HEAD~1 precious &&
> + test_path_is_missing precious
> +'
> +
>  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


[PATCH v8 10/11] ref-filter: implement '--contains' option

2015-07-07 Thread Karthik Nayak
'tag -l' and 'branch -l' have two different ways of finding
out if a certain ref contains a commit. Implement both these
methods in ref-filter and give the caller of ref-filter API
the option to pick which implementation to be used.

'branch -l' uses 'is_descendant_of()' from commit.c which is
left as the default implementation to be used.

'tag -l' uses a more specific algorithm since ffc4b80. This
implementation is used whenever the 'with_commit_tag_algo' bit
is set in 'struct ref_filter'.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/tag.c |   5 +++
 ref-filter.c  | 114 +-
 ref-filter.h  |   3 ++
 3 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 767162e..071d001 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -86,6 +86,11 @@ static int in_commit_list(const struct commit_list *want, 
struct commit *c)
return 0;
 }
 
+/*
+ * The entire code segment for supporting the --contains option has been
+ * copied over to ref-filter.{c,h}. This will be deleted evetually when
+ * we port tag.c to use ref-filter APIs.
+ */
 enum contains_result {
CONTAINS_UNKNOWN = -1,
CONTAINS_NO = 0,
diff --git a/ref-filter.c b/ref-filter.c
index 148b7cd..dd0709d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -818,6 +818,114 @@ static void get_ref_atom_value(struct ref_array_item 
*ref, int atom, struct atom
*v = &ref->value[atom];
 }
 
+enum contains_result {
+   CONTAINS_UNKNOWN = -1,
+   CONTAINS_NO = 0,
+   CONTAINS_YES = 1
+};
+
+/*
+ * Mimicking the real stack, this stack lives on the heap, avoiding stack
+ * overflows.
+ *
+ * At each recursion step, the stack items points to the commits whose
+ * ancestors are to be inspected.
+ */
+struct contains_stack {
+   int nr, alloc;
+   struct contains_stack_entry {
+   struct commit *commit;
+   struct commit_list *parents;
+   } *contains_stack;
+};
+
+static int in_commit_list(const struct commit_list *want, struct commit *c)
+{
+   for (; want; want = want->next)
+   if (!hashcmp(want->item->object.sha1, c->object.sha1))
+   return 1;
+   return 0;
+}
+
+/*
+ * Test whether the candidate or one of its parents is contained in the list.
+ * Do not recurse to find out, though, but return -1 if inconclusive.
+ */
+static enum contains_result contains_test(struct commit *candidate,
+   const struct commit_list *want)
+{
+   /* was it previously marked as containing a want commit? */
+   if (candidate->object.flags & TMP_MARK)
+   return 1;
+   /* or marked as not possibly containing a want commit? */
+   if (candidate->object.flags & UNINTERESTING)
+   return 0;
+   /* or are we it? */
+   if (in_commit_list(want, candidate)) {
+   candidate->object.flags |= TMP_MARK;
+   return 1;
+   }
+
+   if (parse_commit(candidate) < 0)
+   return 0;
+
+   return -1;
+}
+
+static void push_to_contains_stack(struct commit *candidate, struct 
contains_stack *contains_stack)
+{
+   ALLOC_GROW(contains_stack->contains_stack, contains_stack->nr + 1, 
contains_stack->alloc);
+   contains_stack->contains_stack[contains_stack->nr].commit = candidate;
+   contains_stack->contains_stack[contains_stack->nr++].parents = 
candidate->parents;
+}
+
+static enum contains_result contains_tag_algo(struct commit *candidate,
+   const struct commit_list *want)
+{
+   struct contains_stack contains_stack = { 0, 0, NULL };
+   int result = contains_test(candidate, want);
+
+   if (result != CONTAINS_UNKNOWN)
+   return result;
+
+   push_to_contains_stack(candidate, &contains_stack);
+   while (contains_stack.nr) {
+   struct contains_stack_entry *entry = 
&contains_stack.contains_stack[contains_stack.nr - 1];
+   struct commit *commit = entry->commit;
+   struct commit_list *parents = entry->parents;
+
+   if (!parents) {
+   commit->object.flags |= UNINTERESTING;
+   contains_stack.nr--;
+   }
+   /*
+* If we just popped the stack, parents->item has been marked,
+* therefore contains_test will return a meaningful 0 or 1.
+*/
+   else switch (contains_test(parents->item, want)) {
+   case CONTAINS_YES:
+   commit->object.flags |= TMP_MARK;
+   contains_stack.nr--;
+   break;
+   case CONTAINS_NO:
+   entry->parents = parents->next;
+   break;
+   case CONTAINS_UNKNOWN:
+   push_to_contains_stack(parents->item, &contains_stack);
+  

[PATCH v8 11/11] for-each-ref: add '--contains' option

2015-07-07 Thread Karthik Nayak
Add the '--contains' option provided by 'ref-filter'. The '--contains'
option lists only refs which contain the mentioned commit (HEAD if no
commit is explicitly given).

Add documentation and tests for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  5 +
 builtin/for-each-ref.c |  2 ++
 t/t6302-for-each-ref-filter.sh | 15 +++
 3 files changed, 22 insertions(+)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 2842195..e49d578 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 'git for-each-ref' [--count=] [--shell|--perl|--python|--tcl]
   [(--sort=)...] [--format=] [...]
   [--points-at ] [(--merged | --no-merged) []]
+  [--contains []]
 
 DESCRIPTION
 ---
@@ -74,6 +75,10 @@ OPTIONS
Only list refs whose tips are not reachable from the
specified commit (HEAD if not specified).
 
+--contains []::
+   Only list tags which contain the specified commit (HEAD if not
+   specified).
+
 FIELD NAMES
 ---
 
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 7521850..40f343b 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@ static char const * const for_each_ref_usage[] = {
N_("git for-each-ref [] []"),
N_("git for-each-ref [--points-at ]"),
N_("git for-each-ref [(--merged | --no-merged) []]"),
+   N_("git for-each-ref [--contains []]"),
NULL
 };
 
@@ -41,6 +42,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char 
*prefix)
 parse_opt_object_name),
OPT_MERGED(&filter, N_("print only refs that are merged")),
OPT_NO_MERGED(&filter, N_("print only refs that are not 
merged")),
+   OPT_CONTAINS(&filter.with_commit, N_("print only refs which 
contain the commit")),
OPT_END(),
};
 
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 73dbf53..9969a08 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -60,4 +60,19 @@ test_expect_success 'filtering with --no-merged' '
test_cmp expect actual
 '
 
+test_expect_success 'filtering with --contains' '
+   cat >expect <<-\EOF &&
+   refs/heads/master
+   refs/heads/side
+   refs/odd/spot
+   refs/tags/double-tag
+   refs/tags/four
+   refs/tags/signed-tag
+   refs/tags/three
+   refs/tags/two
+   EOF
+   git for-each-ref --format="%(refname)" --contains=two >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.4.5

--
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 v8 08/11] parse-option: rename parse_opt_with_commit()

2015-07-07 Thread Karthik Nayak
Rename parse_opt_with_commit() to parse_opt_commits() to show
that it can be used to obtain a list of commits and is not
constricted to usage of '--contains' option.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c   | 4 ++--
 builtin/tag.c  | 4 ++--
 parse-options-cb.c | 2 +-
 parse-options.h| 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index e63102e..ae9a0eb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -832,13 +832,13 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
OPTION_CALLBACK, 0, "contains", &with_commit, 
N_("commit"),
N_("print only branches that contain the commit"),
PARSE_OPT_LASTARG_DEFAULT,
-   parse_opt_with_commit, (intptr_t)"HEAD",
+   parse_opt_commits, (intptr_t)"HEAD",
},
{
OPTION_CALLBACK, 0, "with", &with_commit, N_("commit"),
N_("print only branches that contain the commit"),
PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT,
-   parse_opt_with_commit, (intptr_t) "HEAD",
+   parse_opt_commits, (intptr_t) "HEAD",
},
OPT__ABBREV(&abbrev),
 
diff --git a/builtin/tag.c b/builtin/tag.c
index 280981f..7af45a0 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -603,13 +603,13 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
OPTION_CALLBACK, 0, "contains", &with_commit, 
N_("commit"),
N_("print only tags that contain the commit"),
PARSE_OPT_LASTARG_DEFAULT,
-   parse_opt_with_commit, (intptr_t)"HEAD",
+   parse_opt_commits, (intptr_t)"HEAD",
},
{
OPTION_CALLBACK, 0, "with", &with_commit, N_("commit"),
N_("print only tags that contain the commit"),
PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT,
-   parse_opt_with_commit, (intptr_t)"HEAD",
+   parse_opt_commits, (intptr_t)"HEAD",
},
{
OPTION_CALLBACK, 0, "points-at", &points_at, 
N_("object"),
diff --git a/parse-options-cb.c b/parse-options-cb.c
index de75411..632f10f 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -77,7 +77,7 @@ int parse_opt_verbosity_cb(const struct option *opt, const 
char *arg,
return 0;
 }
 
-int parse_opt_with_commit(const struct option *opt, const char *arg, int unset)
+int parse_opt_commits(const struct option *opt, const char *arg, int unset)
 {
unsigned char sha1[20];
struct commit *commit;
diff --git a/parse-options.h b/parse-options.h
index 1478818..583690c 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -224,7 +224,7 @@ extern int parse_opt_expiry_date_cb(const struct option *, 
const char *, int);
 extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
 extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
 extern int parse_opt_object_name(const struct option *, const char *, int);
-extern int parse_opt_with_commit(const struct option *, const char *, int);
+extern int parse_opt_commits(const struct option *, const char *, int);
 extern int parse_opt_tertiary(const struct option *, const char *, int);
 extern int parse_opt_string_list(const struct option *, const char *, int);
 extern int parse_opt_noop_cb(const struct option *, const char *, int);
-- 
2.4.5

--
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 v8 06/11] ref-filter: implement '--merged' and '--no-merged' options

2015-07-07 Thread Karthik Nayak
In 'branch -l' we have '--merged' option which only lists refs (branches)
merged into the named commit and '--no-merged' option which only lists
refs (branches) not merged into the named commit. Implement these two
options in ref-filter.{c,h} so that other commands can benefit from this.

Based-on-patch-by: Jeff King 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c |  4 
 ref-filter.c | 73 
 ref-filter.h |  8 +++
 3 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ddd90e6..e63102e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -635,6 +635,10 @@ static int print_ref_list(int kinds, int detached, int 
verbose, int abbrev, stru
cb.pattern = pattern;
cb.ret = 0;
for_each_rawref(append_ref, &cb);
+   /*
+* The following implementation is currently duplicated in ref-filter. 
It
+* will eventually be removed when we port branch.c to use ref-filter 
APIs.
+*/
if (merge_filter != NO_FILTER) {
struct commit *filter;
filter = lookup_commit_reference_gently(merge_filter_ref, 0);
diff --git a/ref-filter.c b/ref-filter.c
index b4753ab..148b7cd 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -9,6 +9,7 @@
 #include "tag.h"
 #include "quote.h"
 #include "ref-filter.h"
+#include "revision.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
@@ -898,6 +899,7 @@ static int ref_filter_handler(const char *refname, const 
struct object_id *oid,
struct ref_filter_cbdata *ref_cbdata = cb_data;
struct ref_filter *filter = ref_cbdata->filter;
struct ref_array_item *ref;
+   struct commit *commit = NULL;
 
if (flag & REF_BAD_NAME) {
  warning("ignoring ref with broken name %s", refname);
@@ -916,11 +918,23 @@ static int ref_filter_handler(const char *refname, const 
struct object_id *oid,
return 0;
 
/*
+* A merge filter is applied on refs pointing to commits. Hence
+* obtain the commit using the 'oid' available and discard all
+* non-commits early. The actual filtering is done later.
+*/
+   if (filter->merge_commit) {
+   commit = lookup_commit_reference_gently(oid->hash, 1);
+   if (!commit)
+   return 0;
+   }
+
+   /*
 * We do not open the object yet; sort may only need refname
 * to do its job and the resulting list may yet to be pruned
 * by maxcount logic.
 */
ref = new_ref_array_item(refname, oid->hash, flag);
+   ref->commit = commit;
 
REALLOC_ARRAY(ref_cbdata->array->items, ref_cbdata->array->nr + 1);
ref_cbdata->array->items[ref_cbdata->array->nr++] = ref;
@@ -946,6 +960,50 @@ void ref_array_clear(struct ref_array *array)
array->nr = array->alloc = 0;
 }
 
+static void do_merge_filter(struct ref_filter_cbdata *ref_cbdata)
+{
+   struct rev_info revs;
+   int i, old_nr;
+   struct ref_filter *filter = ref_cbdata->filter;
+   struct ref_array *array = ref_cbdata->array;
+   struct commit **to_clear = xcalloc(sizeof(struct commit *), array->nr);
+
+   init_revisions(&revs, NULL);
+
+   for (i = 0; i < array->nr; i++) {
+   struct ref_array_item *item = array->items[i];
+   add_pending_object(&revs, &item->commit->object, item->refname);
+   to_clear[i] = item->commit;
+   }
+
+   filter->merge_commit->object.flags |= UNINTERESTING;
+   add_pending_object(&revs, &filter->merge_commit->object, "");
+
+   revs.limited = 1;
+   if (prepare_revision_walk(&revs))
+   die(_("revision walk setup failed"));
+
+   old_nr = array->nr;
+   array->nr = 0;
+
+   for (i = 0; i < old_nr; i++) {
+   struct ref_array_item *item = array->items[i];
+   struct commit *commit = item->commit;
+
+   int is_merged = !!(commit->object.flags & UNINTERESTING);
+
+   if (is_merged == (filter->merge == REF_FILTER_MERGED_INCLUDE))
+   array->items[array->nr++] = array->items[i];
+   else
+   free_array_item(item);
+   }
+
+   for (i = 0; i < old_nr; i++)
+   clear_commit_marks(to_clear[i], ALL_REV_FLAGS);
+   clear_commit_marks(filter->merge_commit, ALL_REV_FLAGS);
+   free(to_clear);
+}
+
 /*
  * API for filtering a set of refs. Based on the type of refs the user
  * has requested, we iterate through those refs and apply filters
@@ -955,17 +1013,24 @@ void ref_array_clear(struct ref_array *array)
 int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned 
int type)
 {
struct ref_filter_cbdata ref_cbdata;
+   int ret = 0;
 
ref_cbdata.array = array;
  

[PATCH v8 07/11] for-each-ref: add '--merged' and '--no-merged' options

2015-07-07 Thread Karthik Nayak
Add the '--merged' and '--no-merged' options provided by 'ref-filter'.
The '--merged' option lets the user to only list refs merged into the
named commit. The '--no-merged' option lets the user to only list refs
not merged into the named commit.

Add documentation and tests for the same.

Based-on-patch-by: Jeff King 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt | 10 +-
 builtin/for-each-ref.c |  3 +++
 t/t6302-for-each-ref-filter.sh | 23 +++
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index ff0283b..2842195 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git for-each-ref' [--count=] [--shell|--perl|--python|--tcl]
   [(--sort=)...] [--format=] [...]
-  [--points-at ]
+  [--points-at ] [(--merged | --no-merged) []]
 
 DESCRIPTION
 ---
@@ -66,6 +66,14 @@ OPTIONS
 --points-at ::
Only list refs which points at the given object.
 
+--merged []::
+   Only list refs whose tips are reachable from the
+   specified commit (HEAD if not specified).
+
+--no-merged []::
+   Only list refs whose tips are not reachable from the
+   specified commit (HEAD if not specified).
+
 FIELD NAMES
 ---
 
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index ae5419e..7521850 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -8,6 +8,7 @@
 static char const * const for_each_ref_usage[] = {
N_("git for-each-ref [] []"),
N_("git for-each-ref [--points-at ]"),
+   N_("git for-each-ref [(--merged | --no-merged) []]"),
NULL
 };
 
@@ -38,6 +39,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char 
*prefix)
OPT_CALLBACK(0, "points-at", &filter.points_at,
 N_("object"), N_("print only refs which points at 
the given object"),
 parse_opt_object_name),
+   OPT_MERGED(&filter, N_("print only refs that are merged")),
+   OPT_NO_MERGED(&filter, N_("print only refs that are not 
merged")),
OPT_END(),
};
 
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 457991f..73dbf53 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -37,4 +37,27 @@ test_expect_success 'check signed tags with --points-at' '
test_cmp expect actual
 '
 
+test_expect_success 'filtering with --merged' '
+   cat >expect <<-\EOF &&
+   refs/heads/master
+   refs/odd/spot
+   refs/tags/one
+   refs/tags/three
+   refs/tags/two
+   EOF
+   git for-each-ref --format="%(refname)" --merged=master >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'filtering with --no-merged' '
+   cat >expect <<-\EOF &&
+   refs/heads/side
+   refs/tags/double-tag
+   refs/tags/four
+   refs/tags/signed-tag
+   EOF
+   git for-each-ref --format="%(refname)" --no-merged=master >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.4.5

--
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 v8 09/11] parse-options.h: add macros for '--contains' option

2015-07-07 Thread Karthik Nayak
Add a macro for using the '--contains' option in parse-options.h
also include an optional '--with' option macro which performs the
same action as '--contains'.

Make tag.c and branch.c use this new macro.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c | 14 ++
 builtin/tag.c| 14 ++
 parse-options.h  |  7 +++
 3 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ae9a0eb..c443cd8 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -828,18 +828,8 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
OPT__COLOR(&branch_use_color, N_("use colored output")),
OPT_SET_INT('r', "remotes", &kinds, N_("act on 
remote-tracking branches"),
REF_REMOTE_BRANCH),
-   {
-   OPTION_CALLBACK, 0, "contains", &with_commit, 
N_("commit"),
-   N_("print only branches that contain the commit"),
-   PARSE_OPT_LASTARG_DEFAULT,
-   parse_opt_commits, (intptr_t)"HEAD",
-   },
-   {
-   OPTION_CALLBACK, 0, "with", &with_commit, N_("commit"),
-   N_("print only branches that contain the commit"),
-   PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT,
-   parse_opt_commits, (intptr_t) "HEAD",
-   },
+   OPT_CONTAINS(&with_commit, N_("print only branches that contain 
the commit")),
+   OPT_WITH(&with_commit, N_("print only branches that contain the 
commit")),
OPT__ABBREV(&abbrev),
 
OPT_GROUP(N_("Specific git-branch actions:")),
diff --git a/builtin/tag.c b/builtin/tag.c
index 7af45a0..767162e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -595,23 +595,13 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
 
OPT_GROUP(N_("Tag listing options")),
OPT_COLUMN(0, "column", &colopts, N_("show tag list in 
columns")),
+   OPT_CONTAINS(&with_commit, N_("print only tags that contain the 
commit")),
+   OPT_WITH(&with_commit, N_("print only tags that contain the 
commit")),
{
OPTION_CALLBACK, 0, "sort", &tag_sort, N_("type"), 
N_("sort tags"),
PARSE_OPT_NONEG, parse_opt_sort
},
{
-   OPTION_CALLBACK, 0, "contains", &with_commit, 
N_("commit"),
-   N_("print only tags that contain the commit"),
-   PARSE_OPT_LASTARG_DEFAULT,
-   parse_opt_commits, (intptr_t)"HEAD",
-   },
-   {
-   OPTION_CALLBACK, 0, "with", &with_commit, N_("commit"),
-   N_("print only tags that contain the commit"),
-   PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT,
-   parse_opt_commits, (intptr_t)"HEAD",
-   },
-   {
OPTION_CALLBACK, 0, "points-at", &points_at, 
N_("object"),
N_("print only tags of the object"), 0, 
parse_opt_object_name
},
diff --git a/parse-options.h b/parse-options.h
index 583690c..7ea22b2 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -246,5 +246,12 @@ extern int parse_opt_noop_cb(const struct option *, const 
char *, int);
OPT_COLOR_FLAG(0, "color", (var), (h))
 #define OPT_COLUMN(s, l, v, h) \
{ OPTION_CALLBACK, (s), (l), (v), N_("style"), (h), PARSE_OPT_OPTARG, 
parseopt_column_callback }
+#define _OPT_CONTAINS_OR_WITH(name, variable, help, flag) \
+   { OPTION_CALLBACK, 0, name, (variable), N_("commit"), (help), \
+ PARSE_OPT_LASTARG_DEFAULT | flag, \
+ parse_opt_commits, (intptr_t) "HEAD" \
+   }
+#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 0)
+#define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN)
 
 #endif
-- 
2.4.5

--
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 v8 05/11] ref-filter: add parse_opt_merge_filter()

2015-07-07 Thread Karthik Nayak
Add 'parse_opt_merge_filter()' to parse '--merged' and '--no-merged'
options and write macros for the same.

This is copied from 'builtin/branch.c' which will eventually be removed
when we port 'branch.c' to use ref-filter APIs.

Based-on-patch-by: Jeff King 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c |  4 
 ref-filter.c | 19 +++
 ref-filter.h | 11 +++
 3 files changed, 34 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index b42e5b6..ddd90e6 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -745,6 +745,10 @@ static void rename_branch(const char *oldname, const char 
*newname, int force)
strbuf_release(&newsection);
 }
 
+/*
+ * This function is duplicated in ref-filter. It will eventually be removed
+ * when we port branch.c to use ref-filter APIs.
+ */
 static int opt_parse_merge_filter(const struct option *opt, const char *arg, 
int unset)
 {
merge_filter = ((opt->long_name[0] == 'n')
diff --git a/ref-filter.c b/ref-filter.c
index 58e532c..b4753ab 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1139,3 +1139,22 @@ int parse_opt_ref_sorting(const struct option *opt, 
const char *arg, int unset)
s->atom = parse_ref_filter_atom(arg, arg+len);
return 0;
 }
+
+int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset)
+{
+   struct ref_filter *rf = opt->value;
+   unsigned char sha1[20];
+
+   rf->merge = starts_with(opt->long_name, "no")
+   ? REF_FILTER_MERGED_OMIT
+   : REF_FILTER_MERGED_INCLUDE;
+
+   if (get_sha1(arg, sha1))
+   die(_("malformed object name %s"), arg);
+
+   rf->merge_commit = lookup_commit_reference_gently(sha1, 0);
+   if (!rf->merge_commit)
+   return opterror(opt, "must point to a commit", 0);
+
+   return 0;
+}
diff --git a/ref-filter.h b/ref-filter.h
index c2856b8..443cfa7 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -50,6 +50,15 @@ struct ref_filter_cbdata {
struct ref_filter *filter;
 };
 
+/*  Macros for checking --merged and --no-merged options */
+#define _OPT_MERGED_NO_MERGED(option, filter, h) \
+   { OPTION_CALLBACK, 0, option, (filter), N_("commit"), (h), \
+ PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
+ parse_opt_merge_filter, (intptr_t) "HEAD" \
+   }
+#define OPT_MERGED(f, h) _OPT_MERGED_NO_MERGED("merged", f, h)
+#define OPT_NO_MERGED(f, h) _OPT_MERGED_NO_MERGED("no-merged", f, h)
+
 /*
  * API for filtering a set of refs. Based on the type of refs the user
  * has requested, we iterate through those refs and apply filters
@@ -71,5 +80,7 @@ void show_ref_array_item(struct ref_array_item *info, const 
char *format, int qu
 int parse_opt_ref_sorting(const struct option *opt, const char *arg, int 
unset);
 /*  Default sort option based on refname */
 struct ref_sorting *ref_default_sorting(void);
+/*  Function to parse --merged and --no-merged options */
+int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 
 #endif /*  REF_FILTER_H  */
-- 
2.4.5

--
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: Git grep does not support multi-byte characters (like UTF-8)

2015-07-07 Thread Junio C Hamano
Plamen Totev  writes:

> pickaxe search also uses kwsearch so the case insensitive search with
> it does not work (e.g. git log -i -S).  Maybe this is a less of a
> problem here as one is expected to search for exact string (hence
> knows the case)

You reasoned correctly, I think.  Pickaxe, as one of the building
blocks to implement Linus's ultimate change tracking tool [*1*],
should never pay attention to "-i".  It is a step to finding the
commit that touches the exact code block given (i.e. "how do you
drill down?" part of $gmane/217 message).

Thanks.

[Footnote]
*1* http://article.gmane.org/gmane.comp.version-control.git/217
--
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 v8 02/11] tag: libify parse_opt_points_at()

2015-07-07 Thread Karthik Nayak
Rename 'parse_opt_points_at()' to 'parse_opt_object_name()' and
move it from 'tag.c' to 'parse-options'. This now acts as a common
parse_opt function which accepts an objectname and stores it into
a sha1_array.

Based-on-patch-by: Jeff King 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/tag.c  | 21 ++---
 parse-options-cb.c | 17 +
 parse-options.h|  1 +
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 5f6cdc5..e36c43e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -546,23 +546,6 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const 
char *name)
return check_refname_format(sb->buf, 0);
 }
 
-static int parse_opt_points_at(const struct option *opt 
__attribute__((unused)),
-   const char *arg, int unset)
-{
-   unsigned char sha1[20];
-
-   if (unset) {
-   sha1_array_clear(&points_at);
-   return 0;
-   }
-   if (!arg)
-   return error(_("switch 'points-at' requires an object"));
-   if (get_sha1(arg, sha1))
-   return error(_("malformed object name '%s'"), arg);
-   sha1_array_append(&points_at, sha1);
-   return 0;
-}
-
 static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
 {
int *sort = opt->value;
@@ -625,8 +608,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
parse_opt_with_commit, (intptr_t)"HEAD",
},
{
-   OPTION_CALLBACK, 0, "points-at", NULL, N_("object"),
-   N_("print only tags of the object"), 0, 
parse_opt_points_at
+   OPTION_CALLBACK, 0, "points-at", &points_at, 
N_("object"),
+   N_("print only tags of the object"), 0, 
parse_opt_object_name
},
OPT_END()
};
diff --git a/parse-options-cb.c b/parse-options-cb.c
index be8c413..de75411 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -4,6 +4,7 @@
 #include "commit.h"
 #include "color.h"
 #include "string-list.h"
+#include "sha1-array.h"
 
 /*- some often used options -*/
 
@@ -92,6 +93,22 @@ int parse_opt_with_commit(const struct option *opt, const 
char *arg, int unset)
return 0;
 }
 
+int parse_opt_object_name(const struct option *opt, const char *arg, int unset)
+{
+   unsigned char sha1[20];
+
+   if (unset) {
+   sha1_array_clear(opt->value);
+   return 0;
+   }
+   if (!arg)
+   return -1;
+   if (get_sha1(arg, sha1))
+   return error(_("malformed object name '%s'"), arg);
+   sha1_array_append(opt->value, sha1);
+   return 0;
+}
+
 int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
 {
int *target = opt->value;
diff --git a/parse-options.h b/parse-options.h
index ca865f6..1478818 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -223,6 +223,7 @@ extern int parse_opt_approxidate_cb(const struct option *, 
const char *, int);
 extern int parse_opt_expiry_date_cb(const struct option *, const char *, int);
 extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
 extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
+extern int parse_opt_object_name(const struct option *, const char *, int);
 extern int parse_opt_with_commit(const struct option *, const char *, int);
 extern int parse_opt_tertiary(const struct option *, const char *, int);
 extern int parse_opt_string_list(const struct option *, const char *, int);
-- 
2.4.5

--
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 v8 01/11] t6302: for-each-ref tests for ref-filter APIs

2015-07-07 Thread Karthik Nayak
Add a test suite for testing the ref-filter APIs used
by for-each-ref. We just intialize the test suite for now.
More tests will be added in the following patches as more
options are added to for-each-ref.

Based-on-patch-by: Jeff King 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 t/t6302-for-each-ref-filter.sh | 20 
 1 file changed, 20 insertions(+)
 create mode 100755 t/t6302-for-each-ref-filter.sh

diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
new file mode 100755
index 000..44d2f24
--- /dev/null
+++ b/t/t6302-for-each-ref-filter.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+test_description='test for-each-refs usage of ref-filter APIs'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-gpg.sh
+
+test_expect_success 'setup some history and refs' '
+   test_commit one &&
+   test_commit two &&
+   test_commit three &&
+   git checkout -b side &&
+   test_commit four &&
+   git tag -s -m "A signed tag message" signed-tag &&
+   git tag -s -m "Annonated doubly" double-tag signed-tag &&
+   git checkout master &&
+   git update-ref refs/odd/spot master
+'
+
+test_done
-- 
2.4.5

--
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] index-pack: fix allocation of sorted_by_pos array

2015-07-07 Thread Jeff King
On Tue, Jul 07, 2015 at 08:49:19AM -0700, Junio C Hamano wrote:

> Duy Nguyen  writes:
> 
> > I keep tripping over this "real_type vs type" in this code. What do
> > you think about renaming "type" field to "in_pack_type" and
> > "real_type" to "canon_type" (or "final_type")? "Real" does not really
> > say anything in this context..
> 
> An unqualified name "type" does bother me for the word to express
> what representation the piece of data uses (i.e. is it a delta, or
> is it a base object of "tree" type, or what).  I think I tried to
> unconfuse myself by saying "representation type" in in-code
> comments, reviews and log messages when it is not clear which kind
> between "in-pack representation" or "Git object type of that stored
> data" a sentence is talking about, and I agree "in_pack_type" would
> be a vast improvement over just "type".

I think this is doubly confusing because pack-objects _does_ use
in_pack_type. And its "type" is therefore the "real" object type. Which
is the opposite of index-pack, which uses "type" for the in-pack type.
So at the very least, we should harmonize these two uses.

> Especially, if the other one is renamed with "in_pack_" prefix,
> "real_type" is not just clear enough but is probably better because
> it explains what it is from its "meaning" (i.e. it is the type of
> the Git object, not how it is represented in the pack-stream) than
> "final_type" that is named after "how" it is computed (i.e. it makes
> sense to you only if you know that an in-pack type "this is delta"
> does not have the full information and you have to traverse the
> delta chain and you will finally find out what it is when you hit
> the base representation).

Yeah, I agree real_type is fine when paired with in_pack_type. We might
consider modifying pack-objects.h to match (on top of just moving
index-pack to in_pack_type).

-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 v8 03/11] ref-filter: implement '--points-at' option

2015-07-07 Thread Karthik Nayak
In 'tag -l' we have '--points-at' option which lets users
list only tags of a given object. Implement this option in
'ref-filter.{c,h}' so that other commands can benefit from this.

This is duplicated from tag.c, we will eventually remove that
when we port tag.c to use ref-filter APIs.

Based-on-patch-by: Jeff King 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/tag.c |  4 
 ref-filter.c  | 35 +++
 ref-filter.h  |  1 +
 3 files changed, 40 insertions(+)

diff --git a/builtin/tag.c b/builtin/tag.c
index e36c43e..280981f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -56,6 +56,10 @@ static int match_pattern(const char **patterns, const char 
*ref)
return 0;
 }
 
+/*
+ * This is currently duplicated in ref-filter.c, and will eventually be
+ * removed as we port tag.c to use the ref-filter APIs.
+ */
 static const unsigned char *match_points_at(const char *refname,
const unsigned char *sha1)
 {
diff --git a/ref-filter.c b/ref-filter.c
index c4004db..58e532c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -842,6 +842,38 @@ static int match_name_as_path(const char **pattern, const 
char *refname)
return 0;
 }
 
+/*
+ * Given a ref (sha1, refname), check if the ref belongs to the array
+ * of sha1s. If the given ref is a tag, check if the given tag points
+ * at one of the sha1s in the given sha1 array.
+ * the given sha1_array.
+ * NEEDSWORK:
+ * 1. Only a single level of inderection is obtained, we might want to
+ * change this to account for multiple levels (e.g. annotated tags
+ * pointing to annotated tags pointing to a commit.)
+ * 2. As the refs are cached we might know what refname peels to without
+ * the need to parse the object via parse_object(). peel_ref() might be a
+ * more efficient alternative to obtain the pointee.
+ */
+static const unsigned char *match_points_at(struct sha1_array *points_at,
+   const unsigned char *sha1,
+   const char *refname)
+{
+   const unsigned char *tagged_sha1 = NULL;
+   struct object *obj;
+
+   if (sha1_array_lookup(points_at, sha1) >= 0)
+   return sha1;
+   obj = parse_object(sha1);
+   if (!obj)
+   die(_("malformed object at '%s'"), refname);
+   if (obj->type == OBJ_TAG)
+   tagged_sha1 = ((struct tag *)obj)->tagged->sha1;
+   if (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)
+   return tagged_sha1;
+   return NULL;
+}
+
 /* Allocate space for a new ref_array_item and copy the objectname and flag to 
it */
 static struct ref_array_item *new_ref_array_item(const char *refname,
 const unsigned char 
*objectname,
@@ -880,6 +912,9 @@ static int ref_filter_handler(const char *refname, const 
struct object_id *oid,
if (*filter->name_patterns && 
!match_name_as_path(filter->name_patterns, refname))
return 0;
 
+   if (filter->points_at.nr && !match_points_at(&filter->points_at, 
oid->hash, refname))
+   return 0;
+
/*
 * We do not open the object yet; sort may only need refname
 * to do its job and the resulting list may yet to be pruned
diff --git a/ref-filter.h b/ref-filter.h
index 6997984..c2856b8 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -42,6 +42,7 @@ struct ref_array {
 
 struct ref_filter {
const char **name_patterns;
+   struct sha1_array points_at;
 };
 
 struct ref_filter_cbdata {
-- 
2.4.5

--
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 v8 04/11] for-each-ref: add '--points-at' option

2015-07-07 Thread Karthik Nayak
Add the '--points-at' option provided by 'ref-filter'. The
option lets the user to list only refs which points at the
given object.

Add documentation and tests for the same.

Based-on-patch-by: Jeff King 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  3 +++
 builtin/for-each-ref.c |  9 +++--
 t/t6302-for-each-ref-filter.sh | 20 
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 7f8d9a5..ff0283b 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git for-each-ref' [--count=] [--shell|--perl|--python|--tcl]
   [(--sort=)...] [--format=] [...]
+  [--points-at ]
 
 DESCRIPTION
 ---
@@ -62,6 +63,8 @@ OPTIONS
the specified host language.  This is meant to produce
a scriptlet that can directly be `eval`ed.
 
+--points-at ::
+   Only list refs which points at the given object.
 
 FIELD NAMES
 ---
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 7919206..ae5419e 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -7,6 +7,7 @@
 
 static char const * const for_each_ref_usage[] = {
N_("git for-each-ref [] []"),
+   N_("git for-each-ref [--points-at ]"),
NULL
 };
 
@@ -34,9 +35,15 @@ int cmd_for_each_ref(int argc, const char **argv, const char 
*prefix)
OPT_STRING(  0 , "format", &format, N_("format"), N_("format to 
use for the output")),
OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
N_("field name to sort on"), 
&parse_opt_ref_sorting),
+   OPT_CALLBACK(0, "points-at", &filter.points_at,
+N_("object"), N_("print only refs which points at 
the given object"),
+parse_opt_object_name),
OPT_END(),
};
 
+   memset(&array, 0, sizeof(array));
+   memset(&filter, 0, sizeof(filter));
+
parse_options(argc, argv, prefix, opts, for_each_ref_usage, 0);
if (maxcount < 0) {
error("invalid --count argument: `%d'", maxcount);
@@ -55,8 +62,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char 
*prefix)
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
 
-   memset(&array, 0, sizeof(array));
-   memset(&filter, 0, sizeof(filter));
filter.name_patterns = argv;
filter_refs(&array, &filter, FILTER_REFS_ALL | 
FILTER_REFS_INCLUDE_BROKEN);
ref_array_sort(sorting, &array);
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 44d2f24..457991f 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -17,4 +17,24 @@ test_expect_success 'setup some history and refs' '
git update-ref refs/odd/spot master
 '
 
+test_expect_success 'filtering with --points-at' '
+   cat >expect <<-\EOF &&
+   refs/heads/master
+   refs/odd/spot
+   refs/tags/three
+   EOF
+   git for-each-ref --format="%(refname)" --points-at=master >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'check signed tags with --points-at' '
+   sed -e "s/Z$//" >expect <<-\EOF &&
+   refs/heads/side Z
+   refs/tags/four Z
+   refs/tags/signed-tag four
+   EOF
+   git for-each-ref --format="%(refname) %(*subject)" --points-at=side 
>actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.4.5

--
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 v8 00/11] add options to for-each-ref

2015-07-07 Thread Karthik Nayak
v7 of this patch series can be found here :
http://article.gmane.org/gmane.comp.version-control.git/273233

This is a continuation of my GSoC project to unify git tag -l, git
branch -l and for-each-ref. Continued from this patch series :
http://thread.gmane.org/gmane.comp.version-control.git/271563

Changes in v8:
[04/11]: Change "pertain to" to "points at", Grammatical change.
[05/11], [04/11]: Random spaces left in macro definition
[10/11]: Mention the movement of code from tag.c

Thanks to Matthieu Moy for suggestions on the previous versions.

-- 
Regards,
Karthik Nayak
--
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] index-pack: fix allocation of sorted_by_pos array

2015-07-07 Thread Junio C Hamano
Duy Nguyen  writes:

> I keep tripping over this "real_type vs type" in this code. What do
> you think about renaming "type" field to "in_pack_type" and
> "real_type" to "canon_type" (or "final_type")? "Real" does not really
> say anything in this context..

An unqualified name "type" does bother me for the word to express
what representation the piece of data uses (i.e. is it a delta, or
is it a base object of "tree" type, or what).  I think I tried to
unconfuse myself by saying "representation type" in in-code
comments, reviews and log messages when it is not clear which kind
between "in-pack representation" or "Git object type of that stored
data" a sentence is talking about, and I agree "in_pack_type" would
be a vast improvement over just "type".

To me personally real- and final- mean about the same thing
(i.e. what is the real type of the object that is stored?) in the
context of this codepath.

Especially, if the other one is renamed with "in_pack_" prefix,
"real_type" is not just clear enough but is probably better because
it explains what it is from its "meaning" (i.e. it is the type of
the Git object, not how it is represented in the pack-stream) than
"final_type" that is named after "how" it is computed (i.e. it makes
sense to you only if you know that an in-pack type "this is delta"
does not have the full information and you have to traverse the
delta chain and you will finally find out what it is when you hit
the base representation).

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


No one understands diff3 "Temporary merge branch" conflict markers

2015-07-07 Thread Edward Anderson
I have the diff3 conflictstyle enabled and want to be able to
understand how to understand its output when there are criss-cross
merges requiring temporary merge branches.  Eg:

<<< HEAD
  print(A);
||| merged common ancestors
<<< Temporary merge branch 1
  print(B);
===
  print(C);
>>> feature

I have combed Google but have been able to find no good explanation.
People are hitting these and asking for help, but the suggestions
are weak and fail to explain anything.


Background
--

Our workflow often causes criss-cross merges. In this more extreme
(but real) case, I have 16 ambiguously "best" merge bases:

$ git merge-base --all HEAD feature | wc -l
16

The 'resolve' strategy isn't an option for this many merge-bases:

$ git merge -s resolve feature
fatal: I cannot read more than 8 trees
Merge with strategy resolve failed.

That brings me back to the recursive strategy.

https://lwn.net/Articles/210045/ "Branching and merging with git"
describes what a criss-cross merge is and how the recursive strategy
deals with them by merging the multiple merge-bases to form a single
temporary merge base.

> The classic confusing case is called a "criss-cross merge", and
> looks like this:
>
>  o--b-o-o--B
> /\ /
>  o--o--o  X
> \/ \
>  o--a-o-o--A
>
> There are two common ancestors of A and B, marked a and b in the
> graph above.  And they're not the same.  You could use either one
> and get reasonable results, but how to choose?
>
> The details are too advanced for this discussion, but the default
> "recursive" merge strategy that git uses solves the answer by
> merging a and b into a temporary commit and using *that* as the
> merge base.
>
> Of course, a and b could have the same problem, so merging them
> could require another merge of still-older commits.  This is why the
> algorithm is called "recursive."

With the diff3 conflictstyle showing the merged common ancestors and
the recursive merge strategy, the merged common ancestors may have a
conflict

Conflict #1
---

 1 <<< HEAD
 2 unless admin
 3   fail Unauthorized.new("Admin only")
 4 end
 5 ||| merged common ancestors
 6 <<< Temporary merge branch 1
 7 unless admin
 8   fail Unauthorized.new("Admin only")
 9 end
10 ||| merged common ancestors
11 unless admin
12   fail Unauthorized.new
13 end
14 ===
15 fail Unauthorized.new unless admin
16 >>> Temporary merge branch 2
17 ===
18 unless admin
19 fail Unauthorized.new("Admin only")
20   fail Unauthorized.new
21 end
22 >>> feature

It seems lines 6-16 are a conflict that occurred when merging the
merge-bases.  That conflict could be resolved by merging the change in
Temporary merge branch 1 (add "Admin only") with Temporary merge
branch 2 (convert multi-line unless to single-line) as this:

   fail Unauthorized.new("Admin only") unless admin

Thus, you might thing that you could refactor the above conflict to be
this:

 1 <<< HEAD
 2 unless admin
 3   fail Unauthorized.new("Admin only")
 4 end
 5 ||| merged common ancestors
 6 fail Unauthorized.new("Admin only") unless admin
 7 ===
 8 unless admin
 9 fail Unauthorized.new("Admin only")
10   fail Unauthorized.new
11 end
12 >>> feature

However this would be resolved by merging HEAD's apparent change
(break single-line unless onto multiple lines) and feature's change,
which appears to be a botched up conflict resolution.

Obviously I'm reading this wrong. What is the correct way?

Conflict #2


 1 <<< HEAD
 2 unless feature.enabled_for_user?(UserIdLookup.new(params).user_id)
 3   fail Unauthorized.new("Requires setting #{label}.")
 4 ||| merged common ancestors
 5 <<< Temporary merge branch 1
 6 unless feature.enabled_for_user?(params[:user_id])
 7   fail Unauthorized.new("Requires setting #{label}.")
 8 ===
 9 unless feature.enabled_for_user?(params[:user_id])
10   fail Unauthorized.new("Requires setting #{label}.")
11 >>> feature

This is the full conflict, and it doesn't seem to balance. In this
case, there's only one Temporary merge branch. Can the line marker on
line 5 just be ignored?  If so, then this is easy to resolve, but I am
left puzzled as to why the sections in lines 6-7 and 9-10 are
identical.  Is the Temporary merge branch 1 marker here simply because
other conflicts had multiple Temporary merge branches that had to be
labeled?  Or is the 0 lines between lines 4 and 5 a significant
deletion?

---

The git community needs some light and understanding shed on this
topic, on how these conflicts are to be read and understood. I'm
looking for 

Re: [PATCH] Change strbuf_read_file() to return ssize_t

2015-07-07 Thread Jeff King
On Fri, Jul 03, 2015 at 03:59:32PM +0200, Michael Haggerty wrote:

> It is currently declared to return int, which could overflow for large
> files.
> 
> Signed-off-by: Michael Haggerty 
> ---
> This patch is against maint, but it also rebases against master
> without conflict.
> 
> I couldn't find any way to exploit this bug. Most callers only use
> this function for locally-generated files in the first place. And the
> correct length of the file is available in strbuf::len, so most
> callers only use the return value for a "< 0" check. And they don't do
> anything risky on the error path.

FWIW, I also looked for problem areas, but couldn't find anything
interesting. But this seems like an obviously good thing to be doing
anyway.

I also wondered if any callers needed to adjust their storage for the
return type to ssize_t (i.e., are we just moving the truncation up one
assignment). But there is only a single caller that assigns the result,
and it uses an ssize_t already.

-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] grep: use regcomp() for icase search with non-ascii patterns

2015-07-07 Thread Plamen Totev

On 07.07. 2015 at 02:02, Duy Nguyen  wrote: 
> On Tue, Jul 7, 2015 at 3:10 AM, René Scharfe  wrote: 
> > Am 06.07.2015 um 14:42 schrieb Nguyễn Thái Ngọc Duy: 

> > So the optimization before this patch was that if a string was searched for 
> > without -F then it would be treated as a fixed string anyway unless it 
> > contained regex special characters. Searching for fixed strings using the 
> > kwset functions is faster than using regcomp and regexec, which makes the 
> > exercise worthwhile. 
> > 
> > Your patch disables the optimization if non-ASCII characters are searched 
> > for because kwset handles case transformations only for ASCII chars. 
> > 
> > Another consequence of this limitation is that -Fi (explicit 
> > case-insensitive fixed-string search) doesn't work properly with non-ASCII 
> > chars neither. How can we handle this one? Fall back to regcomp by 
> > escaping all special characters? Or at least warn? 
> 
> Hehe.. I noticed it too shortly after sending the patch. I was torn 
> between simply documenting the limitation and waiting for the next 
> person to come and fix it, or quoting the regex then passing to 
> regcomp. GNU grep does the quoting in this case, but that code is 
> GPLv3 so we can't simply copy over. It could be a problem if we need 
> to quote a regex in a multibyte charset where ascii is not a subset. 
> But i guess we can just go with utf-8.. 

I played a little bit with the code and I came up with this function to escape
regular expressions in  utf-8. Hope it helps.

static void escape_regexp(const char *pattern, size_t len,
                char **new_pattern, size_t *new_len)
{
        const char *p = pattern;
        char *np = *new_pattern = xmalloc(2 * len);
        int chrlen;
        *new_len = len;

        while (len) {
                chrlen = mbs_chrlen(&p, &len, "utf-8");
                if (chrlen == 1 && is_regex_special(*pattern))
                        *np++ = '\\';

                memcpy(np, pattern, chrlen);
                np += chrlen;
                pattern = p;
        }

        *new_len = np - *new_pattern;
}

--
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 v5 34/44] builtin-am: invoke applypatch-msg hook

2015-07-07 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07),
git-am.sh will invoke the applypatch-msg hooks just after extracting the
patch message. If the applypatch-msg hook exits with a non-zero status,
git-am.sh abort before even applying the patch to the index.

Re-implement this in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 2255a1e..f856d3b 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -491,6 +491,27 @@ static void am_destroy(const struct am_state *state)
 }
 
 /**
+ * Runs applypatch-msg hook. Returns its exit code.
+ */
+static int run_applypatch_msg_hook(struct am_state *state)
+{
+   int ret;
+
+   assert(state->msg);
+   ret = run_hook_le(NULL, "applypatch-msg", am_path(state, 
"final-commit"), NULL);
+
+   if (!ret) {
+   free(state->msg);
+   state->msg = NULL;
+   if (read_commit_msg(state) < 0)
+   die(_("'%s' was deleted by the applypatch-msg hook"),
+   am_path(state, "final-commit"));
+   }
+
+   return ret;
+}
+
+/**
  * Runs post-rewrite hook. Returns it exit code.
  */
 static int run_post_rewrite_hook(const struct am_state *state)
@@ -1434,6 +1455,9 @@ static void am_run(struct am_state *state)
write_author_script(state);
write_commit_msg(state);
 
+   if (run_applypatch_msg_hook(state))
+   exit(1);
+
say(state, stdout, _("Applying: %.*s"), linelen(state->msg), 
state->msg);
 
apply_status = run_apply(state, NULL);
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 19/44] builtin-am: implement --3way, am.threeWay

2015-07-07 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07),
git-am.sh supported the --3way option, and if set, would attempt to do a
3-way merge if the initial patch application fails.

Since d96a275 (git-am: add am.threeWay config variable, 2015-06-04), the
setting am.threeWay configures if the --3way option is set by default.

Since 5d86861 (am -3: list the paths that needed 3-way fallback,
2012-03-28), in a 3-way merge git-am.sh would list the paths that needed
3-way fallback, so that the user can review them more carefully to spot
mismerges.

Re-implement the above in builtin/am.c.

Signed-off-by: Paul Tan 
---

Notes:
v5

* s/a index/an index/

 builtin/am.c | 157 +--
 1 file changed, 153 insertions(+), 4 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 511e4dd..af22c35 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -19,6 +19,8 @@
 #include "unpack-trees.h"
 #include "branch.h"
 #include "sequencer.h"
+#include "revision.h"
+#include "merge-recursive.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -82,6 +84,8 @@ struct am_state {
/* number of digits in patch filename */
int prec;
 
+   int threeway;
+
int quiet;
 
int append_signoff;
@@ -102,6 +106,8 @@ static void am_state_init(struct am_state *state, const 
char *dir)
state->dir = xstrdup(dir);
 
state->prec = 4;
+
+   git_config_get_bool("am.threeway", &state->threeway);
 }
 
 /**
@@ -371,6 +377,9 @@ static void am_load(struct am_state *state)
 
read_commit_msg(state);
 
+   read_state_file(&sb, state, "threeway", 1);
+   state->threeway = !strcmp(sb.buf, "t");
+
read_state_file(&sb, state, "quiet", 1);
state->quiet = !strcmp(sb.buf, "t");
 
@@ -555,6 +564,8 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
die(_("Failed to split patches."));
}
 
+   write_file(am_path(state, "threeway"), 1, state->threeway ? "t" : "f");
+
write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f");
 
write_file(am_path(state, "sign"), 1, state->append_signoff ? "t" : 
"f");
@@ -790,16 +801,34 @@ finish:
 }
 
 /**
- * Applies current patch with git-apply. Returns 0 on success, -1 otherwise.
+ * Applies current patch with git-apply. Returns 0 on success, -1 otherwise. If
+ * `index_file` is not NULL, the patch will be applied to that index.
  */
-static int run_apply(const struct am_state *state)
+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(&cp.env_array, "GIT_INDEX_FILE=%s", 
index_file);
+
+   /*
+* 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;
+   }
+
argv_array_push(&cp.args, "apply");
-   argv_array_push(&cp.args, "--index");
+
+   if (index_file)
+   argv_array_push(&cp.args, "--cached");
+   else
+   argv_array_push(&cp.args, "--index");
+
argv_array_push(&cp.args, am_path(state, "patch"));
 
if (run_command(&cp))
@@ -807,8 +836,106 @@ static int run_apply(const struct am_state *state)
 
/* Reload index as git-apply will have modified it. */
discard_cache();
+   read_cache_from(index_file ? index_file : get_index_file());
+
+   return 0;
+}
+
+/**
+ * Builds an index that contains just the blobs needed for a 3way merge.
+ */
+static int build_fake_ancestor(const struct am_state *state, const char 
*index_file)
+{
+   struct child_process cp = CHILD_PROCESS_INIT;
+
+   cp.git_cmd = 1;
+   argv_array_push(&cp.args, "apply");
+   argv_array_pushf(&cp.args, "--build-fake-ancestor=%s", index_file);
+   argv_array_push(&cp.args, am_path(state, "patch"));
+
+   if (run_command(&cp))
+   return -1;
+
+   return 0;
+}
+
+/**
+ * Attempt a threeway merge, using index_path as the temporary index.
+ */
+static int fall_back_threeway(const struct am_state *state, const char 
*index_path)
+{
+   unsigned char orig_tree[GIT_SHA1_RAWSZ], his_tree[GIT_SHA1_RAWSZ],
+ our_tree[GIT_SHA1_RAWSZ];
+   const unsigned char *bases[1] = {orig_tree};
+   struct merge_options o;
+   struct commit *result;
+   char *his_tree_name;
+
+   if (get_sha1("HEAD", our_tree) < 0)
+   hashcpy(our_tree, EMPTY_TREE_SHA1_BIN);
+
+   if (build_fake_ancestor(state, index_path))
+   return error("could not build fake ancestor");
+
+   discard_cache();
+   read_cache_from(index_path);
+
+   if (write_index_as_tree(orig_tree, &the_index, index_path, 

[PATCH v5 44/44] builtin-am: remove redirection to git-am.sh

2015-07-07 Thread Paul Tan
At the beginning of the rewrite of git-am.sh to C, in order to not break
existing test scripts that depended on a functional git-am, a
redirection to git-am.sh was introduced that would activate if the
environment variable _GIT_USE_BUILTIN_AM was not defined.

Now that all of git-am.sh's functionality has been re-implemented in
builtin/am.c, remove this redirection, and retire git-am.sh into
contrib/examples/.

Signed-off-by: Paul Tan 
---
 Makefile|  1 -
 builtin/am.c| 15 ---
 git-am.sh => contrib/examples/git-am.sh |  0
 git.c   |  7 +--
 4 files changed, 1 insertion(+), 22 deletions(-)
 rename git-am.sh => contrib/examples/git-am.sh (100%)

diff --git a/Makefile b/Makefile
index ff9bdc0..005d771 100644
--- a/Makefile
+++ b/Makefile
@@ -466,7 +466,6 @@ TEST_PROGRAMS_NEED_X =
 # interactive shell sessions without exporting it.
 unexport CDPATH
 
-SCRIPT_SH += git-am.sh
 SCRIPT_SH += git-bisect.sh
 SCRIPT_SH += git-difftool--helper.sh
 SCRIPT_SH += git-filter-branch.sh
diff --git a/builtin/am.c b/builtin/am.c
index a0982d9..c548129 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2239,21 +2239,6 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
-   /*
-* NEEDSWORK: Once all the features of git-am.sh have been
-* re-implemented in builtin/am.c, this preamble can be removed.
-*/
-   if (!getenv("_GIT_USE_BUILTIN_AM")) {
-   const char *path = mkpath("%s/git-am", git_exec_path());
-
-   if (sane_execvp(path, (char **)argv) < 0)
-   die_errno("could not exec %s", path);
-   } else {
-   prefix = setup_git_directory();
-   trace_repo_setup(prefix);
-   setup_work_tree();
-   }
-
git_config(git_default_config, NULL);
 
am_state_init(&state, git_path("rebase-apply"));
diff --git a/git-am.sh b/contrib/examples/git-am.sh
similarity index 100%
rename from git-am.sh
rename to contrib/examples/git-am.sh
diff --git a/git.c b/git.c
index e84e1a1..8420e43 100644
--- a/git.c
+++ b/git.c
@@ -370,12 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, 
const char **argv)
 
 static struct cmd_struct commands[] = {
{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
-   /*
-* NEEDSWORK: Once the redirection to git-am.sh in builtin/am.c has
-* been removed, this entry should be changed to
-* RUN_SETUP | NEED_WORK_TREE
-*/
-   { "am", cmd_am },
+   { "am", cmd_am, RUN_SETUP | NEED_WORK_TREE },
{ "annotate", cmd_annotate, RUN_SETUP },
{ "apply", cmd_apply, RUN_SETUP_GENTLY },
{ "archive", cmd_archive },
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 13/44] builtin-am: implement --abort

2015-07-07 Thread Paul Tan
Since 3e5057a (git am --abort, 2008-07-16), git-am supported the --abort
option that will rewind HEAD back to the original commit. Re-implement
this through am_abort().

Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure
and warn, 2010-12-21), to prevent commits made since the last failure
from being lost, git-am will not rewind HEAD back to the original
commit if HEAD moved since the last failure. Re-implement this through
safe_to_abort().

Signed-off-by: Paul Tan 
---
 builtin/am.c | 102 +--
 1 file changed, 99 insertions(+), 3 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 5087094..1db95f2 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -507,6 +507,8 @@ static int split_mail(struct am_state *state, enum 
patch_format patch_format,
 static void am_setup(struct am_state *state, enum patch_format patch_format,
const char **paths)
 {
+   unsigned char curr_head[GIT_SHA1_RAWSZ];
+
if (!patch_format)
patch_format = detect_patch_format(paths);
 
@@ -523,6 +525,14 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
die(_("Failed to split patches."));
}
 
+   if (!get_sha1("HEAD", curr_head)) {
+   write_file(am_path(state, "abort-safety"), 1, "%s", 
sha1_to_hex(curr_head));
+   update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, 
UPDATE_REFS_DIE_ON_ERR);
+   } else {
+   write_file(am_path(state, "abort-safety"), 1, "%s", "");
+   delete_ref("ORIG_HEAD", NULL, 0);
+   }
+
/*
 * NOTE: Since the "next" and "last" files determine if an am_state
 * session is in progress, they should be written last.
@@ -539,6 +549,8 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
  */
 static void am_next(struct am_state *state)
 {
+   unsigned char head[GIT_SHA1_RAWSZ];
+
if (state->author_name)
free(state->author_name);
state->author_name = NULL;
@@ -559,6 +571,11 @@ static void am_next(struct am_state *state)
unlink(am_path(state, "author-script"));
unlink(am_path(state, "final-commit"));
 
+   if (!get_sha1("HEAD", head))
+   write_file(am_path(state, "abort-safety"), 1, "%s", 
sha1_to_hex(head));
+   else
+   write_file(am_path(state, "abort-safety"), 1, "%s", "");
+
state->cur++;
write_file(am_path(state, "next"), 1, "%d", state->cur);
 }
@@ -791,10 +808,14 @@ static void am_run(struct am_state *state)
const char *argv_gc_auto[] = {"gc", "--auto", NULL};
struct strbuf sb = STRBUF_INIT;
 
+   unlink(am_path(state, "dirtyindex"));
+
refresh_and_write_cache();
 
-   if (index_has_changes(&sb))
+   if (index_has_changes(&sb)) {
+   write_file(am_path(state, "dirtyindex"), 1, "t");
die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf);
+   }
 
strbuf_release(&sb);
 
@@ -982,6 +1003,74 @@ static void am_skip(struct am_state *state)
 }
 
 /**
+ * Returns true if it is safe to reset HEAD to the ORIG_HEAD, false otherwise.
+ *
+ * It is not safe to reset HEAD when:
+ * 1. git-am previously failed because the index was dirty.
+ * 2. HEAD has moved since git-am previously failed.
+ */
+static int safe_to_abort(const struct am_state *state)
+{
+   struct strbuf sb = STRBUF_INIT;
+   unsigned char abort_safety[GIT_SHA1_RAWSZ], head[GIT_SHA1_RAWSZ];
+
+   if (file_exists(am_path(state, "dirtyindex")))
+   return 0;
+
+   if (read_state_file(&sb, state, "abort-safety", 1) > 0) {
+   if (get_sha1_hex(sb.buf, abort_safety))
+   die(_("could not parse %s"), am_path(state, 
"abort_safety"));
+   } else
+   hashclr(abort_safety);
+
+   if (get_sha1("HEAD", head))
+   hashclr(head);
+
+   if (!hashcmp(head, abort_safety))
+   return 1;
+
+   error(_("You seem to have moved HEAD since the last 'am' failure.\n"
+   "Not rewinding to ORIG_HEAD"));
+
+   return 0;
+}
+
+/**
+ * Aborts the current am session if it is safe to do so.
+ */
+static void am_abort(struct am_state *state)
+{
+   unsigned char curr_head[GIT_SHA1_RAWSZ], orig_head[GIT_SHA1_RAWSZ];
+   int has_curr_head, has_orig_head;
+   const char *curr_branch;
+
+   if (!safe_to_abort(state)) {
+   am_destroy(state);
+   return;
+   }
+
+   curr_branch = resolve_refdup("HEAD", 0, curr_head, NULL);
+   has_curr_head = !is_null_sha1(curr_head);
+   if (!has_curr_head)
+   hashcpy(curr_head, EMPTY_TREE_SHA1_BIN);
+
+   has_orig_head = !get_sha1("ORIG_HEAD", orig_head);
+   if (!has_orig_head)
+   hashcpy(orig_head, EMPTY_TREE_SHA1_BIN);
+
+   clean_index(curr_head, orig_head);
+
+   i

[PATCH v5 24/44] builtin-am: implement -k/--keep, --keep-non-patch

2015-07-07 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07),
git-am.sh supported the -k/--keep option to pass the -k option to
git-mailsplit.

Since f7e5ea1 (am: learn passing -b to mailinfo, 2012-01-16), git-am.sh
supported the --keep-non-patch option to pass the -b option to
git-mailsplit.

Re-implement these two options in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 52 
 1 file changed, 52 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 0b441fb..4e1ee7f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -68,6 +68,12 @@ enum patch_format {
PATCH_FORMAT_MBOX
 };
 
+enum keep_type {
+   KEEP_FALSE = 0,
+   KEEP_TRUE,  /* pass -k flag to git-mailinfo */
+   KEEP_NON_PATCH  /* pass -b flag to git-mailinfo */
+};
+
 struct am_state {
/* state directory path */
char *dir;
@@ -94,6 +100,9 @@ struct am_state {
 
int utf8;
 
+   /* one of the enum keep_type values */
+   int keep;
+
/* override error message when patch failure occurs */
const char *resolvemsg;
 
@@ -397,6 +406,14 @@ static void am_load(struct am_state *state)
read_state_file(&sb, state, "utf8", 1);
state->utf8 = !strcmp(sb.buf, "t");
 
+   read_state_file(&sb, state, "keep", 1);
+   if (!strcmp(sb.buf, "t"))
+   state->keep = KEEP_TRUE;
+   else if (!strcmp(sb.buf, "b"))
+   state->keep = KEEP_NON_PATCH;
+   else
+   state->keep = KEEP_FALSE;
+
state->rebasing = !!file_exists(am_path(state, "rebasing"));
 
strbuf_release(&sb);
@@ -560,6 +577,7 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
const char **paths)
 {
unsigned char curr_head[GIT_SHA1_RAWSZ];
+   const char *str;
 
if (!patch_format)
patch_format = detect_patch_format(paths);
@@ -588,6 +606,22 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
 
write_file(am_path(state, "utf8"), 1, state->utf8 ? "t" : "f");
 
+   switch (state->keep) {
+   case KEEP_FALSE:
+   str = "f";
+   break;
+   case KEEP_TRUE:
+   str = "t";
+   break;
+   case KEEP_NON_PATCH:
+   str = "b";
+   break;
+   default:
+   die("BUG: invalid value for state->keep");
+   }
+
+   write_file(am_path(state, "keep"), 1, "%s", str);
+
if (state->rebasing)
write_file(am_path(state, "rebasing"), 1, "%s", "");
else
@@ -760,6 +794,20 @@ static int parse_mail(struct am_state *state, const char 
*mail)
 
argv_array_push(&cp.args, "mailinfo");
argv_array_push(&cp.args, state->utf8 ? "-u" : "-n");
+
+   switch (state->keep) {
+   case KEEP_FALSE:
+   break;
+   case KEEP_TRUE:
+   argv_array_push(&cp.args, "-k");
+   break;
+   case KEEP_NON_PATCH:
+   argv_array_push(&cp.args, "-b");
+   break;
+   default:
+   die("BUG: invalid value for state->keep");
+   }
+
argv_array_push(&cp.args, am_path(state, "msg"));
argv_array_push(&cp.args, am_path(state, "patch"));
 
@@ -1476,6 +1524,10 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
N_("add a Signed-off-by line to the commit message")),
OPT_BOOL('u', "utf8", &state.utf8,
N_("recode into utf8 (default)")),
+   OPT_SET_INT('k', "keep", &state.keep,
+   N_("pass -k flag to git-mailinfo"), KEEP_TRUE),
+   OPT_SET_INT(0, "keep-non-patch", &state.keep,
+   N_("pass -b flag to git-mailinfo"), KEEP_NON_PATCH),
OPT_CALLBACK(0, "patch-format", &patch_format, N_("format"),
N_("format the patch(es) are in"),
parse_opt_patchformat),
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 31/44] builtin-am: implement -S/--gpg-sign, commit.gpgsign

2015-07-07 Thread Paul Tan
Since 3b4e395 (am: add the --gpg-sign option, 2014-02-01), git-am.sh
supported the --gpg-sign option, and would pass it to git-commit-tree,
thus GPG-signing the commit object.

Re-implement this option in builtin/am.c.

git-commit-tree would also sign the commit by default if the
commit.gpgsign setting is true. Since we do not run commit-tree, we
re-implement this behavior by handling the commit.gpgsign setting
ourselves.

Helped-by: Stefan Beller 
Signed-off-by: Paul Tan 
---

Notes:
v5

* Renamed local "sign_commit" variable in am_state_init() to "gpgsign"
  to aid code review.

 builtin/am.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index d7bb159..81b3e31 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -124,6 +124,8 @@ struct am_state {
 
int ignore_date;
 
+   const char *sign_commit;
+
int rebasing;
 };
 
@@ -133,6 +135,8 @@ struct am_state {
  */
 static void am_state_init(struct am_state *state, const char *dir)
 {
+   int gpgsign;
+
memset(state, 0, sizeof(*state));
 
assert(dir);
@@ -149,6 +153,9 @@ static void am_state_init(struct am_state *state, const 
char *dir)
state->scissors = SCISSORS_UNSET;
 
argv_array_init(&state->git_apply_opts);
+
+   if (!git_config_get_bool("commit.gpgsign", &gpgsign))
+   state->sign_commit = gpgsign ? "" : NULL;
 }
 
 /**
@@ -1266,7 +1273,7 @@ static void do_commit(const struct am_state *state)
state->ignore_date ? "" : state->author_date, 1);
 
if (commit_tree(state->msg, state->msg_len, tree, parents, commit,
-   author, NULL))
+   author, state->sign_commit))
die(_("failed to write commit object"));
 
reflog_msg = getenv("GIT_REFLOG_ACTION");
@@ -1688,6 +1695,9 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
N_("lie about committer date")),
OPT_BOOL(0, "ignore-date", &state.ignore_date,
N_("use current timestamp for author date")),
+   { OPTION_STRING, 'S', "gpg-sign", &state.sign_commit, 
N_("key-id"),
+ N_("GPG-sign commits"),
+ PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing,
N_("(internal use for git-rebase)")),
OPT_END()
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 41/44] builtin-am: implement -i/--interactive

2015-07-07 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07),
git-am.sh supported the --interactive mode. After parsing the patch mail
and extracting the patch, commit message and authorship info, an
interactive session will begin that allows the user to choose between:

* applying the patch

* applying the patch and all subsequent patches (by disabling
  interactive mode in subsequent patches)

* skipping the patch

* editing the commit message

Since f89ad67 (Add [v]iew patch in git-am interactive., 2005-10-25),
git-am.sh --interactive also supported viewing the patch to be applied.

When --resolved-ing in --interactive mode, we need to take care to
update the patch with the contents of the index, such that the correct
patch will be displayed when the patch is viewed in interactive mode.

Re-implement the above in builtin/am.c

Signed-off-by: Paul Tan 
---

Notes:
Can't be tested because even with test_terminal isatty(0) still returns
false.

 builtin/am.c | 106 ++-
 1 file changed, 105 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index 55962c6..2866328 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -25,6 +25,7 @@
 #include "log-tree.h"
 #include "notes-utils.h"
 #include "rerere.h"
+#include "prompt.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -118,6 +119,8 @@ struct am_state {
/* number of digits in patch filename */
int prec;
 
+   int interactive;
+
int threeway;
 
int quiet;
@@ -1212,7 +1215,7 @@ static void NORETURN die_user_resolve(const struct 
am_state *state)
if (state->resolvemsg) {
printf_ln("%s", state->resolvemsg);
} else {
-   const char *cmdline = "git am";
+   const char *cmdline = state->interactive ? "git am -i" : "git 
am";
 
printf_ln(_("When you have resolved this problem, run \"%s 
--continue\"."), cmdline);
printf_ln(_("If you prefer to skip this patch, run \"%s 
--skip\" instead."), cmdline);
@@ -1445,6 +1448,36 @@ static void write_commit_patch(const struct am_state 
*state, struct commit *comm
 }
 
 /**
+ * Writes the diff of the index against HEAD as a patch to the state
+ * directory's "patch" file.
+ */
+static void write_index_patch(const struct am_state *state)
+{
+   struct tree *tree;
+   unsigned char head[GIT_SHA1_RAWSZ];
+   struct rev_info rev_info;
+   FILE *fp;
+
+   if (!get_sha1_tree("HEAD", head))
+   tree = lookup_tree(head);
+   else
+   tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
+
+   fp = xfopen(am_path(state, "patch"), "w");
+   init_revisions(&rev_info, NULL);
+   rev_info.diff = 1;
+   rev_info.disable_stdin = 1;
+   rev_info.no_commit_id = 1;
+   rev_info.diffopt.output_format = DIFF_FORMAT_PATCH;
+   rev_info.diffopt.use_color = 0;
+   rev_info.diffopt.file = fp;
+   rev_info.diffopt.close_file = 1;
+   add_pending_object(&rev_info, &tree->object, "");
+   diff_setup_done(&rev_info.diffopt);
+   run_diff_index(&rev_info, 1);
+}
+
+/**
  * Like parse_mail(), but parses the mail by looking up its commit ID
  * directly. This is used in --rebasing mode to bypass git-mailinfo's munging
  * of patches.
@@ -1681,6 +1714,65 @@ static void do_commit(const struct am_state *state)
 }
 
 /**
+ * Interactively prompt the user on whether the current patch should be
+ * applied.
+ *
+ * Returns 0 if the user chooses to apply the patch, 1 if the user chooses to
+ * skip it.
+ */
+static int do_interactive(struct am_state *state)
+{
+   assert(state->msg);
+
+   if (!isatty(0))
+   die(_("cannot be interactive without stdin connected to a 
terminal."));
+
+   for (;;) {
+   const char *reply;
+
+   puts(_("Commit Body is:"));
+   puts("--");
+   printf("%s", state->msg);
+   puts("--");
+
+   /*
+* TRANSLATORS: Make sure to include [y], [n], [e], [v] and [a]
+* in your translation. The program will only accept English
+* input at this point.
+*/
+   reply = git_prompt(_("Apply? [y]es/[n]o/[e]dit/[v]iew 
patch/[a]ccept all: "), PROMPT_ECHO);
+
+   if (!reply) {
+   continue;
+   } else if (*reply == 'y' || *reply == 'Y') {
+   return 0;
+   } else if (*reply == 'a' || *reply == 'A') {
+   state->interactive = 0;
+   return 0;
+   } else if (*reply == 'n' || *reply == 'N') {
+   return 1;
+   } else if (*reply == 'e' || *reply == 'E') {
+   struct strbuf msg = STRBUF_INIT;
+
+   if (!launch_editor(am_path(state, "final-

[PATCH v5 14/44] builtin-am: reject patches when there's a session in progress

2015-07-07 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am
would error out if the user gave it mbox(s) on the command-line, but
there was a session in progress.

Since c95b138 (Fix git-am safety checks, 2006-09-15), git-am would
detect if the user attempted to feed it a mbox via stdin, by checking if
stdin is not a tty and there is no resume command given.

Re-implement the above two safety checks.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 1db95f2..e066a97 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1147,9 +1147,24 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
if (read_index_preload(&the_index, NULL) < 0)
die(_("failed to read the index"));
 
-   if (am_in_progress(&state))
+   if (am_in_progress(&state)) {
+   /*
+* Catch user error to feed us patches when there is a session
+* in progress:
+*
+* 1. mbox path(s) are provided on the command-line.
+* 2. stdin is not a tty: the user is trying to feed us a patch
+*from standard input. This is somewhat unreliable -- stdin
+*could be /dev/null for example and the caller did not
+*intend to feed us a patch but wanted to continue
+*unattended.
+*/
+   if (argc || (resume == RESUME_FALSE && !isatty(0)))
+   die(_("previous rebase directory %s still exists but 
mbox given."),
+   state.dir);
+
am_load(&state);
-   else {
+   } else {
struct argv_array paths = ARGV_ARRAY_INIT;
int i;
 
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 40/44] builtin-am: support and auto-detect mercurial patches

2015-07-07 Thread Paul Tan
Since 0cfd112 (am: preliminary support for hg patches, 2011-08-29),
git-am.sh could convert mercurial patches to an RFC2822 mail patch
suitable for parsing with git-mailinfo, and queue them in the state
directory for application.

Since 15ced75 (git-am foreign patch support: autodetect some patch
formats, 2009-05-27), git-am.sh was able to auto-detect mercurial
patches by checking if the file begins with the line:

# HG changeset patch

Re-implement the above in builtin/am.c.

Helped-by: Stefan Beller 
Signed-off-by: Paul Tan 
---

Notes:
v5

* v4 had a math fail in the timestamp conversion. Fixed the math.

* In C89, it is implementation defined whether integer division rounds
  towards 0 or towards negative infinity. To be safe, we do the
  timestamp conversion with positive integers only, and then negate the
  result appropriately.

 builtin/am.c | 74 +++-
 1 file changed, 73 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index 7a56005..55962c6 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -81,7 +81,8 @@ enum patch_format {
PATCH_FORMAT_UNKNOWN = 0,
PATCH_FORMAT_MBOX,
PATCH_FORMAT_STGIT,
-   PATCH_FORMAT_STGIT_SERIES
+   PATCH_FORMAT_STGIT_SERIES,
+   PATCH_FORMAT_HG
 };
 
 enum keep_type {
@@ -692,6 +693,11 @@ static int detect_patch_format(const char **paths)
goto done;
}
 
+   if (!strcmp(l1.buf, "# HG changeset patch")) {
+   ret = PATCH_FORMAT_HG;
+   goto done;
+   }
+
strbuf_reset(&l2);
strbuf_getline_crlf(&l2, fp);
strbuf_reset(&l3);
@@ -890,6 +896,68 @@ static int split_mail_stgit_series(struct am_state *state, 
const char **paths,
 }
 
 /**
+ * A split_patches_conv() callback that converts a mercurial patch to a RFC2822
+ * message suitable for parsing with git-mailinfo.
+ */
+static int hg_patch_to_mail(FILE *out, FILE *in, int keep_cr)
+{
+   struct strbuf sb = STRBUF_INIT;
+
+   while (!strbuf_getline(&sb, in, '\n')) {
+   const char *str;
+
+   if (skip_prefix(sb.buf, "# User ", &str))
+   fprintf(out, "From: %s\n", str);
+   else if (skip_prefix(sb.buf, "# Date ", &str)) {
+   unsigned long timestamp;
+   long tz, tz2;
+   char *end;
+
+   errno = 0;
+   timestamp = strtoul(str, &end, 10);
+   if (errno)
+   return error(_("invalid timestamp"));
+
+   if (!skip_prefix(end, " ", &str))
+   return error(_("invalid Date line"));
+
+   errno = 0;
+   tz = strtol(str, &end, 10);
+   if (errno)
+   return error(_("invalid timezone offset"));
+
+   if (*end)
+   return error(_("invalid Date line"));
+
+   /*
+* mercurial's timezone is in seconds west of UTC,
+* however git's timezone is in hours + minutes east of
+* UTC. Convert it.
+*/
+   tz2 = labs(tz) / 3600 * 100 + labs(tz) % 3600 / 60;
+   if (tz > 0)
+   tz2 = -tz2;
+
+   fprintf(out, "Date: %s\n", show_date(timestamp, tz2, 
DATE_RFC2822));
+   } else if (starts_with(sb.buf, "# ")) {
+   continue;
+   } else {
+   fprintf(out, "\n%s\n", sb.buf);
+   break;
+   }
+   }
+
+   strbuf_reset(&sb);
+   while (strbuf_fread(&sb, 8192, in) > 0) {
+   fwrite(sb.buf, 1, sb.len, out);
+   strbuf_reset(&sb);
+   }
+
+   strbuf_release(&sb);
+   return 0;
+}
+
+/**
  * Splits a list of files/directories into individual email patches. Each path
  * in `paths` must be a file/directory that is formatted according to
  * `patch_format`.
@@ -921,6 +989,8 @@ static int split_mail(struct am_state *state, enum 
patch_format patch_format,
return split_mail_conv(stgit_patch_to_mail, state, paths, 
keep_cr);
case PATCH_FORMAT_STGIT_SERIES:
return split_mail_stgit_series(state, paths, keep_cr);
+   case PATCH_FORMAT_HG:
+   return split_mail_conv(hg_patch_to_mail, state, paths, keep_cr);
default:
die("BUG: invalid patch_format");
}
@@ -1955,6 +2025,8 @@ static int parse_opt_patchformat(const struct option 
*opt, const char *arg, int
*opt_value = PATCH_FORMAT_STGIT;
else if (!strcmp(arg, "stgit-series"))
*opt_value = PATCH_FORMAT_STGIT_SERIES;
+   else if (!strcmp(arg, "hg

[PATCH v5 26/44] builtin-am: support --keep-cr, am.keepcr

2015-07-07 Thread Paul Tan
Since ad2c928 (git-am: Add command line parameter `--keep-cr` passing it
to git-mailsplit, 2010-02-27), git-am.sh supported the --keep-cr option
and would pass it to git-mailsplit.

Since e80d4cb (git-am: Add am.keepcr and --no-keep-cr to override it,
2010-02-27), git-am.sh supported the am.keepcr config setting, which
controls whether --keep-cr is on by default.

Re-implement the above in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 099c6ed..59a7a2a 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -530,7 +530,7 @@ done:
  * Splits out individual email patches from `paths`, where each path is either
  * a mbox file or a Maildir. Returns 0 on success, -1 on failure.
  */
-static int split_mail_mbox(struct am_state *state, const char **paths)
+static int split_mail_mbox(struct am_state *state, const char **paths, int 
keep_cr)
 {
struct child_process cp = CHILD_PROCESS_INIT;
struct strbuf last = STRBUF_INIT;
@@ -540,6 +540,8 @@ static int split_mail_mbox(struct am_state *state, const 
char **paths)
argv_array_pushf(&cp.args, "-d%d", state->prec);
argv_array_pushf(&cp.args, "-o%s", state->dir);
argv_array_push(&cp.args, "-b");
+   if (keep_cr)
+   argv_array_push(&cp.args, "--keep-cr");
argv_array_push(&cp.args, "--");
argv_array_pushv(&cp.args, paths);
 
@@ -564,14 +566,22 @@ static int split_mail_mbox(struct am_state *state, const 
char **paths)
  * state->cur will be set to the index of the first mail, and state->last will
  * be set to the index of the last mail.
  *
+ * Set keep_cr to 0 to convert all lines ending with \r\n to end with \n, 1
+ * to disable this behavior, -1 to use the default configured setting.
+ *
  * Returns 0 on success, -1 on failure.
  */
 static int split_mail(struct am_state *state, enum patch_format patch_format,
-   const char **paths)
+   const char **paths, int keep_cr)
 {
+   if (keep_cr < 0) {
+   keep_cr = 0;
+   git_config_get_bool("am.keepcr", &keep_cr);
+   }
+
switch (patch_format) {
case PATCH_FORMAT_MBOX:
-   return split_mail_mbox(state, paths);
+   return split_mail_mbox(state, paths, keep_cr);
default:
die("BUG: invalid patch_format");
}
@@ -582,7 +592,7 @@ static int split_mail(struct am_state *state, enum 
patch_format patch_format,
  * Setup a new am session for applying patches
  */
 static void am_setup(struct am_state *state, enum patch_format patch_format,
-   const char **paths)
+   const char **paths, int keep_cr)
 {
unsigned char curr_head[GIT_SHA1_RAWSZ];
const char *str;
@@ -598,7 +608,7 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
if (mkdir(state->dir, 0777) < 0 && errno != EEXIST)
die_errno(_("failed to create directory '%s'"), state->dir);
 
-   if (split_mail(state, patch_format, paths) < 0) {
+   if (split_mail(state, patch_format, paths, keep_cr) < 0) {
am_destroy(state);
die(_("Failed to split patches."));
}
@@ -1520,6 +1530,7 @@ enum resume_mode {
 int cmd_am(int argc, const char **argv, const char *prefix)
 {
struct am_state state;
+   int keep_cr = -1;
int patch_format = PATCH_FORMAT_UNKNOWN;
enum resume_mode resume = RESUME_FALSE;
 
@@ -1543,6 +1554,12 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
N_("pass -b flag to git-mailinfo"), KEEP_NON_PATCH),
OPT_BOOL('m', "message-id", &state.message_id,
N_("pass -m flag to git-mailinfo")),
+   { OPTION_SET_INT, 0, "keep-cr", &keep_cr, NULL,
+ N_("pass --keep-cr flag to git-mailsplit for mbox format"),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
+   { OPTION_SET_INT, 0, "no-keep-cr", &keep_cr, NULL,
+ N_("do not pass --keep-cr flag to git-mailsplit independent 
of am.keepcr"),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
OPT_CALLBACK(0, "patch-format", &patch_format, N_("format"),
N_("format the patch(es) are in"),
parse_opt_patchformat),
@@ -1637,7 +1654,7 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
argv_array_push(&paths, mkpath("%s/%s", prefix, 
argv[i]));
}
 
-   am_setup(&state, patch_format, paths.argv);
+   am_setup(&state, patch_format, paths.argv, keep_cr);
 
argv_array_clear(&paths);
}
-- 
2.5.0.rc1.76.gf60a929

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a messag

[PATCH v5 20/44] builtin-am: implement --rebasing mode

2015-07-07 Thread Paul Tan
Since 3041c32 (am: --rebasing, 2008-03-04), git-am.sh supported the
--rebasing option, which is used internally by git-rebase to tell git-am
that it is being used for its purpose. It would create the empty file
$state_dir/rebasing to help "completion" scripts tell if the ongoing
operation is am or rebase.

As of 0fbb95d (am: don't call mailinfo if $rebasing, 2012-06-26),
--rebasing also implies --3way as well.

Since a1549e1 (am: return control to caller, for housekeeping,
2013-05-12), git-am.sh would only clean up the state directory when it
is not --rebasing, instead deferring cleanup to git-rebase.sh.

Re-implement the above in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 31 +++
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index af22c35..b68dcc5 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -92,6 +92,8 @@ struct am_state {
 
/* override error message when patch failure occurs */
const char *resolvemsg;
+
+   int rebasing;
 };
 
 /**
@@ -386,6 +388,8 @@ static void am_load(struct am_state *state)
read_state_file(&sb, state, "sign", 1);
state->append_signoff = !strcmp(sb.buf, "t");
 
+   state->rebasing = !!file_exists(am_path(state, "rebasing"));
+
strbuf_release(&sb);
 }
 
@@ -564,18 +568,29 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
die(_("Failed to split patches."));
}
 
+   if (state->rebasing)
+   state->threeway = 1;
+
write_file(am_path(state, "threeway"), 1, state->threeway ? "t" : "f");
 
write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f");
 
write_file(am_path(state, "sign"), 1, state->append_signoff ? "t" : 
"f");
 
+   if (state->rebasing)
+   write_file(am_path(state, "rebasing"), 1, "%s", "");
+   else
+   write_file(am_path(state, "applying"), 1, "%s", "");
+
if (!get_sha1("HEAD", curr_head)) {
write_file(am_path(state, "abort-safety"), 1, "%s", 
sha1_to_hex(curr_head));
-   update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, 
UPDATE_REFS_DIE_ON_ERR);
+   if (!state->rebasing)
+   update_ref("am", "ORIG_HEAD", curr_head, NULL, 0,
+   UPDATE_REFS_DIE_ON_ERR);
} else {
write_file(am_path(state, "abort-safety"), 1, "%s", "");
-   delete_ref("ORIG_HEAD", NULL, 0);
+   if (!state->rebasing)
+   delete_ref("ORIG_HEAD", NULL, 0);
}
 
/*
@@ -1057,8 +1072,14 @@ next:
am_next(state);
}
 
-   am_destroy(state);
-   run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+   /*
+* In rebasing mode, it's up to the caller to take care of
+* housekeeping.
+*/
+   if (!state->rebasing) {
+   am_destroy(state);
+   run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+   }
 }
 
 /**
@@ -1330,6 +1351,8 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
OPT_CMDMODE(0, "abort", &resume,
N_("restore the original branch and abort the patching 
operation."),
RESUME_ABORT),
+   OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing,
+   N_("(internal use for git-rebase)")),
OPT_END()
};
 
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 38/44] builtin-am: support and auto-detect StGit patches

2015-07-07 Thread Paul Tan
Since c574e68 (git-am foreign patch support: StGIT support, 2009-05-27),
git-am.sh supported converting StGit patches into RFC2822 mail patches
that can be parsed with git-mailinfo.

Implement this by introducing two functions in builtin/am.c:
stgit_patch_to_mail() and split_mail_conv().

stgit_patch_to_mail() is a callback function for split_mail_conv(), and
contains the logic for converting an StGit patch into an RFC2822 mail
patch.

split_mail_conv() implements the logic to go through each file in the
`paths` list, reading from stdin where specified, and calls the callback
function to write the converted patch to the corresponding output file
in the state directory. This interface should be generic enough to
support other foreign patch formats in the future.

Since 15ced75 (git-am foreign patch support: autodetect some patch
formats, 2009-05-27), git-am.sh is able to auto-detect StGit patches.
Re-implement this in builtin/am.c.

Helped-by: Eric Sunshine 
Signed-off-by: Paul Tan 
---

Notes:
v5

* Rewrite of the loop in str_isspace() to be clearer.

 builtin/am.c | 132 ++-
 1 file changed, 131 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index 6b388de..0bb1875 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -65,9 +65,22 @@ static int linelen(const char *msg)
return strchrnul(msg, '\n') - msg;
 }
 
+/**
+ * Returns true if `str` consists of only whitespace, false otherwise.
+ */
+static int str_isspace(const char *str)
+{
+   for (; *str; str++)
+   if (!isspace(*str))
+   return 0;
+
+   return 1;
+}
+
 enum patch_format {
PATCH_FORMAT_UNKNOWN = 0,
-   PATCH_FORMAT_MBOX
+   PATCH_FORMAT_MBOX,
+   PATCH_FORMAT_STGIT
 };
 
 enum keep_type {
@@ -646,6 +659,8 @@ static int detect_patch_format(const char **paths)
 {
enum patch_format ret = PATCH_FORMAT_UNKNOWN;
struct strbuf l1 = STRBUF_INIT;
+   struct strbuf l2 = STRBUF_INIT;
+   struct strbuf l3 = STRBUF_INIT;
FILE *fp;
 
/*
@@ -671,6 +686,23 @@ static int detect_patch_format(const char **paths)
goto done;
}
 
+   strbuf_reset(&l2);
+   strbuf_getline_crlf(&l2, fp);
+   strbuf_reset(&l3);
+   strbuf_getline_crlf(&l3, fp);
+
+   /*
+* If the second line is empty and the third is a From, Author or Date
+* entry, this is likely an StGit patch.
+*/
+   if (l1.len && !l2.len &&
+   (starts_with(l3.buf, "From:") ||
+starts_with(l3.buf, "Author:") ||
+starts_with(l3.buf, "Date:"))) {
+   ret = PATCH_FORMAT_STGIT;
+   goto done;
+   }
+
if (l1.len && is_mail(fp)) {
ret = PATCH_FORMAT_MBOX;
goto done;
@@ -711,6 +743,100 @@ static int split_mail_mbox(struct am_state *state, const 
char **paths, int keep_
 }
 
 /**
+ * Callback signature for split_mail_conv(). The foreign patch should be
+ * read from `in`, and the converted patch (in RFC2822 mail format) should be
+ * written to `out`. Return 0 on success, or -1 on failure.
+ */
+typedef int (*mail_conv_fn)(FILE *out, FILE *in, int keep_cr);
+
+/**
+ * Calls `fn` for each file in `paths` to convert the foreign patch to the
+ * RFC2822 mail format suitable for parsing with git-mailinfo.
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+static int split_mail_conv(mail_conv_fn fn, struct am_state *state,
+   const char **paths, int keep_cr)
+{
+   static const char *stdin_only[] = {"-", NULL};
+   int i;
+
+   if (!*paths)
+   paths = stdin_only;
+
+   for (i = 0; *paths; paths++, i++) {
+   FILE *in, *out;
+   const char *mail;
+   int ret;
+
+   if (!strcmp(*paths, "-"))
+   in = stdin;
+   else
+   in = fopen(*paths, "r");
+
+   if (!in)
+   return error(_("could not open '%s' for reading: %s"),
+   *paths, strerror(errno));
+
+   mail = mkpath("%s/%0*d", state->dir, state->prec, i + 1);
+
+   out = fopen(mail, "w");
+   if (!out)
+   return error(_("could not open '%s' for writing: %s"),
+   mail, strerror(errno));
+
+   ret = fn(out, in, keep_cr);
+
+   fclose(out);
+   fclose(in);
+
+   if (ret)
+   return error(_("could not parse patch '%s'"), *paths);
+   }
+
+   state->cur = 1;
+   state->last = i;
+   return 0;
+}
+
+/**
+ * A split_mail_conv() callback that converts an StGit patch to an RFC2822
+ * message suitable for parsing with git-mailinfo.
+ */
+static int stgit_patch_to_mail(FILE *out, FILE *in, int keep_cr)
+{
+   struct strbuf sb =

[PATCH v5 16/44] builtin-am: exit with user friendly message on failure

2015-07-07 Thread Paul Tan
Since ced9456 (Give the user a hint for how to continue in the case that
git-am fails because it requires user intervention, 2006-05-02), git-am
prints additional information on how the user can re-invoke git-am to
resume patch application after resolving the failure. Re-implement this
through the die_user_resolve() function.

Since cc12005 (Make git rebase interactive help match documentation.,
2006-05-13), git-am supports the --resolvemsg option which is used by
git-rebase to override the message printed out when git-am fails.
Re-implement this option.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 32 
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 61c2ad3..7eb23b9 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -82,6 +82,9 @@ struct am_state {
int prec;
 
int quiet;
+
+   /* override error message when patch failure occurs */
+   const char *resolvemsg;
 };
 
 /**
@@ -668,6 +671,25 @@ static int index_has_changes(struct strbuf *sb)
 }
 
 /**
+ * Dies with a user-friendly message on how to proceed after resolving the
+ * problem. This message can be overridden with state->resolvemsg.
+ */
+static void NORETURN die_user_resolve(const struct am_state *state)
+{
+   if (state->resolvemsg) {
+   printf_ln("%s", state->resolvemsg);
+   } else {
+   const char *cmdline = "git am";
+
+   printf_ln(_("When you have resolved this problem, run \"%s 
--continue\"."), cmdline);
+   printf_ln(_("If you prefer to skip this patch, run \"%s 
--skip\" instead."), cmdline);
+   printf_ln(_("To restore the original branch and stop patching, 
run \"%s --abort\"."), cmdline);
+   }
+
+   exit(128);
+}
+
+/**
  * Parses `mail` using git-mailinfo, extracting its patch and authorship info.
  * state->msg will be set to the patch message. state->author_name,
  * state->author_email and state->author_date will be set to the patch author's
@@ -727,7 +749,7 @@ static int parse_mail(struct am_state *state, const char 
*mail)
 
if (is_empty_file(am_path(state, "patch"))) {
printf_ln(_("Patch is empty. Was it split wrong?"));
-   exit(128);
+   die_user_resolve(state);
}
 
strbuf_addstr(&msg, "\n\n");
@@ -868,7 +890,7 @@ static void am_run(struct am_state *state)
printf_ln(_("The copy of the patch that failed 
is found in: %s"),
am_path(state, "patch"));
 
-   exit(128);
+   die_user_resolve(state);
}
 
do_commit(state);
@@ -902,13 +924,13 @@ static void am_resolve(struct am_state *state)
printf_ln(_("No changes - did you forget to use 'git add'?\n"
"If there is nothing left to stage, chances are that 
something else\n"
"already introduced the same changes; you might want to 
skip this patch."));
-   exit(128);
+   die_user_resolve(state);
}
 
if (unmerged_cache()) {
printf_ln(_("You still have unmerged paths in your index.\n"
"Did you forget to use 'git add'?"));
-   exit(128);
+   die_user_resolve(state);
}
 
do_commit(state);
@@ -1132,6 +1154,8 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
OPT_CALLBACK(0, "patch-format", &patch_format, N_("format"),
N_("format the patch(es) are in"),
parse_opt_patchformat),
+   OPT_STRING(0, "resolvemsg", &state.resolvemsg, NULL,
+   N_("override error message when patch failure occurs")),
OPT_CMDMODE(0, "continue", &resume,
N_("continue applying patches after resolving a 
conflict"),
RESUME_RESOLVED),
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 43/44] builtin-am: check for valid committer ident

2015-07-07 Thread Paul Tan
When commit_tree() is called, if the user does not have an explicit
committer ident configured, it will attempt to construct a default
committer ident based on the user's and system's info (e.g. gecos field,
hostname etc.) However, if a default committer ident is unable to be
constructed, commit_tree() will die(), but at this point of git-am's
execution, there will already be changes made to the index and work
tree.

This can be confusing to new users, and as such since d64e6b0 (Keep
Porcelainish from failing by broken ident after making changes.,
2006-02-18) git-am.sh will check to see if the committer ident has been
configured, or a default one can be constructed, before even starting to
apply patches.

Re-implement this in builtin/am.c.

Signed-off-by: Paul Tan 
---

Notes:
v5

* modified commit message

 builtin/am.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index ad07ba4..a0982d9 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2264,6 +2264,9 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
fprintf_ln(stderr, _("The -b/--binary option has been a no-op 
for long time, and\n"
"it will be removed. Please do not use it 
anymore."));
 
+   /* Ensure a valid committer ident can be constructed */
+   git_committer_info(IDENT_STRICT);
+
if (read_index_preload(&the_index, NULL) < 0)
die(_("failed to read the index"));
 
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 37/44] builtin-am: rerere support

2015-07-07 Thread Paul Tan
git-am.sh will call git-rerere at the following events:

* "git rerere" when a three-way merge fails to record the conflicted
  automerge results. Since 8389b52 (git-rerere: reuse recorded resolve.,
  2006-01-28)

  * Since cb6020b (Teach --[no-]rerere-autoupdate option to merge,
revert and friends, 2009-12-04), git-am.sh supports the
--[no-]rerere-autoupdate option as well, and would pass it to
git-rerere.

* "git rerere" when --resolved, to record the hand resolution. Since
  f131dd4 (rerere: record (or avoid misrecording) resolved, skipped or
  aborted rebase/am, 2006-12-08)

* "git rerere clear" when --skip-ing. Since f131dd4 (rerere: record (or
  avoid misrecording) resolved, skipped or aborted rebase/am,
  2006-12-08)

* "git rerere clear" when --abort-ing. Since 3e5057a (git am --abort,
  2008-07-16)

Re-implement the above in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index fdaf0a7..6b388de 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -24,6 +24,7 @@
 #include "revision.h"
 #include "log-tree.h"
 #include "notes-utils.h"
+#include "rerere.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -128,6 +129,8 @@ struct am_state {
 
int ignore_date;
 
+   int allow_rerere_autoupdate;
+
const char *sign_commit;
 
int rebasing;
@@ -1352,6 +1355,7 @@ static int fall_back_threeway(const struct am_state 
*state, const char *index_pa
o.verbosity = 0;
 
if (merge_recursive_generic(&o, our_tree, his_tree, 1, bases, &result)) 
{
+   rerere(state->allow_rerere_autoupdate);
free(his_tree_name);
return error(_("Failed to merge in the changes."));
}
@@ -1551,6 +1555,8 @@ static void am_resolve(struct am_state *state)
die_user_resolve(state);
}
 
+   rerere(0);
+
do_commit(state);
 
am_next(state);
@@ -1649,12 +1655,29 @@ static int clean_index(const unsigned char *head, const 
unsigned char *remote)
 }
 
 /**
+ * Resets rerere's merge resolution metadata.
+ */
+static void am_rerere_clear(void)
+{
+   struct string_list merge_rr = STRING_LIST_INIT_DUP;
+   int fd = setup_rerere(&merge_rr, 0);
+
+   if (fd < 0)
+   return;
+
+   rerere_clear(&merge_rr);
+   string_list_clear(&merge_rr, 1);
+}
+
+/**
  * Resume the current am session by skipping the current patch.
  */
 static void am_skip(struct am_state *state)
 {
unsigned char head[GIT_SHA1_RAWSZ];
 
+   am_rerere_clear();
+
if (get_sha1("HEAD", head))
hashcpy(head, EMPTY_TREE_SHA1_BIN);
 
@@ -1712,6 +1735,8 @@ static void am_abort(struct am_state *state)
return;
}
 
+   am_rerere_clear();
+
curr_branch = resolve_refdup("HEAD", 0, curr_head, NULL);
has_curr_head = !is_null_sha1(curr_head);
if (!has_curr_head)
@@ -1839,6 +1864,7 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
N_("lie about committer date")),
OPT_BOOL(0, "ignore-date", &state.ignore_date,
N_("use current timestamp for author date")),
+   OPT_RERERE_AUTOUPDATE(&state.allow_rerere_autoupdate),
{ OPTION_STRING, 'S', "gpg-sign", &state.sign_commit, 
N_("key-id"),
  N_("GPG-sign commits"),
  PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 32/44] builtin-am: invoke post-rewrite hook

2015-07-07 Thread Paul Tan
Since 96e1948 (rebase: invoke post-rewrite hook, 2010-03-12), git-am.sh
will invoke the post-rewrite hook after it successfully finishes
applying all the queued patches.

To do this, when parsing a mail to extract its patch and metadata, in
--rebasing mode git-am.sh will also store the original commit ID in the
$state_dir/original-commit file. Once it applies and commits the patch,
the original commit ID, and the new commit ID, will be appended to the
$state_dir/rewritten file.

Once all of the queued mail have been processed, git-am.sh will then
invoke the post-rewrite hook with the contents of the
$state_dir/rewritten file.

Re-implement this in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 55 +++
 1 file changed, 55 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 81b3e31..c234ee9 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -95,6 +95,9 @@ struct am_state {
char *msg;
size_t msg_len;
 
+   /* when --rebasing, records the original commit the patch came from */
+   unsigned char orig_commit[GIT_SHA1_RAWSZ];
+
/* number of digits in patch filename */
int prec;
 
@@ -427,6 +430,11 @@ static void am_load(struct am_state *state)
 
read_commit_msg(state);
 
+   if (read_state_file(&sb, state, "original-commit", 1) < 0)
+   hashclr(state->orig_commit);
+   else if (get_sha1_hex(sb.buf, state->orig_commit) < 0)
+   die(_("could not parse %s"), am_path(state, "original-commit"));
+
read_state_file(&sb, state, "threeway", 1);
state->threeway = !strcmp(sb.buf, "t");
 
@@ -482,6 +490,30 @@ static void am_destroy(const struct am_state *state)
 }
 
 /**
+ * Runs post-rewrite hook. Returns it exit code.
+ */
+static int run_post_rewrite_hook(const struct am_state *state)
+{
+   struct child_process cp = CHILD_PROCESS_INIT;
+   const char *hook = find_hook("post-rewrite");
+   int ret;
+
+   if (!hook)
+   return 0;
+
+   argv_array_push(&cp.args, hook);
+   argv_array_push(&cp.args, "rebase");
+
+   cp.in = xopen(am_path(state, "rewritten"), O_RDONLY);
+   cp.stdout_to_stderr = 1;
+
+   ret = run_command(&cp);
+
+   close(cp.in);
+   return ret;
+}
+
+/**
  * Determines if the file looks like a piece of RFC2822 mail by grabbing all
  * non-indented lines and checking if they look like they begin with valid
  * header field names.
@@ -759,6 +791,9 @@ static void am_next(struct am_state *state)
unlink(am_path(state, "author-script"));
unlink(am_path(state, "final-commit"));
 
+   hashclr(state->orig_commit);
+   unlink(am_path(state, "original-commit"));
+
if (!get_sha1("HEAD", head))
write_file(am_path(state, "abort-safety"), 1, "%s", 
sha1_to_hex(head));
else
@@ -1078,6 +1113,8 @@ static void write_commit_patch(const struct am_state 
*state, struct commit *comm
  * directly. This is used in --rebasing mode to bypass git-mailinfo's munging
  * of patches.
  *
+ * state->orig_commit will be set to the original commit ID.
+ *
  * Will always return 0 as the patch should never be skipped.
  */
 static int parse_mail_rebase(struct am_state *state, const char *mail)
@@ -1094,6 +1131,10 @@ static int parse_mail_rebase(struct am_state *state, 
const char *mail)
 
write_commit_patch(state, commit);
 
+   hashcpy(state->orig_commit, commit_sha1);
+   write_file(am_path(state, "original-commit"), 1, "%s",
+   sha1_to_hex(commit_sha1));
+
return 0;
 }
 
@@ -1285,6 +1326,15 @@ static void do_commit(const struct am_state *state)
 
update_ref(sb.buf, "HEAD", commit, ptr, 0, UPDATE_REFS_DIE_ON_ERR);
 
+   if (state->rebasing) {
+   FILE *fp = xfopen(am_path(state, "rewritten"), "a");
+
+   assert(!is_null_sha1(state->orig_commit));
+   fprintf(fp, "%s ", sha1_to_hex(state->orig_commit));
+   fprintf(fp, "%s\n", sha1_to_hex(commit));
+   fclose(fp);
+   }
+
strbuf_release(&sb);
 }
 
@@ -1367,6 +1417,11 @@ next:
am_next(state);
}
 
+   if (!is_empty_file(am_path(state, "rewritten"))) {
+   assert(state->rebasing);
+   run_post_rewrite_hook(state);
+   }
+
/*
 * In rebasing mode, it's up to the caller to take care of
 * housekeeping.
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 21/44] builtin-am: bypass git-mailinfo when --rebasing

2015-07-07 Thread Paul Tan
Since 5e835ca (rebase: do not munge commit log message, 2008-04-16),
git am --rebasing no longer gets the commit log message from the patch,
but reads it directly from the commit identified by the "From " header
line.

Since 43c2325 (am: use get_author_ident_from_commit instead of mailinfo
when rebasing, 2010-06-16), git am --rebasing also gets the author name,
email and date directly from the commit.

Since 0fbb95d (am: don't call mailinfo if $rebasing, 2012-06-26), git am
--rebasing does not use git-mailinfo to get the patch body, but rather
generates it directly from the commit itself.

The above 3 commits introduced a separate parse_mail() code path in
git-am.sh's --rebasing mode that bypasses git-mailinfo. Re-implement
this code path in builtin/am.c as parse_mail_rebase().

Signed-off-by: Paul Tan 
---
 builtin/am.c | 134 ++-
 1 file changed, 132 insertions(+), 2 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index b68dcc5..06ded5d 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -21,6 +21,8 @@
 #include "sequencer.h"
 #include "revision.h"
 #include "merge-recursive.h"
+#include "revision.h"
+#include "log-tree.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -816,6 +818,129 @@ finish:
 }
 
 /**
+ * Sets commit_id to the commit hash where the mail was generated from.
+ * Returns 0 on success, -1 on failure.
+ */
+static int get_mail_commit_sha1(unsigned char *commit_id, const char *mail)
+{
+   struct strbuf sb = STRBUF_INIT;
+   FILE *fp = xfopen(mail, "r");
+   const char *x;
+
+   if (strbuf_getline(&sb, fp, '\n'))
+   return -1;
+
+   if (!skip_prefix(sb.buf, "From ", &x))
+   return -1;
+
+   if (get_sha1_hex(x, commit_id) < 0)
+   return -1;
+
+   strbuf_release(&sb);
+   fclose(fp);
+   return 0;
+}
+
+/**
+ * Sets state->msg, state->author_name, state->author_email, state->author_date
+ * to the commit's respective info.
+ */
+static void get_commit_info(struct am_state *state, struct commit *commit)
+{
+   const char *buffer, *ident_line, *author_date, *msg;
+   size_t ident_len;
+   struct ident_split ident_split;
+   struct strbuf sb = STRBUF_INIT;
+
+   buffer = logmsg_reencode(commit, NULL, get_commit_output_encoding());
+
+   ident_line = find_commit_header(buffer, "author", &ident_len);
+
+   if (split_ident_line(&ident_split, ident_line, ident_len) < 0) {
+   strbuf_add(&sb, ident_line, ident_len);
+   die(_("invalid ident line: %s"), sb.buf);
+   }
+
+   assert(!state->author_name);
+   if (ident_split.name_begin) {
+   strbuf_add(&sb, ident_split.name_begin,
+   ident_split.name_end - ident_split.name_begin);
+   state->author_name = strbuf_detach(&sb, NULL);
+   } else
+   state->author_name = xstrdup("");
+
+   assert(!state->author_email);
+   if (ident_split.mail_begin) {
+   strbuf_add(&sb, ident_split.mail_begin,
+   ident_split.mail_end - ident_split.mail_begin);
+   state->author_email = strbuf_detach(&sb, NULL);
+   } else
+   state->author_email = xstrdup("");
+
+   author_date = show_ident_date(&ident_split, DATE_NORMAL);
+   strbuf_addstr(&sb, author_date);
+   assert(!state->author_date);
+   state->author_date = strbuf_detach(&sb, NULL);
+
+   assert(!state->msg);
+   msg = strstr(buffer, "\n\n");
+   if (!msg)
+   die(_("unable to parse commit %s"), 
sha1_to_hex(commit->object.sha1));
+   state->msg = xstrdup(msg + 2);
+   state->msg_len = strlen(state->msg);
+}
+
+/**
+ * Writes `commit` as a patch to the state directory's "patch" file.
+ */
+static void write_commit_patch(const struct am_state *state, struct commit 
*commit)
+{
+   struct rev_info rev_info;
+   FILE *fp;
+
+   fp = xfopen(am_path(state, "patch"), "w");
+   init_revisions(&rev_info, NULL);
+   rev_info.diff = 1;
+   rev_info.abbrev = 0;
+   rev_info.disable_stdin = 1;
+   rev_info.show_root_diff = 1;
+   rev_info.diffopt.output_format = DIFF_FORMAT_PATCH;
+   rev_info.no_commit_id = 1;
+   DIFF_OPT_SET(&rev_info.diffopt, BINARY);
+   DIFF_OPT_SET(&rev_info.diffopt, FULL_INDEX);
+   rev_info.diffopt.use_color = 0;
+   rev_info.diffopt.file = fp;
+   rev_info.diffopt.close_file = 1;
+   add_pending_object(&rev_info, &commit->object, "");
+   diff_setup_done(&rev_info.diffopt);
+   log_tree_commit(&rev_info, commit);
+}
+
+/**
+ * Like parse_mail(), but parses the mail by looking up its commit ID
+ * directly. This is used in --rebasing mode to bypass git-mailinfo's munging
+ * of patches.
+ *
+ * Will always return 0 as the patch should never be skipped.
+ */
+static int parse_mail_rebase(struct am_state *state, const char *mail)
+{

[PATCH v5 39/44] builtin-am: support and auto-detect StGit series files

2015-07-07 Thread Paul Tan
Since c574e68 (git-am foreign patch support: StGIT support, 2009-05-27),
git-am.sh is able to read a single StGit series file and, for each StGit
patch listed in the file, convert the StGit patch into a RFC2822 mail
patch suitable for parsing with git-mailinfo, and queue them in the
state directory for applying.

Since 15ced75 (git-am foreign patch support: autodetect some patch
formats, 2009-05-27), git-am.sh is able to auto-detect StGit series
files by checking to see if the file starts with the string:

# This series applies on GIT commit

Re-implement the above in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 59 ++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/builtin/am.c b/builtin/am.c
index 0bb1875..7a56005 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -80,7 +80,8 @@ static int str_isspace(const char *str)
 enum patch_format {
PATCH_FORMAT_UNKNOWN = 0,
PATCH_FORMAT_MBOX,
-   PATCH_FORMAT_STGIT
+   PATCH_FORMAT_STGIT,
+   PATCH_FORMAT_STGIT_SERIES
 };
 
 enum keep_type {
@@ -686,6 +687,11 @@ static int detect_patch_format(const char **paths)
goto done;
}
 
+   if (starts_with(l1.buf, "# This series applies on GIT commit")) {
+   ret = PATCH_FORMAT_STGIT_SERIES;
+   goto done;
+   }
+
strbuf_reset(&l2);
strbuf_getline_crlf(&l2, fp);
strbuf_reset(&l3);
@@ -837,6 +843,53 @@ static int stgit_patch_to_mail(FILE *out, FILE *in, int 
keep_cr)
 }
 
 /**
+ * This function only supports a single StGit series file in `paths`.
+ *
+ * Given an StGit series file, converts the StGit patches in the series into
+ * RFC2822 messages suitable for parsing with git-mailinfo, and queues them in
+ * the state directory.
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+static int split_mail_stgit_series(struct am_state *state, const char **paths,
+   int keep_cr)
+{
+   const char *series_dir;
+   char *series_dir_buf;
+   FILE *fp;
+   struct argv_array patches = ARGV_ARRAY_INIT;
+   struct strbuf sb = STRBUF_INIT;
+   int ret;
+
+   if (!paths[0] || paths[1])
+   return error(_("Only one StGIT patch series can be applied at 
once"));
+
+   series_dir_buf = xstrdup(*paths);
+   series_dir = dirname(series_dir_buf);
+
+   fp = fopen(*paths, "r");
+   if (!fp)
+   return error(_("could not open '%s' for reading: %s"), *paths,
+   strerror(errno));
+
+   while (!strbuf_getline(&sb, fp, '\n')) {
+   if (*sb.buf == '#')
+   continue; /* skip comment lines */
+
+   argv_array_push(&patches, mkpath("%s/%s", series_dir, sb.buf));
+   }
+
+   fclose(fp);
+   strbuf_release(&sb);
+   free(series_dir_buf);
+
+   ret = split_mail_conv(stgit_patch_to_mail, state, patches.argv, 
keep_cr);
+
+   argv_array_clear(&patches);
+   return ret;
+}
+
+/**
  * Splits a list of files/directories into individual email patches. Each path
  * in `paths` must be a file/directory that is formatted according to
  * `patch_format`.
@@ -866,6 +919,8 @@ static int split_mail(struct am_state *state, enum 
patch_format patch_format,
return split_mail_mbox(state, paths, keep_cr);
case PATCH_FORMAT_STGIT:
return split_mail_conv(stgit_patch_to_mail, state, paths, 
keep_cr);
+   case PATCH_FORMAT_STGIT_SERIES:
+   return split_mail_stgit_series(state, paths, keep_cr);
default:
die("BUG: invalid patch_format");
}
@@ -1898,6 +1953,8 @@ static int parse_opt_patchformat(const struct option 
*opt, const char *arg, int
*opt_value = PATCH_FORMAT_MBOX;
else if (!strcmp(arg, "stgit"))
*opt_value = PATCH_FORMAT_STGIT;
+   else if (!strcmp(arg, "stgit-series"))
+   *opt_value = PATCH_FORMAT_STGIT_SERIES;
else
return error(_("Invalid value for --patch-format: %s"), arg);
return 0;
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 42/44] builtin-am: implement legacy -b/--binary option

2015-07-07 Thread Paul Tan
The -b/--binary option was initially implemented in 087b674 (git-am:
--binary; document --resume and --binary., 2005-11-16). The option will
pass the --binary flag to git-apply to allow it to apply binary patches.

However, in 2b6eef9 (Make apply --binary a no-op., 2006-09-06), --binary
was been made a no-op in git-apply. Following that, since cb3a160
(git-am: ignore --binary option, 2008-08-09), the --binary option in
git-am is ignored as well.

In 6c15a1c (am: officially deprecate -b/--binary option, 2012-03-13),
the --binary option was tweaked to its present behavior: when set, the
message:

The -b/--binary option has been a no-op for long time, and it
will be removed. Please do not use it anymore.

will be printed.

Re-implement this in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 2866328..ad07ba4 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2144,6 +2144,7 @@ enum resume_mode {
 int cmd_am(int argc, const char **argv, const char *prefix)
 {
struct am_state state;
+   int binary = -1;
int keep_cr = -1;
int patch_format = PATCH_FORMAT_UNKNOWN;
enum resume_mode resume = RESUME_FALSE;
@@ -2157,6 +2158,8 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
struct option options[] = {
OPT_BOOL('i', "interactive", &state.interactive,
N_("run interactively")),
+   OPT_HIDDEN_BOOL('b', "binary", &binary,
+   N_("(historical option -- no-op")),
OPT_BOOL('3', "3way", &state.threeway,
N_("allow fall back on 3way merging if needed")),
OPT__QUIET(&state.quiet, N_("be quiet")),
@@ -2257,6 +2260,10 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
 
argc = parse_options(argc, argv, prefix, options, usage, 0);
 
+   if (binary >= 0)
+   fprintf_ln(stderr, _("The -b/--binary option has been a no-op 
for long time, and\n"
+   "it will be removed. Please do not use it 
anymore."));
+
if (read_index_preload(&the_index, NULL) < 0)
die(_("failed to read the index"));
 
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 30/44] builtin-am: implement --committer-date-is-author-date

2015-07-07 Thread Paul Tan
Since 3f01ad6 (am: Add --committer-date-is-author-date option,
2009-01-22), git-am.sh implemented the --committer-date-is-author-date
option, which tells git-am to use the timestamp recorded in the email
message as both author and committer date.

Re-implement this option in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index e9eacf0..d7bb159 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -120,6 +120,8 @@ struct am_state {
/* override error message when patch failure occurs */
const char *resolvemsg;
 
+   int committer_date_is_author_date;
+
int ignore_date;
 
int rebasing;
@@ -1259,6 +1261,10 @@ static void do_commit(const struct am_state *state)
state->ignore_date ? NULL : state->author_date,
IDENT_STRICT);
 
+   if (state->committer_date_is_author_date)
+   setenv("GIT_COMMITTER_DATE",
+   state->ignore_date ? "" : state->author_date, 1);
+
if (commit_tree(state->msg, state->msg_len, tree, parents, commit,
author, NULL))
die(_("failed to write commit object"));
@@ -1677,6 +1683,9 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
OPT_CMDMODE(0, "abort", &resume,
N_("restore the original branch and abort the patching 
operation."),
RESUME_ABORT),
+   OPT_BOOL(0, "committer-date-is-author-date",
+   &state.committer_date_is_author_date,
+   N_("lie about committer date")),
OPT_BOOL(0, "ignore-date", &state.ignore_date,
N_("use current timestamp for author date")),
OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing,
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 23/44] builtin-am: implement -u/--utf8

2015-07-07 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07),
git-am.sh supported the -u,--utf8 option. If set, the -u option will be
passed to git-mailinfo to re-code the commit log message and authorship
in the charset specified by i18n.commitencoding. If unset, the -n option
will be passed to git-mailinfo, which disables the re-encoding.

Since d84029b (--utf8 is now default for 'git-am', 2007-01-08), --utf8
is specified by default in git-am.sh.

Re-implement the above in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index c92bff4..0b441fb 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -92,6 +92,8 @@ struct am_state {
 
int append_signoff;
 
+   int utf8;
+
/* override error message when patch failure occurs */
const char *resolvemsg;
 
@@ -112,6 +114,8 @@ static void am_state_init(struct am_state *state, const 
char *dir)
state->prec = 4;
 
git_config_get_bool("am.threeway", &state->threeway);
+
+   state->utf8 = 1;
 }
 
 /**
@@ -390,6 +394,9 @@ static void am_load(struct am_state *state)
read_state_file(&sb, state, "sign", 1);
state->append_signoff = !strcmp(sb.buf, "t");
 
+   read_state_file(&sb, state, "utf8", 1);
+   state->utf8 = !strcmp(sb.buf, "t");
+
state->rebasing = !!file_exists(am_path(state, "rebasing"));
 
strbuf_release(&sb);
@@ -579,6 +586,8 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
 
write_file(am_path(state, "sign"), 1, state->append_signoff ? "t" : 
"f");
 
+   write_file(am_path(state, "utf8"), 1, state->utf8 ? "t" : "f");
+
if (state->rebasing)
write_file(am_path(state, "rebasing"), 1, "%s", "");
else
@@ -750,6 +759,7 @@ static int parse_mail(struct am_state *state, const char 
*mail)
cp.out = xopen(am_path(state, "info"), O_WRONLY | O_CREAT, 0777);
 
argv_array_push(&cp.args, "mailinfo");
+   argv_array_push(&cp.args, state->utf8 ? "-u" : "-n");
argv_array_push(&cp.args, am_path(state, "msg"));
argv_array_push(&cp.args, am_path(state, "patch"));
 
@@ -1464,6 +1474,8 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
OPT__QUIET(&state.quiet, N_("be quiet")),
OPT_BOOL('s', "signoff", &state.append_signoff,
N_("add a Signed-off-by line to the commit message")),
+   OPT_BOOL('u', "utf8", &state.utf8,
+   N_("recode into utf8 (default)")),
OPT_CALLBACK(0, "patch-format", &patch_format, N_("format"),
N_("format the patch(es) are in"),
parse_opt_patchformat),
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 12/44] builtin-am: implement --skip

2015-07-07 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), git-am
supported resuming from a failed patch application by skipping the
current patch. Re-implement this feature by introducing am_skip().

Signed-off-by: Paul Tan 
---
 builtin/am.c | 121 ++-
 1 file changed, 119 insertions(+), 2 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index f21565b..5087094 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -16,6 +16,8 @@
 #include "commit.h"
 #include "diff.h"
 #include "diffcore.h"
+#include "unpack-trees.h"
+#include "branch.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -872,6 +874,114 @@ static void am_resolve(struct am_state *state)
 }
 
 /**
+ * Performs a checkout fast-forward from `head` to `remote`. If `reset` is
+ * true, any unmerged entries will be discarded. Returns 0 on success, -1 on
+ * failure.
+ */
+static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
+{
+   struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+   struct unpack_trees_options opts;
+   struct tree_desc t[2];
+
+   if (parse_tree(head) || parse_tree(remote))
+   return -1;
+
+   hold_locked_index(lock_file, 1);
+
+   refresh_cache(REFRESH_QUIET);
+
+   memset(&opts, 0, sizeof(opts));
+   opts.head_idx = 1;
+   opts.src_index = &the_index;
+   opts.dst_index = &the_index;
+   opts.update = 1;
+   opts.merge = 1;
+   opts.reset = reset;
+   opts.fn = twoway_merge;
+   init_tree_desc(&t[0], head->buffer, head->size);
+   init_tree_desc(&t[1], remote->buffer, remote->size);
+
+   if (unpack_trees(2, t, &opts)) {
+   rollback_lock_file(lock_file);
+   return -1;
+   }
+
+   if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+   die(_("unable to write new index file"));
+
+   return 0;
+}
+
+/**
+ * Clean the index without touching entries that are not modified between
+ * `head` and `remote`.
+ */
+static int clean_index(const unsigned char *head, const unsigned char *remote)
+{
+   struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+   struct tree *head_tree, *remote_tree, *index_tree;
+   unsigned char index[GIT_SHA1_RAWSZ];
+   struct pathspec pathspec;
+
+   head_tree = parse_tree_indirect(head);
+   if (!head_tree)
+   return error(_("Could not parse object '%s'."), 
sha1_to_hex(head));
+
+   remote_tree = parse_tree_indirect(remote);
+   if (!remote_tree)
+   return error(_("Could not parse object '%s'."), 
sha1_to_hex(remote));
+
+   read_cache_unmerged();
+
+   if (fast_forward_to(head_tree, head_tree, 1))
+   return -1;
+
+   if (write_cache_as_tree(index, 0, NULL))
+   return -1;
+
+   index_tree = parse_tree_indirect(index);
+   if (!index_tree)
+   return error(_("Could not parse object '%s'."), 
sha1_to_hex(index));
+
+   if (fast_forward_to(index_tree, remote_tree, 0))
+   return -1;
+
+   memset(&pathspec, 0, sizeof(pathspec));
+
+   hold_locked_index(lock_file, 1);
+
+   if (read_tree(remote_tree, 0, &pathspec)) {
+   rollback_lock_file(lock_file);
+   return -1;
+   }
+
+   if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
+   die(_("unable to write new index file"));
+
+   remove_branch_state();
+
+   return 0;
+}
+
+/**
+ * Resume the current am session by skipping the current patch.
+ */
+static void am_skip(struct am_state *state)
+{
+   unsigned char head[GIT_SHA1_RAWSZ];
+
+   if (get_sha1("HEAD", head))
+   hashcpy(head, EMPTY_TREE_SHA1_BIN);
+
+   if (clean_index(head, head))
+   die(_("failed to clean index"));
+
+   am_next(state);
+   am_run(state);
+}
+
+/**
  * parse_options() callback that validates and sets opt->value to the
  * PATCH_FORMAT_* enum value corresponding to `arg`.
  */
@@ -888,7 +998,8 @@ static int parse_opt_patchformat(const struct option *opt, 
const char *arg, int
 
 enum resume_mode {
RESUME_FALSE = 0,
-   RESUME_RESOLVED
+   RESUME_RESOLVED,
+   RESUME_SKIP
 };
 
 int cmd_am(int argc, const char **argv, const char *prefix)
@@ -899,7 +1010,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 
const char * const usage[] = {
N_("git am [options] [(|)...]"),
-   N_("git am [options] --continue"),
+   N_("git am [options] (--continue | --skip)"),
NULL
};
 
@@ -913,6 +1024,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
OPT_CMDMODE('r', "resolved", &resume,
N_("synonyms for --continue"),
RESUME_RESOLVED),
+   OPT_CMDMODE(0, "skip", &resume,
+

[PATCH v5 28/44] builtin-am: pass git-apply's options to git-apply

2015-07-07 Thread Paul Tan
git-am.sh recognizes some of git-apply's options, and would pass them to
git-apply:

* --whitespace, since 8c31cb8 (git-am: --whitespace=x option.,
  2006-02-28)

* -C, since 67dad68 (add -C[NUM] to git-am, 2007-02-08)

* -p, since 2092a1f (Teach git-am to pass -p option down to git-apply,
  2007-02-11)

* --directory, since b47dfe9 (git-am: add --directory= option,
  2009-01-11)

* --reject, since b80da42 (git-am: implement --reject option passed to
  git-apply, 2009-01-23)

* --ignore-space-change, --ignore-whitespace, since 86c91f9 (git apply:
  option to ignore whitespace differences, 2009-08-04)

* --exclude, since 77e9e49 (am: pass exclude down to apply, 2011-08-03)

* --include, since 58725ef (am: support --include option, 2012-03-28)

* --reject, since b80da42 (git-am: implement --reject option passed to
  git-apply, 2009-01-23)

Re-implement support for these options in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 2e46a06..6006010 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -115,6 +115,8 @@ struct am_state {
/* one of the enum scissors_type values */
int scissors;
 
+   struct argv_array git_apply_opts;
+
/* override error message when patch failure occurs */
const char *resolvemsg;
 
@@ -141,6 +143,8 @@ static void am_state_init(struct am_state *state, const 
char *dir)
git_config_get_bool("am.messageid", &state->message_id);
 
state->scissors = SCISSORS_UNSET;
+
+   argv_array_init(&state->git_apply_opts);
 }
 
 /**
@@ -162,6 +166,8 @@ static void am_state_release(struct am_state *state)
 
if (state->msg)
free(state->msg);
+
+   argv_array_clear(&state->git_apply_opts);
 }
 
 /**
@@ -441,6 +447,11 @@ static void am_load(struct am_state *state)
else
state->scissors = SCISSORS_UNSET;
 
+   read_state_file(&sb, state, "apply-opt", 1);
+   argv_array_clear(&state->git_apply_opts);
+   if (sq_dequote_to_argv_array(sb.buf, &state->git_apply_opts) < 0)
+   die(_("could not parse %s"), am_path(state, "apply-opt"));
+
state->rebasing = !!file_exists(am_path(state, "rebasing"));
 
strbuf_release(&sb);
@@ -615,6 +626,7 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
 {
unsigned char curr_head[GIT_SHA1_RAWSZ];
const char *str;
+   struct strbuf sb = STRBUF_INIT;
 
if (!patch_format)
patch_format = detect_patch_format(paths);
@@ -677,6 +689,9 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
 
write_file(am_path(state, "scissors"), 1, "%s", str);
 
+   sq_quote_argv(&sb, state->git_apply_opts.argv, 0);
+   write_file(am_path(state, "apply-opt"), 1, "%s", sb.buf);
+
if (state->rebasing)
write_file(am_path(state, "rebasing"), 1, "%s", "");
else
@@ -701,6 +716,8 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
write_file(am_path(state, "next"), 1, "%d", state->cur);
 
write_file(am_path(state, "last"), 1, "%d", state->last);
+
+   strbuf_release(&sb);
 }
 
 /**
@@ -1093,6 +1110,8 @@ static int run_apply(const struct am_state *state, const 
char *index_file)
 
argv_array_push(&cp.args, "apply");
 
+   argv_array_pushv(&cp.args, state->git_apply_opts.argv);
+
if (index_file)
argv_array_push(&cp.args, "--cached");
else
@@ -1119,6 +1138,7 @@ static int build_fake_ancestor(const struct am_state 
*state, const char *index_f
 
cp.git_cmd = 1;
argv_array_push(&cp.args, "apply");
+   argv_array_pushv(&cp.args, state->git_apply_opts.argv);
argv_array_pushf(&cp.args, "--build-fake-ancestor=%s", index_file);
argv_array_push(&cp.args, am_path(state, "patch"));
 
@@ -1610,9 +1630,36 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
OPT_BOOL('c', "scissors", &state.scissors,
N_("strip everything before a scissors line")),
+   OPT_PASSTHRU_ARGV(0, "whitespace", &state.git_apply_opts, 
N_("action"),
+   N_("pass it through git-apply"),
+   0),
+   OPT_PASSTHRU_ARGV(0, "ignore-space-change", 
&state.git_apply_opts, NULL,
+   N_("pass it through git-apply"),
+   PARSE_OPT_NOARG),
+   OPT_PASSTHRU_ARGV(0, "ignore-whitespace", 
&state.git_apply_opts, NULL,
+   N_("pass it through git-apply"),
+   PARSE_OPT_NOARG),
+   OPT_PASSTHRU_ARGV(0, "directory", &state.git_apply_opts, 
N_("root"),
+   N_("pass it through git-apply"),
+   0),
+

[PATCH v5 35/44] builtin-am: invoke pre-applypatch hook

2015-07-07 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07),
git-am.sg will invoke the pre-applypatch hook after applying the patch
to the index, but before a commit is made. Should the hook exit with a
non-zero status, git am will exit.

Re-implement this in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index f856d3b..407775c 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1374,6 +1374,9 @@ static void do_commit(const struct am_state *state)
const char *reflog_msg, *author;
struct strbuf sb = STRBUF_INIT;
 
+   if (run_hook_le(NULL, "pre-applypatch", NULL))
+   exit(1);
+
if (write_cache_as_tree(tree, 0, NULL))
die(_("git write-tree failed to write a tree"));
 
-- 
2.5.0.rc1.76.gf60a929

--
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 v5 36/44] builtin-am: invoke post-applypatch hook

2015-07-07 Thread Paul Tan
Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07),
git-am.sh will invoke the post-applypatch hook after the patch is
applied and a commit is made. The exit code of the hook is ignored.

Re-implement this in builtin/am.c.

Signed-off-by: Paul Tan 
---
 builtin/am.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 407775c..fdaf0a7 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1418,6 +1418,8 @@ static void do_commit(const struct am_state *state)
fclose(fp);
}
 
+   run_hook_le(NULL, "post-applypatch", NULL);
+
strbuf_release(&sb);
 }
 
-- 
2.5.0.rc1.76.gf60a929

--
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


  1   2   >