Re: [PATCH] blame: display a more helpful error message if the file was deleted

2015-12-16 Thread Junio C Hamano
Alex Henrie  writes:

> `git blame 22414770 generate-cmdlist.perl` currently results in:
> fatal: cannot stat path '22414770': No such file or directory
>
> This patch changes the error message to:
> fatal: ambiguous argument 'generate-cmdlist.perl': unknown revision
> or path not in the working tree.
> Use '--' to separate paths from revisions, like this:
> 'git  [...] -- [...]'"
>
> That way, the user knows to rewrite the command as
> `git blame 22414770 -- generate-cmdlist.perl`.

It is unfortunate that the command line parser of 'blame' needs to
cater to an oddball syntax, 'git annotate  []'.

This change still makes us emit the following funny errors:

$ ./git blame  generate-cmdlist.prel
fatal: bad revision 'generate-cmdlist.prel'
$ ./git blame generate-cmdlist.prel 
fatal: bad revision ''

and gives the users a wrong impression that the early argument is
the path and then comes the revision.

I'll take the patch as-is, because the original is not better, i.e.

$ git blame  generate-cmdlist.prel
fatal: cannot stat path '': No such file or directory
$ git blame generate-cmdlist.prel 
fatal: cannot stat path 'generate-cmdlist.prel': No such file or directory

and the change is not making things worse.

But I think in the longer term we should deprecate and eventually
remove 'git annotate' with its odd syntax, so that we can lose the
special case we have in the command line parser of 'git blame'.

Thanks.

>
> Signed-off-by: Alex Henrie 
> ---
>  builtin/blame.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 1df13cf..f070272 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2683,8 +2683,6 @@ parse_done:
>   argv[argc - 1] = "--";
>  
>   setup_work_tree();
> - if (!file_exists(path))
> - die_errno("cannot stat path '%s'", path);
>   }
>  
>   revs.disable_stdin = 1;
--
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: [PATCHv2 0/7] Rerolling sb/submodule-parallel-fetch for the time after 2.7

2015-12-16 Thread Junio C Hamano
Stefan Beller  writes:

> I am sending out a new version for replacing sb/submodule-parallel-fetch for
> the time after the 2.7 release.
>
> * Dropped the patch, which introduces xread_nonblock
> * strbuf_read_once uses xread now. This is safe as we poll before using
>   strbuf_read_once, so we know we won't stall.

"That is only true for the current callers" was my first reaction,
but it is safe and sensible even for future callers.  If they have
something better to do than getting stuck before reading some, they
ought to be checking for POLLIN before calling this, and if not,
they do not have to be forced to spin for themselves until this
returns some new payload.

I like that.

> * have the commit message reworded for "run-command: add an
>   asynchronous parallel child processor" with Johannes' suggestion.

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 1/2] push: Fully test --recurse-submodules on command line overrides config

2015-12-16 Thread Stefan Beller
On Thu, Dec 3, 2015 at 5:10 AM, Mike Crowe  wrote:
> t5531 only checked that the push.recurseSubmodules config option was
> overridden by passing --recurse-submodules=check on the command line.
> Add new tests for overriding with --recurse-submodules=no,
> --no-recurse-submodules and --recurse-submodules=push too.
>
> Also correct minor typo in test commit message.
>
> Signed-off-by: Mike Crowe 

This looks good to me.

Thanks,
Stefan


> ---
>  t/t5531-deep-submodule-push.sh | 32 
>  1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
> index 9fda7b0..721be32 100755
> --- a/t/t5531-deep-submodule-push.sh
> +++ b/t/t5531-deep-submodule-push.sh
> @@ -126,24 +126,48 @@ test_expect_success 'push succeeds if submodule commit 
> not on remote but using o
> )
>  '
>
> -test_expect_success 'push fails if submodule commit not on remote using 
> check from cmdline overriding config' '
> +test_expect_success 'push recurse-submodules on command line overrides 
> config' '
> (
> cd work/gar/bage &&
> >recurse-check-on-command-line-overriding-config &&
> git add recurse-check-on-command-line-overriding-config &&
> -   git commit -m "Recurse on command-line overridiing config 
> junk"
> +   git commit -m "Recurse on command-line overriding config junk"
> ) &&
> (
> cd work &&
> git add gar/bage &&
> git commit -m "Recurse on command-line overriding config for 
> gar/bage" &&
> +
> +   # Ensure that we can override on-demand in the config
> +   # to just check submodules
> test_must_fail git -c push.recurseSubmodules=on-demand push 
> --recurse-submodules=check ../pub.git master &&
> # Check that the supermodule commit did not get there
> git fetch ../pub.git &&
> git diff --quiet FETCH_HEAD master^ &&
> # Check that the submodule commit did not get there
> -   cd gar/bage &&
> -   git diff --quiet origin/master master^
> +   (cd gar/bage && git diff --quiet origin/master master^) &&
> +
> +   # Ensure that we can override check in the config to
> +   # disable submodule recursion entirely
> +   (cd gar/bage && git diff --quiet origin/master master^) &&
> +   git -c push.recurseSubmodules=on-demand push 
> --recurse-submodules=no ../pub.git master &&
> +   git fetch ../pub.git &&
> +   git diff --quiet FETCH_HEAD master &&
> +   (cd gar/bage && git diff --quiet origin/master master^) &&
> +
> +   # Ensure that we can override check in the config to
> +   # disable submodule recursion entirely (alternative form)
> +   git -c push.recurseSubmodules=on-demand push 
> --no-recurse-submodules ../pub.git master &&
> +   git fetch ../pub.git &&
> +   git diff --quiet FETCH_HEAD master &&
> +   (cd gar/bage && git diff --quiet origin/master master^) &&
> +
> +   # Ensure that we can override check in the config to
> +   # push the submodule too
> +   git -c push.recurseSubmodules=check push 
> --recurse-submodules=on-demand ../pub.git master &&
> +   git fetch ../pub.git &&
> +   git diff --quiet FETCH_HEAD master &&
> +   (cd gar/bage && git diff --quiet origin/master master)
> )
>  '
>
> --
> 2.1.4
>
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] push: add '-d' as shorthand for '--delete'

2015-12-16 Thread Junio C Hamano
Jeff King  writes:

> At this point, it seems that "--delete" is useful, and nothing else has
> been proposed for "-d" in the intervening years. It seems like a
> reasonable use of the flag to me.

I think there were two (and a half) reasons why we didn't let
"--delete" use a short-and-sweet "-d", and I agree that "something
else that is more useful did not come" removes one of them.

The other reason was to avoid the chance of fat-fingering, because
deleting is destructive, and it is even harder to recover from if
the damage is done remotely (and the remaining one-half is that
deleting is a rare event).

Even though I do not think the need for the "safety" has been
reduced over time to warrant this change, a similarity with "branch"
that has "-d/--delete" would be a good enough argument to support
this change.

Thanks.

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


Re: [PATCH v2 05/11] ref-filter: skip deref specifier in match_atom_name()

2015-12-16 Thread Eric Sunshine
On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak  wrote:
> In upcoming patches we make calls to match_atom_name() with the '*'
> deref specifier still attached to the atom name. This causes
> undesirable errors, hence, if present skip over the '*' deref
> specifier in the atom name.

I'd drop the second sentence since it doesn't add much or any value.
Instead, you might want to explain that skipping '*' is done as a
convenience.

Subsequent patches will call match_atom_name() with the '*' deref
specifier still attached to the atom name so, as a convenience,
skip over it on their behalf.

> Signed-off-by: Karthik Nayak 
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -37,6 +37,10 @@ static int match_atom_name(const char *name, const char 
> *atom_name, const char *
>  {
> const char *body;
>
> +   /*  skip the deref specifier*/

Too many spaces before "skip".
Too few spaces after "specifier".

> +   if (name[0] == '*')
> +   name++;
> +
> if (!skip_prefix(name, atom_name, ))
> return 0; /* doesn't even begin with "atom_name" */
> if (!body[0]) {
> --
> 2.6.4
--
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] credential-store: don't pass strerror to die_errno()

2015-12-16 Thread Jeff King
On Wed, Dec 16, 2015 at 12:22:55PM +0100, SZEDER Gábor wrote:

> Signed-off-by: SZEDER Gábor 
> ---
> 
> v2 fixed this, but it fell on the floor, I suppose because of the
> maintainer switch.  Anyway, I should have noticed it while the patch
> was still cooking, sorry.

Oops. I do remember seeing Eric point out the problem and I think I was
planning to fix it up before merging. But I didn't make a note in
whats-cooking, so Junio had no idea of my plan.

>  credential-store.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Patch is obviously correct. Thanks.

-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] mingw: emulate write(2) that fails with a EPIPE

2015-12-16 Thread Junio C Hamano
Johannes Schindelin  writes:

> On Windows, when writing to a pipe fails, errno is always
> EINVAL. However, Git expects it to be EPIPE.
>
> According to the documentation, there are two cases in which write()
> triggers EINVAL: the buffer is NULL, or the length is odd but the mode
> is 16-bit Unicode (the broken pipe is not mentioned as possible cause).
> Git never sets the file mode to anything but binary, therefore we know
> that errno should actually be EPIPE if it is EINVAL and the buffer is
> not NULL.

Makes sense.

>  int mingw_fflush(FILE *stream);
>  #define fflush mingw_fflush
>  
> +static inline ssize_t mingw_write(int fd, const void *buf, size_t len)
> +{
> + ssize_t result = write(fd, buf, len);
> +
> + if (result < 0 && errno == EINVAL && buf) {
> + /* check if fd is a pipe */
> + HANDLE h = (HANDLE) _get_osfhandle(fd);
> + if (GetFileType(h) == FILE_TYPE_PIPE)
> + errno = EPIPE;
> + else
> + errno = EINVAL;
> + }
> +
> + return result;
> +}
> +
> +#define write mingw_write
> +

It strikes me a bit strange to see this inlined compared to what
appears in the context.  Shouldn't the implementation be done in
compat/mingw.c like all others?

>  int mingw_access(const char *filename, int mode);
>  #undef access
>  #define access mingw_access
--
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 01/11] strbuf: introduce strbuf_split_str_without_term()

2015-12-16 Thread Eric Sunshine
On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak  wrote:
> strbuf: introduce strbuf_split_str_without_term()

s/without/omit/

> The current implementation of 'strbuf_split_buf()' includes the
> terminator at the end of each strbuf post splitting. Add an option
> wherein we can drop the terminator if desired. In this context
> introduce a wrapper function 'strbuf_split_str_without_term()' which

s/without/omit/

> splits a given string into strbufs without including the terminator.
>
> Signed-off-by: Karthik Nayak 
> ---
> diff --git a/strbuf.c b/strbuf.c
> @@ -115,7 +115,7 @@ void strbuf_tolower(struct strbuf *sb)
>  struct strbuf **strbuf_split_buf(const char *str, size_t slen,
> -int terminator, int max)
> +int terminator, int max, int omit_term)
>  {
> struct strbuf **ret = NULL;
> size_t nr = 0, alloc = 0;
> @@ -123,14 +123,15 @@ struct strbuf **strbuf_split_buf(const char *str, 
> size_t slen,
>
> while (slen) {
> int len = slen;
> +   const char *end = NULL;
> if (max <= 0 || nr + 1 < max) {
> -   const char *end = memchr(str, terminator, slen);
> +   end = memchr(str, terminator, slen);
> if (end)
> len = end - str + 1;
> }
> t = xmalloc(sizeof(struct strbuf));
> strbuf_init(t, len);
> -   strbuf_add(t, str, len);
> +   strbuf_add(t, str, len - !!end * !!omit_term);

This version of the patch with its minimal[1] change is much easier to
review for correctness than the original attempt[2].

[1]: http://article.gmane.org/gmane.comp.version-control.git/281882
[2]: http://article.gmane.org/gmane.comp.version-control.git/281189

> ALLOC_GROW(ret, nr + 2, alloc);
> ret[nr++] = t;
> str += len;
> diff --git a/strbuf.h b/strbuf.h
> index 6ae7a72..a865a74 100644
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -450,11 +450,12 @@ static inline int strbuf_strip_suffix(struct strbuf 
> *sb, const char *suffix)
>  /**
>   * Split str (of length slen) at the specified terminator character.
>   * Return a null-terminated array of pointers to strbuf objects
> - * holding the substrings.  The substrings include the terminator,
> - * except for the last substring, which might be unterminated if the
> - * original string did not end with a terminator.  If max is positive,
> - * then split the string into at most max substrings (with the last
> - * substring containing everything following the (max-1)th terminator
> + * holding the substrings.  The substrings include the terminator if
> + * the value of omit_term is '0' else the terminator is excluded.  The

Perhaps just say "if omit_term is false" rather than "if the value of ... is 0".

> + * last substring, might be unterminated if the original string did
> + * not end with a terminator.  If max is positive, then split the

This bit about the last substring is perhaps too disconnected from the
previous sentence. What if you re-word the entire thing something like
this:

If omit_term is true, the terminator will be stripped from all
substrings. Otherwise, substrings will include the terminator,
except for the final substring, if the original string lacked a
terminator.

> + * string into at most max substrings (with the last substring
> + * containing everything following the (max-1)th terminator
>   * character).
>   *
>   * The most generic form is `strbuf_split_buf`, which takes an arbitrary
> @@ -465,19 +466,25 @@ static inline int strbuf_strip_suffix(struct strbuf 
> *sb, const char *suffix)
>   * For lighter-weight alternatives, see string_list_split() and
>   * string_list_split_in_place().
>   */
> -extern struct strbuf **strbuf_split_buf(const char *, size_t,
> -   int terminator, int max);
> +extern struct strbuf **strbuf_split_buf(const char *str, size_t slen,
> +   int terminator, int max, int 
> omit_term);
> +
> +static inline struct strbuf **strbuf_split_str_omit_term(const char *str,
> +   int terminator, 
> int max)
> +{
> +   return strbuf_split_buf(str, strlen(str), terminator, max, 1);
> +}
>
>  static inline struct strbuf **strbuf_split_str(const char *str,
>int terminator, int max)
>  {
> -   return strbuf_split_buf(str, strlen(str), terminator, max);
> +   return strbuf_split_buf(str, strlen(str), terminator, max, 0);
>  }
>
>  static inline struct strbuf **strbuf_split_max(const struct strbuf *sb,
> int terminator, int max)
>  {
> -   return strbuf_split_buf(sb->buf, sb->len, terminator, max);
> +   return 

Re: [PATCH] mingw: emulate write(2) that fails with a EPIPE

2015-12-16 Thread Junio C Hamano
Eric Sunshine  writes:

>> +   if (result < 0 && errno == EINVAL && buf) {
>
> Here, errno is EINVAL...
>
>> +   /* check if fd is a pipe */
>> +   HANDLE h = (HANDLE) _get_osfhandle(fd);
>> +   if (GetFileType(h) == FILE_TYPE_PIPE)
>> +   errno = EPIPE;
>> +   else
>> +   errno = EINVAL;
>
> Does any of the code between the outer 'if' and this point clobber
> errno, or are you merely assigning EINVAL for robustness against
> future changes?

I do think it is a good idea to reassign 'errno' here to stress on
the fact that we return EPIPE only when we positively know that we
were reading from a pipe, and otherwise we won't change the original
errno at all.

But at the same time, if other things that are called before we
figure out if we were reading from a pipe could affect errno, I
wonder if that is an indication of a bug--an error return from
system functions that the code is not responding to.  For example,
can _get_osfhandle() fail and when it does what would we see?
Perhaps NULL in h?

--
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] mingw: emulate write(2) that fails with a EPIPE

2015-12-16 Thread Eric Sunshine
On Wednesday, December 16, 2015, Johannes Schindelin
 wrote:
> On Windows, when writing to a pipe fails, errno is always
> EINVAL. However, Git expects it to be EPIPE.
>
> According to the documentation, there are two cases in which write()
> triggers EINVAL: the buffer is NULL, or the length is odd but the mode
> is 16-bit Unicode (the broken pipe is not mentioned as possible cause).
> Git never sets the file mode to anything but binary, therefore we know
> that errno should actually be EPIPE if it is EINVAL and the buffer is
> not NULL.
>
> See https://msdn.microsoft.com/en-us/library/1570wh78.aspx for more
> details.
>
> This works around t5571.11 failing with v2.6.4 on Windows.
>
> Signed-off-by: Johannes Schindelin 
> ---
> diff --git a/compat/mingw.h b/compat/mingw.h
> @@ -210,6 +210,24 @@ FILE *mingw_freopen (const char *filename, const char 
> *otype, FILE *stream);
> +static inline ssize_t mingw_write(int fd, const void *buf, size_t len)
> +{
> +   ssize_t result = write(fd, buf, len);
> +
> +   if (result < 0 && errno == EINVAL && buf) {

Here, errno is EINVAL...

> +   /* check if fd is a pipe */
> +   HANDLE h = (HANDLE) _get_osfhandle(fd);
> +   if (GetFileType(h) == FILE_TYPE_PIPE)
> +   errno = EPIPE;
> +   else
> +   errno = EINVAL;

Does any of the code between the outer 'if' and this point clobber
errno, or are you merely assigning EINVAL for robustness against
future changes?

> +   }
> +
> +   return result;
> +}
> +
> +#define write mingw_write
> +
> --
> 2.6.3.windows.1.300.g1c25e49
--
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 03/11] ref-filter: introduce struct used_atom

2015-12-16 Thread Eric Sunshine
On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak  wrote:
> Introduce the 'used_array' structure which would replace the existing
> implementation of 'used_array' (which a list of atoms). This helps us
> parse atom's before hand and store required details into the
> 'used_array' for future usage.

All my v1 review comments[1] about the commit message still apply to
this version.

[1]: http://article.gmane.org/gmane.comp.version-control.git/281860

> Also introduce a parsing function for each atom in valid_atom. Using
> this we can define special parsing functions for each of the atoms.

This is a conceptually distinct change which probably deserves its own
patch. In particular, the new patch would add this field to
valid_atom[] *and* add the code which invokes the custom parser. (That
code is currently commingled with introduction of the color parser in
patch 6/11.)

More below...

> Signed-off-by: Karthik Nayak 
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -16,9 +16,27 @@
> +/*
> + * An atom is a valid field atom listed below, possibly prefixed with
> + * a "*" to denote deref_tag().
> + *
> + * We parse given format string and sort specifiers, and make a list
> + * of properties that we need to extract out of objects.  ref_array_item
> + * structure will hold an array of values extracted that can be
> + * indexed with the "atom number", which is an index into this
> + * array.
> + */
> +static struct used_atom {
> +   const char *str;
> +   cmp_type type;
> +} *used_atom;
> +static int used_atom_cnt, need_tagged, need_symref;
> +static int need_color_reset_at_eol;
> +
>  static struct {
> const char *name;
> cmp_type cmp_type;
> +   void (*parser)(struct used_atom *atom);
>  } valid_atom[] = {
> { "refname" },
> { "objecttype" },
> @@ -786,7 +788,8 @@ static void populate_value(struct ref_array_item *ref)
>
> /* Fill in specials first */
> for (i = 0; i < used_atom_cnt; i++) {
> -   const char *name = used_atom[i];
> +   struct used_atom *atom = _atom[i];
> +   const char *name = atom->str;

Same question as my previous review[1]: Why not just:

const char *name = used_atom[i].str;

?

> struct atom_value *v = >value[i];
> int deref = 0;
> const char *refname;
--
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] rebase: add --verify-signatures

2015-12-16 Thread Junio C Hamano
Alexander 'z33ky' Hirsch <1ze...@gmail.com> writes:

> On Thu, Dec 10, 2015 at 11:53:45AM -0800, Junio C Hamano wrote:
>
>> In a workflow that is built around "pull --rebase", you are _given_
>> the authoritative history with all the good things from another
>> place and then you rebuild your own work on top of it.  The sanity
>> and cleanliness of what you built on top is given, and rejecting it
>> at that point would not help you make further progress in any way,
>> as that is a published history that is shared and more authoritative
>> than what you have.
>
> Well, the rejection would not refer to the work you put on top, but to
> the commits you want to base your work on.
> If validation fails, then an empty commit that is signed can be
> committed on top of the previously unsigned branch if commit rewriting
> is not allowed.

I do not quite understand how that would help anything.  I do not
personally believe in projects that wants to sign each and every
commit, but to them, "an empty signed commit on top" would not fix
anything once they have an unsigned commit at the tip of a public
branch.  And for those that care about only the tip to be signed,
instead of adding such an empty commit, you would rebuild and sign
your work on top of that unsigned public tip and push back---at
which point the tip of the public branch would have a signature from
you.  So such an empty signed commit would either not help, or not
necessary, to make the resulting history kosher again.

--
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: What's cooking in git.git (Dec 2015, #05; Tue, 15)

2015-12-16 Thread Junio C Hamano
Torsten Bögershausen  writes:

> On 15.12.15 23:48, Junio C Hamano wrote:
>> * tb/ls-files-eol (2015-11-28) 2 commits
>>  - convert.c: mark a file-local function static
>>  - ls-files: Add eol diagnostics
>> 
>>  Add options to ls-files to help diagnose end-of-line problems.
>> 
>>  This latest round hasn't gotten any review yet.
>> 
>>  Waiting for review.
>
> The latest review are here: $gmane/281785 $gmane/282061
> And a re-roll is planned the next weeks, sorry for delay.

Thanks.  In the meantime, I'll pick up v6 $gmane/282061 and replace
the above with 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/2] git-svn: test for git-svn prefixed globs

2015-12-16 Thread Victor Leschuk
Hello Eric, sorry, I just copy pasted your old t9108-git-svn-glob.sh, 
changed branch names to be prefixed and added test for "exact" prefix 
match. If it is necessary I can rewrite it according to current guidelines.


On 12/17/2015 12:28 AM, Eric Wong wrote:

Thanks for this work.  Most things look fine with 1/2, comments
on 2/2 below...

Victor Leschuk  wrote:

Add test for git-svn prefixed globs.

Why a separate patch?  Unless there's some documentation purpose
for a regression, usually tests and a feature should be added
atomically in the same commit.


--- /dev/null
+++ b/t/t9168-git-svn-prefixed-glob.sh
@@ -0,0 +1,136 @@
+#!/bin/sh
+test_description='git svn globbing refspecs with prefixed globs'
+. ./lib-git-svn.sh
+
+cat > expect.end <
We prefer redirects in new code to be in the form of ">foo" (no space)
(or ">>foo" for append).

It wasn't in the old tests, either, but Documentation/CodingGuidelines
favors this for new code.


+the end
+hi
+start a new branch
+initial
+EOF

All the setup code be checked for errors with '&&' as well.


+   test "`git rev-parse refs/remotes/tags/t_end~1`" = \
+   "`git rev-parse refs/remotes/branches/b_start`" &&
+   test "`git rev-parse refs/remotes/branches/b_start~2`" = \
+   "`git rev-parse refs/remotes/trunk`" &&

And we prefer $(command) instead of `command` for nestability
as Documentation/CodingGuidelines suggests.

(yeah, most of the old tests don't follow the guidelines, but the
  guidelines also warn against fixup patches for them).

Thanks again.


--
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] rebase: add --verify-signatures

2015-12-16 Thread Alexander 'z33ky' Hirsch
On Wed, Dec 16, 2015 at 10:12:12AM -0800, Junio C Hamano wrote:
> I do not quite understand how that would help anything.  I do not
> personally believe in projects that wants to sign each and every
> commit, but to them, "an empty signed commit on top" would not fix
> anything once they have an unsigned commit at the tip of a public
> branch.  And for those that care about only the tip to be signed,
> instead of adding such an empty commit, you would rebuild and sign
> your work on top of that unsigned public tip and push back---at
> which point the tip of the public branch would have a signature from
> you.  So such an empty signed commit would either not help, or not
> necessary, to make the resulting history kosher again.
> 

Checking all commits was a mistake I made because of misinterpreting the
git-merge code. Only the tip should be checked for a signature.
And the reason to get it signed instead of just signing the commits
rebased on top is to defer to the judgement of the author of the branch
you're rebasing onto instead of checking the unsigned commits for
validity yourself.

As I understand it, this is the same reason for the existence of
--verify-signatures for git-merge. Otherwise the same argument could be
made for git-merge - just do whatever cleanup you need after merging and
sign it yourself.
Or maybe I haven't grasped what --verify-signatures is for.
--
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 0/4] Fix git-gui when recentrepo list has duplicates

2015-12-16 Thread Philip Oakley
git-gui won't start if either there are duplicate invalid entries or
the user selects a duplicated entry from the gui.recentrepo list. Such
duplicates have been found in the wild.

V2 corrects the spellings, grammar and quoting errors in the commit
messages as noted by Eric Sunshine ($gmane/282430 & $gmane/282432)

Also patch 1/4 title has been shortened

This re-roll will hopefully give Pat an opportunity to consider including
the fix into the git-gui for Git V2.7

V1 series at $gmane/282360


Philip Oakley (4):
  git-gui: de-dup entries from gui.recentrepo
  git gui: cope with duplicates in _get_recentrepo
  git gui: de-dup selected repo from recentrepo history
  git gui: allow for a long recentrepo list

 git-gui/lib/choose_repository.tcl | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

-- 
2.6.4.windows.1

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


Re: [PATCH 2/2] git-svn: test for git-svn prefixed globs

2015-12-16 Thread Eric Wong
Thanks for this work.  Most things look fine with 1/2, comments
on 2/2 below...

Victor Leschuk  wrote:
> Add test for git-svn prefixed globs.

Why a separate patch?  Unless there's some documentation purpose
for a regression, usually tests and a feature should be added
atomically in the same commit.

> --- /dev/null
> +++ b/t/t9168-git-svn-prefixed-glob.sh
> @@ -0,0 +1,136 @@
> +#!/bin/sh
> +test_description='git svn globbing refspecs with prefixed globs'
> +. ./lib-git-svn.sh
> +
> +cat > expect.end >foo" for append).

It wasn't in the old tests, either, but Documentation/CodingGuidelines
favors this for new code.

> +the end
> +hi
> +start a new branch
> +initial
> +EOF

All the setup code be checked for errors with '&&' as well.

> + test "`git rev-parse refs/remotes/tags/t_end~1`" = \
> + "`git rev-parse refs/remotes/branches/b_start`" &&
> + test "`git rev-parse refs/remotes/branches/b_start~2`" = \
> + "`git rev-parse refs/remotes/trunk`" &&

And we prefer $(command) instead of `command` for nestability
as Documentation/CodingGuidelines suggests.

(yeah, most of the old tests don't follow the guidelines, but the
 guidelines also warn against fixup patches for them).

Thanks again.
--
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 10/17] remote.c: read $GIT_DIR/remotes/* with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
These files can be edited with a DOS editor, leaving CR at the end
of the line if read with strbuf_getline().

Signed-off-by: Junio C Hamano 
---
 remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 1101f82..e88d936 100644
--- a/remote.c
+++ b/remote.c
@@ -256,7 +256,7 @@ static void read_remotes_file(struct remote *remote)
if (!f)
return;
remote->origin = REMOTE_REMOTES;
-   while (strbuf_getline(, f, '\n') != EOF) {
+   while (strbuf_getline_crlf(, f) != EOF) {
const char *v;
 
strbuf_rtrim();
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 15/17] send-pack: read list of refs with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
Signed-off-by: Junio C Hamano 
---
 builtin/send-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index f6e5d64..02e6e24 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -212,7 +212,7 @@ int cmd_send_pack(int argc, const char **argv, const char 
*prefix)
argv_array_push(_refspecs, buf);
} else {
struct strbuf line = STRBUF_INIT;
-   while (strbuf_getline(, stdin, '\n') != EOF)
+   while (strbuf_getline_crlf(, stdin) != EOF)
argv_array_push(_refspecs, line.buf);
strbuf_release();
}
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 17/17] test-sha1-array: read command stream with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
The input to this command comes from a pipeline in t0064, whose
upstream has bunch of "echo"s.  It is not unreasonable to expect
that it may be fed CRLF lines on DOSsy systems.

Signed-off-by: Junio C Hamano 
---
 test-sha1-array.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test-sha1-array.c b/test-sha1-array.c
index ddc491e..87b04de 100644
--- a/test-sha1-array.c
+++ b/test-sha1-array.c
@@ -11,7 +11,7 @@ int main(int argc, char **argv)
struct sha1_array array = SHA1_ARRAY_INIT;
struct strbuf line = STRBUF_INIT;
 
-   while (strbuf_getline(, stdin, '\n') != EOF) {
+   while (strbuf_getline_crlf(, stdin) != EOF) {
const char *arg;
unsigned char sha1[20];
 
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 13/17] cat-file: read batch stream with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
It is possible to prepare a text file with a DOS editor and feed it
as a batch command stream to the command.

Signed-off-by: Junio C Hamano 
---
 builtin/cat-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index c0fd8db..a2e75ad 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -401,7 +401,7 @@ static int batch_objects(struct batch_options *opt)
save_warning = warn_on_object_refname_ambiguity;
warn_on_object_refname_ambiguity = 0;
 
-   while (strbuf_getline(, stdin, '\n') != EOF) {
+   while (strbuf_getline_crlf(, stdin) != EOF) {
if (data.split_on_whitespace) {
/*
 * Split at first whitespace, tying off the beginning
-- 
2.7.0-rc1-83-ga8b6b9e

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


[PATCH v2 16/17] grep: read -f file with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
List of patterns file could come from a DOS editor.

This is iffy; you may actually be trying to find a line with ^M in
it on a system whose line ending is LF.  You can of course work it
around by having a line that has "^M^M^J", let the strbuf_getline_crlf() eat
the last "^M^J", leaving just the single "^M" as the pattern.

Signed-off-by: Junio C Hamano 
---
 builtin/grep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index d04f440..df162f1 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -562,7 +562,7 @@ static int file_callback(const struct option *opt, const 
char *arg, int unset)
patterns = from_stdin ? stdin : fopen(arg, "r");
if (!patterns)
die_errno(_("cannot open '%s'"), arg);
-   while (strbuf_getline(, patterns, '\n') == 0) {
+   while (strbuf_getline_crlf(, patterns) == 0) {
/* ignore empty line like grep does */
if (sb.len == 0)
continue;
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 09/17] ident.c: read /etc/mailname with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
Just in case /etc/mailname file was edited with a DOS editor,
read it with strbuf_getline_crlf() so that a stray CR is not included
as the last character of the mail hostname.

We _might_ want to more aggressively discard whitespace characters
around the line with strbuf_trim(), but that is a bit outside the
scope of this series.

Signed-off-by: Junio C Hamano 
---
 ident.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ident.c b/ident.c
index 5ff1aad..9018df0 100644
--- a/ident.c
+++ b/ident.c
@@ -55,7 +55,7 @@ static int add_mailname_host(struct strbuf *buf)
strerror(errno));
return -1;
}
-   if (strbuf_getline(, mailname, '\n') == EOF) {
+   if (strbuf_getline_crlf(, mailname) == EOF) {
if (ferror(mailname))
warning("cannot read /etc/mailname: %s",
strerror(errno));
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 06/17] hash-object: read --stdin-paths with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
The list of paths could have been written with a DOS editor.

Signed-off-by: Junio C Hamano 
---
 builtin/hash-object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 43b098b..57c743d 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -60,7 +60,7 @@ static void hash_stdin_paths(const char *type, int 
no_filters, unsigned flags,
 {
struct strbuf buf = STRBUF_INIT, nbuf = STRBUF_INIT;
 
-   while (strbuf_getline(, stdin, '\n') != EOF) {
+   while (strbuf_getline_crlf(, stdin) != EOF) {
if (buf.buf[0] == '"') {
strbuf_reset();
if (unquote_c_style(, buf.buf, NULL))
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 00/17] Peace with CRLF

2015-12-16 Thread Junio C Hamano
We have too many topics titled "War on something"; let's try to make
peace for a change.

This is a reroll with small changes of the previous one that appears
here:

  http://thread.gmane.org/gmane.comp.version-control.git/280401

which is a continuation to $gmane/275735, which is filed in the
archive under another mailing list:

  http://thread.gmane.org/gmane.comp.version-control.msysgit/21773

We read "text" files from filesystem (or from the standard input) in
various places in the code.  Some of them are written by us, but
others are often prepared by an editor driven by human user.  Even
though we write (and expect) lines in these text files to be
terminated with LF (not CRLF), the end-user's editor can be told to
use CRLF termination, and on some platforms that may be the default.

Because many codepaths (e.g. commit log message) first pass the
contents of such a file through stripspace(), and as a side effect
of discarding the whitespace at the end of each line, lines
terminated with CRLF are normalized to LF terminated lines (but we
do not lose a lone CR in the middle of a non-blank string), this is
not a problem in many codepaths.  But not all of the codepaths are
safe.

Typically, we use strbuf_getline() to read these "text" files, which
reads a single line up to the first LF into a buffer, discard that
LF at the end, and returns (an incomplete last line gets returned
as-is).  In theory, these places that expect to read "text", we
should be able to update the logic to read a line up to the first
LF, discard that LF, together with a CR if one appears immediately
before that LF, without breaking anything.

I inspected all the callsites of this function to see if it is safe
to use such an updated logic at these callsites, and did not find
anything problematic.  I could update strbuf_getline() in place, but
just to be extra careful, this series instead introduces another
helper, strbuf_getline_crlf(), that is aware of this CRLF business,
and convert the ones that are safe to update as we verify.

 * This series converts only the callers of strbuf_getline() that
   would misbehave when fed a file with CRLF-terminated lines and
   use the data with an unwanted CR appended at the end.  With the
   update the code should work as intended with such a file, without
   breaking the expected behaviour when working on a file with
   LF-terminated lines.

 * Callers of strbuf_getline() that expect to only read from our own
   output do not have to accommodate CRLF-terminated lines, but they
   can be updated to do so safely if we do not rely on the ability
   to express a payload that has CR at the end.  For example, the
   insn sheet "rebase -i" uses typically ends each line with a
   commit log summary that we ourselves do not read or use, so even
   if the end-user on a platform with LF lines deliberately does
   insert \r at the end of the line and strbuf_gets() removed that
   \r from the payload, no unexpected behaviour should happen.

   This series does not touch them, but it may be a good GSoC
   microproject to convert them to use strbuf_getline_crlf().

 * Callers of strbuf_getline() that call strbuf_trim() immediately
   on the result before doing anything, or otherwise have logic that
   tolerates whitespaces at the end of the line, can continue using
   strbuf_getline() and will not misbehave on CRLF-terminated lines.

   This series does not touch them; converting them to use
   strbuf_getline_crlf() would a good way to document them as
   dealing with "text".  While doing so, they can also lose their
   custom logic that happens to make CRLF-terminated lines work.

 * A caller of strbuf_getline() that wants to only see LF-terminated
   lines (in other words, "ABC\r\n" must be treated as a line with 4
   bytes payload on it, A B C and CR), would be broken if we replace
   it with strbuf_getline_crlf().  This series does not touch them,
   and no follow-up series should, either.

An ideal endgame would be to retire the current strbuf_getline() and
use strbuf_getline_crlf() everywhere.  For that to happen, follow-up
topics to cover the second and the third class of callers above need
to show that there is no remaining callers to strbuf_getline() that
falls into the fourth category.

Even better, once you say "line", as in strbuf_getline(), you ought
to be talking about "text", so we could rename strbuf_getline_crlf()
back to strbuf_getline() after all of the above happens.


Junio C Hamano (17):
  strbuf: make strbuf_getline_crlf() global
  check-attr, check-ignore, checkout-index: read paths with
strbuf_getline_crlf()
  update-index: read --index-info with strbuf_getline_crlf()
  update-index: read list of paths with strbuf_getline_crlf() under
--stdin
  mktree: read textual tree representation with strbuf_getline_crlf()
  hash-object: read --stdin-paths with strbuf_getline_crlf()
  revision: read --stdin with strbuf_getline_crlf()
  rev-parse: read parseopt spec with 

[PATCH v2 14/17] column: read lines with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
Multiple lines read here are concatenated on a single line to form a
multi-column output line.  We do not want to have a CR at the end,
even if the input file consists of CRLF terminated lines.

Signed-off-by: Junio C Hamano 
---
 builtin/column.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/column.c b/builtin/column.c
index 449413c..3205aa9 100644
--- a/builtin/column.c
+++ b/builtin/column.c
@@ -51,7 +51,7 @@ int cmd_column(int argc, const char **argv, const char 
*prefix)
die(_("--command must be the first argument"));
}
finalize_colopts(, -1);
-   while (!strbuf_getline(, stdin, '\n'))
+   while (!strbuf_getline_crlf(, stdin))
string_list_append(, sb.buf);
 
print_columns(, colopts, );
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 02/17] check-attr, check-ignore, checkout-index: read paths with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
These commands read list of paths from their standard input under
the --stdin option (in order to avoid busting limit on the length of
the command line).

When they are using text input mode (i.e. line_termination is set to
'\n'), we should try to be more friendly to our DOSsy friends and
accept lines with CRLF endings.

It is tempting to lift this logic to strbuf_getline() and not
introduce a separate strbuf_getline_crlf(), but that can lead to silent
misconversion.

Signed-off-by: Junio C Hamano 
---
 builtin/check-attr.c | 4 +++-
 builtin/check-ignore.c   | 5 -
 builtin/checkout-index.c | 4 +++-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 265c9ba..4c44d8f 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -77,7 +77,9 @@ static void check_attr_stdin_paths(const char *prefix, int 
cnt,
 
strbuf_init(, 0);
strbuf_init(, 0);
-   while (strbuf_getline(, stdin, line_termination) != EOF) {
+   while ((line_termination
+   ? strbuf_getline_crlf(, stdin)
+   : strbuf_getline(, stdin, '\0')) != EOF) {
if (line_termination && buf.buf[0] == '"') {
strbuf_reset();
if (unquote_c_style(, buf.buf, NULL))
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 43f3617..862ced1 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -122,7 +122,10 @@ static int check_ignore_stdin_paths(struct dir_struct 
*dir, const char *prefix)
 
strbuf_init(, 0);
strbuf_init(, 0);
-   while (strbuf_getline(, stdin, line_termination) != EOF) {
+
+   while ((line_termination
+   ? strbuf_getline_crlf(, stdin)
+   : strbuf_getline(, stdin, '\0')) != EOF) {
if (line_termination && buf.buf[0] == '"') {
strbuf_reset();
if (unquote_c_style(, buf.buf, NULL))
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 8028c37..27d65f2 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -258,7 +258,9 @@ int cmd_checkout_index(int argc, const char **argv, const 
char *prefix)
if (all)
die("git checkout-index: don't mix '--all' and 
'--stdin'");
 
-   while (strbuf_getline(, stdin, line_termination) != EOF) {
+   while ((line_termination
+   ? strbuf_getline_crlf(, stdin)
+   : strbuf_getline(, stdin, '\0')) != EOF) {
char *p;
if (line_termination && buf.buf[0] == '"') {
strbuf_reset();
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 12/17] transport-helper: read helper response with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
Our implementation of helpers never use CRLF line endings, and they
do not depend on the ability to place a CR as payload at the end of
the line, so this is essentially a no-op for in-tree users.  However,
this allows third-party implementation of helpers to give us their
line with CRLF line ending (they cannot expect us to feed CRLF to
them, though).

Signed-off-by: Junio C Hamano 
---
 transport-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport-helper.c b/transport-helper.c
index 63d5427..7de52e1 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -54,7 +54,7 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer, 
const char *name)
strbuf_reset(buffer);
if (debug)
fprintf(stderr, "Debug: Remote helper: Waiting...\n");
-   if (strbuf_getline(buffer, helper, '\n') == EOF) {
+   if (strbuf_getline_crlf(buffer, helper) == EOF) {
if (debug)
fprintf(stderr, "Debug: Remote helper quit.\n");
return 1;
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 04/17] update-index: read list of paths with strbuf_getline_crlf() under --stdin

2015-12-16 Thread Junio C Hamano
Signed-off-by: Junio C Hamano 
---
 builtin/update-index.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index a7a9a7e..3a6c5b2 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1075,7 +1075,10 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
struct strbuf buf = STRBUF_INIT, nbuf = STRBUF_INIT;
 
setup_work_tree();
-   while (strbuf_getline(, stdin, line_termination) != EOF) {
+
+   while ((line_termination
+   ? strbuf_getline_crlf(, stdin)
+   : strbuf_getline(, stdin, '\0')) != EOF) {
char *p;
if (line_termination && buf.buf[0] == '"') {
strbuf_reset();
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 03/17] update-index: read --index-info with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
Signed-off-by: Junio C Hamano 
---
 builtin/update-index.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7431938..a7a9a7e 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -473,7 +473,9 @@ static void read_index_info(int line_termination)
struct strbuf buf = STRBUF_INIT;
struct strbuf uq = STRBUF_INIT;
 
-   while (strbuf_getline(, stdin, line_termination) != EOF) {
+   while ((line_termination
+   ? strbuf_getline_crlf(, stdin)
+   : strbuf_getline(, stdin, '\0')) != EOF) {
char *ptr, *tab;
char *path_name;
unsigned char sha1[20];
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 07/17] revision: read --stdin with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
Reading with getwholeline() and manually stripping the terminating
'\n' would leave CR at the end of the line if the input comes from
a DOS editor.

Contrasting this with the previous few changes, one may realize that
the way "log" family of commands read the paths with --stdin looks
inconsistent and sloppy.  It does not allow us to C-quote a textual
input, and it does not accept NUL-terminated records.  These are
unfortunately way too late to fix X-<.

Signed-off-by: Junio C Hamano 
---
 revision.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/revision.c b/revision.c
index 2236463..651a34b 100644
--- a/revision.c
+++ b/revision.c
@@ -1641,10 +1641,7 @@ static void append_prune_data(struct cmdline_pathspec 
*prune, const char **av)
 static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb,
 struct cmdline_pathspec *prune)
 {
-   while (strbuf_getwholeline(sb, stdin, '\n') != EOF) {
-   int len = sb->len;
-   if (len && sb->buf[len - 1] == '\n')
-   sb->buf[--len] = '\0';
+   while (strbuf_getline_crlf(sb, stdin) != EOF) {
ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
prune->path[prune->nr++] = xstrdup(sb->buf);
}
@@ -1661,10 +1658,8 @@ static void read_revisions_from_stdin(struct rev_info 
*revs,
warn_on_object_refname_ambiguity = 0;
 
strbuf_init(, 1000);
-   while (strbuf_getwholeline(, stdin, '\n') != EOF) {
+   while (strbuf_getline_crlf(, stdin) != EOF) {
int len = sb.len;
-   if (len && sb.buf[len - 1] == '\n')
-   sb.buf[--len] = '\0';
if (!len)
break;
if (sb.buf[0] == '-') {
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 08/17] rev-parse: read parseopt spec with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
"rev-parse --parseopt" specification is clearly text and we
should anticipate that we may be fed CRLF lines.

Signed-off-by: Junio C Hamano 
---
 builtin/rev-parse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 02d747d..f2cf99c 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -386,7 +386,7 @@ static int cmd_parseopt(int argc, const char **argv, const 
char *prefix)
 
/* get the usage up to the first line with a -- on it */
for (;;) {
-   if (strbuf_getline(, stdin, '\n') == EOF)
+   if (strbuf_getline_crlf(, stdin) == EOF)
die("premature end of input");
ALLOC_GROW(usage, unb + 1, usz);
if (!strcmp("--", sb.buf)) {
@@ -399,7 +399,7 @@ static int cmd_parseopt(int argc, const char **argv, const 
char *prefix)
}
 
/* parse: (|,|)[*=?!]*? SP+  */
-   while (strbuf_getline(, stdin, '\n') != EOF) {
+   while (strbuf_getline_crlf(, stdin) != EOF) {
const char *s;
const char *help;
struct option *o;
-- 
2.7.0-rc1-83-ga8b6b9e

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


[PATCH v2 11/17] clone/sha1_file: read info/alternates with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
$GIT_OBJECT_DIRECTORY/info/alternates is a text file that can be
edited with a DOS editor.  We do not want to use the real path with
CR appended at the end.

Signed-off-by: Junio C Hamano 
---
 builtin/clone.c | 2 +-
 sha1_file.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 9eaecd9..0a85243 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -339,7 +339,7 @@ static void copy_alternates(struct strbuf *src, struct 
strbuf *dst,
FILE *in = fopen(src->buf, "r");
struct strbuf line = STRBUF_INIT;
 
-   while (strbuf_getline(, in, '\n') != EOF) {
+   while (strbuf_getline_crlf(, in) != EOF) {
char *abs_path;
if (!line.len || line.buf[0] == '#')
continue;
diff --git a/sha1_file.c b/sha1_file.c
index 50896ff..7a748f2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -395,7 +395,7 @@ void add_to_alternates_file(const char *reference)
struct strbuf line = STRBUF_INIT;
int found = 0;
 
-   while (strbuf_getline(, in, '\n') != EOF) {
+   while (strbuf_getline_crlf(, in) != EOF) {
if (!strcmp(reference, line.buf)) {
found = 1;
break;
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 05/17] mktree: read textual tree representation with strbuf_getline_crlf()

2015-12-16 Thread Junio C Hamano
The input can come from a DOS editor.

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

diff --git a/builtin/mktree.c b/builtin/mktree.c
index a964d6b..c6cafb6 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -157,7 +157,9 @@ int cmd_mktree(int ac, const char **av, const char *prefix)
 
while (!got_eof) {
while (1) {
-   if (strbuf_getline(, stdin, line_termination) == 
EOF) {
+   if ((line_termination
+? strbuf_getline_crlf(, stdin)
+: strbuf_getline(, stdin, '\0')) == EOF) {
got_eof = 1;
break;
}
-- 
2.7.0-rc1-83-ga8b6b9e

--
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 06/11] ref-filter: introduce color_atom_parser()

2015-12-16 Thread Eric Sunshine
On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak  wrote:
> Introduce color_atom_parser() which will parse a "color" atom and
> store its color in the "use_atom" structure for further usage in

Same comment as last time: s/use_atom/used_atom/

> 'populate_value()'.

s/'//g

More below...

> Helped-by: Ramsay Jones 
> Signed-off-by: Karthik Nayak 
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -29,6 +29,9 @@ typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } 
> cmp_type;
>  static struct used_atom {
> const char *str;
> cmp_type type;
> +   union {
> +   const char *color;
> +   } u;
>  } *used_atom;
>  static int used_atom_cnt, need_tagged, need_symref;
>  static int need_color_reset_at_eol;
> @@ -53,6 +56,13 @@ static int match_atom_name(const char *name, const char 
> *atom_name, const char *
> +static void color_atom_parser(struct used_atom *atom)
> +{
> +   match_atom_name(atom->str, "color", >u.color);
> +   if (!atom->u.color)
> +   die(_("expected format: %%(color:)"));
> +}
> +
> @@ -833,12 +846,10 @@ static void populate_value(struct ref_array_item *ref)
> refname = branch_get_push(branch, NULL);
> if (!refname)
> continue;
> -   } else if (match_atom_name(name, "color", )) {
> +   } else if (starts_with(name, "color:")) {
> char color[COLOR_MAXLEN] = "";
>
> -   if (!valp)
> -   die(_("expected format: %%(color:)"));
> -   if (color_parse(valp, color) < 0)
> +   if (color_parse(atom->u.color, color) < 0)

It would make a lot more sense to invoke color_parse() with the
unchanging argument to "color:" just once in color_atom_parser()
rather than doing it here repeatedly. (Also, you'd drop 'const' from
used_atom.u.color declaration.)

> die(_("unable to parse format"));
> v->s = xstrdup(color);

Does v->s get freed each time through the loop? If not, then, assuming
you parse the color just once in color_atom_parser(), then you could
just assign the parsed color directly to v->s rather than duplicating
it.

> continue;
> --
> 2.6.4
--
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-gui: do not use obsolete `git merge $msg HEAD $branch'

2015-12-16 Thread Johannes Sixt
Use the modern `git merge' invocation pattern.

Since both `git merge' and `git fmt-merge-msg' obey the merge.log
configuration, instruct the former not to generate the log summary to
avoid that it appears twice in the commit message.

Signed-off-by: Johannes Sixt 
---
 lib/merge.tcl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/merge.tcl b/lib/merge.tcl
index 460d32f..f512456 100644
--- a/lib/merge.tcl
+++ b/lib/merge.tcl
@@ -115,8 +115,9 @@ method _start {} {
set cmd [list git]
lappend cmd merge
lappend cmd --strategy=recursive
+   lappend cmd --no-log
+   lappend cmd -m
lappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]]
-   lappend cmd HEAD
lappend cmd $name
 
ui_status [mc "Merging %s and %s..." $current_branch $stitle]
-- 
2.6.2.337.ga235d84

--
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 07/11] ref-filter: introduce align_atom_parser()

2015-12-16 Thread Eric Sunshine
On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak  wrote:
> Introduce align_atom_parser() which will parse an "align" atom and
> store the required alignment position and width in the "use_atom"
> structure for further usage in 'populate_value()'.
>
> Helped-by: Ramsay Jones 
> Signed-off-by: Karthik Nayak 
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -63,6 +69,49 @@ static void color_atom_parser(struct used_atom *atom)
> +static align_type parse_align_position(const char *s)
> +{
> +   if (!strcmp(s, "right"))
> +   return ALIGN_RIGHT;
> +   else if (!strcmp(s, "middle"))
> +   return ALIGN_MIDDLE;
> +   else if (!strcmp(s, "left"))
> +   return ALIGN_LEFT;
> +   return -1;
> +}
> +
> +static void align_atom_parser(struct used_atom *atom)
> +{
> +   struct align *align = >u.align;
> +   const char *buf = NULL;
> +   struct strbuf **s, **to_free;
> +   int width = -1;
> +
> +   match_atom_name(atom->str, "align", );
> +   if (!buf)
> +   die(_("expected format: %%(align:,)"));
> +   s = to_free = strbuf_split_str_omit_term(buf, ',', 0);
> +
> +   align->position = ALIGN_LEFT;
> +
> +   while (*s) {
> +   int position;
> +
> +   if (!strtoul_ui(s[0]->buf, 10, (unsigned int *)))

This casting is pretty ugly. It probably would be cleaner to declare
'width' as 'unsigned int' and initialize it with ~0U than to do this
ugly and potentially dangerous casting. Likewise, below where you
check 'width < 0', you'd check instead for ~0U. However, such a change
should not be part of the current patch, but rather as a separate
preparatory patch.

> +   ;
> +   else if ((position = parse_align_position(s[0]->buf)) > 0)

Shouldn't this be '>=' rather than '>'?

This likely would have been easier to spot if parse_align_position()
had been factored out in its own patch, as suggested by my v1
review[1], since the caller would have been trivially inspectable
rather than having to keep track of both code movement and changes.

[1]: http://article.gmane.org/gmane.comp.version-control.git/281916

> +   align->position = position;
> +   else
> +   die(_("unrecognized %%(align) argument: %s"), 
> s[0]->buf);
> +   s++;
> +   }
> +
> +   if (width < 0)
> +   die(_("positive width expected with the %%(align) atom"));
> +   align->width = width;
> +   strbuf_list_free(to_free);
> +}
> +
>  static struct {
> const char *name;
> cmp_type cmp_type;
> @@ -880,35 +924,7 @@ static void populate_value(struct ref_array_item *ref)
> v->s = " ";
> continue;
> } else if (match_atom_name(name, "align", )) {
> -   struct align *align = >u.align;
> -   struct strbuf **s, **to_free;
> -   int width = -1;
> -
> -   if (!valp)
> -   die(_("expected format: 
> %%(align:,)"));
> -
> -   s = to_free = strbuf_split_str_omit_term(valp, ',', 
> 0);
> -
> -   align->position = ALIGN_LEFT;
> -
> -   while (*s) {
> -   if (!strtoul_ui(s[0]->buf, 10, (unsigned int 
> *)))
> -   ;
> -   else if (!strcmp(s[0]->buf, "left"))
> -   align->position = ALIGN_LEFT;
> -   else if (!strcmp(s[0]->buf, "right"))
> -   align->position = ALIGN_RIGHT;
> -   else if (!strcmp(s[0]->buf, "middle"))
> -   align->position = ALIGN_MIDDLE;
> -   else
> -   die(_("improper format entered 
> align:%s"), s[0]->buf);
> -   s++;
> -   }
> -
> -   if (width < 0)
> -   die(_("positive width expected with the 
> %%(align) atom"));
> -   align->width = width;
> -   strbuf_list_free(to_free);
> +   v->u.align = atom->u.align;
> v->handler = align_atom_handler;
> continue;
> } else if (!strcmp(name, "end")) {
> --
> 2.6.4
--
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: Odd rebase behavior

2015-12-16 Thread John Keeping
On Tue, Dec 15, 2015 at 09:17:30PM -0600, David A. Greene wrote:
> According to the rebase man page, rebase gathers commits as in "git log
> ..HEAD."  However, that is not what happens in the tests
> below.  Some of the commits disappear.
> 
> The test basically does this:
> 
> - Setup a master project and a subproject, merged via a subtree-like
>   merge (this is how git-subtree does it).
> 
> - Add some commits to the subproject directory after the subtree merge,
>   to create some history not in the original subproject.
> 
> - filter-branch --subdirectory-filter to extract commits from the
>   subproject directory.
> 
> - Rebase those commits back on to the original subproject repository.
> 
> The above loses all commits made after the subproject is merged into
> the main project.
[snip]
> # Does not preserve master4 and master5.
> test_expect_success 'Rebase default' '
>   git checkout -b rebase-default master &&
>   git filter-branch --prune-empty -f --subdirectory-filter files_subtree 
> &&
>   git commit -m "Empty commit" --allow-empty &&
>   git rebase -Xsubtree=files_subtree  --preserve-merges --onto 
> files-master master &&

It seems that the problem is introduces by --preserve-merges (and
-Xsubtree causes something interesting to happen as well).  I see the
following behaviour:

git rebase --onto files-master master

Works (master4 and master5 preserved).

git rebase --preserve-merges --onto files-master master

Behaves as described above (master4 and master5 are lost).

git rebase -Xsubtree=files_subtree --onto files-master master

fatal: Could not parse object 
'b15c4133fc3146e1330c84159886f0f7a09fbf43^'
Unknown exit code (128) from command: git-merge-recursive 
b15c4133fc3146e1330c84159886f0f7a09fbf43^ -- HEAD 
b15c4133fc3146e1330c84159886f0f7a09fbf43 

git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master

Same as the version with only --preserve-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: [PATCH 1/2] push: Fully test --recurse-submodules on command line overrides config

2015-12-16 Thread Stefan Beller
On Wed, Dec 16, 2015 at 2:41 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> On Thu, Dec 3, 2015 at 5:10 AM, Mike Crowe  wrote:
>>> t5531 only checked that the push.recurseSubmodules config option was
>>> overridden by passing --recurse-submodules=check on the command line.
>>> Add new tests for overriding with --recurse-submodules=no,
>>> --no-recurse-submodules and --recurse-submodules=push too.
>>>
>>> Also correct minor typo in test commit message.
>>>
>>> Signed-off-by: Mike Crowe 
>>
>> This looks good to me.
>>
>> Thanks,
>> Stefan
>
> Thanks.  Does "This" refer to 1/2 alone or the whole series?

Yes. :)

"This" is applicable to both patches. We had the discussion on 2/2 about me
misreading a line a few days earlier, but apart from that it looked good, too.
--
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 v1 1/4] git-gui: remove duplicate entries from .gitconfig's gui.recentrepo

2015-12-16 Thread Philip Oakley

From: "Eric Sunshine" 

On Monday, December 14, 2015, Philip Oakley  wrote:

The git gui's recent repo list may become contaminated with duplicate
entries. The git gui would barf when attempting to remove one entry.
Remove them all - there is no option within 'git config' to selectively
remove one of the entries.

This issue was reported on the 'Git User' list
(https://groups.google.com/forum/#!topic/git-users/msev4KsQGFc,
Warning: gui.recentrepo has multiply values while executing).


s/multiply/multiple/


The original report has that mistake. I'll quote the thread title.



On startup the gui checks that entries in the recentrepo list are still
valid repos and deletes thoses that are not. If duplicate entries are


s/thoses/those/

OK




present the 'git config --unset' will barf and this prevents the gui


s/present the/present, then/


OK
[my first glance thought the change was to the 'prevents the' at the end...]



from starting.

Subsequent patches fix other parts of recentrepo logic used for syncing
internal lists with the external .gitconfig.

Reported-by: Alexey Astakhov 
Signed-off-by: Philip Oakley 
---
diff --git a/git-gui/lib/choose_repository.tcl 
b/git-gui/lib/choose_repository.tcl

index 75d1da8..133ca0a 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -247,7 +247,7 @@ proc _get_recentrepos {} {

 proc _unset_recentrepo {p} {
regsub -all -- {([()\[\]{}\.^$+*?\\])} $p {\\\1} p
-   git config --global --unset gui.recentrepo "^$p\$"
+   git config --global --unset-all gui.recentrepo "^$p\$"
load_config 1
 }

--

Will re-roll to give Pat the opportunity to include in any 2.7 update.

Philip 


--
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 2/4] git gui: cope with duplicates in _get_recentrepo

2015-12-16 Thread Philip Oakley
_get_recentrepo will fail if duplicate invalid entries are present
in the recentrepo config list. The previous commit fixed the
'git config' limitations in _unset_recentrepo by unsetting all config
entries, however this code would fail on the second attempt to unset it.

Refactor the code to pre-sort and de-duplicate the recentrepo list to
avoid a potential second unset attempt.

Signed-off-by: Philip Oakley 
---
 git-gui/lib/choose_repository.tcl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-gui/lib/choose_repository.tcl 
b/git-gui/lib/choose_repository.tcl
index 133ca0a..aa87bcc 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -235,14 +235,14 @@ method _invoke_next {} {
 
 proc _get_recentrepos {} {
set recent [list]
-   foreach p [get_config gui.recentrepo] {
+   foreach p [lsort -unique [get_config gui.recentrepo]] {
if {[_is_git [file join $p .git]]} {
lappend recent $p
} else {
_unset_recentrepo $p
}
}
-   return [lsort $recent]
+   return $recent
 }
 
 proc _unset_recentrepo {p} {
-- 
2.6.4.windows.1

--
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 3/4] git gui: de-dup selected repo from recentrepo history

2015-12-16 Thread Philip Oakley
When the gui/user selects a repo for display, that repo is brought to
the end of the recentrepo config list. The logic can fail if there are
duplicate old entries for the repo (you cannot unset a single config
entry when duplicates are present).

Similarly, the maxrecentrepo logic could fail if older duplicate entries
are present.

The first commit of this series ({this}~2) fixed the config unsetting
issue. Rather than manipulating a local copy of the $recent list (one
cannot know how many entries were removed), simply re-read it.

Signed-off-by: Philip Oakley 
---
 git-gui/lib/choose_repository.tcl | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/git-gui/lib/choose_repository.tcl 
b/git-gui/lib/choose_repository.tcl
index aa87bcc..ad7a888 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -262,12 +262,11 @@ proc _append_recentrepos {path} {
set i [lsearch $recent $path]
if {$i >= 0} {
_unset_recentrepo $path
-   set recent [lreplace $recent $i $i]
}
 
-   lappend recent $path
git config --global --add gui.recentrepo $path
load_config 1
+   set recent [get_config gui.recentrepo]
 
if {[set maxrecent [get_config gui.maxrecentrepo]] eq {}} {
set maxrecent 10
@@ -275,7 +274,7 @@ proc _append_recentrepos {path} {
 
while {[llength $recent] > $maxrecent} {
_unset_recentrepo [lindex $recent 0]
-   set recent [lrange $recent 1 end]
+   set recent [get_config gui.recentrepo]
}
 }
 
-- 
2.6.4.windows.1

--
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 4/4] git gui: allow for a long recentrepo list

2015-12-16 Thread Philip Oakley
The gui.recentrepo list may be longer than the maxrecent setting.
Allow extra space to show any extra entries.

In an ideal world, the git gui would limit the number of entries
to the maxrecent setting, however the recentrepo config list may
have been extended outside of the gui, or the maxrecent setting changed
to a reduced value. Further, when testing the gui's recentrepo
logic it is useful to show these extra, but valid, entries.

Signed-off-by: Philip Oakley 
---
word usage corrected.
Eric's comments $gmane/282432
---
 git-gui/lib/choose_repository.tcl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-gui/lib/choose_repository.tcl 
b/git-gui/lib/choose_repository.tcl
index ad7a888..a08ed4f 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -153,7 +153,7 @@ constructor pick {} {
-background [get_bg_color $w_body.recentlabel] \
-wrap none \
-width 50 \
-   -height $maxrecent
+   -height [expr {$maxrecent + 5}]
$w_recentlist tag conf link \
-foreground blue \
-underline 1
-- 
2.6.4.windows.1

--
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 1/4] git-gui: de-dup entries from gui.recentrepo

2015-12-16 Thread Philip Oakley
The git gui's recent repo list may become contaminated with duplicate
entries. The git gui would barf when attempting to remove one entry.
Remove them all - there is no option within 'git config' to selectively
remove one of the entries.

This issue was reported on the 'Git User' list
(https://groups.google.com/forum/#!topic/git-users/msev4KsQGFc,
"Warning: gui.recentrepo has multiply values while executing").

On startup the gui checks that entries in the recentrepo list are still
valid repos and deletes those that are not. If duplicate entries are
present, then 'git config --unset' will barf and this prevents the gui
from starting.

Subsequent patches fix other parts of recentrepo logic used for syncing
internal lists with the external .gitconfig.

Reported-by: Alexey Astakhov 
Signed-off-by: Philip Oakley 
---
tightened the patch title
user report thread title now quoted
spelling corrected
fixed grammar (comma)
Eric's comments $gmane/282430
---
 git-gui/lib/choose_repository.tcl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-gui/lib/choose_repository.tcl 
b/git-gui/lib/choose_repository.tcl
index 75d1da8..133ca0a 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -247,7 +247,7 @@ proc _get_recentrepos {} {
 
 proc _unset_recentrepo {p} {
regsub -all -- {([()\[\]{}\.^$+*?\\])} $p {\\\1} p
-   git config --global --unset gui.recentrepo "^$p\$"
+   git config --global --unset-all gui.recentrepo "^$p\$"
load_config 1
 }
 
-- 
2.6.4.windows.1

--
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] submodule: Port resolve_relative_url from shell to C

2015-12-16 Thread Stefan Beller
On Wed, Dec 9, 2015 at 10:48 PM, Johannes Sixt  wrote:
> Am 10.12.2015 um 02:07 schrieb Stefan Beller:
>>
>> This reimplements the helper function `resolve_relative_url` in shell
>> in C. This functionality is needed in C for introducing the groups
>> feature later on. When using groups, the user should not need to run
>> `git submodule init`, but it should be implicit at all appropriate places,
>> which are all in C code. As the we would not just call out to `git
>> submodule init`, but do a more fine grained structure there, we actually
>> need all the init functionality in C before attempting the groups
>> feature. To get the init functionality in C, rewriting the
>> resolve_relative_url subfunction is a major step.
>
>
> I see lots of '/', but no is_dir_sep() in the C version. Did you consider
> that local URLs can use a backslash as path separator on Windows? In the
> shell version, this did not matter because bash converts the backslashes to
> forward slashes for us. But when rewritten in C, this does not happen.

I see. That's a pity. :(

>
> Valid URLs are
>
>   D:\foo\bar.git
>   \\server\share\foo\bar
>   ..\..\foo\bar

I am staring at the code in desperation as backslashes
in Linux are valid for file names, i.e.:
"/tmp/testfile\" is a valid filename

Now look at the code where the first slash occurs, it's

if (strip_suffix(remoteurl, "/", ))
remoteurl[len] = '\0';

The intention is to strip off the last character if it is a directory separator.
So in the unix world we want to keep "/tmp/testfile\" as it is,
whereas in Windows
we want to chop off the backslash (because there is no file with a
backslash allowed,
it is the directory separator?)

So what I think I am going to do for next round is something like

static int has_same_dir_prefix(const char *str, const char **out)
{
#ifdef GIT_WINDOWS_NATIVE
return skip_prefix(str, "./", out)
   || skip_prefix(str, ".\\", out);
#else
return skip_prefix(str, "./", out);
#endif
}

static int has_upper_dir_prefix(const char *str, const char **out)
{
#ifdef GIT_WINDOWS_NATIVE
return skip_prefix(str, "../", out)
   || skip_prefix(str, "..\\", out);
#else
return skip_prefix(str, "../", out);
#endif
}

in the submodule helper function, or alternatively in the wrapper.c ?
and then rely on these functions being accurate.

Would that approach make sense?

Thanks,
Stefan


>
> and all of them with some or all backslashes replaced by forward slashes.
>
> See also connect.c:url_is_local_not_ssh, which ensures that the first
> example above is considered a local path with a drive letter, not a remote
> ssh path.
>
>
>>
>> This also improves the performance:
>> (Best out of 3) time ./t7400-submodule-basic.sh
>> Before:
>> real0m9.575s
>> user0m2.683s
>> sys 0m6.773s
>> After:
>> real0m9.293s
>> user0m2.691s
>> sys 0m6.549s
>>
>> Signed-off-by: Stefan Beller 
>> ---
>>
>>   This applies on origin/master, and I'd carry as its own feature branch
>>   as I am nowhere near done with the groups feature after reading Jens
>> feedback.
>>   (It took me a while to identify this as a next best step.)
>>
>>   Thanks,
>>   Stefan
>>
>>   builtin/submodule--helper.c | 120
>> 
>>   git-submodule.sh|  81 ++
>>   2 files changed, 124 insertions(+), 77 deletions(-)
>>
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> index f4c3eff..f48b5b5 100644
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -9,6 +9,125 @@
>>   #include "submodule-config.h"
>>   #include "string-list.h"
>>   #include "run-command.h"
>> +#include "remote.h"
>> +#include "refs.h"
>> +
>> +static const char *get_default_remote(void)
>> +{
>> +   char *dest = NULL;
>> +   unsigned char sha1[20];
>> +   int flag;
>> +   struct strbuf sb = STRBUF_INIT;
>> +   const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, );
>> +
>> +   if (!refname)
>> +   die("No such ref: HEAD");
>> +
>> +   refname = shorten_unambiguous_ref(refname, 0);
>> +   strbuf_addf(, "branch.%s.remote", refname);
>> +   if (git_config_get_string(sb.buf, ))
>> +   return "origin";
>> +   else
>> +   return xstrdup(dest);
>> +}
>> +
>> +/*
>> + * The function takes at most 2 arguments. The first argument is the
>> + * URL that navigates to the submodule origin repo. When relative, this
>> URL
>> + * is relative to the superproject origin URL repo. The second up_path
>> + * argument, if specified, is the relative path that navigates
>> + * from the submodule working tree to the superproject working tree.
>> + *
>> + * The output of the function is the origin URL of the submodule.
>> + *
>> + * The output will either be an absolute URL or filesystem path (if the
>> + * 

Re: [PATCH 1/2] push: Fully test --recurse-submodules on command line overrides config

2015-12-16 Thread Junio C Hamano
Stefan Beller  writes:

> On Thu, Dec 3, 2015 at 5:10 AM, Mike Crowe  wrote:
>> t5531 only checked that the push.recurseSubmodules config option was
>> overridden by passing --recurse-submodules=check on the command line.
>> Add new tests for overriding with --recurse-submodules=no,
>> --no-recurse-submodules and --recurse-submodules=push too.
>>
>> Also correct minor typo in test commit message.
>>
>> Signed-off-by: Mike Crowe 
>
> This looks good to me.
>
> Thanks,
> Stefan

Thanks.  Does "This" refer to 1/2 alone or the whole series?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] push: add '-d' as shorthand for '--delete'

2015-12-16 Thread Jeff King
On Wed, Dec 16, 2015 at 12:29:38PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > At this point, it seems that "--delete" is useful, and nothing else has
> > been proposed for "-d" in the intervening years. It seems like a
> > reasonable use of the flag to me.
> 
> I think there were two (and a half) reasons why we didn't let
> "--delete" use a short-and-sweet "-d", and I agree that "something
> else that is more useful did not come" removes one of them.
> 
> The other reason was to avoid the chance of fat-fingering, because
> deleting is destructive, and it is even harder to recover from if
> the damage is done remotely (and the remaining one-half is that
> deleting is a rare event).
> 
> Even though I do not think the need for the "safety" has been
> reduced over time to warrant this change, a similarity with "branch"
> that has "-d/--delete" would be a good enough argument to support
> this change.

Thanks for the input, I hadn't considered "safety" at all. We do have
safety measures on "git branch -d" that we don't have here. I guess we
could implement something similar (e.g., see if the to-be-deleted branch
is merged elsewhere; of course we might not have the objects locally at
all). On the other hand, you can already screw yourself pretty badly
with "push -f".

So I think it's probably OK to add "-d".

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


[PATCH v2 01/17] strbuf: make strbuf_getline_crlf() global

2015-12-16 Thread Junio C Hamano
Often we read "text" files that are supplied by the end user
(e.g. commit log message that was edited with $GIT_EDITOR upon
'git commit -e'), and in some environments lines in a text file
are terminated with CRLF.  Existing strbuf_getline() knows to read
a single line and then strip the terminating byte from the result,
but it is handy to have a version that is more tailored for a "text"
input that takes both '\n' and '\r\n' as line terminator (aka
 in POSIX lingo) and returns the body of the line after
stripping .

Recently reimplemented "git am" uses such a function implemented
privately; move it to strbuf.[ch] and make it available for others.

Signed-off-by: Junio C Hamano 
---
 builtin/am.c | 15 ---
 strbuf.c | 16 ++--
 strbuf.h |  7 +++
 3 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 4e396c8..94a533a 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -45,21 +45,6 @@ static int is_empty_file(const char *filename)
 }
 
 /**
- * Like strbuf_getline(), but treats both '\n' and "\r\n" as line terminators.
- */
-static int strbuf_getline_crlf(struct strbuf *sb, FILE *fp)
-{
-   if (strbuf_getwholeline(sb, fp, '\n'))
-   return EOF;
-   if (sb->buf[sb->len - 1] == '\n') {
-   strbuf_setlen(sb, sb->len - 1);
-   if (sb->len > 0 && sb->buf[sb->len - 1] == '\r')
-   strbuf_setlen(sb, sb->len - 1);
-   }
-   return 0;
-}
-
-/**
  * Returns the length of the first line of msg.
  */
 static int linelen(const char *msg)
diff --git a/strbuf.c b/strbuf.c
index d76f0ae..7ad5ea4 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -505,8 +505,20 @@ int strbuf_getline(struct strbuf *sb, FILE *fp, int term)
 {
if (strbuf_getwholeline(sb, fp, term))
return EOF;
-   if (sb->buf[sb->len-1] == term)
-   strbuf_setlen(sb, sb->len-1);
+   if (sb->buf[sb->len - 1] == term)
+   strbuf_setlen(sb, sb->len - 1);
+   return 0;
+}
+
+int strbuf_getline_crlf(struct strbuf *sb, FILE *fp)
+{
+   if (strbuf_getwholeline(sb, fp, '\n'))
+   return EOF;
+   if (sb->buf[sb->len - 1] == '\n') {
+   strbuf_setlen(sb, sb->len - 1);
+   if (sb->len && sb->buf[sb->len - 1] == '\r')
+   strbuf_setlen(sb, sb->len - 1);
+   }
return 0;
 }
 
diff --git a/strbuf.h b/strbuf.h
index 7123fca..d84c866 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -388,6 +388,13 @@ extern int strbuf_readlink(struct strbuf *sb, const char 
*path, size_t hint);
  */
 extern int strbuf_getline(struct strbuf *, FILE *, int);
 
+/*
+ * Similar to strbuf_getline(), but uses '\n' as the terminator,
+ * and additionally treats a '\r' that comes immediately before '\n'
+ * as part of the terminator.
+ */
+extern int strbuf_getline_crlf(struct strbuf *, FILE *);
+
 /**
  * Like `strbuf_getline`, but keeps the trailing terminator (if
  * any) in the buffer.
-- 
2.7.0-rc1-83-ga8b6b9e

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


[PATCHv2] submodule: Port resolve_relative_url from shell to C

2015-12-16 Thread Stefan Beller
This reimplements the helper function `resolve_relative_url` in shell
in C. This functionality is needed in C for introducing the groups
feature later on. When using groups, the user should not need to run
`git submodule init`, but it should be implicit at all appropriate places,
which are all in C code. As the we would not just call out to `git
submodule init`, but do a more fine grained structure there, we actually
need all the init functionality in C before attempting the groups
feature. To get the init functionality in C, rewriting the
resolve_relative_url subfunction is a major step.

This also improves the performance:
(Best out of 3) time ./t7400-submodule-basic.sh
Before:
real0m9.575s
user0m2.683s
sys 0m6.773s
After:
real0m9.293s
user0m2.691s
sys 0m6.549s

That's about 3%.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 151 
 git-submodule.sh|  81 ++--
 2 files changed, 155 insertions(+), 77 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..b925bed 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -9,6 +9,156 @@
 #include "submodule-config.h"
 #include "string-list.h"
 #include "run-command.h"
+#include "remote.h"
+#include "refs.h"
+
+static const char *get_default_remote(void)
+{
+   char *dest = NULL;
+   unsigned char sha1[20];
+   int flag;
+   struct strbuf sb = STRBUF_INIT;
+   const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, );
+
+   if (!refname)
+   die("No such ref: HEAD");
+
+   refname = shorten_unambiguous_ref(refname, 0);
+   strbuf_addf(, "branch.%s.remote", refname);
+   if (git_config_get_string(sb.buf, ))
+   return "origin";
+   else
+   return xstrdup(dest);
+}
+
+static int has_same_dir_prefix(const char *str, const char **out)
+{
+#ifdef GIT_WINDOWS_NATIVE
+   return skip_prefix(str, "./", out)
+   || skip_prefix(str, ".\\", out);
+#else
+   return skip_prefix(str, "./", out);
+#endif
+}
+
+static int has_upper_dir_prefix(const char *str, const char **out)
+{
+#ifdef GIT_WINDOWS_NATIVE
+   return skip_prefix(str, "../", out)
+   || skip_prefix(str, "..\\", out);
+#else
+   return skip_prefix(str, "../", out);
+#endif
+}
+
+static char *last_dir_separator(const char *str)
+{
+#ifdef GIT_WINDOWS_NATIVE
+   return strrchr(str, "/")
+   || strrchr(str, "\\");
+#else
+   return strrchr(str, '/');
+#endif
+}
+
+/*
+ * The function takes at most 2 arguments. The first argument is the
+ * URL that navigates to the submodule origin repo. When relative, this URL
+ * is relative to the superproject origin URL repo. The second up_path
+ * argument, if specified, is the relative path that navigates
+ * from the submodule working tree to the superproject working tree.
+ *
+ * The output of the function is the origin URL of the submodule.
+ *
+ * The output will either be an absolute URL or filesystem path (if the
+ * superproject origin URL is an absolute URL or filesystem path,
+ * respectively) or a relative file system path (if the superproject
+ * origin URL is a relative file system path).
+ *
+ * When the output is a relative file system path, the path is either
+ * relative to the submodule working tree, if up_path is specified, or to
+ * the superproject working tree otherwise.
+ */
+static const char *relative_url(const char *url, const char *up_path)
+{
+   int is_relative = 0;
+   size_t len;
+   char *remoteurl = NULL;
+   char *sep = "/";
+   const char *out;
+   struct strbuf sb = STRBUF_INIT;
+   const char *remote = get_default_remote();
+   strbuf_addf(, "remote.%s.url", remote);
+
+   if (git_config_get_string(sb.buf, ))
+   /* the repository is its own authoritative upstream */
+   remoteurl = xgetcwd();
+
+   len = strlen(remoteurl);
+   if (is_dir_sep(remoteurl[len]))
+   remoteurl[len] = '\0';
+
+   if (strchr(remoteurl, ':') || is_dir_sep(*remoteurl))
+   is_relative = 0;
+   else if (has_same_dir_prefix(remoteurl, ) ||
+   has_upper_dir_prefix(remoteurl, ))
+   is_relative = 1;
+   else {
+   is_relative = 1;
+   strbuf_reset();
+   strbuf_addf(, "./%s", remoteurl);
+   remoteurl = strbuf_detach(, NULL);
+   }
+
+   while (url) {
+   if (has_upper_dir_prefix(url, )) {
+   char *rfind;
+   url = out;
+
+   rfind = last_dir_separator(remoteurl);
+   if (rfind)
+   *rfind = '\0';
+   else {
+   rfind = strrchr(remoteurl, ':');
+   if 

[PATCHv2] Porting resolve_relative_url from shell to C

2015-12-16 Thread Stefan Beller
A new version of the patch, which spells out more its intent and
may actually work in Windows.

Any comment welcome,
Thanks,
Stefan

Stefan Beller (1):
  submodule: Port resolve_relative_url from shell to C

 builtin/submodule--helper.c | 151 
 git-submodule.sh|  81 ++--
 2 files changed, 155 insertions(+), 77 deletions(-)

interdiff to previous version:

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f48b5b5..b925bed 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -31,6 +31,36 @@ static const char *get_default_remote(void)
return xstrdup(dest);
 }
 
+static int has_same_dir_prefix(const char *str, const char **out)
+{
+#ifdef GIT_WINDOWS_NATIVE
+   return skip_prefix(str, "./", out)
+   || skip_prefix(str, ".\\", out);
+#else
+   return skip_prefix(str, "./", out);
+#endif
+}
+
+static int has_upper_dir_prefix(const char *str, const char **out)
+{
+#ifdef GIT_WINDOWS_NATIVE
+   return skip_prefix(str, "../", out)
+   || skip_prefix(str, "..\\", out);
+#else
+   return skip_prefix(str, "../", out);
+#endif
+}
+
+static char *last_dir_separator(const char *str)
+{
+#ifdef GIT_WINDOWS_NATIVE
+   return strrchr(str, "/")
+   || strrchr(str, "\\");
+#else
+   return strrchr(str, '/');
+#endif
+}
+
 /*
  * The function takes at most 2 arguments. The first argument is the
  * URL that navigates to the submodule origin repo. When relative, this URL
@@ -64,13 +94,14 @@ static const char *relative_url(const char *url, const char 
*up_path)
/* the repository is its own authoritative upstream */
remoteurl = xgetcwd();
 
-   if (strip_suffix(remoteurl, "/", ))
+   len = strlen(remoteurl);
+   if (is_dir_sep(remoteurl[len]))
remoteurl[len] = '\0';
 
-   if (strchr(remoteurl, ':') || skip_prefix(remoteurl, "/", ))
+   if (strchr(remoteurl, ':') || is_dir_sep(*remoteurl))
is_relative = 0;
-   else if (skip_prefix(remoteurl, "./", ) ||
-   skip_prefix(remoteurl, "../", ))
+   else if (has_same_dir_prefix(remoteurl, ) ||
+   has_upper_dir_prefix(remoteurl, ))
is_relative = 1;
else {
is_relative = 1;
@@ -80,11 +111,11 @@ static const char *relative_url(const char *url, const 
char *up_path)
}
 
while (url) {
-   if (skip_prefix(url, "../", )) {
+   if (has_upper_dir_prefix(url, )) {
char *rfind;
url = out;
 
-   rfind = strrchr(remoteurl, '/');
+   rfind = last_dir_separator(remoteurl);
if (rfind)
*rfind = '\0';
else {
@@ -99,7 +130,7 @@ static const char *relative_url(const char *url, const char 
*up_path)
remoteurl = ".";
}
}
-   } else if (skip_prefix(url, "./", ))
+   } else if (has_same_dir_prefix(url, ))
url = out;
else
break;
@@ -107,7 +138,7 @@ static const char *relative_url(const char *url, const char 
*up_path)
strbuf_reset();
strbuf_addf(, "%s%s%s", remoteurl, sep, url);
 
-   if (!skip_prefix(sb.buf, "./", ))
+   if (!has_same_dir_prefix(sb.buf, ))
out = sb.buf;
out = xstrdup(out);
 

-- 
2.7.0.rc1.2.gfc39790.dirty

--
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] revision.c: propagate tag names from pending array

2015-12-16 Thread Jeff King
When we unwrap a tag to find its commit for a traversal, we
do not propagate the "name" field of the tag in the pending
array (i.e., the ref name the user gave us in the first
place) to the commit (instead, we use an empty string). This
means that "git log --source" will never show the tag-name
for commits we reach through it.

This was broken in 2073949 (traverse_commit_list: support
pending blobs/trees with paths, 2014-10-15). That commit
tried to be careful and avoid propagating the path
information for a tag (which would be nonsensical) to trees
and blobs. But it should not have cut off the "name" field,
which should carry forward to children.

Note that this does mean that the "name" field will carry
forward to blobs and trees, too. Whereas prior to 2073949,
we always gave them an empty string. This is the right thing
to do, but in practice no callers probably use it (since now
we have an explicit separate "path" field, which was the
point of 2073949).

We add tests here not only for the broken case, but also a
basic sanity test of "log --source" in general, which did
not have any coverage in the test suite.

Reported-by: Raymundo 
Signed-off-by: Jeff King 
---
This was reported several weeks ago, but I needed to take the time to
convince myself this wasn't regressing any cases. I'm pretty sure it's
the right thing to do.

The regression is in v2.2.0, so this is not urgent to make it into v2.7
before release, but it is definitely maint-worthy.

 revision.c |  3 +--
 t/t4202-log.sh | 29 +
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index 9404a05..0a282f5 100644
--- a/revision.c
+++ b/revision.c
@@ -294,9 +294,8 @@ static struct commit *handle_commit(struct rev_info *revs,
/*
 * We'll handle the tagged object by looping or dropping
 * through to the non-tag handlers below. Do not
-* propagate data from the tag's pending entry.
+* propagate path data from the tag's pending entry.
 */
-   name = "";
path = NULL;
mode = 0;
}
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 6ede069..a0f80af 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -908,4 +908,33 @@ test_expect_success 'log diagnoses bogus HEAD' '
test_i18ngrep broken stderr
 '
 
+test_expect_success 'set up --source tests' '
+   git checkout --orphan source-a &&
+   test_commit one &&
+   test_commit two &&
+   git checkout -b source-b HEAD^ &&
+   test_commit three
+'
+
+test_expect_success 'log --source paints branch names' '
+   cat >expect <<-\EOF &&
+   09e12a9 source-b three
+   8e393e1 source-a two
+   1ac6c77 source-b one
+   EOF
+   git log --oneline --source source-a source-b >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'log --source paints tag names' '
+   git tag -m tagged source-tag &&
+   cat >expect <<-\EOF &&
+   09e12a9 source-tag three
+   8e393e1 source-a two
+   1ac6c77 source-tag one
+   EOF
+   git log --oneline --source source-tag source-a >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.7.0.rc1.339.gca82f22
--
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: 'Comma in -to/-cc entry' on valid addresses

2015-12-16 Thread Matthieu Moy
Andy Lutomirski  writes:

> $ git send-email --to='"Address, Valid" '
> Comma in --to entry: "Address, Valid" '
> $ git --version
> git version 2.5.0
>
> This appears to be a recent regression.

It's not: the check was introduced in 79ee555 (Check and document the
options to prevent mistakes., 2006-06-21).

The check was _removed_ in Git 2.6.0, which happily accepts your
command. IOW, the bug is already fixed :-).

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


[RFC] Git Developer Summit, April 4th, 2016, NYC

2015-12-16 Thread Jeff King
GitHub is organizing another Git Merge conference this coming April in
New York City, USA. The organizers have offered to host a developer's
summit similar to what we did last year in Paris (one day, developers
sitting around chatting).

The venue has not been booked yet, and they've asked for input before
doing so.  Specifically, they need a rough headcount to book something
of the appropriate size. So I'd like to get a list of people who are
potentially interested in coming (you can respond here, or email me
off-list). This isn't a commitment, so don't be afraid to say you're
interested if you're not sure yet. But at the same time, be realistic. :)

In the past we've more or less limited attendance to people involved in
the development of Git, alternate implementations (e.g., JGit, libgit2),
and surrounding tools (e.g., I'd welcome discussion about distributed
code review systems built around git). If you're not sure if you'd be
welcome, please ask (here, or feel free to email me off-list).

If you need or want money to help with travel expenses, it's possible
that GitHub or the Git project itself may be able to help out. There's
nothing particularly organized yet, but if you are interested, you can
either email me or the Git project committee ().
I think we'll need to see how many requests there are before we have a
sense of how many can be filled (but in past years, there have not been
many requests, so please don't be shy because you think we're
overburdened).

All of the details are open for discussion, including the attendance
policy and Git project policy for doling out money. The format in years
past has been mostly unconference, and centered around round-table
discussions. I've asked for more round-table style seating (as opposed
to theater-style for presentations). There will probably be a projector.
I've also asked to have a developer's dinner (last year there was an
ad-hoc thing, but I thought it might be easier if somebody organized it
in advance).

Let me know if there are additional requests. It's early still, but some
things are easier to accommodate earlier than later. And if anybody is
interested in organizing any part of it (e.g., figuring out the dinner
ourselves rather than leaving it to GitHub organizers), let me know.

I hope to see people there!

-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


'Comma in -to/-cc entry' on valid addresses

2015-12-16 Thread Andy Lutomirski
$ git send-email --to='"Address, Valid" '
Comma in --to entry: "Address, Valid" '
$ git --version
git version 2.5.0

This appears to be a recent regression.

For an authority on why the current behavior is wrong, see RFC 2822
section 3.4, which defines mailbox and mailbox-list as well as section
3.2.6, which indicates that a phrase can contain a quoted-string.


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


Re: [PATCH 2/2] git-svn: test for git-svn prefixed globs

2015-12-16 Thread Eric Wong
Victor Leschuk  wrote:
> Hello Eric, sorry, I just copy pasted your old
> t9108-git-svn-glob.sh, changed branch names to be prefixed and added
> test for "exact" prefix match. If it is necessary I can rewrite it
> according to current guidelines.

Yes, please do if you can.  I can do it if you're willing to wait a bit.
--
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 15/16] refs: add LMDB refs backend

2015-12-16 Thread David Turner
On Wed, 2015-12-16 at 17:00 -0800, Jonathan Nieder wrote:
> David Turner wrote:
> 
> > --- a/Documentation/config.txt
> > +++ b/Documentation/config.txt
> > @@ -496,6 +496,13 @@ core.repositoryFormatVersion::
> > Internal variable identifying the repository format and
> > layout
> > version.
> >  
> > +core.refsBackendType::
> > +   Type of refs backend. Default is to use the original files
> > +   based backend. Set to 'lmdb' to activate the lmdb database
> > +   backend.  If you use the lmdb backend,
> > +   core.repositoryFormatVersion must be set to 1, and
> > +   extensions.refBackend must be set to 'lmdb'.
> 
> If core.refsBackendType names one backend and extensions.refBackend
> names another, which wins?

I've simplified this to just use extensions.refsBackendType for
everything.  Will re-roll once I finish getting comments on the rest of
this series.
--
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 15/16] refs: add LMDB refs backend

2015-12-16 Thread Jonathan Nieder
David Turner wrote:

> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -496,6 +496,13 @@ core.repositoryFormatVersion::
>   Internal variable identifying the repository format and layout
>   version.
>  
> +core.refsBackendType::
> + Type of refs backend. Default is to use the original files
> + based backend. Set to 'lmdb' to activate the lmdb database
> + backend.  If you use the lmdb backend,
> + core.repositoryFormatVersion must be set to 1, and
> + extensions.refBackend must be set to 'lmdb'.

If core.refsBackendType names one backend and extensions.refBackend
names another, which wins?

Thanks,
Jonathan
--
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: [PATCHv2] Porting resolve_relative_url from shell to C

2015-12-16 Thread Torsten Bögershausen
On 17.12.15 01:26, Stefan Beller wrote:
> A new version of the patch, which spells out more its intent and
> may actually work in Windows.
> 
> Any comment welcome,
> Thanks,
> Stefan
> 
> Stefan Beller (1):
>   submodule: Port resolve_relative_url from shell to C
> 
>  builtin/submodule--helper.c | 151 
> 
>  git-submodule.sh|  81 ++--
>  2 files changed, 155 insertions(+), 77 deletions(-)
> 
> interdiff to previous version:
> 
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index f48b5b5..b925bed 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -31,6 +31,36 @@ static const char *get_default_remote(void)
>   return xstrdup(dest);
>  }
>  
> +static int has_same_dir_prefix(const char *str, const char **out)
> +{
> +#ifdef GIT_WINDOWS_NATIVE
Should that be
if defined(GIT_WINDOWS_NATIVE) || defined(__CYGWIN__)

--
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 v1 1/4] git-gui: remove duplicate entries from .gitconfig's gui.recentrepo

2015-12-16 Thread Eric Sunshine
On Wed, Dec 16, 2015 at 6:41 PM, Philip Oakley  wrote:
> From: "Eric Sunshine" 
>> On Monday, December 14, 2015, Philip Oakley  wrote:
>>> The git gui's recent repo list may become contaminated with duplicate
>>> entries. The git gui would barf when attempting to remove one entry.
>>> Remove them all - there is no option within 'git config' to selectively
>>> remove one of the entries.
>>>
>>> This issue was reported on the 'Git User' list
>>> (https://groups.google.com/forum/#!topic/git-users/msev4KsQGFc,
>>> Warning: gui.recentrepo has multiply values while executing).
>>
>> s/multiply/multiple/
>
> The original report has that mistake. I'll quote the thread title.

Ah, I had checked the screenshot included with the original report and
saw that it was correctly spelled, thus assumed that the typo was
yours. I didn't pay much attention to the title in the original
report, which indeed already contains this misspelling.
--
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 7/8] config: add core.untrackedCache

2015-12-16 Thread Jeff King
On Tue, Dec 15, 2015 at 10:05:57PM -0800, Junio C Hamano wrote:

> Junio C Hamano  writes:
> 
> > This is why the_index.has_untracked_cache is not just a simple "Do I
> > want to use this feature?" boolean configuration.  The index also
> > stores the real data, and "Am I using this feature?" bit goes hand
> > in hand with that real data.  Thinking that this is merely a boolean
> > configuration is the real source of the confusion, and introducing a
> > config that overrules what the user has stored in the index needs to
> > add complexity.
> >
> > The additional complexity may (or may not) be justifiable, but in
> > any case "all other things being equal, this is a config" feels like
> > a flawed observation.
> 
> To put it another way, the "bit" in the index (i.e. the presence of
> the cached data) is "Am I using the feature now?".  The effect of
> the feature has to (and is designed to) persist, as it is a cache
> and you do not want a stale cache to give you wrong answers.  There
> is no "Do I want to use the feature?" preference, in other words.
> 
> And I do not mind creating such a preference bit as a configuration.
> 
> That is why I suggested such a configuration to cause the equivalent
> of "update-index --untracked-cache" when a new index is created from
> scratch (as opposed to the case where the previously created cache
> data is carried forward across read_cache() -> do things to the
> index -> write_cache() flow).  Doing it that way will not have to
> involve additional complexity that comes from the desire that
> setting a single configuration on (or off) has to suddenly change
> the behaviour of an index file that is already using (or not using)
> the feature.

I think we may actually be thinking of the same thing. Naively, I would
expect:

  - if there is untracked cache data in the index, we will make use of
it when enumerating untracked files (and my understanding is that if
it is stale, we can detect that)

  - if core.untrackedCache is set, we will update and write out an
untracked cache when we are enumerating the untracked files

  - if there is cache data in the index but that config flag is not set,
presumably we would not update it (we could even explicitly drop it,
but my understanding is that is not necessary for correctness, but
only as a possible optimization).

You could have a config option for "if there is a cache there, pretend
it isn't and ignore it", but I don't see much point.

-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] rebase: add --verify-signatures

2015-12-16 Thread Alexander 'z33ky' Hirsch
On Thu, Dec 10, 2015 at 11:53:45AM -0800, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > Alexander 'z33ky' Hirsch <1ze...@gmail.com> writes:
> >
> >> +  if test -n "$rebase_root"
> >> +  then
> >> +  foreign_revisions="$orig_head..$onto"
> >> +  else
> >> +  foreign_revisions="$orig_head..${restrict_revision-$upstream}"
> >> +  fi
> >> +
> >> +  for cmt in $(git rev-list --reverse "$foreign_revisions")
> >> +  do
> >> +  if ! git log -1 --pretty=format:'%G?%n%GS' "$cmt" |
> >
> > I do not think this matches what the corresponding option in "git
> > merge" does, where the only tips of the histories being merged are
> > checked.

Oh, indeed. I saw the loop in merge.c and by brain went there without
any further thought. This would be easy to fix though.

> Having said that, I somehow doubt that verify-signatures is a
> feature that is desirable in a workflow around "pull --rebase" in
> the larger picture.  If you step back a bit, in a "merge" based
> workflow, you are the keeper of the sanity, cleanliness, and all the
> good things in the authoritative history when doing a "git pull".
> That is why you would want to validate what gets merged from another
> place and in that context having --verify-signatures may make sense
> (and it might even make more sense if the code did so for all new
> commits, not just the tip, but that is a separate topic).  If the
> validation fails, you would tell the owner of that side branch you
> just attempted to pull from to get her act together before asking to
> be pulled again.  There is a clear path to make further progress
> after the validation fails.
> 
> In a workflow that is built around "pull --rebase", you are _given_
> the authoritative history with all the good things from another
> place and then you rebuild your own work on top of it.  The sanity
> and cleanliness of what you built on top is given, and rejecting it
> at that point would not help you make further progress in any way,
> as that is a published history that is shared and more authoritative
> than what you have.

Well, the rejection would not refer to the work you put on top, but to
the commits you want to base your work on.
If validation fails, then an empty commit that is signed can be
committed on top of the previously unsigned branch if commit rewriting
is not allowed.

> Hence, while I 100% agree with Brian's "it is not there because
> nobody bothered to add the corresponding option on the rebase side",
> I do not necessarily think "nobody bothered" is the same as "they
> were too lazy"--perhaps some people thought about doing it, and then
> decided not to, because the option made no sense when they stepped
> back to look at the larger picture.

That's why I was asking in my first mail if such an addition would make
sense. I don't really have an agenda or a pressing need for this
feature, I just noticed that a `git pull --rebase --verify-signatures`
did not complain when it looked like it ought to.
If this patch gets rejected then I will propose one which makes git-pull
warn, or even error, when both --rebase and --verify-signatures is
passed.

Regards,
Alexander Hirsch
--
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: Help debugging git-svn

2015-12-16 Thread Edmundo Carmona Antoranz
On Wed, Dec 16, 2015 at 1:41 AM, Eric Wong  wrote:
>
> Any chance you can reproduce this on a Linux system?
> I do not use non-Free systems and have no debugging experience
> there at all.
>

My wish But it's a big resounding "no".

>> With my very flawed knowledge of perl I have seen that the process is
>> getting to Ra.pm around here:
>
> It could also be a bug in the SVN bindings or the port of
> Perl.  Which versions are you running?

I'm working on git for windows 2.6.3. I think I could use cygwin, just
in case (I used it before).

>
>
> I've also been wondering about the motivation of SVN developers to do a
> good job on maintaining their Perl bindings (given git-svn is likely the
> main user of them).
> We've certainly had problems in the past with SVN breaking and
> causing git-svn to segfault around 2011-2012; but it's been a while...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] Fix tyop in the 2.7.0 release notes

2015-12-16 Thread SZEDER Gábor
/^Subject:/ s/tyop/typo/

;)

--
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] mingw: emulate write(2) that fails with a EPIPE

2015-12-16 Thread Johannes Schindelin
On Windows, when writing to a pipe fails, errno is always
EINVAL. However, Git expects it to be EPIPE.

According to the documentation, there are two cases in which write()
triggers EINVAL: the buffer is NULL, or the length is odd but the mode
is 16-bit Unicode (the broken pipe is not mentioned as possible cause).
Git never sets the file mode to anything but binary, therefore we know
that errno should actually be EPIPE if it is EINVAL and the buffer is
not NULL.

See https://msdn.microsoft.com/en-us/library/1570wh78.aspx for more
details.

This works around t5571.11 failing with v2.6.4 on Windows.

Signed-off-by: Johannes Schindelin 
---
 compat/mingw.h | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/compat/mingw.h b/compat/mingw.h
index 738865c..2aca347 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -210,6 +210,24 @@ FILE *mingw_freopen (const char *filename, const char 
*otype, FILE *stream);
 int mingw_fflush(FILE *stream);
 #define fflush mingw_fflush
 
+static inline ssize_t mingw_write(int fd, const void *buf, size_t len)
+{
+   ssize_t result = write(fd, buf, len);
+
+   if (result < 0 && errno == EINVAL && buf) {
+   /* check if fd is a pipe */
+   HANDLE h = (HANDLE) _get_osfhandle(fd);
+   if (GetFileType(h) == FILE_TYPE_PIPE)
+   errno = EPIPE;
+   else
+   errno = EINVAL;
+   }
+
+   return result;
+}
+
+#define write mingw_write
+
 int mingw_access(const char *filename, int mode);
 #undef access
 #define access mingw_access
-- 
2.6.3.windows.1.300.g1c25e49
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] git-svn: test for git-svn prefixed globs

2015-12-16 Thread Victor Leschuk
Add test for git-svn prefixed globs.

Signed-off-by: Victor Leschuk 
---
 t/t9168-git-svn-prefixed-glob.sh | 136 +++
 1 file changed, 136 insertions(+)
 create mode 100755 t/t9168-git-svn-prefixed-glob.sh

diff --git a/t/t9168-git-svn-prefixed-glob.sh b/t/t9168-git-svn-prefixed-glob.sh
new file mode 100755
index 000..979ecd9
--- /dev/null
+++ b/t/t9168-git-svn-prefixed-glob.sh
@@ -0,0 +1,136 @@
+#!/bin/sh
+test_description='git svn globbing refspecs with prefixed globs'
+. ./lib-git-svn.sh
+
+cat > expect.end < trunk/src/a/readme &&
+   echo "goodbye world" > trunk/src/b/readme &&
+   svn_cmd import -m "initial" trunk "$svnrepo"/trunk &&
+   svn_cmd co "$svnrepo" tmp &&
+   (
+   cd tmp &&
+   mkdir branches tags &&
+   svn_cmd add branches tags &&
+   svn_cmd cp trunk branches/b_start &&
+   svn_cmd commit -m "start a new branch" &&
+   svn_cmd up &&
+   echo "hi" >> branches/b_start/src/b/readme &&
+   poke branches/b_start/src/b/readme &&
+   echo "hey" >> branches/b_start/src/a/readme &&
+   poke branches/b_start/src/a/readme &&
+   svn_cmd commit -m "hi" &&
+   svn_cmd up &&
+   svn_cmd cp branches/b_start tags/t_end &&
+   echo "bye" >> tags/t_end/src/b/readme &&
+   poke tags/t_end/src/b/readme &&
+   echo "aye" >> tags/t_end/src/a/readme &&
+   poke tags/t_end/src/a/readme &&
+   svn_cmd commit -m "the end" &&
+   echo "byebye" >> tags/t_end/src/b/readme &&
+   poke tags/t_end/src/b/readme &&
+   svn_cmd commit -m "nothing to see here"
+   ) &&
+   git config --add svn-remote.svn.url "$svnrepo" &&
+   git config --add svn-remote.svn.fetch \
+"trunk/src/a:refs/remotes/trunk" &&
+   git config --add svn-remote.svn.branches \
+"branches/b_*/src/a:refs/remotes/branches/b_*" &&
+   git config --add svn-remote.svn.tags\
+"tags/t_*/src/a:refs/remotes/tags/t_*" &&
+   git svn multi-fetch &&
+   git log --pretty=oneline refs/remotes/tags/t_end | \
+   sed -e "s/^.\{41\}//" > output.end &&
+   test_cmp expect.end output.end &&
+   test "`git rev-parse refs/remotes/tags/t_end~1`" = \
+   "`git rev-parse refs/remotes/branches/b_start`" &&
+   test "`git rev-parse refs/remotes/branches/b_start~2`" = \
+   "`git rev-parse refs/remotes/trunk`" &&
+   test_must_fail git rev-parse refs/remotes/tags/t_end@3
+   '
+
+echo try to try > expect.two
+echo nothing to see here >> expect.two
+cat expect.end >> expect.two
+
+test_expect_success 'test left-hand-side only prefixed globbing' '
+   git config --add svn-remote.two.url "$svnrepo" &&
+   git config --add svn-remote.two.fetch trunk:refs/remotes/two/trunk &&
+   git config --add svn-remote.two.branches \
+"branches/b_*:refs/remotes/two/branches/*" &&
+   git config --add svn-remote.two.tags \
+"tags/t_*:refs/remotes/two/tags/*" &&
+   (
+   cd tmp &&
+   echo "try try" >> tags/t_end/src/b/readme &&
+   poke tags/t_end/src/b/readme &&
+   svn_cmd commit -m "try to try"
+   ) &&
+   git svn fetch two &&
+   test `git rev-list refs/remotes/two/tags/t_end | wc -l` -eq 6 &&
+   test `git rev-list refs/remotes/two/branches/b_start | wc -l` -eq 3 &&
+   test `git rev-parse refs/remotes/two/branches/b_start~2` = \
+`git rev-parse refs/remotes/two/trunk` &&
+   test `git rev-parse refs/remotes/two/tags/t_end~3` = \
+`git rev-parse refs/remotes/two/branches/b_start` &&
+   git log --pretty=oneline refs/remotes/two/tags/t_end | \
+   sed -e "s/^.\{41\}//" > output.two &&
+   test_cmp expect.two output.two
+   '
+
+test_expect_success 'test prefixed globs do not match just prefix' '
+   git config --add svn-remote.three.url "$svnrepo" &&
+   git config --add svn-remote.three.fetch \
+trunk:refs/remotes/three/trunk &&
+   git config --add svn-remote.three.branches \
+"branches/b_*:refs/remotes/three/branches/*" &&
+   git config --add svn-remote.three.tags \
+"tags/t_*:refs/remotes/three/tags/*" &&
+   (
+   cd tmp &&
+   svn_cmd cp trunk branches/b_ &&
+   echo "You should never see me" >> branches/b_/src/a/readme &&
+   poke branches/b_/src/a/readme &&
+   svn_cmd commit -m "Never seen branch commit" &&
+   svn_cmd up &&
+   svn_cmd cp branches/b_ tags/t_ &&
+   echo "You should never see mee too" >> tags/t_/src/a/readme &&
+  

[PATCH 1/2] git-svn: support for prefixed globs in config

2015-12-16 Thread Victor Leschuk
Introduce prefixed globs for branches and tags in git-svn.

Signed-off-by: Victor Leschuk 
---
 Documentation/git-svn.txt | 5 +
 perl/Git/SVN/GlobSpec.pm  | 9 -
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 0c0f60b..529cffe 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -1034,6 +1034,7 @@ listed below are allowed:
url = http://server.org/svn
fetch = trunk/project-a:refs/remotes/project-a/trunk
branches = branches/*/project-a:refs/remotes/project-a/branches/*
+   branches = branches/release_*:refs/remotes/project-a/branches/release_*
tags = tags/*/project-a:refs/remotes/project-a/tags/*
 
 
@@ -1044,6 +1045,10 @@ independent path component (surrounded by '/' or EOL).   
This
 type of configuration is not automatically created by 'init' and
 should be manually entered with a text-editor or using 'git config'.
 
+Also note that prefixed globs (e.g. 'release_*') match everything after prefix
+but do not match exact prefix. For example:
+'release_*' will match 'release_1' or 'release_v1' but will not match 
'release_'.
+
 It is also possible to fetch a subset of branches or tags by using a
 comma-separated list of names within braces. For example:
 
diff --git a/perl/Git/SVN/GlobSpec.pm b/perl/Git/SVN/GlobSpec.pm
index c95f5d7..a136090 100644
--- a/perl/Git/SVN/GlobSpec.pm
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -11,16 +11,15 @@ sub new {
my $die_msg = "Only one set of wildcard directories " .
"(e.g. '*' or '*/*/*') is supported: '$glob'\n";
for my $part (split(m|/|, $glob)) {
-   if ($part =~ /\*/ && $part ne "*") {
-   die "Invalid pattern in '$glob': $part\n";
-   } elsif ($pattern_ok && $part =~ /[{}]/ &&
+   if ($pattern_ok && $part =~ /[{}]/ &&
 $part !~ /^\{[^{}]+\}/) {
die "Invalid pattern in '$glob': $part\n";
}
-   if ($part eq "*") {
+   if ($part =~ /(\w*)\*/) {
die $die_msg if $state eq "right";
$state = "pattern";
-   push(@patterns, "[^/]*");
+   my $pat = $1 ? "${1}[^/]+" : "[^/]*";
+   push(@patterns, $pat);
} elsif ($pattern_ok && $part =~ /^\{(.*)\}$/) {
die $die_msg if $state eq "right";
$state = "pattern";
-- 
2.7.0.rc0.21.gb793f61

--
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 0/2] git-svn: add support for prefixed globs in config

2015-12-16 Thread Victor Leschuk
There are existing old SVN repos which use patterns in branch (and tag) names
to indicate some information. For example: branches/release_01, 
branches/release_02, etc,
however non-patterned branches co-exist with them (like branches/dev).

If someone maintains git mirror of such a repo it is reasonable to mirror only
several branches (for example those who match some pattern) and in current 
situation
it leads to messy and error-prone git config like:

branches = branches/{release_20,release_21,release_22,...

It would be useful to have an opportunity to write

branches = branches/release_*

instead of this.

Thus I suggest to add support for such 'prefixed' globs into git-svn.

Victor Leschuk (2):
  Introduce prefixed globs for branches and tags in git-svn.
  Add test for git-svn prefixed globs.

 Documentation/git-svn.txt|   5 ++
 perl/Git/SVN/GlobSpec.pm |   9 ++-
 t/t9168-git-svn-prefixed-glob.sh | 136 +++
 3 files changed, 145 insertions(+), 5 deletions(-)
 create mode 100755 t/t9168-git-svn-prefixed-glob.sh

-- 
2.7.0.rc0.21.gb793f61

--
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/RFC 00/10] ref-filter: use parsing functions

2015-12-16 Thread Karthik Nayak
On Mon, Dec 14, 2015 at 3:25 AM, Eric Sunshine  wrote:
> On Sun, Dec 13, 2015 at 4:31 AM, Karthik Nayak  wrote:
>> On Sun, Dec 13, 2015 at 11:10 AM, Eric Sunshine  
>> wrote:
>>> If the intention is to rid that inner loop of much of the expensive
>>> processing, then wouldn't we want to introduce an enum of valid atoms
>>> which is to be a member of 'struct used_atom', and have
>>> populate_value() switch on the enum value rather than doing all the
>>> expensive strcmp()s and starts_with()?
>>>
>>> enum atom_type {
>>> AT_REFNAME,
>>> AT_OBJECTTYPE,
>>> ...
>>> AT_COLOR,
>>> AT_ALIGN
>>> };
>>>
>>> static struct used_atom {
>>> enum atom_type atom;
>>> cmp_type cmp;
>>> union {
>>> char *color; /* parsed color */
>>> struct align align;
>>> enum { ... } remote_ref;
>>> struct {
>>> enum { ... } portion;
>>> unsigned int nlines;
>>> } contents;
>>> int short_objname;
>>> } u;
>>> } *used_atom;
>>>
>>> In fact, the 'cmp_type cmp' field can be dropped altogether since it
>>> can just as easily be looked up when needed by keeping 'enum
>>> atom_type' and valid_atoms[] in-sync and indexing into valid_atoms[]
>>> by atom_type:
>>>
>>> struct used_atom *a = ...;
>>> cmp_type cmp = valid_atoms[a->atom].cmp_type;
>>>
>>> As a bonus, an 'enum atom_type' would resolve the problem of how
>>> starts_with() is abused in populate_value() for certain cases
>>> (assuming I'm understanding the logic), such as how matching of
>>> "color" could incorrectly match some yet-to-be-added atom named
>>> "colorize".
>>
>> This actually makes sense to me, especially how we can eliminate most of
>> the starts_with() in populate_value(). Well then I guess I'll work on this 
>> and
>> see what I can come up with, Thanks for the review :)
>
> The 'enum atom_type' approach doesn't necessarily need to be part of
> this patch series; it may make sense to layer it atop the current
> series, which is already long enough to make review onerous. Patches
> in the current series are things you would want to do anyhow for the
> 'enum atom_type' approach, and there isn't really anything in the
> current series which should present a roadblock for layering 'enum
> atom_type' atop. Plus, by layering 'enum atom_type' atop, you get a
> chance to polish the current series (based upon review comments) and
> let it settle down, which should make working on 'enum atom_type'
> easier.

Ya could do that, I have the series ready then, I was gonna work on the
'enum atom_type' but I'll work on top of this, like you suggested.

Thanks.

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


[PATCH v2 00/11] ref-filter: use parsing functions

2015-12-16 Thread Karthik Nayak
This series cleans up populate_value() in ref-filter, by moving out
the parsing part of atoms to separate parsing functions. This ensures
that parsing is only done once and also improves the modularity of the
code.
The previous version can be found here:
http://thread.gmane.org/gmane.comp.version-control.git/281180

Changes in this version:
1. Re-ordered the patch series and split patches.
2. Used 'enums' rather than 'structs' in 'used_atom' structure.
3. Made all functions static to remove global scope (Thanks to Ramsay Jones).
4. Added "subject" and "body" atom under contents_atom_parser.
5. Changes in error messages, function names and variable names.
6. Changes in comments and documentation.
7. Added extensive test cases for the %(align) atom.

Thanks to Eric and Junio for suggestions on v1.
TODO: Implement an enum to denote current atom in used_atom,
so as to get rid of repetitive checks performed.
(http://article.gmane.org/gmane.comp.version-control.git/282320).

Karthik Nayak (11):
  strbuf: introduce strbuf_split_str_without_term()
  ref-filter: use strbuf_split_str_omit_term()
  ref-filter: introduce struct used_atom
  ref-fitler: bump match_atom() name to the top
  ref-filter: skip deref specifier in match_atom_name()
  ref-filter: introduce color_atom_parser()
  ref-filter: introduce align_atom_parser()
  ref-filter: introduce prefixes for the align atom
  ref-filter: introduce remote_ref_atom_parser()
  ref-filter: introduce contents_atom_parser()
  ref-filter: introduce objectname_atom_parser()

 Documentation/git-for-each-ref.txt |  20 +-
 ref-filter.c   | 441 ++---
 strbuf.c   |   7 +-
 strbuf.h   |  25 ++-
 t/t6302-for-each-ref-filter.sh | 162 ++
 5 files changed, 462 insertions(+), 193 deletions(-)

Interdiff:

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 56ffdc1..9af0f53 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -133,8 +133,10 @@ align::
`width=` and `position=` in any order
separated by a comma, where the `` is either left,
right or middle, default being left and `` is the total
-   length of the content with alignment. The prefix for the
-   arguments is not mandatory. If the contents length is more
+   length of the content with alignment. For brevity, the
+   "width=" and/or "position=" prefixes may be omitted, and bare
+and  used instead.  For instance,
+   `%(align:,)`. If the contents length is more
than the width then no alignment is performed. If used with
'--quote' everything in between %(align:...) and %(end) is
quoted, but if nested then only the topmost level performs
diff --git a/ref-filter.c b/ref-filter.c
index 3bd3e45..a8fd178 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -37,23 +37,13 @@ static struct used_atom {
union {
const char *color;
struct align align;
+   enum { RR_SHORTEN, RR_TRACK, RR_TRACKSHORT, RR_NORMAL }
+   remote_ref;
struct {
-   unsigned int shorten : 1,
-   track : 1,
-   trackshort : 1;
-   } remote_ref;
-   struct {
-   unsigned int subject : 1,
-   body : 1,
-   signature : 1,
-   all : 1,
-   lines : 1,
-   no_lines;
+   enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, 
C_SUB } option;
+   unsigned int no_lines;
} contents;
-   struct {
-   unsigned int shorten : 1,
-   full : 1;
-   } objectname;
+   enum { O_FULL, O_SHORT } objectname;
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
@@ -79,81 +69,89 @@ static int match_atom_name(const char *name, const char 
*atom_name, const char *
return 1;
 }
 
-void color_atom_parser(struct used_atom *atom)
+static void color_atom_parser(struct used_atom *atom)
 {
match_atom_name(atom->str, "color", >u.color);
if (!atom->u.color)
die(_("expected format: %%(color:)"));
 }
 
-void remote_ref_atom_parser(struct used_atom *atom)
+static void remote_ref_atom_parser(struct used_atom *atom)
 {
const char *buf;
 
buf = strchr(atom->str, ':');
+   atom->u.remote_ref = RR_NORMAL;
if (!buf)
return;
buf++;
if (!strcmp(buf, "short"))
-   atom->u.remote_ref.shorten = 1;
+   atom->u.remote_ref = RR_SHORTEN;
else if (!strcmp(buf, "track"))
-   atom->u.remote_ref.track = 1;
+   

[PATCH v2 04/11] ref-fitler: bump match_atom() name to the top

2015-12-16 Thread Karthik Nayak
Bump match_atom() to the top for usage in further patches.

Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 043a02a..f4a6414 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -33,6 +33,22 @@ static struct used_atom {
 static int used_atom_cnt, need_tagged, need_symref;
 static int need_color_reset_at_eol;
 
+static int match_atom_name(const char *name, const char *atom_name, const char 
**val)
+{
+   const char *body;
+
+   if (!skip_prefix(name, atom_name, ))
+   return 0; /* doesn't even begin with "atom_name" */
+   if (!body[0]) {
+   *val = NULL; /* %(atom_name) and no customization */
+   return 1;
+   }
+   if (body[0] != ':')
+   return 0; /* "atom_namefoo" is not "atom_name" or 
"atom_name:..." */
+   *val = body + 1; /* "atom_name:val" */
+   return 1;
+}
+
 static struct {
const char *name;
cmp_type cmp_type;
@@ -260,22 +276,6 @@ static void end_atom_handler(struct atom_value *atomv, 
struct ref_formatting_sta
pop_stack_element(>stack);
 }
 
-static int match_atom_name(const char *name, const char *atom_name, const char 
**val)
-{
-   const char *body;
-
-   if (!skip_prefix(name, atom_name, ))
-   return 0; /* doesn't even begin with "atom_name" */
-   if (!body[0]) {
-   *val = NULL; /* %(atom_name) and no customization */
-   return 1;
-   }
-   if (body[0] != ':')
-   return 0; /* "atom_namefoo" is not "atom_name" or 
"atom_name:..." */
-   *val = body + 1; /* "atom_name:val" */
-   return 1;
-}
-
 /*
  * In a format string, find the next occurrence of %(atom).
  */
-- 
2.6.4

--
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 03/11] ref-filter: introduce struct used_atom

2015-12-16 Thread Karthik Nayak
Introduce the 'used_array' structure which would replace the existing
implementation of 'used_array' (which a list of atoms). This helps us
parse atom's before hand and store required details into the
'used_array' for future usage.

Also introduce a parsing function for each atom in valid_atom. Using
this we can define special parsing functions for each of the atoms.

Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 63 +++-
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 6263710..043a02a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -16,9 +16,27 @@
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
+/*
+ * An atom is a valid field atom listed below, possibly prefixed with
+ * a "*" to denote deref_tag().
+ *
+ * We parse given format string and sort specifiers, and make a list
+ * of properties that we need to extract out of objects.  ref_array_item
+ * structure will hold an array of values extracted that can be
+ * indexed with the "atom number", which is an index into this
+ * array.
+ */
+static struct used_atom {
+   const char *str;
+   cmp_type type;
+} *used_atom;
+static int used_atom_cnt, need_tagged, need_symref;
+static int need_color_reset_at_eol;
+
 static struct {
const char *name;
cmp_type cmp_type;
+   void (*parser)(struct used_atom *atom);
 } valid_atom[] = {
{ "refname" },
{ "objecttype" },
@@ -92,21 +110,6 @@ struct atom_value {
 };
 
 /*
- * An atom is a valid field atom listed above, possibly prefixed with
- * a "*" to denote deref_tag().
- *
- * We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects.  ref_array_item
- * structure will hold an array of values extracted that can be
- * indexed with the "atom number", which is an index into this
- * array.
- */
-static const char **used_atom;
-static cmp_type *used_atom_type;
-static int used_atom_cnt, need_tagged, need_symref;
-static int need_color_reset_at_eol;
-
-/*
  * Used to parse format string and sort specifiers
  */
 int parse_ref_filter_atom(const char *atom, const char *ep)
@@ -122,8 +125,8 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
 
/* Do we have the atom already used elsewhere? */
for (i = 0; i < used_atom_cnt; i++) {
-   int len = strlen(used_atom[i]);
-   if (len == ep - atom && !memcmp(used_atom[i], atom, len))
+   int len = strlen(used_atom[i].str);
+   if (len == ep - atom && !memcmp(used_atom[i].str, atom, len))
return i;
}
 
@@ -150,12 +153,11 @@ int parse_ref_filter_atom(const char *atom, const char 
*ep)
at = used_atom_cnt;
used_atom_cnt++;
REALLOC_ARRAY(used_atom, used_atom_cnt);
-   REALLOC_ARRAY(used_atom_type, used_atom_cnt);
-   used_atom[at] = xmemdupz(atom, ep - atom);
-   used_atom_type[at] = valid_atom[i].cmp_type;
+   used_atom[at].str = xmemdupz(atom, ep - atom);
+   used_atom[at].type = valid_atom[i].cmp_type;
if (*atom == '*')
need_tagged = 1;
-   if (!strcmp(used_atom[at], "symref"))
+   if (!strcmp(used_atom[at].str, "symref"))
need_symref = 1;
return at;
 }
@@ -315,7 +317,7 @@ int verify_ref_format(const char *format)
at = parse_ref_filter_atom(sp + 2, ep);
cp = ep + 1;
 
-   if (skip_prefix(used_atom[at], "color:", ))
+   if (skip_prefix(used_atom[at].str, "color:", ))
need_color_reset_at_eol = !!strcmp(color, "reset");
}
return 0;
@@ -359,7 +361,7 @@ static void grab_common_values(struct atom_value *val, int 
deref, struct object
int i;
 
for (i = 0; i < used_atom_cnt; i++) {
-   const char *name = used_atom[i];
+   const char *name = used_atom[i].str;
struct atom_value *v = [i];
if (!!deref != (*name == '*'))
continue;
@@ -383,7 +385,7 @@ static void grab_tag_values(struct atom_value *val, int 
deref, struct object *ob
struct tag *tag = (struct tag *) obj;
 
for (i = 0; i < used_atom_cnt; i++) {
-   const char *name = used_atom[i];
+   const char *name = used_atom[i].str;
struct atom_value *v = [i];
if (!!deref != (*name == '*'))
continue;
@@ -405,7 +407,7 @@ static void grab_commit_values(struct atom_value *val, int 
deref, struct object
struct commit *commit = (struct commit *) obj;
 
for (i = 0; i < used_atom_cnt; i++) {
-   const char *name = used_atom[i];
+   const char *name = used_atom[i].str;
struct atom_value *v = [i];
if (!!deref != (*name == '*'))
  

[PATCH v2 02/11] ref-filter: use strbuf_split_str_omit_term()

2015-12-16 Thread Karthik Nayak
Use the newly introduced strbuf_split_str_omit_term() rather than
using strbuf_split_str() and manually removing the ',' terminator.

Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index e205dd2..6263710 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -869,18 +869,11 @@ static void populate_value(struct ref_array_item *ref)
if (!valp)
die(_("expected format: 
%%(align:,)"));
 
-   /*
-* TODO: Implement a function similar to 
strbuf_split_str()
-* which would omit the separator from the end of each 
value.
-*/
-   s = to_free = strbuf_split_str(valp, ',', 0);
+   s = to_free = strbuf_split_str_omit_term(valp, ',', 0);
 
align->position = ALIGN_LEFT;
 
while (*s) {
-   /*  Strip trailing comma */
-   if (s[1])
-   strbuf_setlen(s[0], s[0]->len - 1);
if (!strtoul_ui(s[0]->buf, 10, (unsigned int 
*)))
;
else if (!strcmp(s[0]->buf, "left"))
-- 
2.6.4

--
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 06/11] ref-filter: introduce color_atom_parser()

2015-12-16 Thread Karthik Nayak
Introduce color_atom_parser() which will parse a "color" atom and
store its color in the "use_atom" structure for further usage in
'populate_value()'.

Helped-by: Ramsay Jones 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 7d33b83..3b61c62 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -29,6 +29,9 @@ typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 static struct used_atom {
const char *str;
cmp_type type;
+   union {
+   const char *color;
+   } u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
 static int need_color_reset_at_eol;
@@ -53,6 +56,13 @@ static int match_atom_name(const char *name, const char 
*atom_name, const char *
return 1;
 }
 
+static void color_atom_parser(struct used_atom *atom)
+{
+   match_atom_name(atom->str, "color", >u.color);
+   if (!atom->u.color)
+   die(_("expected format: %%(color:)"));
+}
+
 static struct {
const char *name;
cmp_type cmp_type;
@@ -90,7 +100,7 @@ static struct {
{ "symref" },
{ "flag" },
{ "HEAD" },
-   { "color" },
+   { "color", FIELD_STR, color_atom_parser },
{ "align" },
{ "end" },
 };
@@ -175,6 +185,9 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
REALLOC_ARRAY(used_atom, used_atom_cnt);
used_atom[at].str = xmemdupz(atom, ep - atom);
used_atom[at].type = valid_atom[i].cmp_type;
+   memset(_atom[at].u, 0, sizeof(used_atom[at].u));
+   if (valid_atom[i].parser)
+   valid_atom[i].parser(_atom[at]);
if (*atom == '*')
need_tagged = 1;
if (!strcmp(used_atom[at].str, "symref"))
@@ -833,12 +846,10 @@ static void populate_value(struct ref_array_item *ref)
refname = branch_get_push(branch, NULL);
if (!refname)
continue;
-   } else if (match_atom_name(name, "color", )) {
+   } else if (starts_with(name, "color:")) {
char color[COLOR_MAXLEN] = "";
 
-   if (!valp)
-   die(_("expected format: %%(color:)"));
-   if (color_parse(valp, color) < 0)
+   if (color_parse(atom->u.color, color) < 0)
die(_("unable to parse format"));
v->s = xstrdup(color);
continue;
-- 
2.6.4

--
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 05/11] ref-filter: skip deref specifier in match_atom_name()

2015-12-16 Thread Karthik Nayak
In upcoming patches we make calls to match_atom_name() with the '*'
deref specifier still attached to the atom name. This causes
undesirable errors, hence, if present skip over the '*' deref
specifier in the atom name.

Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index f4a6414..7d33b83 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -37,6 +37,10 @@ static int match_atom_name(const char *name, const char 
*atom_name, const char *
 {
const char *body;
 
+   /*  skip the deref specifier*/
+   if (name[0] == '*')
+   name++;
+
if (!skip_prefix(name, atom_name, ))
return 0; /* doesn't even begin with "atom_name" */
if (!body[0]) {
-- 
2.6.4

--
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 01/11] strbuf: introduce strbuf_split_str_without_term()

2015-12-16 Thread Karthik Nayak
The current implementation of 'strbuf_split_buf()' includes the
terminator at the end of each strbuf post splitting. Add an option
wherein we can drop the terminator if desired. In this context
introduce a wrapper function 'strbuf_split_str_without_term()' which
splits a given string into strbufs without including the terminator.

Signed-off-by: Karthik Nayak 
---
 strbuf.c |  7 ---
 strbuf.h | 25 -
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/strbuf.c b/strbuf.c
index b552a13..b62508e 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -115,7 +115,7 @@ void strbuf_tolower(struct strbuf *sb)
 }
 
 struct strbuf **strbuf_split_buf(const char *str, size_t slen,
-int terminator, int max)
+int terminator, int max, int omit_term)
 {
struct strbuf **ret = NULL;
size_t nr = 0, alloc = 0;
@@ -123,14 +123,15 @@ struct strbuf **strbuf_split_buf(const char *str, size_t 
slen,
 
while (slen) {
int len = slen;
+   const char *end = NULL;
if (max <= 0 || nr + 1 < max) {
-   const char *end = memchr(str, terminator, slen);
+   end = memchr(str, terminator, slen);
if (end)
len = end - str + 1;
}
t = xmalloc(sizeof(struct strbuf));
strbuf_init(t, len);
-   strbuf_add(t, str, len);
+   strbuf_add(t, str, len - !!end * !!omit_term);
ALLOC_GROW(ret, nr + 2, alloc);
ret[nr++] = t;
str += len;
diff --git a/strbuf.h b/strbuf.h
index 6ae7a72..a865a74 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -450,11 +450,12 @@ static inline int strbuf_strip_suffix(struct strbuf *sb, 
const char *suffix)
 /**
  * Split str (of length slen) at the specified terminator character.
  * Return a null-terminated array of pointers to strbuf objects
- * holding the substrings.  The substrings include the terminator,
- * except for the last substring, which might be unterminated if the
- * original string did not end with a terminator.  If max is positive,
- * then split the string into at most max substrings (with the last
- * substring containing everything following the (max-1)th terminator
+ * holding the substrings.  The substrings include the terminator if
+ * the value of omit_term is '0' else the terminator is excluded.  The
+ * last substring, might be unterminated if the original string did
+ * not end with a terminator.  If max is positive, then split the
+ * string into at most max substrings (with the last substring
+ * containing everything following the (max-1)th terminator
  * character).
  *
  * The most generic form is `strbuf_split_buf`, which takes an arbitrary
@@ -465,19 +466,25 @@ static inline int strbuf_strip_suffix(struct strbuf *sb, 
const char *suffix)
  * For lighter-weight alternatives, see string_list_split() and
  * string_list_split_in_place().
  */
-extern struct strbuf **strbuf_split_buf(const char *, size_t,
-   int terminator, int max);
+extern struct strbuf **strbuf_split_buf(const char *str, size_t slen,
+   int terminator, int max, int omit_term);
+
+static inline struct strbuf **strbuf_split_str_omit_term(const char *str,
+   int terminator, int 
max)
+{
+   return strbuf_split_buf(str, strlen(str), terminator, max, 1);
+}
 
 static inline struct strbuf **strbuf_split_str(const char *str,
   int terminator, int max)
 {
-   return strbuf_split_buf(str, strlen(str), terminator, max);
+   return strbuf_split_buf(str, strlen(str), terminator, max, 0);
 }
 
 static inline struct strbuf **strbuf_split_max(const struct strbuf *sb,
int terminator, int max)
 {
-   return strbuf_split_buf(sb->buf, sb->len, terminator, max);
+   return strbuf_split_buf(sb->buf, sb->len, terminator, max, 0);
 }
 
 static inline struct strbuf **strbuf_split(const struct strbuf *sb,
-- 
2.6.4

--
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 08/11] ref-filter: introduce prefixes for the align atom

2015-12-16 Thread Karthik Nayak
Introduce optional prefixes "width=" and "position=" for the align atom
so that the atom can be used as "%(align:width=,position=)".

Add Documetation and tests for the same.

Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  20 +++--
 ref-filter.c   |  13 ++-
 t/t6302-for-each-ref-filter.sh | 162 +
 3 files changed, 185 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index c6f073c..9af0f53 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -129,14 +129,18 @@ color::
 
 align::
Left-, middle-, or right-align the content between
-   %(align:...) and %(end). The "align:" is followed by ``
-   and `` in any order separated by a comma, where the
-   `` is either left, right or middle, default being
-   left and `` is the total length of the content with
-   alignment. If the contents length is more than the width then
-   no alignment is performed. If used with '--quote' everything
-   in between %(align:...) and %(end) is quoted, but if nested
-   then only the topmost level performs quoting.
+   %(align:...) and %(end). The "align:" is followed by
+   `width=` and `position=` in any order
+   separated by a comma, where the `` is either left,
+   right or middle, default being left and `` is the total
+   length of the content with alignment. For brevity, the
+   "width=" and/or "position=" prefixes may be omitted, and bare
+and  used instead.  For instance,
+   `%(align:,)`. If the contents length is more
+   than the width then no alignment is performed. If used with
+   '--quote' everything in between %(align:...) and %(end) is
+   quoted, but if nested then only the topmost level performs
+   quoting.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/ref-filter.c b/ref-filter.c
index a44673c..985423b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -96,10 +96,19 @@ static void align_atom_parser(struct used_atom *atom)
 
while (*s) {
int position;
+   buf = s[0]->buf;
 
-   if (!strtoul_ui(s[0]->buf, 10, (unsigned int *)))
+   if (skip_prefix(buf, "position=", )) {
+   position = parse_align_position(buf);
+   if (position == -1)
+   die(_("unrecognized position:%s"), buf);
+   align->position = position;
+   } else if (skip_prefix(buf, "width=", )) {
+   if (strtoul_ui(buf, 10, (unsigned int *)))
+   die(_("unrecognized width:%s"), buf);
+   } else if (!strtoul_ui(buf, 10, (unsigned int *)))
;
-   else if ((position = parse_align_position(s[0]->buf)) > 0)
+   else if ((position = parse_align_position(buf)) != -1)
align->position = position;
else
die(_("unrecognized %%(align) argument: %s"), 
s[0]->buf);
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index fe4796c..bcb6771 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -133,6 +133,168 @@ test_expect_success 'right alignment' '
test_cmp expect actual
 '
 
+test_expect_success 'alignment with "position" prefix' '
+   cat >expect <<-\EOF &&
+   |  refname is refs/heads/master|refs/heads/master
+   |refname is refs/heads/side|refs/heads/side
+   |  refname is refs/odd/spot|refs/odd/spot
+   |refname is refs/tags/double-tag|refs/tags/double-tag
+   | refname is refs/tags/four|refs/tags/four
+   |  refname is refs/tags/one|refs/tags/one
+   |refname is refs/tags/signed-tag|refs/tags/signed-tag
+   |refname is refs/tags/three|refs/tags/three
+   |  refname is refs/tags/two|refs/tags/two
+   EOF
+   git for-each-ref --format="|%(align:30,position=right)refname is 
%(refname)%(end)|%(refname)" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'alignment with "position" prefix' '
+   cat >expect <<-\EOF &&
+   |  refname is refs/heads/master|refs/heads/master
+   |refname is refs/heads/side|refs/heads/side
+   |  refname is refs/odd/spot|refs/odd/spot
+   |refname is refs/tags/double-tag|refs/tags/double-tag
+   | refname is refs/tags/four|refs/tags/four
+   |  refname is refs/tags/one|refs/tags/one
+   |refname is refs/tags/signed-tag|refs/tags/signed-tag
+   |refname is refs/tags/three|refs/tags/three
+   |  refname is refs/tags/two|refs/tags/two
+   EOF
+   git for-each-ref 

Re: [PATCH 2/2] Fix tyop in the 2.7.0 release notes

2015-12-16 Thread Johannes Schindelin
Hi Gábor,

On Wed, 16 Dec 2015, SZEDER Gábor wrote:

> /^Subject:/ s/tyop/typo/

That was actually on purpose... I guess I made it a habit in another
project to call it tyop from time to time to take out the edge (many
scientists are pretty bda at speling ;-)).

But sure, if that joke is lost, let's not have a tyop in the comit mesage
;-)

Ciao,
Dscho

No auto CRLF conversion in Commit Message comments

2015-12-16 Thread Harfoot A.J.
Hi there,

I'm new to Git, so apologies if this is already available, but after some 
searching and experimenting I haven't been able to resolve it.

I am running Git 2.6.4.windows.1 on Windows 7 64 bit.

I have the global configuration variable core.autocrlf=true

When I commit, the commit message template file is loaded into my configured 
text editor (in this case Windows Notepad), however the comment lines are 
terminated with LF, not CRLF, and so are not displayed as new lines by Notepad.
In the opposite direction, the output of 'git log' redirected to a file has all 
EOL characters set to LF, when originally entered as CRLF, so automatic 
conversion is working

I have tried to generate a custom commit message using the commit.template 
variable, but the commented lines are appended to this file, creating a mixture 
of EOL characters!

Is there a way to fix this inconsistency?

Cheers,

Andy

-- 

Andy Harfoot

GeoData Institute
University of Southampton
Southampton
SO17 1BJ

Tel:  +44 (0)23 8059 2719
Fax:  +44 (0)23 8059 2849

www.geodata.soton.ac.uk--
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 07/11] ref-filter: introduce align_atom_parser()

2015-12-16 Thread Karthik Nayak
Introduce align_atom_parser() which will parse an "align" atom and
store the required alignment position and width in the "use_atom"
structure for further usage in 'populate_value()'.

Helped-by: Ramsay Jones 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 86 +++-
 1 file changed, 51 insertions(+), 35 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 3b61c62..a44673c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -16,6 +16,11 @@
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
+struct align {
+   align_type position;
+   unsigned int width;
+};
+
 /*
  * An atom is a valid field atom listed below, possibly prefixed with
  * a "*" to denote deref_tag().
@@ -31,6 +36,7 @@ static struct used_atom {
cmp_type type;
union {
const char *color;
+   struct align align;
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
@@ -63,6 +69,49 @@ static void color_atom_parser(struct used_atom *atom)
die(_("expected format: %%(color:)"));
 }
 
+static align_type parse_align_position(const char *s)
+{
+   if (!strcmp(s, "right"))
+   return ALIGN_RIGHT;
+   else if (!strcmp(s, "middle"))
+   return ALIGN_MIDDLE;
+   else if (!strcmp(s, "left"))
+   return ALIGN_LEFT;
+   return -1;
+}
+
+static void align_atom_parser(struct used_atom *atom)
+{
+   struct align *align = >u.align;
+   const char *buf = NULL;
+   struct strbuf **s, **to_free;
+   int width = -1;
+
+   match_atom_name(atom->str, "align", );
+   if (!buf)
+   die(_("expected format: %%(align:,)"));
+   s = to_free = strbuf_split_str_omit_term(buf, ',', 0);
+
+   align->position = ALIGN_LEFT;
+
+   while (*s) {
+   int position;
+
+   if (!strtoul_ui(s[0]->buf, 10, (unsigned int *)))
+   ;
+   else if ((position = parse_align_position(s[0]->buf)) > 0)
+   align->position = position;
+   else
+   die(_("unrecognized %%(align) argument: %s"), 
s[0]->buf);
+   s++;
+   }
+
+   if (width < 0)
+   die(_("positive width expected with the %%(align) atom"));
+   align->width = width;
+   strbuf_list_free(to_free);
+}
+
 static struct {
const char *name;
cmp_type cmp_type;
@@ -101,17 +150,12 @@ static struct {
{ "flag" },
{ "HEAD" },
{ "color", FIELD_STR, color_atom_parser },
-   { "align" },
+   { "align", FIELD_STR, align_atom_parser },
{ "end" },
 };
 
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
 
-struct align {
-   align_type position;
-   unsigned int width;
-};
-
 struct contents {
unsigned int lines;
struct object_id oid;
@@ -880,35 +924,7 @@ static void populate_value(struct ref_array_item *ref)
v->s = " ";
continue;
} else if (match_atom_name(name, "align", )) {
-   struct align *align = >u.align;
-   struct strbuf **s, **to_free;
-   int width = -1;
-
-   if (!valp)
-   die(_("expected format: 
%%(align:,)"));
-
-   s = to_free = strbuf_split_str_omit_term(valp, ',', 0);
-
-   align->position = ALIGN_LEFT;
-
-   while (*s) {
-   if (!strtoul_ui(s[0]->buf, 10, (unsigned int 
*)))
-   ;
-   else if (!strcmp(s[0]->buf, "left"))
-   align->position = ALIGN_LEFT;
-   else if (!strcmp(s[0]->buf, "right"))
-   align->position = ALIGN_RIGHT;
-   else if (!strcmp(s[0]->buf, "middle"))
-   align->position = ALIGN_MIDDLE;
-   else
-   die(_("improper format entered 
align:%s"), s[0]->buf);
-   s++;
-   }
-
-   if (width < 0)
-   die(_("positive width expected with the 
%%(align) atom"));
-   align->width = width;
-   strbuf_list_free(to_free);
+   v->u.align = atom->u.align;
v->handler = align_atom_handler;
continue;
} else if (!strcmp(name, "end")) {
-- 
2.6.4

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


[PATCH v2 11/11] ref-filter: introduce objectname_atom_parser()

2015-12-16 Thread Karthik Nayak
Introduce objectname_atom_parser() which will parse the
'%(objectname)' atom and store information into the 'used_atom'
structure based on the modifiers used along with the atom.

Helped-by: Ramsay Jones 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 39 ---
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index ce02933..a8fd178 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -43,6 +43,7 @@ static struct used_atom {
enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, 
C_SUB } option;
unsigned int no_lines;
} contents;
+   enum { O_FULL, O_SHORT } objectname;
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
@@ -124,6 +125,21 @@ static void contents_atom_parser(struct used_atom *atom)
die(_("unrecognized %%(contents) argument: %s"), buf);
 }
 
+static void objectname_atom_parser(struct used_atom *atom)
+{
+   const char * buf;
+
+   if (match_atom_name(atom->str, "objectname", ))
+   atom->u.objectname = O_FULL;
+   if (!buf)
+   return;
+
+   if (!strcmp(buf, "short"))
+   atom->u.objectname = O_SHORT;
+   else
+   die(_("unrecognized %%(objectname) argument: %s"), buf);
+}
+
 static align_type parse_align_position(const char *s)
 {
if (!strcmp(s, "right"))
@@ -184,7 +200,7 @@ static struct {
{ "refname" },
{ "objecttype" },
{ "objectsize", FIELD_ULONG },
-   { "objectname" },
+   { "objectname", FIELD_STR, objectname_atom_parser },
{ "tree" },
{ "parent" },
{ "numparent", FIELD_ULONG },
@@ -461,15 +477,16 @@ static void *get_obj(const unsigned char *sha1, struct 
object **obj, unsigned lo
 }
 
 static int grab_objectname(const char *name, const unsigned char *sha1,
-   struct atom_value *v)
+  struct atom_value *v, struct used_atom *atom)
 {
-   if (!strcmp(name, "objectname")) {
-   v->s = xstrdup(sha1_to_hex(sha1));
-   return 1;
-   }
-   if (!strcmp(name, "objectname:short")) {
-   v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));
-   return 1;
+   if (starts_with(name, "objectname")) {
+   if (atom->u.objectname == O_SHORT) {
+   v->s = xstrdup(find_unique_abbrev(sha1, 
DEFAULT_ABBREV));
+   return 1;
+   } else if (atom->u.objectname == O_FULL) {
+   v->s = xstrdup(sha1_to_hex(sha1));
+   return 1;
+   }
}
return 0;
 }
@@ -493,7 +510,7 @@ static void grab_common_values(struct atom_value *val, int 
deref, struct object
v->s = xstrfmt("%lu", sz);
}
else if (deref)
-   grab_objectname(name, obj->sha1, v);
+   grab_objectname(name, obj->sha1, v, _atom[i]);
}
 }
 
@@ -999,7 +1016,7 @@ static void populate_value(struct ref_array_item *ref)
v->s = xstrdup(buf + 1);
}
continue;
-   } else if (!deref && grab_objectname(name, ref->objectname, v)) 
{
+   } else if (!deref && grab_objectname(name, ref->objectname, v, 
atom)) {
continue;
} else if (!strcmp(name, "HEAD")) {
const char *head;
-- 
2.6.4

--
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 09/11] ref-filter: introduce remote_ref_atom_parser()

2015-12-16 Thread Karthik Nayak
Introduce remote_ref_atom_parser() which will parse the '%(upstream)'
and '%(push)' atoms and store information into the 'used_atom'
structure based on the modifiers used along with the corresponding
atom.

Helped-by: Ramsay Jones 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 105 ---
 1 file changed, 64 insertions(+), 41 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 985423b..44670e3 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -37,6 +37,8 @@ static struct used_atom {
union {
const char *color;
struct align align;
+   enum { RR_SHORTEN, RR_TRACK, RR_TRACKSHORT, RR_NORMAL }
+   remote_ref;
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
@@ -69,6 +71,25 @@ static void color_atom_parser(struct used_atom *atom)
die(_("expected format: %%(color:)"));
 }
 
+static void remote_ref_atom_parser(struct used_atom *atom)
+{
+   const char *buf;
+
+   buf = strchr(atom->str, ':');
+   atom->u.remote_ref = RR_NORMAL;
+   if (!buf)
+   return;
+   buf++;
+   if (!strcmp(buf, "short"))
+   atom->u.remote_ref = RR_SHORTEN;
+   else if (!strcmp(buf, "track"))
+   atom->u.remote_ref = RR_TRACK;
+   else if (!strcmp(buf, "trackshort"))
+   atom->u.remote_ref = RR_TRACKSHORT;
+   else
+   die(_("unrecognized format: %%(%s)"), atom->str);
+}
+
 static align_type parse_align_position(const char *s)
 {
if (!strcmp(s, "right"))
@@ -153,8 +174,8 @@ static struct {
{ "subject" },
{ "body" },
{ "contents" },
-   { "upstream" },
-   { "push" },
+   { "upstream", FIELD_STR, remote_ref_atom_parser },
+   { "push", FIELD_STR, remote_ref_atom_parser },
{ "symref" },
{ "flag" },
{ "HEAD" },
@@ -835,6 +856,42 @@ static inline char *copy_advance(char *dst, const char 
*src)
return dst;
 }
 
+static void fill_remote_ref_details(struct used_atom *atom, const char 
*refname,
+   struct branch *branch, const char **s)
+{
+   int num_ours, num_theirs;
+   if (atom->u.remote_ref == RR_SHORTEN)
+   *s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
+   else if (atom->u.remote_ref == RR_TRACK) {
+   if (stat_tracking_info(branch, _ours,
+  _theirs, NULL))
+   return;
+   if (!num_ours && !num_theirs)
+   *s = "";
+   else if (!num_ours)
+   *s = xstrfmt("[behind %d]", num_theirs);
+   else if (!num_theirs)
+   *s = xstrfmt("[ahead %d]", num_ours);
+   else
+   *s = xstrfmt("[ahead %d, behind %d]",
+num_ours, num_theirs);
+   } else if (atom->u.remote_ref == RR_TRACKSHORT) {
+   if (stat_tracking_info(branch, _ours,
+  _theirs, NULL))
+   return;
+
+   if (!num_ours && !num_theirs)
+   *s = "=";
+   else if (!num_ours)
+   *s = "<";
+   else if (!num_theirs)
+   *s = ">";
+   else
+   *s = "<>";
+   } else if (atom->u.remote_ref == RR_NORMAL)
+   *s = refname;
+}
+
 /*
  * Parse the object referred by ref, and grab needed value.
  */
@@ -889,6 +946,8 @@ static void populate_value(struct ref_array_item *ref)
refname = branch_get_upstream(branch, NULL);
if (!refname)
continue;
+   fill_remote_ref_details(atom, refname, branch, >s);
+   continue;
} else if (starts_with(name, "push")) {
const char *branch_name;
if (!skip_prefix(ref->refname, "refs/heads/",
@@ -899,6 +958,8 @@ static void populate_value(struct ref_array_item *ref)
refname = branch_get_push(branch, NULL);
if (!refname)
continue;
+   fill_remote_ref_details(atom, refname, branch, >s);
+   continue;
} else if (starts_with(name, "color:")) {
char color[COLOR_MAXLEN] = "";
 
@@ -944,49 +1005,11 @@ static void populate_value(struct ref_array_item *ref)
 
formatp = strchr(name, ':');
if (formatp) {
-   int num_ours, num_theirs;
-
formatp++;
if (!strcmp(formatp, "short"))
refname = 

[PATCH v2 10/11] ref-filter: introduce contents_atom_parser()

2015-12-16 Thread Karthik Nayak
Introduce contents_atom_parser() which will parse the '%(contents)'
atom and store information into the 'used_atom' structure based on the
modifiers used along with the atom.

Helped-by: Ramsay Jones 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 77 +---
 1 file changed, 48 insertions(+), 29 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 44670e3..ce02933 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -39,6 +39,10 @@ static struct used_atom {
struct align align;
enum { RR_SHORTEN, RR_TRACK, RR_TRACKSHORT, RR_NORMAL }
remote_ref;
+   struct {
+   enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, 
C_SUB } option;
+   unsigned int no_lines;
+   } contents;
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
@@ -90,6 +94,36 @@ static void remote_ref_atom_parser(struct used_atom *atom)
die(_("unrecognized format: %%(%s)"), atom->str);
 }
 
+static void contents_atom_parser(struct used_atom *atom)
+{
+   const char * buf;
+
+   if (match_atom_name(atom->str, "contents", ))
+   atom->u.contents.option = C_BARE;
+   else if (match_atom_name(atom->str, "subject", )) {
+   atom->u.contents.option = C_SUB;
+   return;
+   } else if (match_atom_name(atom->str, "body", )) {
+   atom->u.contents.option = C_BODY_DEP;
+   return;
+   }
+   if (!buf)
+   return;
+
+   if (!strcmp(buf, "body"))
+   atom->u.contents.option = C_BODY;
+   else if (!strcmp(buf, "signature"))
+   atom->u.contents.option = C_SIG;
+   else if (!strcmp(buf, "subject"))
+   atom->u.contents.option = C_SUB;
+   else if (skip_prefix(buf, "lines=", )) {
+   atom->u.contents.option = C_LINES;
+   if (strtoul_ui(buf, 10, >u.contents.no_lines))
+   die(_("positive value expected contents:lines=%s"), 
buf);
+   } else
+   die(_("unrecognized %%(contents) argument: %s"), buf);
+}
+
 static align_type parse_align_position(const char *s)
 {
if (!strcmp(s, "right"))
@@ -171,9 +205,9 @@ static struct {
{ "taggerdate", FIELD_TIME },
{ "creator" },
{ "creatordate", FIELD_TIME },
-   { "subject" },
-   { "body" },
-   { "contents" },
+   { "subject", FIELD_STR, contents_atom_parser },
+   { "body", FIELD_STR, contents_atom_parser },
+   { "contents", FIELD_STR, contents_atom_parser },
{ "upstream", FIELD_STR, remote_ref_atom_parser },
{ "push", FIELD_STR, remote_ref_atom_parser },
{ "symref" },
@@ -186,11 +220,6 @@ static struct {
 
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
 
-struct contents {
-   unsigned int lines;
-   struct object_id oid;
-};
-
 struct ref_formatting_stack {
struct ref_formatting_stack *prev;
struct strbuf output;
@@ -207,7 +236,6 @@ struct atom_value {
const char *s;
union {
struct align align;
-   struct contents contents;
} u;
void (*handler)(struct atom_value *atomv, struct ref_formatting_state 
*state);
unsigned long ul; /* used for sorting when not FIELD_STR */
@@ -756,20 +784,16 @@ static void grab_sub_body_contents(struct atom_value 
*val, int deref, struct obj
unsigned long sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
 
for (i = 0; i < used_atom_cnt; i++) {
-   const char *name = used_atom[i].str;
+   struct used_atom *atom = _atom[i];
+   const char *name = atom->str;
struct atom_value *v = [i];
-   const char *valp = NULL;
if (!!deref != (*name == '*'))
continue;
if (deref)
name++;
if (strcmp(name, "subject") &&
strcmp(name, "body") &&
-   strcmp(name, "contents") &&
-   strcmp(name, "contents:subject") &&
-   strcmp(name, "contents:body") &&
-   strcmp(name, "contents:signature") &&
-   !starts_with(name, "contents:lines="))
+   !starts_with(name, "contents"))
continue;
if (!subpos)
find_subpos(buf, sz,
@@ -777,28 +801,23 @@ static void grab_sub_body_contents(struct atom_value 
*val, int deref, struct obj
, , ,
, );
 
-   if (!strcmp(name, "subject"))
+   if (atom->u.contents.option == C_SUB)
v->s = copy_subject(subpos, sublen);
-   else if (!strcmp(name, "contents:subject"))
-   

[PATCH] credential-store: don't pass strerror to die_errno()

2015-12-16 Thread SZEDER Gábor
Signed-off-by: SZEDER Gábor 
---

v2 fixed this, but it fell on the floor, I suppose because of the
maintainer switch.  Anyway, I should have noticed it while the patch
was still cooking, sorry.

 credential-store.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/credential-store.c b/credential-store.c
index fc67d16c1088..54c4e0473737 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -64,8 +64,7 @@ static void rewrite_credential_file(const char *fn, struct 
credential *c,
print_line(extra);
parse_credential_file(fn, c, NULL, print_line);
if (commit_lock_file(_lock) < 0)
-   die_errno("unable to write credential store: %s",
- strerror(errno));
+   die_errno("unable to write credential store");
 }
 
 static void store_credential_file(const char *fn, struct credential *c)
-- 
2.7.0.rc0.37.g77d69b9

--
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: Question: Exclusions for AV-Software

2015-12-16 Thread Johannes Schindelin
Hi Roman,

On Wed, 16 Dec 2015, ro...@roume.de wrote:

> do you have any recommendations how exclusions must be set in an anti
> virus software what directories etc.?

If you are talking about a Git for Windows installed into C:\Program Files
(or even C:\Program Files (x86)), it is safe to exclude that location
(after an initial scan, of course) because Git for Windows itself does not
even have write permissions to that location.

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


[PATCH 2/2] Fix tyop in the 2.7.0 release notes

2015-12-16 Thread Johannes Schindelin
Signed-off-by: Johannes Schindelin 
---
 Documentation/RelNotes/2.7.0.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/RelNotes/2.7.0.txt b/Documentation/RelNotes/2.7.0.txt
index a84caba..eb179cc 100644
--- a/Documentation/RelNotes/2.7.0.txt
+++ b/Documentation/RelNotes/2.7.0.txt
@@ -22,7 +22,7 @@ UI, Workflows & Features
 
  * Users who are too busy to type three extra keystrokes to ask for
"git stash show -p" can now set stash.showPatch configuration
-   varible to true to always see the actual patch, not just the list
+   variable to true to always see the actual patch, not just the list
of paths affected with feel for the extent of damage via diffstat.
 
  * "quiltimport" allows to specify the series file by honoring the
-- 
2.6.3.windows.1.300.g1c25e49
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] t3404: fix typo

2015-12-16 Thread Johannes Schindelin
Signed-off-by: Johannes Schindelin 
---
 t/t3404-rebase-interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 544f9ad..11dde66 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -62,7 +62,7 @@ test_expect_success 'setup' '
 
 # "exec" commands are ran with the user shell by default, but this may
 # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
-# to create a file. Unseting SHELL avoids such non-portable behavior
+# to create a file. Unsetting SHELL avoids such non-portable behavior
 # in tests. It must be exported for it to take effect where needed.
 SHELL=
 export SHELL
-- 
2.6.3.windows.1.300.g1c25e49


--
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 0/2] Fix two tyops

2015-12-16 Thread Johannes Schindelin
These two grammar fixes are here solely to ramp up my commit count.

:-)


Johannes Schindelin (2):
  t3404: fix typo
  Fix tyop in the 2.7.0 release notes

 Documentation/RelNotes/2.7.0.txt | 2 +-
 t/t3404-rebase-interactive.sh| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.6.3.windows.1.300.g1c25e49

--
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: What's cooking in git.git (Dec 2015, #05; Tue, 15)

2015-12-16 Thread Johannes Schindelin
Hi Junio,

On Tue, 15 Dec 2015, Junio C Hamano wrote:

> Junio C Hamano  writes:
> 
> > There already was strbuf_getline_crlf(), and I wanted a new name to
> > be conservative.
> 
> When I re-read the series, I realize that the existing one had
> exactly the same semantics as strbuf_gets(), so I think no risk
> would come from reusing that name.  Let me try redoing the series
> when I find time ;-)

v1 is still in my inbox, and I promise it has not been malice that I did
not review it yet. Hopefully I will have enough brain cycles available
when you post v2.

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


[PATCH] dir: mark add_untracked_ident() as a file local symbol

2015-12-16 Thread Ramsay Jones

Signed-off-by: Ramsay Jones 
---

Hi Christian,

If you need to re-roll your 'cc/untracked' branch, could you
please squash the relevant parts of this into your patches.

Thanks!

Also, you may what to consider removing the emulation of uname()
on MinGW, since this is no longer required after your series.

ATB,
Ramsay Jones

 dir.c | 2 +-
 dir.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index 94fba2a..be27afa 100644
--- a/dir.c
+++ b/dir.c
@@ -1908,7 +1908,7 @@ static int treat_leading_path(struct dir_struct *dir,
  * We used to save the location of the work tree and the kernel version,
  * but it was not a good idea, so we now just save an empty string.
  */
-void add_untracked_ident(struct untracked_cache *uc)
+static void add_untracked_ident(struct untracked_cache *uc)
 {
strbuf_addstr(>ident, "");
/* this strbuf contains a list of strings, save NUL too */
diff --git a/dir.h b/dir.h
index 1935b76..531c99a 100644
--- a/dir.h
+++ b/dir.h
@@ -307,7 +307,6 @@ void untracked_cache_add_to_index(struct index_state *, 
const char *);
 void free_untracked_cache(struct untracked_cache *);
 struct untracked_cache *read_untracked_extension(const void *data, unsigned 
long sz);
 void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
-void add_untracked_ident(struct untracked_cache *);
 void add_untracked_cache(void);
 void remove_untracked_cache(void);
 #endif
-- 
2.6.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: No auto CRLF conversion in Commit Message comments

2015-12-16 Thread Johannes Schindelin
Hi A.J.,

On Wed, 16 Dec 2015, Harfoot A.J. wrote:

> I'm new to Git, so apologies if this is already available, but after
> some searching and experimenting I haven't been able to resolve it.
> 
> I am running Git 2.6.4.windows.1 on Windows 7 64 bit.
> 
> I have the global configuration variable core.autocrlf=true
> 
> When I commit, the commit message template file is loaded into my
> configured text editor (in this case Windows Notepad), however the
> comment lines are terminated with LF, not CRLF, and so are not displayed
> as new lines by Notepad.

The core.autocrlf=true setting affects only Git's operation when it adds
file contents to the index or writes files from the index.

However, if you have `core.editor = notepad`, it should Just Work because
there is a `notepad` helper that performs the LF<->CR/LF translation
transparently.

> In the opposite direction, the output of 'git log' redirected to a file
> has all EOL characters set to LF, when originally entered as CRLF, so
> automatic conversion is working

Somehow I doubt that `git log` transforms CR/LF to LF...

> I have tried to generate a custom commit message using the
> commit.template variable, but the commented lines are appended to this
> file, creating a mixture of EOL characters!

I am afraid that the only way to keep that consistent would be to ensure
that your commit.template has the same line endings as your editor
produces.

Ciao,
Johannes
--
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: [PATCHv3] Makefile: add missing phony target

2015-12-16 Thread Matthieu Moy
Elia Pinto  writes:

> Add some missing phony target to Makefile.
>
> Signed-off-by: Elia Pinto 
> Helped-by: Matthieu Moy 
> ---
> This is the third version of this patch.

Thanks. I double-checked with

grep .PHONY Makefile | sed 's/.PHONY: //; s/#.*//' | tr ' ' '\n' | sort | uniq 
-c | sort -n

there are no duplicate .PHONY, and all .PHONY targets are indeed .PHONY
ones, so the patch is now

Reviewed-by: Matthieu Moy 

> Compared to the previous I have added only the missing phony 
> target as suggested by Matthieu Moy

Note: my comments were not meant to be an objection to the additional
refactoring. At least some of the hunks definitely made sense to me, but
they are better done in a separate patch.

-- 
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: (unknown)

2015-12-16 Thread Patrick Steinhardt
On Tue, Dec 15, 2015 at 09:57:50PM -0800, Junio C Hamano wrote:
> David Greene  writes:
> 
> > - If new option --keep-redundant is specified, invoke cherry-pick with
> >   --keep-redundant-commits.
> 
> This came up in the past several weeks, I think; you would need to
> disable patch-equivalence based commit filtering if you really want
> to do a --keep-redundant that is reproducible and/or reliable.

Here are the links to the previous proposal [1] and following
discussion [2] (see 'ps/rebase-keep-empty') if you are
interested.

Patrick

[1]: http://thread.gmane.org/gmane.comp.version-control.git/281515
[2]: http://thread.gmane.org/gmane.comp.version-control.git/281917


signature.asc
Description: Digital signature


[PATCH v2 2/2] push: add '-d' as shorthand for '--delete'

2015-12-16 Thread Patrick Steinhardt
It is possible to delete branches on remotes by specifying the
'--delete' flag. The `git-branch` command, which can be used to
delete local branches with the same '--delete' flag, also accepts
the shorthand '-d'. This may cause confusion for users which are
frequently using the shorthand form of deleting local branches
and subsequently try to use the same shorthand for `git-push`,
which will fail.

Fix this usability issue by adding the '-d' shorthand to
`git-push` and document it.

Signed-off-by: Patrick Steinhardt 
---
 Documentation/git-push.txt | 2 +-
 builtin/push.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index e830c08..6f5d98c 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic] [-n | 
--dry-run] [--receive-pack=]
-  [--repo=] [-f | --force] [--delete] [--prune] [-v | 
--verbose]
+  [--repo=] [-f | --force] [-d | --delete] [--prune] [-v | 
--verbose]
   [-u | --set-upstream]
   [--[no-]signed|--sign=(true|false|if-asked)]
   [--force-with-lease[=[:]]]
diff --git a/builtin/push.c b/builtin/push.c
index 3bda430..093011d 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -540,7 +540,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
OPT_BIT( 0 , "all", , N_("push all refs"), 
TRANSPORT_PUSH_ALL),
OPT_BIT( 0 , "mirror", , N_("mirror all refs"),
(TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
-   OPT_BOOL( 0, "delete", , N_("delete refs")),
+   OPT_BOOL('d', "delete", , N_("delete refs")),
OPT_BOOL( 0 , "tags", , N_("push tags (can't be used with 
--all or --mirror)")),
OPT_BIT('n' , "dry-run", , N_("dry run"), 
TRANSPORT_PUSH_DRY_RUN),
OPT_BIT( 0,  "porcelain", , N_("machine-readable 
output"), TRANSPORT_PUSH_PORCELAIN),
-- 
2.6.4

--
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 1/2] push: add '--delete' flag to synopsis

2015-12-16 Thread Patrick Steinhardt
The delete flag is not mentioned in the synopsis of `git-push`.
Add the flag to make it more discoverable.

Signed-off-by: Patrick Steinhardt 
---

Only change to version 1 [1] is a slightly altered commit message
of the second commit. Previously, it suggested that '--delete' is
the only way to delete a remote ref with `git-push`, while there
is also `git push origin :ref`.

[1]: http://article.gmane.org/gmane.comp.version-control.git/282364

 Documentation/git-push.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 85a4d7d..e830c08 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic] [-n | 
--dry-run] [--receive-pack=]
-  [--repo=] [-f | --force] [--prune] [-v | --verbose]
+  [--repo=] [-f | --force] [--delete] [--prune] [-v | 
--verbose]
   [-u | --set-upstream]
   [--[no-]signed|--sign=(true|false|if-asked)]
   [--force-with-lease[=[:]]]
-- 
2.6.4

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


GIT 2.6.4 is listed on Software Informer

2015-12-16 Thread Kasey Bloome
Good day!

Software.informer.com would like to inform you that your product GIT 2.6.4 has 
been reviewed by our editors http://git.software.informer.com/.

We would be grateful if you place our logo with a link to our review on your 
website. On our part, we can offer featuring your program in our Today's 
Highlight block. This block is shown in the rotator at the top of the main page 
and also on every page of our website in the upper right corner.

We also offer you to take advantage of our free storage by hosting your 
installation package on our servers and listing us as one of the mirror 
downloads for your application. There is a selection of predesigned buttons 
available to fit the look of your website.

Please let me know if you're interested in any of these offers.

We are on the list of the world's 500 most visited websites with over 700,000 
unique visitors every day, so this could get your application some extra 
exposure.

Kind regards,
Kasey Bloome


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