date:20160404

On Mon, Apr 4, 2016 at 9:46 PM, Santiago Torres  wrote:
> Eric Sunshine wrote:
>> > +test_expect_success GPG 'verify multiple tags' '
>> > +   tags="fourth-signed sixth-signed seventh-signed" &&
>> > +   for i in $tags; do
>> > +   git verify-tag -v --raw $i || return 1
>> > +   done >expect.stdout 2>expect.stderr.1 &&
>> > +   grep GOODSIG expect.stderr &&
>> > +   git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
>> > +   grep GOODSIG actual.stderr &&
>>
>> Hmm, I had expected you to adopt Peff's suggestion[1] for the greps:
>>
>> grep '^.GNUPG:.' ...
>>
>> [1]: http://article.gmane.org/gmane.comp.version-control.git/290691
>
> I thought this was an stylistic thing. I can of course adopt this
> suggestion.

It's probably not a big deal, but Peff's suggestion (at least feels
like it) makes the test a bit more comprehensive.

By the way, as the test is heavily inspired by Peff's example, it
might be worth giving him a nod via a Helped-by: just above your
Signed-off-by:.
--
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 v4 6/6] tag.c: Change gpg_verify_tag argument to sha1

On Mon, Apr 4, 2016 at 10:10 PM, Santiago Torres  wrote:
> On Mon, Apr 04, 2016 at 10:00:17PM -0400, Eric Sunshine wrote:
>> On Mon, Apr 4, 2016 at 6:22 PM,   wrote:
>> > -   return gpg_verify_tag(name, GPG_VERIFY_VERBOSE);
>> > +   return gpg_verify_tag(sha1, GPG_VERIFY_VERBOSE);
>>
>> So, by this point, 'name' has already been resolved to 'sha1', thus
>> this change avoids a second resolution of 'name' inside
>> gpg_verify_tag(). Therefore, this is really an optimization, right?
>> Perhaps the intent of the patch would be clearer if the commit message
>> sold it as such. For instance, the commit message might start off:
>>
>> tag: avoid resolving tag name twice
>>
>> and then go on to say that by hefting tag name resolution out of
>> gpg_verify_tag(), the extra resolution can be avoided.
>
> Yep, this is actually true, but something I didn't consider. I think
> that, from what I could draw on [1] and [2], git tag -v is reserved to
> tags only (refs/tags iirc). This patch makes it so that this behavior is
> not lost. I'm not sure if it should be separate from 5/6 though.

Okay, so this is a fix for a regression introduced by the previous
patch. I agree that this is suboptimal. You can avoid this regression
issue altogether by merely re-ordering the patch series. For instance,
the series could be ordered like this:

1. SIGPIPE dance
2. add new t7030 test
3. improve variable name in verify_tag()
4. heft get_sha1() lookup out of verify_tag()
5. move code and publish gpg_verify_tag()
6. re-implement "git-tag -v" in terms of gpg_verify_tag()

Sell patch #4 as a libification preparation step, explaining as
justification that some future clients may already have the sha1 in
hand and would want to avoid duplicate resolution of the tag name.
--
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

support block comments in gitconfig

2016-04-04 Thread Timothee Cour

Could we have block comments in gitconfig?
It's a nice-to-have supported in most languages.
eg:

#{
commented out block
#}

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

Re: [PATCH v5 4/4] pretty: test --expand-tabs

On Mon, Apr 04, 2016 at 05:58:37PM -0700, Junio C Hamano wrote:

> +count_expand ()
> +{

This function takes a lot of unnamed arguments that we process with
"shift". It might be nice to give a brief comment describing them.

> +test_expand ""
> +test_expand --pretty
> +test_expand --pretty=short
> +test_expand --pretty=medium
> +test_expand --pretty=full
> +test_expand --pretty=fuller
> +test_expand --pretty=fuller
> +test_expand --pretty=raw
> +test_expand --pretty=email

Duplicated fuller?

-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 v4 6/6] tag.c: Change gpg_verify_tag argument to sha1

On Mon, Apr 04, 2016 at 10:00:17PM -0400, Eric Sunshine wrote:
> On Mon, Apr 4, 2016 at 6:22 PM,   wrote:
> > tag.c: Change gpg_verify_tag argument to sha1
> 
> s/Change/change/

Sorry I've been consistently missing these... 
> 
> > The gpg_verify_tag function resolves the ref for any existing object.
> > However, git tag -v resolves to only tag-refs. We can provide support
> > for sha1 by moving the refname resolution code out of gpg_verify_tag and
> > allow for the object's sha1 as an argument.
> 
> This description leaves me fairly clueless about why this change is
> being made since justification seems to be lacking. More about this
> below...
> 
> > Signed-off-by: Santiago Torres 
> > ---
> > diff --git a/builtin/tag.c b/builtin/tag.c
> > @@ -104,7 +104,7 @@ static int delete_tag(const char *name, const char *ref,
> >  static int verify_tag(const char *name, const char *ref,
> > const unsigned char *sha1)
> >  {
> > -   return gpg_verify_tag(name, GPG_VERIFY_VERBOSE);
> > +   return gpg_verify_tag(sha1, GPG_VERIFY_VERBOSE);
> >  }
> 
> So, by this point, 'name' has already been resolved to 'sha1', thus
> this change avoids a second resolution of 'name' inside
> gpg_verify_tag(). Therefore, this is really an optimization, right?
> Perhaps the intent of the patch would be clearer if the commit message
> sold it as such. For instance, the commit message might start off:
> 
> tag: avoid resolving tag name twice
> 
> and then go on to say that by hefting tag name resolution out of
> gpg_verify_tag(), the extra resolution can be avoided.

Yep, this is actually true, but something I didn't consider. I think
that, from what I could draw on [1] and [2], git tag -v is reserved to
tags only (refs/tags iirc). This patch makes it so that this behavior is
not lost. I'm not sure if it should be separate from 5/6 though. 
> 
> >  static int do_sign(struct strbuf *buffer)
> > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> > @@ -46,8 +47,12 @@ int cmd_verify_tag(int argc, const char **argv, const 
> > char *prefix)
> > if (verbose)
> > flags |= GPG_VERIFY_VERBOSE;
> >
> > -   while (i < argc)
> > -   if (gpg_verify_tag(argv[i++], flags))
> > +   while (i < argc) {
> > +   if (get_sha1(argv[i++], sha1))
> > +   return error("tag '%s' not found.", argv[i]);
> 
> Why does this 'return' after the first error, but the gpg_verify_tag()
> call below merely sets a 'had_error' flag and continues? I would
> expect this one to set the flag and continue, as well.

This sounds better than what I had thought. I'll set had error and do
continue instead.
> 
 >  {
> > enum object_type type;
> > -   unsigned char sha1[20];
> > char *buf;
> > unsigned long size;
> > int ret;
> >
> > -   if (get_sha1(name, sha1))
> > -   return error("tag '%s' not found.", name);
> > -
> > type = sha1_object_info(sha1, NULL);
> > if (type != OBJ_TAG)
> > -   return error("%s: cannot verify a non-tag object of type 
> > %s.",
> > -   name, typename(type));
> > +   return error("cannot verify a non-tag object of type %s.",
> > +   typename(type));
> 
> This error message becomes much less useful since it now only says
> that there is a problem with *some* tag but doesn't give any
> identifying information. How about including the sha1 in the error
> message?
> 
> >
> > buf = read_sha1_file(sha1, , );
> > if (!buf)
> > -   return error("%s: unable to read file.", name);
> > +   return error("unable to read file.");
> 
> Ditto regarding making this more useful by including the sha1.

yes, I wasn't sure about how to move forward here. I'll replace the name
with the sha1 instead of just removing it.

Thanks!
-Santiago.


[1] https://git.kernel.org/cgit/git/git.git/tree/builtin/tag.c
[2]
http://git.661346.n2.nabble.com/PATCH-v3-0-4-tag-move-PGP-verification-code-to-tag-c-tp7652334p7652437.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 v4 6/6] tag.c: Change gpg_verify_tag argument to sha1

On Mon, Apr 4, 2016 at 6:22 PM,   wrote:
> tag.c: Change gpg_verify_tag argument to sha1

s/Change/change/

> The gpg_verify_tag function resolves the ref for any existing object.
> However, git tag -v resolves to only tag-refs. We can provide support
> for sha1 by moving the refname resolution code out of gpg_verify_tag and
> allow for the object's sha1 as an argument.

This description leaves me fairly clueless about why this change is
being made since justification seems to be lacking. More about this
below...

> Signed-off-by: Santiago Torres 
> ---
> diff --git a/builtin/tag.c b/builtin/tag.c
> @@ -104,7 +104,7 @@ static int delete_tag(const char *name, const char *ref,
>  static int verify_tag(const char *name, const char *ref,
> const unsigned char *sha1)
>  {
> -   return gpg_verify_tag(name, GPG_VERIFY_VERBOSE);
> +   return gpg_verify_tag(sha1, GPG_VERIFY_VERBOSE);
>  }

So, by this point, 'name' has already been resolved to 'sha1', thus
this change avoids a second resolution of 'name' inside
gpg_verify_tag(). Therefore, this is really an optimization, right?
Perhaps the intent of the patch would be clearer if the commit message
sold it as such. For instance, the commit message might start off:

tag: avoid resolving tag name twice

and then go on to say that by hefting tag name resolution out of
gpg_verify_tag(), the extra resolution can be avoided.

>  static int do_sign(struct strbuf *buffer)
> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> @@ -46,8 +47,12 @@ int cmd_verify_tag(int argc, const char **argv, const char 
> *prefix)
> if (verbose)
> flags |= GPG_VERIFY_VERBOSE;
>
> -   while (i < argc)
> -   if (gpg_verify_tag(argv[i++], flags))
> +   while (i < argc) {
> +   if (get_sha1(argv[i++], sha1))
> +   return error("tag '%s' not found.", argv[i]);

Why does this 'return' after the first error, but the gpg_verify_tag()
call below merely sets a 'had_error' flag and continues? I would
expect this one to set the flag and continue, as well.

> +

Style: unnecessary blank line

> +   if (gpg_verify_tag(sha1, flags))
> had_error = 1;
> +   }
> return had_error;
>  }
> diff --git a/tag.c b/tag.c
> @@ -30,25 +30,21 @@ static int run_gpg_verify(const char *buf, unsigned long 
> size, unsigned flags)
> -int gpg_verify_tag(const char *name, unsigned flags)
> +int gpg_verify_tag(const unsigned char *sha1, unsigned flags)
>  {
> enum object_type type;
> -   unsigned char sha1[20];
> char *buf;
> unsigned long size;
> int ret;
>
> -   if (get_sha1(name, sha1))
> -   return error("tag '%s' not found.", name);
> -
> type = sha1_object_info(sha1, NULL);
> if (type != OBJ_TAG)
> -   return error("%s: cannot verify a non-tag object of type %s.",
> -   name, typename(type));
> +   return error("cannot verify a non-tag object of type %s.",
> +   typename(type));

This error message becomes much less useful since it now only says
that there is a problem with *some* tag but doesn't give any
identifying information. How about including the sha1 in the error
message?

>
> buf = read_sha1_file(sha1, , );
> if (!buf)
> -   return error("%s: unable to read file.", name);
> +   return error("unable to read file.");

Ditto regarding making this more useful by including the sha1.

> ret = run_gpg_verify(buf, size, flags);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 4/4] pretty: test --expand-tabs

On Mon, Apr 04, 2016 at 09:10:46PM -0400, Eric Sunshine wrote:

> > +   count=$1 ;# expected tabs
> 
> Why semicolon before the hash here and above?

I am in the habit of doing this, too. I have a vague recollection of
getting bitten by a shell that treated:

  echo foo # bar

or something similar as not-a-comment. But neither bash, dash, nor ksh
seem to. So I'm not sure if it was some other shell in my past, or if I
simply have an irrational fear.

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

Re: [PATCH v5 0/4] Expanding tabs in "git log" output

On Mon, Apr 04, 2016 at 05:58:33PM -0700, Junio C Hamano wrote:

> So here is the fifth and hopefully the final try.  Previous round
> are at $gmane/289694, $gmane/289166, $gmane/288987 and
> $gmane/290222.

With the exception of two minor nits on the final patch, this looks good
to me. Thanks for cleaning up the option-parsing ordering thing; I think
it turned out rather nice.

-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 v4 2/6] t/t7030-verify-tag.sh: Adds validation for multiple tags

> 
> The bulk of this description is merely repeating what the patch itself
> already says, thus is redundant. The entire commit message could
> probably be collapsed to:
> 
> t7030: test verify-tag with multiple tags
> 
> git-verify-tag accepts multiple tags as arguments, however,
> existing tests only ever invoke it with a single tag, so add a
> test invoking it with multiple tags.

Yeah, this is actually clearer. Sorry for the academ-ose commit message.

> > +test_expect_success GPG 'verify multiple tags' '
> > +   tags="fourth-signed sixth-signed seventh-signed" &&
> > +   for i in $tags; do
> > +   git verify-tag -v --raw $i || return 1
> > +   done >expect.stdout 2>expect.stderr.1 &&
> > +   grep GOODSIG expect.stderr &&
> > +   git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
> > +   grep GOODSIG actual.stderr &&
> 
> Hmm, I had expected you to adopt Peff's suggestion[1] for the greps:
> 
> grep '^.GNUPG:.' ...
> 
> [1]: http://article.gmane.org/gmane.comp.version-control.git/290691

I thought this was an stylistic thing. I can of course adopt this
suggestion.

Thanks,
-Santiago

--
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 v4 5/6] tag: use pgp_verify_function in tag -v call

On Mon, Apr 04, 2016 at 09:39:29PM -0400, Eric Sunshine wrote:
> On Mon, Apr 4, 2016 at 6:22 PM,   wrote:
> > Instead of running the verify-tag plumbing command, we use the
> > pgp_verify_tag(). This avoids the usage of an extra fork call. To do
> > this, we extend the number of parameters that tag.c takes, and
> > verify-tag passes. Redundant calls done in the pgp_verify_tag function
> > are removed.
> 
> I'm confused about everything following "an extra fork call" since
> those subsequent sentences don't seem to pertain to this patch. Is
> that leftover gunk from the previous version of this series?

Yes, I thought I had taken this part of the commit message away. 
I'll take it out right away. Apologies for this.

-Santiago.



--
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 v4 5/6] tag: use pgp_verify_function in tag -v call

On Mon, Apr 4, 2016 at 6:22 PM,   wrote:
> Instead of running the verify-tag plumbing command, we use the
> pgp_verify_tag(). This avoids the usage of an extra fork call. To do
> this, we extend the number of parameters that tag.c takes, and
> verify-tag passes. Redundant calls done in the pgp_verify_tag function
> are removed.

I'm confused about everything following "an extra fork call" since
those subsequent sentences don't seem to pertain to this patch. Is
that leftover gunk from the previous version of this series?

> Signed-off-by: Santiago Torres 
> ---
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 1705c94..f4450f8 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -104,13 +104,7 @@ static int delete_tag(const char *name, const char *ref,
>  static int verify_tag(const char *name, const char *ref,
> const unsigned char *sha1)
>  {
> -   const char *argv_verify_tag[] = {"verify-tag",
> -   "-v", "SHA1_HEX", NULL};
> -   argv_verify_tag[2] = sha1_to_hex(sha1);
> -
> -   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
> -   return error(_("could not verify the tag '%s'"), name);
> -   return 0;
> +   return gpg_verify_tag(name, GPG_VERIFY_VERBOSE);
>  }
>
>  static int do_sign(struct strbuf *buffer)
> --
--
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 v4 4/6] tag.c: Replace varialbe name for readability

On Mon, Apr 4, 2016 at 6:22 PM,   wrote:
> tag.c: Replace varialbe name for readability

s/Replace/replace/
s/varialbe/variable/

> The run_gpg_verify function has two variables size, and len. This may
> come off as confusing when reading the code. We clarify which one
> pertains to the length of the tag headers by renaming len to
> payload_length.
>
> Signed-off-by: Santiago Torres 
> ---
--
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 v4 2/6] t/t7030-verify-tag.sh: Adds validation for multiple tags

On Mon, Apr 4, 2016 at 6:22 PM,   wrote:
> t/t7030-verify-tag.sh: Adds validation for multiple tags

Rewrite:

t7030: test verify-tag with multiple tags

The leading "t/" and the trailing "-*.sh" were dropped since they add no value.

> The verify-tag command supports mutliple tag names as an argument.

s/mutliple/multiple/

> However, no previous tests try to verify multiple tags at once. This
> test runs the verify-tag command against three tags separately and then
> compares the result against the invocation with the same three tags as
> an argument. The results shouldn't differ.

The bulk of this description is merely repeating what the patch itself
already says, thus is redundant. The entire commit message could
probably be collapsed to:

t7030: test verify-tag with multiple tags

git-verify-tag accepts multiple tags as arguments, however,
existing tests only ever invoke it with a single tag, so add a
test invoking it with multiple tags.

> Signed-off-by: Santiago Torres 
> ---
> diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
> index 4608e71..f9161332 100755
> --- a/t/t7030-verify-tag.sh
> +++ b/t/t7030-verify-tag.sh
> @@ -112,4 +112,16 @@ test_expect_success GPG 'verify signatures with --raw' '
> +test_expect_success GPG 'verify multiple tags' '
> +   tags="fourth-signed sixth-signed seventh-signed" &&
> +   for i in $tags; do
> +   git verify-tag -v --raw $i || return 1
> +   done >expect.stdout 2>expect.stderr.1 &&
> +   grep GOODSIG expect.stderr &&
> +   git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
> +   grep GOODSIG actual.stderr &&

Hmm, I had expected you to adopt Peff's suggestion[1] for the greps:

grep '^.GNUPG:.' ...

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

> +   test_cmp expect.stdout actual.stdout &&
> +   test_cmp expect.stderr actual.stderr
> +'
> +
>  test_done
> --
> 2.8.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: [PATCH v5 4/4] pretty: test --expand-tabs

On Mon, Apr 4, 2016 at 8:58 PM, Junio C Hamano  wrote:
> The test prepares a simple commit with HT on its log message lines,
> and makes sure that
>
>  - formats that should or should not expand tabs by default do or do
>not expand tabs respectively,
>
>  - with explicit --expand-tabs= and short-hands --expand-tabs
>(equivalent to --expand-tabs=8) and --no-expand-tabs (equivalent
>to --expand-tabs=0) before or after the explicit --pretty=$fmt,
>the tabs are expanded (or not expanded) accordingly.
>
> The tests use the second line of the log message for formats other
> than --pretty=short, primarily because the first line of the email
> format is handled specially to add the [PATCH] prefix, etc. in a
> separate codepath (--pretty=short uses the first line because there
> is no other line to test).
>
> Signed-off-by: Junio C Hamano 
> ---
> diff --git a/t/t4213-log-tabexpand.sh b/t/t4213-log-tabexpand.sh
> @@ -0,0 +1,98 @@
> +count_expand ()
> +{
> +   case " $* " in
> +   *' --pretty=short '*)
> +   line=$title ;;
> +   *)
> +   line=$body ;;
> +   esac
> +   expect=
> +   count=$(( $1 + $2 )) ;# expected spaces
> +   while test $count -gt 0
> +   do
> +   expect="$expect "
> +   count=$(( $count - 1 ))
> +   done
> +   shift 2
> +   count=$1 ;# expected tabs

Why semicolon before the hash here and above?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 0/4] Expanding tabs in "git log" output

So here is the fifth and hopefully the final try.  Previous round
are at $gmane/289694, $gmane/289166, $gmane/288987 and
$gmane/290222.

This round is different from v4 in the following ways.

 * wording changes, grammo- and typo-fixes in the documentation
   (thanks to Eric Sunshine and Jeff King).

 * v4 made --pretty=$fmt to override an earlier --expand-tabs=;
   this round only allows --pretty=$fmt to set the default behaviour
   when an explicit --expand-tabs= is given on the command line
   (thanks to Jeff King).
   
 * comes with a test.

See the end of this cover letter for an interdiff.

Junio C Hamano (3):
  pretty: enable --expand-tabs by default for selected pretty formats
  pretty: allow tweaking tabwidth in --expand-tabs
  pretty: test --expand-tabs

Linus Torvalds (1):
  pretty: expand tabs in indented logs to make things line up properly

 Documentation/pretty-options.txt | 14 ++
 builtin/log.c|  1 +
 commit.h |  1 +
 log-tree.c   |  1 +
 pretty.c | 90 
 revision.c   | 14 ++
 revision.h   |  2 +
 t/t4201-shortlog.sh  |  2 +-
 t/t4213-log-tabexpand.sh | 98 
 9 files changed, 213 insertions(+), 10 deletions(-)
 create mode 100755 t/t4213-log-tabexpand.sh

-- 
2.8.1-251-g9997610


diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 8a944b1..93ad1cd 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -45,16 +45,16 @@ people using 80-column terminals.
 --expand-tabs=::
 --expand-tabs::
 --no-expand-tabs::
-   Perform a tab expansion (replace each tab with enough number
-   of spaces to fill to the next display column that is
-   multiple of '') in the log message before using the message
-   to show in the output.  `--expand-tabs` is a short-hand for
-   `--expand-tabs=8`, and `--no-expand-tabs` is a short-hand for
-   `--expand-tabs=0`, which disables tab expansion.
+   Perform a tab expansion (replace each tab with enough spaces
+   to fill to the next display column that is multiple of '')
+   in the log message before showing it in the output.
+   `--expand-tabs` is a short-hand for `--expand-tabs=8`, and
+   `--no-expand-tabs` is a short-hand for `--expand-tabs=0`,
+   which disables tab expansion.
 +
 By default, tabs are expanded in pretty formats that indent the log
 message by 4 spaces (i.e.  'medium', which is the default, 'full',
-and "fuller').
+and 'fuller').
 
 ifndef::git-rev-list[]
 --notes[=]::
diff --git a/builtin/log.c b/builtin/log.c
index e00cea7..e5775ae 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1281,6 +1281,7 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
git_config(git_format_config, NULL);
init_revisions(, prefix);
rev.commit_format = CMIT_FMT_EMAIL;
+   rev.expand_tabs_in_log_default = 0;
rev.verbose_header = 1;
rev.diff = 1;
rev.max_parents = 1;
diff --git a/commit.h b/commit.h
index 2185c8d..b06db4d 100644
--- a/commit.h
+++ b/commit.h
@@ -147,7 +147,7 @@ struct pretty_print_context {
int preserve_subject;
struct date_mode date_mode;
unsigned date_mode_explicit:1;
-   unsigned expand_tabs_in_log;
+   int expand_tabs_in_log;
int need_8bit_cte;
char *notes_message;
struct reflog_walk_info *reflog_info;
diff --git a/pretty.c b/pretty.c
index b340ecd..87c4497 100644
--- a/pretty.c
+++ b/pretty.c
@@ -173,7 +173,7 @@ void get_commit_format(const char *arg, struct rev_info 
*rev)
 
rev->commit_format = commit_format->format;
rev->use_terminator = commit_format->is_tformat;
-   rev->expand_tabs_in_log = commit_format->expand_tabs_in_log;
+   rev->expand_tabs_in_log_default = commit_format->expand_tabs_in_log;
if (commit_format->format == CMIT_FMT_USERFORMAT) {
save_user_format(rev, commit_format->user_format,
 commit_format->is_tformat);
@@ -1722,6 +1722,9 @@ void pp_remainder(struct pretty_print_context *pp,
strbuf_grow(sb, linelen + indent + 20);
if (indent)
pp_handle_indent(pp, sb, indent, line, linelen);
+   else if (pp->expand_tabs_in_log)
+   strbuf_add_tabexpand(sb, pp->expand_tabs_in_log,
+line, linelen);
else
strbuf_add(sb, line, linelen);
strbuf_addch(sb, '\n');
diff --git a/revision.c b/revision.c
index 4f9ecbe..47e9ee7 100644
--- a/revision.c
+++ b/revision.c
@@ -1412,9 +1412,10 @@ void init_revisions(struct rev_info *revs, const char 
*prefix)
revs->skip_count = -1;
revs->max_count = -1;

[PATCH v5 4/4] pretty: test --expand-tabs

The test prepares a simple commit with HT on its log message lines,
and makes sure that

 - formats that should or should not expand tabs by default do or do
   not expand tabs respectively,

 - with explicit --expand-tabs= and short-hands --expand-tabs
   (equivalent to --expand-tabs=8) and --no-expand-tabs (equivalent
   to --expand-tabs=0) before or after the explicit --pretty=$fmt,
   the tabs are expanded (or not expanded) accordingly.

The tests use the second line of the log message for formats other
than --pretty=short, primarily because the first line of the email
format is handled specially to add the [PATCH] prefix, etc. in a
separate codepath (--pretty=short uses the first line because there
is no other line to test).

Signed-off-by: Junio C Hamano 
---
 t/t4213-log-tabexpand.sh | 98 
 1 file changed, 98 insertions(+)
 create mode 100755 t/t4213-log-tabexpand.sh

diff --git a/t/t4213-log-tabexpand.sh b/t/t4213-log-tabexpand.sh
new file mode 100755
index 000..74ca03a
--- /dev/null
+++ b/t/t4213-log-tabexpand.sh
@@ -0,0 +1,98 @@
+#!/bin/sh
+
+test_description='log/show --expand-tabs'
+
+. ./test-lib.sh
+
+HT="   "
+title='tab indent at the beginning of the title line'
+body='tab indent on a line in the body'
+
+count_expand ()
+{
+   case " $* " in
+   *' --pretty=short '*)
+   line=$title ;;
+   *)
+   line=$body ;;
+   esac
+   expect=
+   count=$(( $1 + $2 )) ;# expected spaces
+   while test $count -gt 0
+   do
+   expect="$expect "
+   count=$(( $count - 1 ))
+   done
+   shift 2
+   count=$1 ;# expected tabs
+   while test $count -gt 0
+   do
+   expect="$expect$HT"
+   count=$(( $count - 1 ))
+   done
+   shift
+   {
+   echo "git show -s $*"
+   echo "$expect$line"
+   } | sed -e 's/ /./g' >expect
+
+   {
+   echo "git show -s $*"
+   git show -s "$@" |
+   sed -n -e "/$line\$/p"
+   } | sed -e 's/ /./g' >actual
+
+   test_cmp expect actual
+}
+
+test_expand ()
+{
+   fmt=$1
+   case "$fmt" in
+   *=raw | *=short | *=email)
+   default="0 1" ;;
+   *)
+   default="8 0" ;;
+   esac
+   case "$fmt" in
+   *=email)
+   in=0 ;;
+   *)
+   in=4 ;;
+   esac
+   test_expect_success "expand/no-expand${fmt:+ for $fmt}" '
+   count_expand $in $default $fmt &&
+   count_expand $in 8 0 $fmt --expand-tabs &&
+   count_expand $in 8 0 --expand-tabs $fmt &&
+   count_expand $in 8 0 $fmt --expand-tabs=8 &&
+   count_expand $in 8 0 --expand-tabs=8 $fmt &&
+   count_expand $in 0 1 $fmt --no-expand-tabs &&
+   count_expand $in 0 1 --no-expand-tabs $fmt &&
+   count_expand $in 0 1 $fmt --expand-tabs=0 &&
+   count_expand $in 0 1 --expand-tabs=0 $fmt &&
+   count_expand $in 4 0 $fmt --expand-tabs=4 &&
+   count_expand $in 4 0 --expand-tabs=4 $fmt
+   '
+}
+
+test_expect_success 'setup' '
+   test_tick &&
+   sed -e "s/Q/$HT/g" <<-EOF >msg &&
+   Q$title
+
+   Q$body
+   EOF
+   git commit --allow-empty -F msg
+'
+
+test_expand ""
+test_expand --pretty
+test_expand --pretty=short
+test_expand --pretty=medium
+test_expand --pretty=full
+test_expand --pretty=fuller
+test_expand --pretty=fuller
+test_expand --pretty=raw
+test_expand --pretty=email
+
+test_done
-- 
2.8.1-251-g9997610

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

[PATCH v5 3/4] pretty: allow tweaking tabwidth in --expand-tabs

When the local convention of the project is to use tab width that is
not 8, it may make sense to allow "git log --expand-tabs=" to
tweak the output to match it.

Signed-off-by: Junio C Hamano 
---
 Documentation/pretty-options.txt |  9 ++---
 commit.h |  2 +-
 pretty.c | 15 ---
 revision.c   |  9 +++--
 4 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index edbb02f..93ad1cd 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -42,16 +42,19 @@ people using 80-column terminals.
verbatim; this means that invalid sequences in the original
commit may be copied to the output.
 
+--expand-tabs=::
 --expand-tabs::
 --no-expand-tabs::
Perform a tab expansion (replace each tab with enough spaces
-   to fill to the next display column that is multiple of 8)
+   to fill to the next display column that is multiple of '')
in the log message before showing it in the output.
+   `--expand-tabs` is a short-hand for `--expand-tabs=8`, and
+   `--no-expand-tabs` is a short-hand for `--expand-tabs=0`,
+   which disables tab expansion.
 +
 By default, tabs are expanded in pretty formats that indent the log
 message by 4 spaces (i.e.  'medium', which is the default, 'full',
-and 'fuller').  `--no-expand-tabs` option can be used to disable
-this.
+and 'fuller').
 
 ifndef::git-rev-list[]
 --notes[=]::
diff --git a/commit.h b/commit.h
index a7ef682..b06db4d 100644
--- a/commit.h
+++ b/commit.h
@@ -147,7 +147,7 @@ struct pretty_print_context {
int preserve_subject;
struct date_mode date_mode;
unsigned date_mode_explicit:1;
-   unsigned expand_tabs_in_log:1;
+   int expand_tabs_in_log;
int need_8bit_cte;
char *notes_message;
struct reflog_walk_info *reflog_info;
diff --git a/pretty.c b/pretty.c
index b7938e0..87c4497 100644
--- a/pretty.c
+++ b/pretty.c
@@ -89,11 +89,11 @@ static void setup_commit_formats(void)
 {
struct cmt_fmt_map builtin_formats[] = {
{ "raw",CMIT_FMT_RAW,   0,  0 },
-   { "medium", CMIT_FMT_MEDIUM,0,  1 },
+   { "medium", CMIT_FMT_MEDIUM,0,  8 },
{ "short",  CMIT_FMT_SHORT, 0,  0 },
{ "email",  CMIT_FMT_EMAIL, 0,  0 },
-   { "fuller", CMIT_FMT_FULLER,0,  1 },
-   { "full",   CMIT_FMT_FULL,  0,  1 },
+   { "fuller", CMIT_FMT_FULLER,0,  8 },
+   { "full",   CMIT_FMT_FULL,  0,  8 },
{ "oneline",CMIT_FMT_ONELINE,   1,  0 }
};
commit_formats_len = ARRAY_SIZE(builtin_formats);
@@ -1645,7 +1645,7 @@ static int pp_utf8_width(const char *start, const char 
*end)
return width;
 }
 
-static void strbuf_add_tabexpand(struct strbuf *sb,
+static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,
 const char *line, int linelen)
 {
const char *tab;
@@ -1666,7 +1666,7 @@ static void strbuf_add_tabexpand(struct strbuf *sb,
strbuf_add(sb, line, tab - line);
 
/* .. and the de-tabified tab */
-   strbuf_addchars(sb, ' ', 8 - (width % 8));
+   strbuf_addchars(sb, ' ', tabwidth - (width % tabwidth));
 
/* Skip over the printed part .. */
linelen -= tab + 1 - line;
@@ -1692,7 +1692,7 @@ static void pp_handle_indent(struct pretty_print_context 
*pp,
 {
strbuf_addchars(sb, ' ', indent);
if (pp->expand_tabs_in_log)
-   strbuf_add_tabexpand(sb, line, linelen);
+   strbuf_add_tabexpand(sb, pp->expand_tabs_in_log, line, linelen);
else
strbuf_add(sb, line, linelen);
 }
@@ -1723,7 +1723,8 @@ void pp_remainder(struct pretty_print_context *pp,
if (indent)
pp_handle_indent(pp, sb, indent, line, linelen);
else if (pp->expand_tabs_in_log)
-   strbuf_add_tabexpand(sb, line, linelen);
+   strbuf_add_tabexpand(sb, pp->expand_tabs_in_log,
+line, linelen);
else
strbuf_add(sb, line, linelen);
strbuf_addch(sb, '\n');
diff --git a/revision.c b/revision.c
index da53b6c..47e9ee7 100644
--- a/revision.c
+++ b/revision.c
@@ -1415,7 +1415,7 @@ void init_revisions(struct rev_info *revs, const char 
*prefix)
revs->expand_tabs_in_log = -1;
 
revs->commit_format = CMIT_FMT_DEFAULT;
-   revs->expand_tabs_in_log_default = 1;
+   revs->expand_tabs_in_log_default = 8;
 
init_grep_defaults();

[PATCH v5 2/4] pretty: enable --expand-tabs by default for selected pretty formats

"git log --pretty={medium,full,fuller}" and "git log" by default
prepend 4 spaces to the log message, so it makes sense to enable
the new "expand-tabs" facility by default for these formats.
Add --no-expand-tabs option to override the new default.

The change alone breaks a test in t4201 that runs "git shortlog"
on the output from "git log", and expects that the output from
"git log" does not do such a tab expansion.  Adjust the test to
explicitly disable expand-tabs with --no-expand-tabs.

Signed-off-by: Junio C Hamano 
---
 Documentation/pretty-options.txt |  6 ++
 builtin/log.c|  1 +
 pretty.c | 18 +++---
 revision.c   |  7 +++
 revision.h   |  3 ++-
 t/t4201-shortlog.sh  |  2 +-
 6 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index d820653..edbb02f 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -43,9 +43,15 @@ people using 80-column terminals.
commit may be copied to the output.
 
 --expand-tabs::
+--no-expand-tabs::
Perform a tab expansion (replace each tab with enough spaces
to fill to the next display column that is multiple of 8)
in the log message before showing it in the output.
++
+By default, tabs are expanded in pretty formats that indent the log
+message by 4 spaces (i.e.  'medium', which is the default, 'full',
+and 'fuller').  `--no-expand-tabs` option can be used to disable
+this.
 
 ifndef::git-rev-list[]
 --notes[=]::
diff --git a/builtin/log.c b/builtin/log.c
index e00cea7..e5775ae 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1281,6 +1281,7 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
git_config(git_format_config, NULL);
init_revisions(, prefix);
rev.commit_format = CMIT_FMT_EMAIL;
+   rev.expand_tabs_in_log_default = 0;
rev.verbose_header = 1;
rev.diff = 1;
rev.max_parents = 1;
diff --git a/pretty.c b/pretty.c
index c8b075d..b7938e0 100644
--- a/pretty.c
+++ b/pretty.c
@@ -16,6 +16,7 @@ static struct cmt_fmt_map {
const char *name;
enum cmit_fmt format;
int is_tformat;
+   int expand_tabs_in_log;
int is_alias;
const char *user_format;
 } *commit_formats;
@@ -87,13 +88,13 @@ static int git_pretty_formats_config(const char *var, const 
char *value, void *c
 static void setup_commit_formats(void)
 {
struct cmt_fmt_map builtin_formats[] = {
-   { "raw",CMIT_FMT_RAW,   0 },
-   { "medium", CMIT_FMT_MEDIUM,0 },
-   { "short",  CMIT_FMT_SHORT, 0 },
-   { "email",  CMIT_FMT_EMAIL, 0 },
-   { "fuller", CMIT_FMT_FULLER,0 },
-   { "full",   CMIT_FMT_FULL,  0 },
-   { "oneline",CMIT_FMT_ONELINE,   1 }
+   { "raw",CMIT_FMT_RAW,   0,  0 },
+   { "medium", CMIT_FMT_MEDIUM,0,  1 },
+   { "short",  CMIT_FMT_SHORT, 0,  0 },
+   { "email",  CMIT_FMT_EMAIL, 0,  0 },
+   { "fuller", CMIT_FMT_FULLER,0,  1 },
+   { "full",   CMIT_FMT_FULL,  0,  1 },
+   { "oneline",CMIT_FMT_ONELINE,   1,  0 }
};
commit_formats_len = ARRAY_SIZE(builtin_formats);
builtin_formats_len = commit_formats_len;
@@ -172,6 +173,7 @@ void get_commit_format(const char *arg, struct rev_info 
*rev)
 
rev->commit_format = commit_format->format;
rev->use_terminator = commit_format->is_tformat;
+   rev->expand_tabs_in_log_default = commit_format->expand_tabs_in_log;
if (commit_format->format == CMIT_FMT_USERFORMAT) {
save_user_format(rev, commit_format->user_format,
 commit_format->is_tformat);
@@ -1720,6 +1722,8 @@ void pp_remainder(struct pretty_print_context *pp,
strbuf_grow(sb, linelen + indent + 20);
if (indent)
pp_handle_indent(pp, sb, indent, line, linelen);
+   else if (pp->expand_tabs_in_log)
+   strbuf_add_tabexpand(sb, line, linelen);
else
strbuf_add(sb, line, linelen);
strbuf_addch(sb, '\n');
diff --git a/revision.c b/revision.c
index e662230..da53b6c 100644
--- a/revision.c
+++ b/revision.c
@@ -1412,8 +1412,10 @@ void init_revisions(struct rev_info *revs, const char 
*prefix)
revs->skip_count = -1;
revs->max_count = -1;
revs->max_parents = -1;
+   revs->expand_tabs_in_log = -1;
 
revs->commit_format = CMIT_FMT_DEFAULT;
+   revs->expand_tabs_in_log_default = 1;

[PATCH v5 1/4] pretty: expand tabs in indented logs to make things line up properly

From: Linus Torvalds 

A commit log message sometimes tries to line things up using tabs,
assuming fixed-width font with the standard 8-place tab settings.
Viewing such a commit however does not work well in "git log", as
we indent the lines by prefixing 4 spaces in front of them.

This should all line up:

  Column 1  Column 2
    
  A B
  ABCD  EFGH
  SPACESInstead of Tabs

Even with multi-byte UTF8 characters:

  Column 1  Column 2
    
  Ä B
  åäö   100
  A Møøse   once bit my sister..

Tab-expand the lines in "git log --expand-tabs" output before
prefixing 4 spaces.

This is based on the patch by Linus Torvalds, but at this step, we
require an explicit command line option to enable the behaviour.

Signed-off-by: Junio C Hamano 
---
 Documentation/pretty-options.txt |  5 +++
 commit.h |  1 +
 log-tree.c   |  1 +
 pretty.c | 71 ++--
 revision.c   |  2 ++
 revision.h   |  1 +
 6 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 4b659ac..d820653 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -42,6 +42,11 @@ people using 80-column terminals.
verbatim; this means that invalid sequences in the original
commit may be copied to the output.
 
+--expand-tabs::
+   Perform a tab expansion (replace each tab with enough spaces
+   to fill to the next display column that is multiple of 8)
+   in the log message before showing it in the output.
+
 ifndef::git-rev-list[]
 --notes[=]::
Show the notes (see linkgit:git-notes[1]) that annotate the
diff --git a/commit.h b/commit.h
index 5d58be0..a7ef682 100644
--- a/commit.h
+++ b/commit.h
@@ -147,6 +147,7 @@ struct pretty_print_context {
int preserve_subject;
struct date_mode date_mode;
unsigned date_mode_explicit:1;
+   unsigned expand_tabs_in_log:1;
int need_8bit_cte;
char *notes_message;
struct reflog_walk_info *reflog_info;
diff --git a/log-tree.c b/log-tree.c
index 60f9839..78a5381 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -683,6 +683,7 @@ void show_log(struct rev_info *opt)
ctx.fmt = opt->commit_format;
ctx.mailmap = opt->mailmap;
ctx.color = opt->diffopt.use_color;
+   ctx.expand_tabs_in_log = opt->expand_tabs_in_log;
ctx.output_encoding = get_log_output_encoding();
if (opt->from_ident.mail_begin && opt->from_ident.name_begin)
ctx.from_ident = >from_ident;
diff --git a/pretty.c b/pretty.c
index 92b2870..c8b075d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1629,6 +1629,72 @@ void pp_title_line(struct pretty_print_context *pp,
strbuf_release();
 }
 
+static int pp_utf8_width(const char *start, const char *end)
+{
+   int width = 0;
+   size_t remain = end - start;
+
+   while (remain) {
+   int n = utf8_width(, );
+   if (n < 0 || !start)
+   return -1;
+   width += n;
+   }
+   return width;
+}
+
+static void strbuf_add_tabexpand(struct strbuf *sb,
+const char *line, int linelen)
+{
+   const char *tab;
+
+   while ((tab = memchr(line, '\t', linelen)) != NULL) {
+   int width = pp_utf8_width(line, tab);
+
+   /*
+* If it wasn't well-formed utf8, or it
+* had characters with badly defined
+* width (control characters etc), just
+* give up on trying to align things.
+*/
+   if (width < 0)
+   break;
+
+   /* Output the data .. */
+   strbuf_add(sb, line, tab - line);
+
+   /* .. and the de-tabified tab */
+   strbuf_addchars(sb, ' ', 8 - (width % 8));
+
+   /* Skip over the printed part .. */
+   linelen -= tab + 1 - line;
+   line = tab + 1;
+   }
+
+   /*
+* Print out everything after the last tab without
+* worrying about width - there's nothing more to
+* align.
+*/
+   strbuf_add(sb, line, linelen);
+}
+
+/*
+ * pp_handle_indent() prints out the intendation, and
+ * the whole line (without the final newline), after
+ * de-tabifying.
+ */
+static void pp_handle_indent(struct pretty_print_context *pp,
+struct strbuf *sb, int indent,
+const char *line, int linelen)
+{
+   strbuf_addchars(sb, ' ', indent);
+   if (pp->expand_tabs_in_log)
+   strbuf_add_tabexpand(sb, line, linelen);
+   else
+   strbuf_add(sb, line, linelen);
+}
+
 void

Re: [GSoC] A late proposal: a modern send-email

2016-04-04 Thread Eric Wong

惠轶群  wrote:
> 2016-03-28 6:00 GMT+08:00 Eric Wong :
> > While Gmail provides SMTP access, it was (last I was told)
> > incompatible with two-factor auth; so I've encountered users
> > unable to send patches with their normal 2FA-enabled accounts.
> 
> That's the origin of this idea of `mailto`.
> 
> In fact, you could send mail via 2FA-enabled accounts via
> "app password" metioned by Javier. But it's annoying to create
> "app password" for every client.

Since it seems possible to use 2FA with gmail, can either you
or Javier submit a patch to Documentation/git-send-email.txt
to update the Gmail-related section where
"Use gmail as the smtp server" is to describe how to use this
"app password"?

It's much easier to do than your entire GSoC proposal and would
be immediately useful to 2FA gmail users out there who don't
know this, yet or aren't running the latest git.  Thanks.





> 
> If there is a `mailto` method to send patch, you could type something
> like
> 
> git send-patch --mailto origin/master..HEAD
> 
> Then, gmail is launched with the content of patch in it. You could edit
> the list of `to` and `cc`(You could even take use of gmail contact). Then
> just send. That's all. No need to SMTP config or "app password" any
> more.
> 
> > Maybe git hackers at Google have enough pull to lobby Gmail's
> > web interface to make it easier to send plain-text patches;
> > but I would love more to see users running local mail clients
> > and even running their own SMTP servers.
> 
> Yes, this should be free with user to pick their favorite mail client.
> 
> >> That may not be a "Git" project, but GSoC is not limited to Git ;-)
> >
> > Completely agreed; email is critical to decentralized development;
> > but I don't believe decentralization is in the best interests of
> > any large and powerful corporation.
> >
> > IMHO, what we need is a SoIS: Summer of Independent Sysadmins :>
> --
> 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 v12 5/5] commit: add a commit.verbose config variable

On Sun, Apr 3, 2016 at 8:58 PM, Eric Sunshine  wrote:
> The fact that the 32 new tests are nearly identical suggests strongly
> that the testing should instead either be table-driven or be done via
> for-loops to systematically cover all cases. Not only would either of
> these approaches be easier to digest, but they would make it easy to
> tell at a glance if any cases were missing. See [2] for an example of
> how the tests can be table-driven, and see the bottom of [3] for an
> example of using for-loops to test systematically (though you'd need
> to use nested for-loops rather than just the one in the example).
>
> I'm leaning toward systematic testing via nested for-loops as the more
> suitable of the two approach for this particular application.

By the way, while this would be a nice change, it doesn't necessarily
have to be part of this series, and could be done as a follow-up by
you or someone else.

(The other changes suggested in the same review, however, ought to be
fixed in this series; in particular, simplifying the "setup" test and
making the first test after "setup" consistent with the remaining
tests.)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-format-patch.txt: don't show -s as shorthand for multiple options

On Mon, Apr 4, 2016 at 6:38 PM, Eric Sunshine  wrote:
> On Mon, Apr 4, 2016 at 4:07 PM, Junio C Hamano  wrote:
>> Eric Sunshine  writes:
 Given that the ifndef/endif block immediately before this part is
 also about excluding -p/-u/--patch when formatting the documentation
 for format-patch, perhaps the attached may be a smaller equivalent?
>>>
>>> Perhaps. I kept self-contained to make it easier to add new options
>>> between the two if need be, but I don't feel strongly about it.
>>
>> I don't either, but the reason why I thought it would make sense to
>> have them in the same block is because hiding --no-patch and --patch
>> are about the same theme: format-patch is about presenting the diff,
>> and neither disabling diff output nor explicitly asking for diff
>> output makes sense.
>
> That's reasonable. Should I re-roll, or would you like to amend it locally?

Okay, I just pulled upon seeing "What's Cooking" and see that you
amended it locally. 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] git-format-patch.txt: don't show -s as shorthand for multiple options

On Mon, Apr 4, 2016 at 4:07 PM, Junio C Hamano  wrote:
> Eric Sunshine  writes:
>>> Given that the ifndef/endif block immediately before this part is
>>> also about excluding -p/-u/--patch when formatting the documentation
>>> for format-patch, perhaps the attached may be a smaller equivalent?
>>
>> Perhaps. I kept self-contained to make it easier to add new options
>> between the two if need be, but I don't feel strongly about it.
>
> I don't either, but the reason why I thought it would make sense to
> have them in the same block is because hiding --no-patch and --patch
> are about the same theme: format-patch is about presenting the diff,
> and neither disabling diff output nor explicitly asking for diff
> output makes sense.

That's reasonable. Should I re-roll, or would you like to amend it locally?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 1/6] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface

From: Santiago Torres 

The verify_signed_buffer comand might cause a SIGPIPE signal when the
gpg child process terminates early (due to a bad keyid, for example) and
git tries to write to it afterwards. Previously, ignoring SIGPIPE was
done on the builtin/verify-tag.c command to avoid this issue. However,
any other caller who wanted to use the verify_signed_buffer command
would have to include this signal call.

Instead, we use sigchain_push(SIGPIPE, SIG_IGN) on the
verify_signed_buffer call (pretty much like in sign_buffer()) so
that any caller is not required to perform this task. This will avoid
possible mistakes by further developers using verify_signed_buffer.

Signed-off-by: Santiago Torres 
---
Note: On this version i moved the sigchain_push below the return error
in the same way that sign_buffer does(). Thanks to Johanes Sixt for catching
this. 

 builtin/verify-tag.c | 3 ---
 gpg-interface.c  | 2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 00663f6..77f070a 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   /* sometimes the program was terminated because this signal
-* was received in the process of writing the gpg input: */
-   signal(SIGPIPE, SIG_IGN);
while (i < argc)
if (verify_tag(argv[i++], flags))
had_error = 1;
diff --git a/gpg-interface.c b/gpg-interface.c
index 3dc2fe3..2259938 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -237,6 +237,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
return error(_("could not run gpg."));
}
 
+   sigchain_push(SIGPIPE, SIG_IGN);
write_in_full(gpg.in, payload, payload_size);
close(gpg.in);
 
@@ -250,6 +251,7 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
close(gpg.out);
 
ret = finish_command();
+   sigchain_pop(SIGPIPE);
 
unlink_or_warn(path);
 
-- 
2.8.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

[PATCH v4 6/6] tag.c: Change gpg_verify_tag argument to sha1

From: Santiago Torres 

The gpg_verify_tag function resolves the ref for any existing object.
However, git tag -v resolves to only tag-refs. We can provide support
for sha1 by moving the refname resolution code out of gpg_verify_tag and
allow for the object's sha1 as an argument.

Signed-off-by: Santiago Torres 
---
 builtin/tag.c|  2 +-
 builtin/verify-tag.c |  9 +++--
 tag.c| 12 
 tag.h|  2 +-
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index f4450f8..398c892 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -104,7 +104,7 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
 {
-   return gpg_verify_tag(name, GPG_VERIFY_VERBOSE);
+   return gpg_verify_tag(sha1, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 7e36d53..2ff01d8 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -30,6 +30,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 {
int i = 1, verbose = 0, had_error = 0;
unsigned flags = 0;
+   unsigned char sha1[20];
const struct option verify_tag_options[] = {
OPT__VERBOSE(, N_("print tag contents")),
OPT_BIT(0, "raw", , N_("print raw gpg status output"), 
GPG_VERIFY_RAW),
@@ -46,8 +47,12 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   while (i < argc)
-   if (gpg_verify_tag(argv[i++], flags))
+   while (i < argc) {
+   if (get_sha1(argv[i++], sha1))
+   return error("tag '%s' not found.", argv[i]);
+
+   if (gpg_verify_tag(sha1, flags))
had_error = 1;
+   }
return had_error;
 }
diff --git a/tag.c b/tag.c
index f6443db..8ac9de5 100644
--- a/tag.c
+++ b/tag.c
@@ -30,25 +30,21 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-int gpg_verify_tag(const char *name, unsigned flags)
+int gpg_verify_tag(const unsigned char *sha1, unsigned flags)
 {
enum object_type type;
-   unsigned char sha1[20];
char *buf;
unsigned long size;
int ret;
 
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
type = sha1_object_info(sha1, NULL);
if (type != OBJ_TAG)
-   return error("%s: cannot verify a non-tag object of type %s.",
-   name, typename(type));
+   return error("cannot verify a non-tag object of type %s.",
+   typename(type));
 
buf = read_sha1_file(sha1, , );
if (!buf)
-   return error("%s: unable to read file.", name);
+   return error("unable to read file.");
 
ret = run_gpg_verify(buf, size, flags);
 
diff --git a/tag.h b/tag.h
index 877f180..cb643b9 100644
--- a/tag.h
+++ b/tag.h
@@ -17,6 +17,6 @@ extern int parse_tag_buffer(struct tag *item, const void 
*data, unsigned long si
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
-extern int gpg_verify_tag(const char *name, unsigned flags);
+extern int gpg_verify_tag(const unsigned char *sha1, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.8.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

[PATCH v4 0/6] tag: move PGP verification code to tag.c

This is a follow up of [1], [2], and [3]:

v4 (this):

Thanks Eric, Peff, and Hannes for the feedback.

* I relocated the sigchain_push call so it comes after the error on
gpg-interface (thanks Hannnes for catching this).
* I updated the unit test to match the discussion on [3]. Now it generates
the expected output of the tag on the fly for comparison. (This is just
copy and paste from [3], but I verified that it works by breaking the
while)
* I split moving the code and renaming the variables into two patches so
these are easier to review.
* I used an adapter on builtin/tag.c instead of redefining all the fn*
declarations everywhere. This introduces an issue with the way git tag -v
resolves refnames though. I added a new commit to restore the previous
behavior of git-tag. I'm not sure if I should've split this into two commits
though.

v3:
Thanks Eric, Jeff, for the feedback.

* I separated the patch in multiple sub-patches.
* I compared the behavior of previous git tag -v and git verify-tag
invocations to make sure the behavior is the same
* I dropped the multi-line comment, as suggested.
* I fixed the issue with the missing brackets in the while (this is
now detected by the test).

v2:

* I moved the pgp-verification code to tag.c
* I added extra arguments so git tag -v and git verify-tag both work
with the same function
* Relocated the SIGPIPE handling code in verify-tag to gpg-interface

v1:

The verify tag function is just a thin wrapper around the verify-tag
command. We can avoid one fork call by doing the verification inside
the tag builtin instead.

This applies on v2.8.0.

Thanks!
-Santiago

[1]
http://git.661346.n2.nabble.com/PATCH-RFC-builtin-tag-c-move-PGP-verification-inside-builtin-td7651529.html#a7651547
[2]
http://git.661346.n2.nabble.com/PATCH-tag-c-move-PGP-verification-code-from-plumbing-td7651562.html

[3]
http://git.661346.n2.nabble.com/PATCH-v3-0-4-tag-move-PGP-verification-code-to-tag-c-td7652334.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

[PATCH v4 5/6] tag: use pgp_verify_function in tag -v call

From: Santiago Torres 

Instead of running the verify-tag plumbing command, we use the
pgp_verify_tag(). This avoids the usage of an extra fork call. To do
this, we extend the number of parameters that tag.c takes, and
verify-tag passes. Redundant calls done in the pgp_verify_tag function
are removed.

Signed-off-by: Santiago Torres 
---
Note: This follows Peff's suggestion to use an adapter, instead of 
changing the the fn* pointer structure to match gpg_verify_tag.

 builtin/tag.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..f4450f8 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -104,13 +104,7 @@ static int delete_tag(const char *name, const char *ref,
 static int verify_tag(const char *name, const char *ref,
const unsigned char *sha1)
 {
-   const char *argv_verify_tag[] = {"verify-tag",
-   "-v", "SHA1_HEX", NULL};
-   argv_verify_tag[2] = sha1_to_hex(sha1);
-
-   if (run_command_v_opt(argv_verify_tag, RUN_GIT_CMD))
-   return error(_("could not verify the tag '%s'"), name);
-   return 0;
+   return gpg_verify_tag(name, GPG_VERIFY_VERBOSE);
 }
 
 static int do_sign(struct strbuf *buffer)
-- 
2.8.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

[PATCH v4 2/6] t/t7030-verify-tag.sh: Adds validation for multiple tags

From: Santiago Torres 

The verify-tag command supports mutliple tag names as an argument.
However, no previous tests try to verify multiple tags at once. This
test runs the verify-tag command against three tags separately and then
compares the result against the invocation with the same three tags as
an argument. The results shouldn't differ.

Signed-off-by: Santiago Torres 
---
Note: In this version we don't check or uniq for the verify output, but
instead compare against an invocation for each of the three tags indivdually.

 t/t7030-verify-tag.sh | 12 
 1 file changed, 12 insertions(+)

diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 4608e71..f9161332 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -112,4 +112,16 @@ test_expect_success GPG 'verify signatures with --raw' '
)
 '
 
+test_expect_success GPG 'verify multiple tags' '
+   tags="fourth-signed sixth-signed seventh-signed" &&
+   for i in $tags; do
+   git verify-tag -v --raw $i || return 1
+   done >expect.stdout 2>expect.stderr.1 &&
+   grep GOODSIG expect.stderr &&
+   git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
+   grep GOODSIG actual.stderr &&
+   test_cmp expect.stdout actual.stdout &&
+   test_cmp expect.stderr actual.stderr
+'
+
 test_done
-- 
2.8.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

[PATCH v4 3/6] builtin/verify-tag: move verification code to tag.c

From: Santiago Torres 

The PGP verification routine for tags could be accessed by other
commands that require it. We do this by moving it to the common tag.c
code. We rename the verify_tag() function to gpg_verify_tag() to avoid
conflicts with the mktag.c function.

Signed-off-by: Santiago Torres 
---
Note: In this version, I just blndly moved code around and renamed
the function's name for ease of review. The following patch only renames
variables.

 builtin/verify-tag.c | 51 +--
 tag.c| 49 +
 tag.h|  1 +
 3 files changed, 51 insertions(+), 50 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 77f070a..7e36d53 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,55 +18,6 @@ static const char * const verify_tag_usage[] = {
NULL
 };
 
-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
-{
-   struct signature_check sigc;
-   int len;
-   int ret;
-
-   memset(, 0, sizeof(sigc));
-
-   len = parse_signature(buf, size);
-
-   if (size == len) {
-   if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
-   return error("no signature found");
-   }
-
-   ret = check_signature(buf, len, buf + len, size - len, );
-   print_signature_buffer(, flags);
-
-   signature_check_clear();
-   return ret;
-}
-
-static int verify_tag(const char *name, unsigned flags)
-{
-   enum object_type type;
-   unsigned char sha1[20];
-   char *buf;
-   unsigned long size;
-   int ret;
-
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
-   type = sha1_object_info(sha1, NULL);
-   if (type != OBJ_TAG)
-   return error("%s: cannot verify a non-tag object of type %s.",
-   name, typename(type));
-
-   buf = read_sha1_file(sha1, , );
-   if (!buf)
-   return error("%s: unable to read file.", name);
-
-   ret = run_gpg_verify(buf, size, flags);
-
-   free(buf);
-   return ret;
-}
-
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
int status = git_gpg_config(var, value, cb);
@@ -96,7 +47,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
flags |= GPG_VERIFY_VERBOSE;
 
while (i < argc)
-   if (verify_tag(argv[i++], flags))
+   if (gpg_verify_tag(argv[i++], flags))
had_error = 1;
return had_error;
 }
diff --git a/tag.c b/tag.c
index d72f742..81e86e6 100644
--- a/tag.c
+++ b/tag.c
@@ -6,6 +6,55 @@
 
 const char *tag_type = "tag";
 
+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+{
+   struct signature_check sigc;
+   int len;
+   int ret;
+
+   memset(, 0, sizeof(sigc));
+
+   len = parse_signature(buf, size);
+
+   if (size == len) {
+   if (flags & GPG_VERIFY_VERBOSE)
+   write_in_full(1, buf, len);
+   return error("no signature found");
+   }
+
+   ret = check_signature(buf, len, buf + len, size - len, );
+   print_signature_buffer(, flags);
+
+   signature_check_clear();
+   return ret;
+}
+
+int gpg_verify_tag(const char *name, unsigned flags)
+{
+   enum object_type type;
+   unsigned char sha1[20];
+   char *buf;
+   unsigned long size;
+   int ret;
+
+   if (get_sha1(name, sha1))
+   return error("tag '%s' not found.", name);
+
+   type = sha1_object_info(sha1, NULL);
+   if (type != OBJ_TAG)
+   return error("%s: cannot verify a non-tag object of type %s.",
+   name, typename(type));
+
+   buf = read_sha1_file(sha1, , );
+   if (!buf)
+   return error("%s: unable to read file.", name);
+
+   ret = run_gpg_verify(buf, size, flags);
+
+   free(buf);
+   return ret;
+}
+
 struct object *deref_tag(struct object *o, const char *warn, int warnlen)
 {
while (o && o->type == OBJ_TAG)
diff --git a/tag.h b/tag.h
index f4580ae..877f180 100644
--- a/tag.h
+++ b/tag.h
@@ -17,5 +17,6 @@ extern int parse_tag_buffer(struct tag *item, const void 
*data, unsigned long si
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
+extern int gpg_verify_tag(const char *name, unsigned flags);
 
 #endif /* TAG_H */
-- 
2.8.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

[PATCH v4 4/6] tag.c: Replace varialbe name for readability

From: Santiago Torres 

The run_gpg_verify function has two variables size, and len. This may
come off as confusing when reading the code. We clarify which one
pertains to the length of the tag headers by renaming len to
payload_length.

Signed-off-by: Santiago Torres 
---
Note: this used to be part of the previous patch on v3. I moved the
renaming part to a single patch to simplify the review. 

 tag.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tag.c b/tag.c
index 81e86e6..f6443db 100644
--- a/tag.c
+++ b/tag.c
@@ -9,20 +9,21 @@ const char *tag_type = "tag";
 static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
 {
struct signature_check sigc;
-   int len;
+   int payload_length;
int ret;
 
memset(, 0, sizeof(sigc));
 
-   len = parse_signature(buf, size);
+   payload_length = parse_signature(buf, size);
 
-   if (size == len) {
+   if (size == payload_length) {
if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
+   write_in_full(1, buf, payload_length);
return error("no signature found");
}
 
-   ret = check_signature(buf, len, buf + len, size - len, );
+   ret = check_signature(buf, payload_length, buf + payload_length,
+   size - payload_length, );
print_signature_buffer(, flags);
 
signature_check_clear();
-- 
2.8.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: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}

2016-04-04 Thread Remi Galan Alfonso

elena petrashen  wrote:
> On Thu, Mar 31, 2016 at 6:31 PM, Remi Galan Alfonso
>  wrote:
> > Elena Petrashen  wrote:
> >> +void delete_branch_advice(const char *name, const char *ref)
> >> +{
> >> +const char fmt[] =
> >> +"\nNote: to restore the deleted branch:\n\ngit branch %s %s\n";
> >
> > Shouldn't that be marked for translation, like is done with the other
> > strings?
> >
> > Thanks,
> > Rémi
> 
> Thank you for letting me know about that! Could you please
> help me out and explain how do I mark it for translation? I tried
> to do it the same way as with the other strings but evidently
> didn't quite succeed.

I am not sure.
I tried to grep similar cases, it seems that you can do the following:

const char fmt[] = N_("\nNote: to restore [...] \ngit branch %s %s\n");
fprintf(stderr, _(fmt), name, ref);

Some similar example in builtin/add.c:

static const char ignore_error[] =
N_("The following paths are ignored by one of your .gitignore 
files:\n");
[...]
fprintf(stderr, _(ignore_error));

Or you can define fmt as a 'const char *' and in that case do the
following:

const char *fmt = _("\nNote: to restore [...] \n git branch %s %s\n");
fprintf(stderr, fmt, name, ref);


In builtin/am.c:
const char *invalid_line = _("Malformed input line: '%s'.");
[...]
ret = error(invalid_line, sb.buf);

I don't know which one is the best way to go though.

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

What's cooking in git.git (Apr 2016, #02; Mon, 4)

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

The tip of 'next' has been rewound, the first maintenance release
v2.8.1 that most everybody can safely ignore has been tagged, and
the first batch of topics that have been cooking in 'next' during
the pre-2.8 freeze period are now in 'master'.

There are a handful of topics that are stuck; they are marked as
"Needs review", "Needs an Ack", "Waiting for reroll" etc. in the
following list.

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

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

--
[New Topics]

* es/format-patch-doc-hide-no-patch (2016-04-04) 1 commit
 - git-format-patch.txt: don't show -s as shorthand for multiple options

 "git format-patch --help" showed `-s` and `--no-patch` as if these
 are valid options to the command.  We already hide `--patch` option
 from the documentation, because format-patch is about showing the
 diff, and the documentation now hides these options as well.

 Will merge to next.


* jk/branch-shortening-funny-symrefs (2016-04-04) 1 commit
 - branch: fix shortening of non-remote symrefs

 A change back in version 2.7 to "git branch" broke display of a
 symbolic refs in a non-standard place in the refs/ hierarchy (we
 expect symbolic refs to appear in refs/remotes/*/HEAD to point at
 the primary branch the remote has, and as .git/HEAD to point at the
 branch we locally checked out).

 Will merge to next.
 and later down to maint-2.7.

--
[Graduated to "master"]

* es/test-gpg-tags (2016-03-06) 4 commits
  (merged to 'next' on 2016-03-15 at 617119f)
 + t6302: skip only signed tags rather than all tests when GPG is missing
 + t6302: also test annotated in addition to signed tags
 + t6302: normalize names and descriptions of signed tags
 + lib-gpg: drop unnecessary "missing GPG" warning

 A test for tags has been restructured so that more parts of it can
 easily be run on a platform without a working GnuPG.


* gf/fetch-pack-direct-object-fetch (2016-03-05) 2 commits
  (merged to 'next' on 2016-03-06 at 5628860)
 + fetch-pack: update the documentation for "..." arguments
  (merged to 'next' on 2016-03-04 at 49199e0)
 + fetch-pack: fix object_id of exact sha1

 Fetching of history by naming a commit object name directly didn't
 work across remote-curl transport.


* jc/index-pack (2016-03-03) 2 commits
  (merged to 'next' on 2016-03-15 at 42efaa7)
 + index-pack: add a helper function to derive .idx/.keep filename
 + Merge branch 'jc/maint-index-pack-keep' into jc/index-pack
 (this branch is used by jc/bundle; uses jc/maint-index-pack-keep; is tangled 
with jc/index-pack-clone-bundle.)

 Code clean-up.


* jc/maint-index-pack-keep (2016-03-03) 1 commit
  (merged to 'next' on 2016-03-04 at bc1d37a)
 + index-pack: correct --keep[=]
 (this branch is used by jc/bundle, jc/index-pack and 
jc/index-pack-clone-bundle.)

 "git index-pack --keep[=] pack-$name.pack" simply did not work.


* jk/add-i-highlight (2016-02-28) 1 commit
  (merged to 'next' on 2016-03-04 at 4ac3aa1)
 + add--interactive: allow custom diff highlighting programs

 A new "interactive.diffFilter" configuration can be used to
 customize the diff shown in "git add -i" session.


* jk/config-get-urlmatch (2016-02-28) 3 commits
  (merged to 'next' on 2016-03-04 at feeb192)
 + Documentation/git-config: fix --get-all description
 + Documentation/git-config: use bulleted list for exit codes
 + config: fail if --get-urlmatch finds no value

 "git config --get-urlmatch", unlike other variants of the "git
 config --get" family, did not signal error with its exit status
 when there was no matching configuration.


* jk/credential-clear-config (2016-02-26) 1 commit
  (merged to 'next' on 2016-03-04 at f7b26b7)
 + credential: let empty credential specs reset helper list

 The credential.helper configuration variable is cumulative and
 there is no good way to override it from the command line.  As
 a special case, giving an empty string as its value now serves
 as the signal to clear the values specified in various files.


* jk/getwholeline-getdelim-empty (2016-03-05) 1 commit
  (merged to 'next' on 2016-03-15 at e70d4bd)
 + strbuf_getwholeline: NUL-terminate getdelim buffer on error

 strbuf_getwholeline() did not NUL-terminate the buffer on certain
 corner cases in its error codepath.


* jk/rev-parse-local-env-vars (2016-02-29) 2 commits
  (merged to 'next' on 2016-03-04 at a0300d5)
 + rev-parse: let some options run outside repository
 + t1515: add tests for rev-parse out-of-repo helpers

 The "--local-env-vars" and "--resolve-git-dir" options of "git
 rev-parse" failed to work outside a repository when

Re: [PATCH 1/4] config --show-origin: report paths with forward slashes

2016-04-04 Thread Johannes Schindelin

Hi Junio,

On Mon, 4 Apr 2016, Junio C Hamano wrote:

> Johannes Sixt  writes:
> 
> > Although I am convinced that the change is not necessary for
> > correctness, I can buy the justification that we should produce forward
> > slashes for consistency. There are a number of occasions where we
> > present paths to the user, and we do show forward-slashes in all cases
> > that I found. We should keep the commit.
> >
> > But then let's do this:
> 
> Sounds like a plan; even though I am mildly against adding more
> platform specific #ifdef to files outside compat/, this patch does
> not.
> 
> Dscho?

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

Re: git diff does not precompose unicode file paths (OS X)

2016-04-04 Thread Alexander Rinass


> On 15 Mar 2016, at 06:45, Torsten Bögershausen  wrote:
> 
> >I created a test case but git diff exits with 0 if it does not recognize the 
> >file >path so the test case always succeeds. Can you give me a hint or one 
> >>example test case?
> 
> The most clean (?) is to compare "git diff" NFC and git diff NFD, they should 
> give the same result:
> for "git diff" something like this would do:
> +
> +# This will test git diff
> +test_expect_success "git diff f.Adiar" '
> +   echo "Modified" >f.$Adiarnfd &&
> +   git diff f.$Adiarnfd >expect &&
> +   git diff f.$Adiarnfc >actual &&
> +   git checkout f.$Adiarnfd &&
> +   test_cmp expect actual
> +’

Thank you!

I had to tweak it a little but it now reproduces the issue and confirms the fix
for diff, diff-index, diff-files and diff-tree.

I have just sent in the full patch.

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

[PATCH] diff: run arguments through precompose_argv

2016-04-04 Thread Alexander Rinass

File paths containing decomposed unicode chars passed to diff
command are not converted to precomposed unicode form.

As a result, no diff is displayed when feeding such a file path to the
diff command.

Opposite to most builtin commands, the diff builtin is missing the
parse_options call, which internally runs arguments through the
precompose_argv call, which ensures all arguments are in precomposed
unicode form.

Fix the problem by adding a precompose_argv call directly, as a call to
parse_options would require additional work to call it.

Also applies to diff-index, diff-files and diff-tree.

Signed-off-by: Alexander Rinass 
Thanks-to: Torsten Bögershausen 
Thanks-to: Junio C Hamano 
---
 builtin/diff-files.c |  1 +
 builtin/diff-index.c |  1 +
 builtin/diff-tree.c  |  2 ++
 builtin/diff.c   |  1 +
 t/t3910-mac-os-precompose.sh | 42 ++
 5 files changed, 47 insertions(+)

diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 8ed2eb8..15c61fd 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -24,6 +24,7 @@ int cmd_diff_files(int argc, const char **argv, const char 
*prefix)
gitmodules_config();
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
+   precompose_argv(argc, argv);
 
argc = setup_revisions(argc, argv, , NULL);
while (1 < argc && argv[1][0] == '-') {
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index d979824..1af373d 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -21,6 +21,7 @@ int cmd_diff_index(int argc, const char **argv, const char 
*prefix)
gitmodules_config();
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
+   precompose_argv(argc, argv);
 
argc = setup_revisions(argc, argv, , NULL);
for (i = 1; i < argc; i++) {
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 2a12b81..806dd7a 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -114,6 +114,8 @@ int cmd_diff_tree(int argc, const char **argv, const char 
*prefix)
opt->disable_stdin = 1;
memset(_r_opt, 0, sizeof(s_r_opt));
s_r_opt.tweak = diff_tree_tweak_rev;
+
+   precompose_argv(argc, argv);
argc = setup_revisions(argc, argv, opt, _r_opt);
 
while (--argc > 0) {
diff --git a/builtin/diff.c b/builtin/diff.c
index 52c98a9..d6b8f98 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -319,6 +319,7 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
if (!no_index)
gitmodules_config();
git_config(git_diff_ui_config, NULL);
+   precompose_argv(argc, argv);
 
init_revisions(, prefix);
 
diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh
index 8319356..26dd5b7 100755
--- a/t/t3910-mac-os-precompose.sh
+++ b/t/t3910-mac-os-precompose.sh
@@ -49,12 +49,54 @@ test_expect_success "setup" '
 test_expect_success "setup case mac" '
git checkout -b mac_os
 '
+# This will test nfd2nfc in git diff
+test_expect_success "git diff f.Adiar" '
+   touch f.$Adiarnfc &&
+   git add f.$Adiarnfc &&
+   echo f.Adiarnfc >f.$Adiarnfc &&
+   git diff f.$Adiarnfd >expect &&
+   git diff f.$Adiarnfc >actual &&
+   test_cmp expect actual &&
+   git reset HEAD f.Adiarnfc &&
+   rm f.$Adiarnfc expect actual
+'
+# This will test nfd2nfc in git diff-files
+test_expect_success "git diff-files f.Adiar" '
+   touch f.$Adiarnfc &&
+   git add f.$Adiarnfc &&
+   echo f.Adiarnfc >f.$Adiarnfc &&
+   git diff-files f.$Adiarnfd >expect &&
+   git diff-files f.$Adiarnfc >actual &&
+   test_cmp expect actual &&
+   git reset HEAD f.Adiarnfc &&
+   rm f.$Adiarnfc expect actual
+'
+# This will test nfd2nfc in git diff-index
+test_expect_success "git diff-index f.Adiar" '
+   touch f.$Adiarnfc &&
+   git add f.$Adiarnfc &&
+   echo f.Adiarnfc >f.$Adiarnfc &&
+   git diff-index HEAD f.$Adiarnfd >expect &&
+   git diff-index HEAD f.$Adiarnfc >actual &&
+   test_cmp expect actual &&
+   git reset HEAD f.Adiarnfc &&
+   rm f.$Adiarnfc expect actual
+'
 # This will test nfd2nfc in readdir()
 test_expect_success "add file Adiarnfc" '
echo f.Adiarnfc >f.$Adiarnfc &&
git add f.$Adiarnfc &&
git commit -m "add f.$Adiarnfc"
 '
+# This will test nfd2nfc in git diff-tree
+test_expect_success "git diff-tree f.Adiar" '
+   echo f.Adiarnfc >>f.$Adiarnfc &&
+   git diff-tree HEAD f.$Adiarnfd >expect &&
+   git diff-tree HEAD f.$Adiarnfc >actual &&
+   test_cmp expect actual &&
+   git checkout f.$Adiarnfc &&
+   rm expect actual
+'
 # This will test nfd2nfc in git stage()
 test_expect_success "stage file d.Adiarnfd/f.Adiarnfd" '
mkdir d.$Adiarnfd &&
-- 
2.7.2

--
To

[PATCH] diff: run arguments through precompose_argv

2016-04-04 Thread Alexander Rinass

I found an issue where file paths containing decomposed unicode chars
are not converted to precomposed unicode form when passed to the diff
command family.

As a result, no diff is displayed.

With the help of Torsten Bögershausen and Junio C Hamano I came up 
with the following patch to fix the issue.

Alexander Rinass (1):
  diff: run arguments through precompose_argv

 builtin/diff-files.c |  1 +
 builtin/diff-index.c |  1 +
 builtin/diff-tree.c  |  2 ++
 builtin/diff.c   |  1 +
 t/t3910-mac-os-precompose.sh | 42 ++
 5 files changed, 47 insertions(+)

-- 
2.7.2

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

Re: [PATCH v3][Outreachy] branch -D: allow - as abbreviation of @{-1}

2016-04-04 Thread elena petrashen

On Thu, Mar 31, 2016 at 6:31 PM, Remi Galan Alfonso
 wrote:
> Elena Petrashen  wrote:
>> +void delete_branch_advice(const char *name, const char *ref)
>> +{
>> +const char fmt[] =
>> +"\nNote: to restore the deleted branch:\n\ngit branch %s %s\n";
>
> Shouldn't that be marked for translation, like is done with the other
> strings?
>
> Thanks,
> Rémi

Thank you for letting me know about that! Could you please
help me out and explain how do I mark it for translation? I tried
to do it the same way as with the other strings but evidently
didn't quite succeed.

Thanks!
Elena
--
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: [Outreachy] Git remote whitelist/blacklist

2016-04-04 Thread elena petrashen

Hi Lars,

thank you so much for reaching out to me! and thank you for your
suggestions too, I'm changing my application to reflect those. It does
really make sense to start the discussion earlier then I scheduled,
and add some extra ideas that could be completed after the
whitelist/blacklist. Probably if those ideas are the extensions to the
whitelist/blacklist, they could be also agreed on when discussing the
overall project scope.

Cheers,
Elena

On Wed, Mar 30, 2016 at 11:21 AM, Lars Schneider
 wrote:
> Hi Elena,
>
> I thought a bit more about your proposal. The Outreachy internship is 
> scheduled for 3 months and I think you would be able to come up with a "Git 
> remote whitelist/blacklist" implementation that the Git list considers to 
> accept within a month. In that case it would be good if you would have a few 
> extra ideas in your Outreachy proposal that you could tackle afterwards. 
> These extras could be extensions to the "whitelist/blacklist" project or a 
> contribution to a completely different part of Git. According to the 
> Outreachy page [1] you can still change your application until April 22.
>
> In addition I wonder if you would be willing to start slowly with the 
> "drafting the implementation" task even before April 22. I, and AFAIK the 
> majority of the other people on the list, do not work full time on Git. That 
> means some email responses might be delayed for reasons unrelated to Git. 
> Therefore I think you will have a better experience if you work with a steady 
> slow pace instead of high burst of list activity :-)
>
> Cheers,
> Lars
>
>
> [1] https://wiki.gnome.org/Outreachy#Submit_an_Application
>
>
>> On 29 Mar 2016, at 21:24, Lars Schneider  wrote:
>>
>> Hi Elena,
>>
>> sorry for the late reply. I think it is great that you are interested in the 
>> Git remote whitelist/blacklist idea. Unfortunately I don't have any 
>> experience with Outreachy and I wonder what kind of feedback you are looking 
>> for.
>>
>> Thanks,
>> Lars
>>
>>
>>> On 26 Mar 2016, at 13:15, elena petrashen  wrote:
>>>
>>> Hi everyone,
>>>
>>> I think I will submit the application as it is now, but still
>>> it would be great to get feedback on it, as I don't think
>>> there was no reply because everything was perfect :(
>>>
>>> Thank you! And have an awesome weekend.
>>>
>>> On Thu, Mar 24, 2016 at 5:50 PM, elena petrashen
>>>  wrote:
 Hi,

 I'm thinking of applying to Outreachy program this round with Git
 and the project I'm really interested in is "Git remote 
 whitelist/blacklist"
 project (http://git.github.io/SoC-2016-Ideas/).
 I have drafted the description/timeline for this project
 and it would be great to get feedback/suggestions.
 (I'm actually a bit confused about the scale of this. The
 Outreachy application doesn't ask for "proposal" in the way
 GSoC seems to, but merely requests "details and the timeline",
 so I'm not sure whether the shorter description of what's planned
 is expected or should I go deeper in detail. I apologize if I
 chose a wrong approach.)

 Thank you!

>> What project(s) are you interested in (these can be in the
 same or different organizations)?
 My preferred project to work on is Git remote whitelist/blacklist
 project listed on http://git.github.io/SoC-2016-Ideas/. I'm really
 interested in doing this project as I think this kind of effort is
 really important: I recently started using git myself, and sometimes
 I was really scared to push something to the location I was not
 supposed to push to. I would really appreciate the opportunity in
 participating in making git a bit more newbie-friendly.

>> Who is a possible mentor for the project you are most interested in?
 Lars Schneider

>> Please describe the details and the timeline of the work you
 plan to accomplish on the project you are most interested in (discuss
 these first with the mentor of the project):
 The goal is to provide a safer environment for newcomers to Git to
 enabling the possibility to modify git config, adding there "allowed"
 and "denied" remotes for pushing. Code, tests, and documentation
 are to be created.

 Timeline:
 0. Analysis
 Apr 22 - May 22 - studying the current code and drafting the
 implementation proposal
 1. Design
 a. May 22-June 1 - discussion with the mentor regarding the task,
 presenting the approach and amending it per mentor's feedback
 b. June 1st-June 15th - communicating with the community
 regarding the suggested changes and agreeing on logic, scope
 and format of changes.
 2. Development
 c. June 15th-July 1st - submitting code for the first basic version,
 amending it according to the feedback
 d. July 1st - July 15th - extending the code to

Re: [PATCH v3 4/4] tag: use pgp_verify_function in tag -v call

On Mon, Apr 04, 2016 at 02:24:48PM -0400, Santiago Torres wrote:

> > I think it can be part of this series, but doesn't have to be. As I
> > understand it, the current code is just handing the name to the `git
> > verify-tag` process, so if we continue to do so, that would be OK.
> 
> IIRC, the current code for git tag -v hands the hex-representation[1] of
> the sha1 to git verify-tag --- I believe that's related to the
> desamgibuation issue I've seen people discuss.  I think this behavior is
> lost unless we add this on top of the patch.

Oh, you're right. I didn't notice that. So yeah, we should make sure in
this series to hand the sha1 over to gpg_verify_tag().

> > Yes, though I'd generally advise against a function taking either a name or
> > a sha1, and ignoring the other option. That often leads to confusing
> > interfaces for the callers. Instead, perhaps just take the sha1, and let
> > the caller do the get_sha1() themselves. Or possibly provide two
> > functions, one of which is a convenience to translate the name to sha1
> > and then call the other.
> 
> I think the former sounds easier. I can replace the name argument and
> move the sha1-resolution code to in verify-tag. git tag -v already
> resolves the tagname to a sha1, so it is easier there.
> 
> Does this sound reasonable?

Yes, I think that is a good solution.

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] git-format-patch.txt: don't show -s as shorthand for multiple options

Eric Sunshine  writes:

>> Given that the ifndef/endif block immediately before this part is
>> also about excluding -p/-u/--patch when formatting the documentation
>> for format-patch, perhaps the attached may be a smaller equivalent?
>
> Perhaps. I kept self-contained to make it easier to add new options
> between the two if need be, but I don't feel strongly about it.

I don't either, but the reason why I thought it would make sense to
have them in the same block is because hiding --no-patch and --patch
are about the same theme: format-patch is about presenting the diff,
and neither disabling diff output nor explicitly asking for diff
output makes sense.

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

Re: [PATCH] git-format-patch.txt: don't show -s as shorthand for multiple options

On Mon, Apr 4, 2016 at 3:32 PM, Junio C Hamano  wrote:
> Eric Sunshine  writes:
>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> @@ -28,10 +28,12 @@ ifdef::git-diff[]
>>  endif::git-diff[]
>>  endif::git-format-patch[]
>>
>> +ifndef::git-format-patch[]
>>  -s::
>>  --no-patch::
>>   Suppress diff output. Useful for commands like `git show` that
>>   show the patch by default, or to cancel the effect of `--patch`.
>> +endif::git-format-patch[]
>
> Given that the ifndef/endif block immediately before this part is
> also about excluding -p/-u/--patch when formatting the documentation
> for format-patch, perhaps the attached may be a smaller equivalent?

Perhaps. I kept self-contained to make it easier to add new options
between the two if need be, but I don't feel strongly about it.

>  Documentation/diff-options.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 306b7e3..42e6620 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -26,12 +26,12 @@ ifndef::git-format-patch[]
>  ifdef::git-diff[]
> This is the default.
>  endif::git-diff[]
> -endif::git-format-patch[]
>
>  -s::
>  --no-patch::
> Suppress diff output. Useful for commands like `git show` that
> show the patch by default, or to cancel the effect of `--patch`.
> +endif::git-format-patch[]
>
>  -U::
>  --unified=::
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-format-patch.txt: don't show -s as shorthand for multiple options

Eric Sunshine  writes:

> Documentation/diff-options.txt | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 306b7e3..6eb591f 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -28,10 +28,12 @@ ifdef::git-diff[]
>  endif::git-diff[]
>  endif::git-format-patch[]
>  
> +ifndef::git-format-patch[]
>  -s::
>  --no-patch::
>   Suppress diff output. Useful for commands like `git show` that
>   show the patch by default, or to cancel the effect of `--patch`.
> +endif::git-format-patch[]

Given that the ifndef/endif block immediately before this part is
also about excluding -p/-u/--patch when formatting the documentation
for format-patch, perhaps the attached may be a smaller equivalent?

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

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 306b7e3..42e6620 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -26,12 +26,12 @@ ifndef::git-format-patch[]
 ifdef::git-diff[]
This is the default.
 endif::git-diff[]
-endif::git-format-patch[]
 
 -s::
 --no-patch::
Suppress diff output. Useful for commands like `git show` that
show the patch by default, or to cancel the effect of `--patch`.
+endif::git-format-patch[]
 
 -U::
 --unified=::
--
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 5/5] t/t5520: test --[no-]autostash with pull.rebase=true

Eric Sunshine  writes:

> Although I'm the one who brought up the idea of "automating" these
> tests, I'm not convinced that it's an improvement in this case, but I
> don't feel so strongly that I'd forbid it.

Another option is to define helper functions to shorten the "manual"
tests, e.g. define:

setup_rebase_test () {
git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file
}

rebase_test_ok () {
git pull $1 . copy &&
test_cmp_rev HEAD^ copy &&
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
}

rebase_test_err () {
test_must_fail git pull $1 . copy 2>err &&
test_i18ngrep "uncommitted changes." err
}

I'm also OK with keeping the "manual" tests.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/4] tag: use pgp_verify_function in tag -v call

On Mon, Apr 04, 2016 at 09:38:54AM -0400, Jeff King wrote:
> On Mon, Apr 04, 2016 at 12:12:04AM -0400, Santiago Torres wrote:
> 
> > > As a side note, it might actually be an improvement for pgp_verify_tag
> > > to take a sha1 (so that git-tag is sure that it is verifying the same
> > > object that it is printing), but that refactoring should probably come
> > > separately, I think.
> > 
> > Just to be sure, this refactoring is something we should still include
> > in this set of patches, right? I think that otherwise we'd lose the
> > desambigutaion that git tag -v does in this patch.
> 
> I think it can be part of this series, but doesn't have to be. As I
> understand it, the current code is just handing the name to the `git
> verify-tag` process, so if we continue to do so, that would be OK.

IIRC, the current code for git tag -v hands the hex-representation[1] of
the sha1 to git verify-tag --- I believe that's related to the
desamgibuation issue I've seen people discuss.  I think this behavior is
lost unless we add this on top of the patch.

> 
> > I also think that most of the rippling is gone if we use and adaptor as
> > you suggested. Should I add a patch on top of this to support a sha1 as
> > part for gpg_verify_tag()?
> 
> Yes, though I'd generally advise against a function taking either a name or
> a sha1, and ignoring the other option. That often leads to confusing
> interfaces for the callers. Instead, perhaps just take the sha1, and let
> the caller do the get_sha1() themselves. Or possibly provide two
> functions, one of which is a convenience to translate the name to sha1
> and then call the other.

I think the former sounds easier. I can replace the name argument and
move the sha1-resolution code to in verify-tag. git tag -v already
resolves the tagname to a sha1, so it is easier there.

Does this sound reasonable? 

Thanks!
-Santiago

[1] https://git.kernel.org/cgit/git/git.git/tree/builtin/tag.c#n109
--
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 5/5] t/t5520: test --[no-]autostash with pull.rebase=true

Mehul Jain  writes:

> On Mon, Apr 4, 2016 at 10:22 PM, Matthieu Moy
>  wrote:
>> I think it would be much simpler to drop the loop, and write instead
>> something like (untested):
>
> I tested it (with few minor changes), and worked fine.
>
> test_autostash () {
> OLDIFS=$IFS
> IFS='='
> set -- $*
> IFS=$OLDIFS

This $IFS dance is not needed. If you need to split variable and value,
then just pass two arguments on the caller side.

-- 
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: RFD: removing git.spec.in (Re: git 2.8.0 tarball rpmbuild error)

2016-04-04 Thread Dennis Kaarsemaker

On za, 2016-04-02 at 20:41 -0700, Junio C Hamano wrote:
> I think by now it is very clear that nobody active in the Git
> development community tests RPM binaries built with git.spec.in we
> have in our tree. I suspect RPM based distros are using their own
> RPM build recipe without paying any attention to what we have in our
> tree, and that is why no packager from RPM land gave any bug report
> and correction before the release happened.

Fedora, RHEL, CentOS, OpenSUSE and Mageia all use their own specfiles
and local patches. I do test and distribute RPM packages based on the
next branch, but use fedora's packaging to do so (which incidentally
also broke and I had to work around this change, but I completely
forgot about git.spec.in).

> I'd propose that during the cycle for the next feature release, we'd
> remove git.spec.in and stop pretending as if we support RPM
> packaging.

I would be in favor of that. In general, I find it much better to use a
distro's packaging and simply transplanting a tarball into it. That
keeps the difference with what the distro provides minimal.

-- 
Dennis Kaarsemaker
www.kaarsemaker.net

--
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] doc: clarify that notes can be attached to any type of stored object

Sebastian Schuberth  writes:

>>> -It is also permitted for a notes ref to point directly to a tree
>>> -object, in which case the history of the notes can be read with
>>> +It is also permitted for a notes ref to point to any other object in
>>> +the object store besides commit objects, that is annotated tags, blobs
>>> +or trees. For the latter, the history of the notes can be read with
>>>  `git log -p -g `.
>>
>> I do not think this is correct place to patch.  The original is not
>> talking about what objects can have notes attached at all.  What it
>> explains is this.
>
> Thanks for the explanation, I was indeed misreading this. I'll try to
> clarify this section then, too. In order to do so, I think we should
> mention how to actually create a  that directly points to a
> tree instead of a commit for the history of notes. Would you have an
> example how to do that?

Interesting.  This came from 9eb3f816 (Documentation/notes: document
format of notes trees, 2010-05-08):

Documentation/notes: document format of notes trees

Separate the specification of the notes format exposed in
git-config.1 from the description of the option; or in other
words, move the explanation for what to expect to find at
refs/notes/commits from git-config.1 to git-notes.1.

Suggested-by: Thomas Rast 
Signed-off-by: Jonathan Nieder 
Signed-off-by: Junio C Hamano 

but I do not find a corresponding sentence that says a notes ref can
point at a tree in the text before the patch.

I highly suspect that "git notes add" and other Porcelain level
commands that manipulate an existing notes tree would be unhappy if
a notes ref is not a commit, as it is clear from the paragraph
before the one under discussion, i.e.

Every notes change creates a new commit at the specified notes ref.
You can therefore inspect the history of the notes by invoking, e.g.,
`git log -p notes/commits`.  Currently the commit message only records
which operation triggered the update, and the commit authorship is
determined according to the usual rules (see linkgit:git-commit[1]).
These details may change in the future.

that in order to create a "new" commit, setting the current one as
its parent, would require that the current one to be a commit and
not a bare tree.  "git notes list" and others that merely read from
the notes tree would probably work.
--
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] comment for a long #ifdef

Ivan Pozdeev  writes:

> ---
> compat/poll/poll.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

As Eric pointed out, please sign-off your patch.

> diff --git a/compat/poll/poll.c b/compat/poll/poll.c
> index db4e03e..5eb0280 100644
> --- a/compat/poll/poll.c
> +++ b/compat/poll/poll.c
> @@ -441,7 +441,7 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
> }
>
> return rc;
> -#else
> +#else /* #ifndef WIN32_NATIVE */
> static struct timeval tv0;
> static HANDLE hEvent;
> WSANETWORKEVENTS ev;
> @@ -622,5 +622,5 @@ restart:
> }
>
> return rc;
> -#endif
> +#endif /* #ifndef WIN32_NATIVE */
> }
> -- 
> 1.9.5.msysgit.1

There also is an #ifndef/#else/#endif in abspath.c that is larger
than a typical patch context; could you include a fix for that, 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: Out Of Place Capitalization In Stash Feedback

Saxon Knight  writes:

> I recently did a git stash save "working on main", and in the
> feedback, I noticed that "On" was capitalized ("...state On
> master...").
> I don't think this is intentional, so I figured I'd try to let someone know.
>
> Here is the output (quotes included by me):
>
> "Saved working directory and index state On master: working on main"

"git stash list" at this point would show

stash@{0}: On master: working on main

In other words, the title of the stash entry is "On master: working
on main", and that is what is shown in the feedback.

I guess the output could quote the title in quotes to avoid this
puzzlement, i.e.

Saved working directory and index state 'On master: working on main'

 git-stash.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-stash.sh b/git-stash.sh
index c7c65e2..6d8eb22 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -266,7 +266,7 @@ save_stash () {
create_stash "$stash_msg" $untracked
store_stash -m "$stash_msg" -q $w_commit ||
die "$(gettext "Cannot save the current status")"
-   say Saved working directory and index state "$stash_msg"
+   say "Saved working directory and index state '$stash_msg'"
 
if test -z "$patch_mode"
then
--
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 5/5] t/t5520: test --[no-]autostash with pull.rebase=true

On Mon, Apr 4, 2016 at 1:36 PM, Mehul Jain  wrote:
> On Mon, Apr 4, 2016 at 10:22 PM, Matthieu Moy
>  wrote:
>> I think it would be much simpler to drop the loop, and write instead
>> something like (untested):
>
> I tested it (with few minor changes), and worked fine.
>
> test_autostash () {
> OLDIFS=$IFS
> IFS='='
> set -- $*
> IFS=$OLDIFS
> expect=$1
> cmd=$2
> config_variable=$3
> value=$4
> test_expect_success "$cmd, $config_variable=$value" '
> if [ "$value" = "" ]; then
> test_unconfig $config_variable
> else
> test_config $config_variable $value
> fi &&
>
> git reset --hard before-rebase &&
> echo dirty >new_file &&
> git add new_file &&
>
> if [ $expect = "ok" ]; then
> git pull $cmd . copy &&
> test_cmp_rev HEAD^ copy &&
> test "$(cat new_file)" = dirty &&
> test "$(cat file)" = "modified again"
> else
> test_must_fail git pull $cmd . copy 2>err &&
> test_i18ngrep "uncommitted changes." err
> fi
> '
> }
>
> test_autostash ok '--rebase' rebase.autostash=true
> test_autostash ok '--rebase --autostash' rebase.autostash=true
> test_autostash ok '--rebase --autostash' rebase.autostash=false
> test_autostash ok '--rebase --autostash' rebase.autostash=
> test_autostash err '--rebase --no-autostash' rebase.autostash=true
> test_autostash err '--rebase --no-autostash' rebase.autostash=false
> test_autostash err '--rebase --no-autostash' rebase.autostash=
> test_autostash ok '--autostash' pull.rebase=true
> test_autostash err '--no-autostash' pull.rebase=true
>
> Perhaps this looks better than the one with the loop. Even better than
> the implementation in v2[1].
>
> I think it would be wise to go with the above script for v3 (as I will
> be doing a re-roll of the series[1]).

This new function is sufficiently complex that it increases cognitive
load enough for me to question if it is really a win for such a small
number of tests. The individual tests, as implemented in the current
round, are quite easy to understand, and don't place any significant
cognitive burden on the reader.

Although I'm the one who brought up the idea of "automating" these
tests, I'm not convinced that it's an improvement in this case, but I
don't feel so strongly that I'd forbid it. So, choose the approach
which seems best to you while weighing comprehension load for people
new to these tests, as well as maintainability costs.
--
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] Fix spelling mistakes

Eric Engestrom  writes:

> I went by word-count: "through" was way more common than "thru", so
> I kept the fix.

That is not a "fix" (as it is not touching "mistakes") but "making
them consistent".  I do not personally feel strongly against such a
change, but please leave them out of "Subject: [PATCH] typofix".

> BTW, I used codespell to fix those, I didn't do it by hand :]
> https://github.com/lucasdemarchi/codespell

Heh, cute ;-)

> I can remove the "thru" fixes if you'd prefer, and resend with only the
> other fixes.

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 5/5] t/t5520: test --[no-]autostash with pull.rebase=true

On Mon, Apr 4, 2016 at 10:22 PM, Matthieu Moy
 wrote:
> I think it would be much simpler to drop the loop, and write instead
> something like (untested):

I tested it (with few minor changes), and worked fine.

test_autostash () {
OLDIFS=$IFS
IFS='='
set -- $*
IFS=$OLDIFS
expect=$1
cmd=$2
config_variable=$3
value=$4
test_expect_success "$cmd, $config_variable=$value" '
if [ "$value" = "" ]; then
test_unconfig $config_variable
else
test_config $config_variable $value
fi &&

git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&

if [ $expect = "ok" ]; then
git pull $cmd . copy &&
test_cmp_rev HEAD^ copy &&
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
else
test_must_fail git pull $cmd . copy 2>err &&
test_i18ngrep "uncommitted changes." err
fi
'
}

test_autostash ok '--rebase' rebase.autostash=true
test_autostash ok '--rebase --autostash' rebase.autostash=true
test_autostash ok '--rebase --autostash' rebase.autostash=false
test_autostash ok '--rebase --autostash' rebase.autostash=
test_autostash err '--rebase --no-autostash' rebase.autostash=true
test_autostash err '--rebase --no-autostash' rebase.autostash=false
test_autostash err '--rebase --no-autostash' rebase.autostash=
test_autostash ok '--autostash' pull.rebase=true
test_autostash err '--no-autostash' pull.rebase=true

Perhaps this looks better than the one with the loop. Even better than
the implementation in v2[1].

I think it would be wise to go with the above script for v3 (as I will
be doing a re-roll of the series[1]).

[1]: http://thread.gmane.org/gmane.comp.version-control.git/290596

Thanks,
Mehul
--
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: [sort-of-BUG] merge-resolve cannot resolve "content/mode" conflict

Jeff King  writes:

> Imagine a merge where one side changes the content of a path and the
> other changes the mode. Here's a minimal reproduction:
>
>   git init repo && cd repo &&
>
>   echo base >file &&
>   git add file &&
>   git commit -m base &&
>
>   echo changed >file &&
>   git commit -am content &&
>
>   git checkout -b side HEAD^
>   chmod +x file &&
>   git commit -am mode
> ...
> This is a leftover from my experiments with merge-resolve versus
> merge-recursive last fall, which resulted in a few actual bug-fixes. I
> looked into fixing this case, too, at that time. It seemed possible, but
> a little more involved than you might think (because the logic is driven
> by a bunch of case statements, and this adds a multiplicative layer to
> the cases; we might need to resolve the permissions, and _then_ see if
> the content can be resolved).

Perhaps I am missing some other codepath in the "multiplicative"
layer, but is this not sufficient?

The convoluted "update-index/chmod" dance is to help those on
filesystems that lack proper executable bits.  Otherwise the last
"update-index --chmod" is not needed.


 git-merge-one-file.sh | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 424b034..36bcdcc 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -142,8 +142,19 @@ case "${1:-.}${2:-.}${3:-.}" in
git checkout-index -f --stage=2 -- "$4" && cat "$src1" >"$4" || exit 1
rm -f -- "$orig" "$src1" "$src2"
 
-   if test "$6" != "$7"
+   # Three-way merge of the permissions
+   perm= ;# assume the result is the same from stage #2, i.e. $6
+   if test "$6" = "$7" || test "$5" = "$7"
+   then
+   : nothing
+   elif test "$5" = "$6"
then
+   case "$7" in
+   100644) perm=-x ;;
+   100755) perm=+x ;;
+   *) echo "ERROR: $7: funny filemode not handled." >&2 ;;
+   esac
+   else
if test -n "$msg"
then
msg="$msg, "
@@ -157,7 +168,17 @@ case "${1:-.}${2:-.}${3:-.}" in
echo "ERROR: $msg in $4" >&2
exit 1
fi
-   exec git update-index -- "$4"
+
+   if test -n "$perm"
+   then
+   chmod "$perm" -- "$4"
+   fi &&
+   git update-index -- "$4" &&
+   if test -n "$perm"
+   then
+   git update-index --chmod="$perm" -- "$4"
+   fi
+   exit
;;
 
 *)
--
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 is an efficient way to get all blobs / trees that have notes attached?

2016-04-04 Thread Johan Herland

On Mon, Apr 4, 2016 at 9:46 AM, Sebastian Schuberth
 wrote:
> On Fri, Apr 1, 2016 at 2:16 PM, Johan Herland  wrote:
>>> 3) Recursively list all blobs / trees (git-ls-tree) and look whether an
>>> object's hash is conatined in our table to get its notes.
>>>
>>> In particular 3) could be expensive for repos with a lot of files as we're
>>> looking at all of them just to see whether they have notes attached.
>>
>> In (3), why would you need to search through _all_ blobs/trees? Would
>> it not be cheaper to simply query the object type of each annotated
>> object from (2)? I.e. something like:
>>
>> for notes_ref in $(git for-each-ref refs/notes | cut -c 49-)
>> do
>> echo "--- $notes_ref ---"
>> for annotated_obj in $(git notes --ref=$notes_ref list | cut -c 41-)
>> do
>> type=$(git cat-file -t "$annotated_obj")
>> if test "$type" != "commit"
>> then
>> echo "$annotated_obj: $type"
>> fi
>> done
>> done
>
> Thanks for the idea. The problem is that I do want to list the notes
> by path of the object they belong to. As a blob could potentially
> belong to more than one path (copies of files in the repo), I do not
> see another way of getting that information other than iterating over
> all blobs and checking what path(s) they belong to.

True; fundamentally what you want is a blob/tree ID -> path(s) mapping,
which is an independent problem, unrelated to to the initial notes lookup.

I don't know of a solution faster than the brute-force search you already
sketched. If this lookup is important to your use case, you could consider
building/caching the required mapping when the notes are added in the
first place, but I don't know if that is possible in your scenario...


...Johan

> --
> Sebastian Schuberth

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

Re: [PATCH] git-format-patch.txt: don't show -s as shorthand for multiple options

Ping?

On Sun, Mar 27, 2016 at 5:26 PM, Eric Sunshine  wrote:
> git-format-patch recognizes -s as shorthand only for --signoff, however,
> its documentation shows -s as shorthand for both --signoff and
> --no-patch. Resolve this confusion by suppressing the bogus -s shorthand
> for --no-patch.
>
> While here, also avoid showing the --no-patch option in git-format-patch
> documentation since it doesn't make sense to ask to suppress the patch
> while at the same time explicitly asking to format the patch (which,
> after all, is the purpose of git-format-patch).
>
> Reported-by: Kevin Brodsky 
> Signed-off-by: Eric Sunshine 
> ---
>
> I haven't quite managed to trace the code yet, but git-format-patch
> oddly does recognize --no-patch, and it appears to act as an alias of
> --no-stat. At any rate, --no-patch seems rather senseless with
> git-format-patch, hence this patch suppresses it in documentation
> altogether.
>
> Documentation/diff-options.txt | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 306b7e3..6eb591f 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -28,10 +28,12 @@ ifdef::git-diff[]
>  endif::git-diff[]
>  endif::git-format-patch[]
>
> +ifndef::git-format-patch[]
>  -s::
>  --no-patch::
> Suppress diff output. Useful for commands like `git show` that
> show the patch by default, or to cancel the effect of `--patch`.
> +endif::git-format-patch[]
>
>  -U::
>  --unified=::
> --
> 2.8.0.rc4.285.gc3ac548
--
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 v12 1/5] t0040-test-parse-options.sh: fix style issues

On Mon, Apr 4, 2016 at 8:45 AM, Pranit Bauva  wrote:
> Okay I will do the change. I was previously unaware about the use of
> '\' before EOF. I googled it now. But I am still confused about its
> use in this scenario. Upto what I understood, it is used where you
> want to expand a variable, substitute a command, arithmethic
> expansion. The use of '\' in the tests I have changed in v12 wrt 11 is
> understood by me as you want to remove the use of escape sequences
> which is justified. But this seems a bit vague. Is it some convention
> in git?

Both 'EOF' and \EOF suppress interpolation and other transformations
in the heredoc content which would otherwise occur with plain EOF. The
'EOF' form is well documented; \EOF not so much, but is used heavily
in git test scripts. So:

x=flormp
echo

Re: [PATCH] comment for a long #ifdef

2016-04-04 Thread Ivan Pozdeev


Signed-off-by: Ivan Pozdeev
---
compat/poll/poll.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/compat/poll/poll.c b/compat/poll/poll.c
index db4e03e..5eb0280 100644
--- a/compat/poll/poll.c
+++ b/compat/poll/poll.c
@@ -441,7 +441,7 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
}

return rc;
-#else
+#else /* #ifndef WIN32_NATIVE */
static struct timeval tv0;
static HANDLE hEvent;
WSANETWORKEVENTS ev;
@@ -622,5 +622,5 @@ restart:
}

return rc;
-#endif
+#endif /* #ifndef WIN32_NATIVE */
}
--
1.9.5.msysgit.1


--
Regards,
Ivan

--
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 0/7] t5520: tests for --[no-]autostash option

On Mon, Apr 4, 2016 at 10:30 PM, Junio C Hamano  wrote:
> Matthieu Moy  writes:
>
>> Mehul Jain  writes:
>>
>>> -test_rebase_autostash () {
>>> +test_pull_autostash () {
>>>  git reset --hard before-rebase &&
>>>  echo dirty >new_file &&
>>>  git add new_file &&
>>> -git pull --rebase --autostash . copy &&
>>> +git pull $@ . copy &&
>>
>> Not strictly needed here, but I'd write "$@" (with the double-quotes)
>> which is the robust way to say "transmit all my arguments without
>> whitespace interpretation".
>
> Yes, these should be "$@" (with the double-quotes).

I will do a re-roll then.

Thanks,
Mehul
--
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 0/7] t5520: tests for --[no-]autostash option

Matthieu Moy  writes:

> Mehul Jain  writes:
>
>> -test_rebase_autostash () {
>> +test_pull_autostash () {
>>  git reset --hard before-rebase &&
>>  echo dirty >new_file &&
>>  git add new_file &&
>> -git pull --rebase --autostash . copy &&
>> +git pull $@ . copy &&
>
> Not strictly needed here, but I'd write "$@" (with the double-quotes)
> which is the robust way to say "transmit all my arguments without
> whitespace interpretation".

Yes, these should be "$@" (with the double-quotes).

> I don't mind for this patch since there's no whitespace to interpret,
> but some people (sysadmins ;-) ) have the bad habit of writting $@, $*
> or "$*" in wrapper scripts and it breaks when you call them with spaces
> so it's better to take good habits IHMO.
--
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 0/7] t5520: tests for --[no-]autostash option

On Mon, Apr 4, 2016 at 1:01 PM, Matthieu Moy
 wrote:
> Mehul Jain  writes:
>
>> -test_rebase_autostash () {
>> +test_pull_autostash () {
>>   git reset --hard before-rebase &&
>>   echo dirty >new_file &&
>>   git add new_file &&
>> - git pull --rebase --autostash . copy &&
>> + git pull $@ . copy &&
>
> Not strictly needed here, but I'd write "$@" (with the double-quotes)
> which is the robust way to say "transmit all my arguments without
> whitespace interpretation".
>
> I don't mind for this patch since there's no whitespace to interpret,
> but some people (sysadmins ;-) ) have the bad habit of writting $@, $*
> or "$*" in wrapper scripts and it breaks when you call them with spaces
> so it's better to take good habits IHMO.

Thanks for the suggestion, I will remember it. I'm relatively new to
shell and therefore didn't know much about the difference
between "$@" and $@, $*, "$*".

Now that I have read[1][2] about it, it won't be repeated.

[1]: 
http://unix.stackexchange.com/questions/41571/what-is-the-difference-between-and/94200#94200
[2]: 
http://unix.stackexchange.com/questions/131766/why-does-my-shell-script-choke-on-whitespace-or-other-special-characters

Thanks,
Mehul
--
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 5/5] t/t5520: test --[no-]autostash with pull.rebase=true

Mehul Jain  writes:

> On Mon, Apr 4, 2016 at 12:58 AM, Eric Sunshine  
> wrote:
>> On Fri, Apr 1, 2016 at 6:27 AM, Mehul Jain  wrote:
>>> In test_autostash() there's a line
>>>
>>> echo test_cmp_rev HEAD^ copy &&
>>>
>>> Originally it should have been
>>>
>>> test_cmp_rev HEAD^ copy &&
>>>
>>> but this raise following error while testing
>>>
>>> ./t5520-pull.sh: 684: eval: diff -u: not found
>>
>> This is caused by the custom IFS=',\t=' which is still in effect when
>> test_cmp_rev() is invoked. You need to restore IFS within the loop
>> itself.
>
> Thanks for pointing it out. I made a mistake by not considering
> the consequences of setting IFS=',\t='. I tried it out again and
> this time all tests passed perfectly.

I think it would be much simpler to drop the loop, and write instead
something like (untested):

test_autostash () {
expect="$1"
cmd="$2"
config_variable="$3"
value="$4"
test_expect_success "$cmd, $config_variable=$value" '
if [ "$value" = "" ]; then
test_unconfig $config_variable
else
test_config $config_variable $value
fi &&

git reset --hard before-rebase &&
echo dirty >new_file &&
git add new_file &&

if [ $expect = "ok" ]; then
git pull '$cmd' . copy &&
test_cmp_rev HEAD^ copy &&
test "$(cat new_file)" = dirty &&
test "$(cat file)" = "modified again"
else
test_must_fail git pull '$cmd' . copy 2>err &&
test_i18ngrep "uncommitted changes." err
fi
'
}

test_autostash ok --rebase rebase.autostash=true
test_autostash ok '--rebase --autostash' rebase.autostash=true
test_autostash ok '--rebase --autostash' rebase.autostash=false
test_autostash ok '--rebase --autostash' rebase.autostash=
test_autostash err '--rebase --no-autostash' rebase.autostash=true
test_autostash err '--rebase --no-autostash' rebase.autostash=false
test_autostash err '--rebase --no-autostash' rebase.autostash=
test_autostash ok --autostash pull.rebase=true
test_autostash err --no-autostash pull.rebase=true

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] imap-send.c: implements the GIT_CURL_DEBUG environment variable

Elia Pinto  writes:

>> My impression is that using GIT_TRACE_* is the more mainstream
>> trend, and it may be beneficial to work any new debugging aid like
>> this one to fit within that mechanism.
>
> I thought about it, and I agree with you. The idea could be
>
> - Call the variable GIT_TRACE_CURL_DEBUG instead

I think GIT_TRACE_CURL_DEBUG is overly verbose; tracing by
definition is a debugging aid.

> - Add the new GIT_TRACE_CURL_VERBOSE variable, keeping
> GIT_CURL_VERBOSE for compatibility

I do not care too deeply either way.

 - GIT_CURL_VERBOSE can stay the same as-is and show its output to
   whatever output channel it spits things out.

 - Or it can be a synonym for GIT_TRACE_CURL=2 (as I understand that
   the VERBOSE output goes to the standard error stream)

If you want tracing as debugging aid and existing CURL_VERBOSE
orthogonal, it would probably make more sense to go the former
route, not linking this new "DEBUG" thing with the existing
"VERBOSE" thing.

> - Documenting these GIT_TRACE_CURL_XXX variables (GIT_CURL_VERBOSE
> it is not even documented i think)

If we decide to leave them untangled, this is not necessary.

> - perhaps use the git trace api in doing these new patches
>
> Look reasonable? It seems reasonable? I'd like your own opinion

Not really sensible as long as you have that "perhaps" in the list.

Something that does not use the trace API shouldn't pretend to by
using GIT_TRACE_* names.

GIT_TRACE_CURL could be your new thing and would decide where to
show its output by using the GIT_TRACE_* api.
--
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 5/5] t/t5520: test --[no-]autostash with pull.rebase=true

On Mon, Apr 4, 2016 at 12:58 AM, Eric Sunshine  wrote:
> On Fri, Apr 1, 2016 at 6:27 AM, Mehul Jain  wrote:
>> In test_autostash() there's a line
>>
>> echo test_cmp_rev HEAD^ copy &&
>>
>> Originally it should have been
>>
>> test_cmp_rev HEAD^ copy &&
>>
>> but this raise following error while testing
>>
>> ./t5520-pull.sh: 684: eval: diff -u: not found
>
> This is caused by the custom IFS=',\t=' which is still in effect when
> test_cmp_rev() is invoked. You need to restore IFS within the loop
> itself.

Thanks for pointing it out. I made a mistake by not considering
the consequences of setting IFS=',\t='. I tried it out again and
this time all tests passed perfectly.

I should  been more careful in the first place while playing
with IFS, but instead of that, I kept on thinking that there is some
other problem with the script which lead to me making foolish
changes in the script like putting an echo before "test_cmp_rev ...".

It was nice of you to take out some time and point it out :)

Also now that I have sent v2[1] of this series, which goes
in different direction as far as implementation of these tests
are concerned. I think the script now is useless (but I
learned a bit about shell while writing it).

Thanks,
Mehul
--
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] doc: clarify that notes can be attached to any type of stored object

Sebastian Schuberth  writes:

> Done, I'll send a patch shortly. But I wanted to list the supported
> object types explicitly, in particular as many guide in the net are
> unclear that only annotated tags are object, and lightweight ones are
> not.

I'd really hate to see an explicit list of what object types there
are, as it is one more place we'd need to update if we ever add new
object types (and we are unlikely to do so anytime soon, which makes
it even more likely that we would forget there is this explicit list
we'd need to update).

- Adds, removes, or reads notes attached to objects, without touching
- the objects themselves.
+ Adds, removes, or reads notes attached to any object (not limited
+ to commit objects), without touching the objects themselves.

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

Re: [PATCH v3 13/16] ref-filter: allow porcelain to translate messages in the output

Matthieu Moy  writes:

> I'm not sure how important it is in this case, but it was in the case of
> setup_unpack_trees_porcelain which I took inspiration from when we
> discussed this (actually, in setup_unpack_trees_porcelain, there's isn't
> any translation even in porcelain).

OK, so paraphrase:

In the most general case, we might want to have one code to issue
different messages between plumbing and Porcelain; further, for
Porcelain messages may or may not want to be translated.

But I suspect that all Porcelain messages should be translatable
in general, so there probably is a room for simplification.

The single macro P_() approach was done without knowing that this
codepath wanted the distinction between the plumbing and Porcelain.

> Note that this can be worked around later by adding another function like
>
> static const char *get_message(const char *porcelain, const char 
> *plumbing)
> {
> return use_porcelain_msg ? porcelain : plumbing;
> }
>
> to be called with get_message(_("this ref was gone"), "gone") or so.

Yes, I think that would be a way to do this properly.  And we do not
have a separate "here is the list of all translatable messages"
table, which is a big plus.

> In summary: both would work. No strong opinion from me, but I slightly
> prefer the version in the patch (i.e. the one I suggested IIRC) to
> Junio's version.

Yup.
--
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] imap-send.c: implements the GIT_CURL_DEBUG environment variable

2016-04-04 Thread Elia Pinto

2016-04-01 17:35 GMT+02:00 Junio C Hamano :
> Elia Pinto  writes:
>
>> Implements the GIT_CURL_DEBUG environment variable to allow a greater
>> degree of detail of GIT_CURL_VERBOSE, in particular the complete
>> transport header and all the data payload exchanged.
>> It might be useful if a particular situation could require a more
>> thorough debugging analysis.
>
> My impression is that using GIT_TRACE_* is the more mainstream
> trend, and it may be beneficial to work any new debugging aid like
> this one to fit within that mechanism.

I thought about it, and I agree with you. The idea could be

- Call the variable GIT_TRACE_CURL_DEBUG instead
- Add the new GIT_TRACE_CURL_VERBOSE variable, keeping
GIT_CURL_VERBOSE for compatibility
- Documenting these GIT_TRACE_CURL_XXX variables (GIT_CURL_VERBOSE
it is not even documented i think)
- perhaps use the git trace api in doing these new patches

Look reasonable? It seems reasonable? I'd like your own opinion

Thank you for your suggestion
>
> I am not saying new GIT_*_DEBUG is wrong.  I just wanted to make
> sure you have considered doing this as a new trace in GIT_TRACE_*
> family and rejected that apporach with a very good reason, in
> which case that rationale deserves to be in the log message.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] config --show-origin: report paths with forward slashes

Johannes Sixt  writes:

> Although I am convinced that the change is not necessary for
> correctness, I can buy the justification that we should produce forward
> slashes for consistency. There are a number of occasions where we
> present paths to the user, and we do show forward-slashes in all cases
> that I found. We should keep the commit.
>
> But then let's do this:

Sounds like a plan; even though I am mildly against adding more
platform specific #ifdef to files outside compat/, this patch does
not.

Dscho?

>
>  8< 
> Subject: [PATCH] Windows: shorten code by re-using convert_slashes()
>
> Make a few more spots more readable by using the recently introduced,
> Windows-specific helper.
>
> Signed-off-by: Johannes Sixt 
> ---
>  abspath.c  | 5 +
>  compat/mingw.c | 9 ++---
>  2 files changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/abspath.c b/abspath.c
> index 5edb4e7..2825de8 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -167,7 +167,6 @@ const char *prefix_filename(const char *pfx, int pfx_len, 
> const char *arg)
>   strbuf_add(, pfx, pfx_len);
>   strbuf_addstr(, arg);
>  #else
> - char *p;
>   /* don't add prefix to absolute paths, but still replace '\' by '/' */
>   strbuf_reset();
>   if (is_absolute_path(arg))
> @@ -175,9 +174,7 @@ const char *prefix_filename(const char *pfx, int pfx_len, 
> const char *arg)
>   else if (pfx_len)
>   strbuf_add(, pfx, pfx_len);
>   strbuf_addstr(, arg);
> - for (p = path.buf + pfx_len; *p; p++)
> - if (*p == '\\')
> - *p = '/';
> + convert_slashes(path.buf + pfx_len);
>  #endif
>   return path.buf;
>  }
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 54c82ec..0413d5c 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -763,15 +763,12 @@ struct tm *localtime_r(const time_t *timep, struct tm 
> *result)
>  
>  char *mingw_getcwd(char *pointer, int len)
>  {
> - int i;
>   wchar_t wpointer[MAX_PATH];
>   if (!_wgetcwd(wpointer, ARRAY_SIZE(wpointer)))
>   return NULL;
>   if (xwcstoutf(pointer, wpointer, len) < 0)
>   return NULL;
> - for (i = 0; pointer[i]; i++)
> - if (pointer[i] == '\\')
> - pointer[i] = '/';
> + convert_slashes(pointer);
>   return pointer;
>  }
>  
> @@ -2112,9 +2109,7 @@ static void setup_windows_environment()
>* executable (by not mistaking the dir separators
>* for escape characters).
>*/
> - for (; *tmp; tmp++)
> - if (*tmp == '\\')
> - *tmp = '/';
> + convert_slashes(tmp);
>   }
>  
>   /* simulate TERM to enable auto-color (see color.c) */
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/4] t/t7030-verify-tag.sh: Adds validation for multiple tags

On Sun, Apr 03, 2016 at 09:38:34PM -0400, Eric Sunshine wrote:

> I think Peff meant that a simple grep would suffice; no need for
> test_i18ngrep. In other words (reproducing Peff's example), something
> like this:
> 
> tags="fourth-signed sixth-signed seventh-signed" &&
> for i in $tags; do
> git verify-tag -v --raw $i || return 1
> done >expect.stdout 2>expect.stderr.1 &&
> grep GOODSIG expect.stderr &&
> git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
> grep GOODSIG actual.stderr &&
> test_cmp expect.stdout actual.stdout &&
> test_cmp expect.stderr actual.stderr

Yep, though I think I would actually have done:

   grep '^.GNUPG:.' ...

or something to just catch all of the gnupg output.

-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 v3 4/4] tag: use pgp_verify_function in tag -v call

On Mon, Apr 04, 2016 at 12:12:04AM -0400, Santiago Torres wrote:

> > As a side note, it might actually be an improvement for pgp_verify_tag
> > to take a sha1 (so that git-tag is sure that it is verifying the same
> > object that it is printing), but that refactoring should probably come
> > separately, I think.
> 
> Just to be sure, this refactoring is something we should still include
> in this set of patches, right? I think that otherwise we'd lose the
> desambigutaion that git tag -v does in this patch.

I think it can be part of this series, but doesn't have to be. As I
understand it, the current code is just handing the name to the `git
verify-tag` process, so if we continue to do so, that would be OK.

> I also think that most of the rippling is gone if we use and adaptor as
> you suggested. Should I add a patch on top of this to support a sha1 as
> part for gpg_verify_tag()?

Yes, though I'd generally advise against a function taking either a name or
a sha1, and ignoring the other option. That often leads to confusing
interfaces for the callers. Instead, perhaps just take the sha1, and let
the caller do the get_sha1() themselves. Or possibly provide two
functions, one of which is a convenience to translate the name to sha1
and then call the other.

-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 1/2] imap-send.c: implements the GIT_CURL_DEBUG environment variable

2016-04-04 Thread Elia Pinto

2016-04-01 16:56 GMT+02:00 Ramsay Jones :
>
>
> On 01/04/16 11:44, Elia Pinto wrote:
>> Implements the GIT_CURL_DEBUG environment variable to allow a greater
>> degree of detail of GIT_CURL_VERBOSE, in particular the complete
>> transport header and all the data payload exchanged.
>> It might be useful if a particular situation could require a more
>> thorough debugging analysis.
>>
>> Signed-off-by: Elia Pinto 
>> ---
>>  imap-send.c | 99 
>> +++--
>>  1 file changed, 97 insertions(+), 2 deletions(-)
>>
>> diff --git a/imap-send.c b/imap-send.c
>> index 4d3b773..cf79e7f 100644
>> --- a/imap-send.c
>> +++ b/imap-send.c
> [snip]
>
>> @@ -1442,8 +1532,13 @@ static CURL *setup_curl(struct imap_server_conf *srvc)
>>
>>   curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
>>
>> - if (0 < verbosity || getenv("GIT_CURL_VERBOSE"))
>> - curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
>> + if (0 < verbosity )
It was already so in the previous code, I have not changed it. If it
is a desirable change it would take another patch
>
> previously it was sufficient to set GIT_CURL_VERBOSE, now I have to
> set verbosity too?

>
> [Does it matter that you change "1L" to "1" in the curl_easy_setopt()
> call? In http.c (line 567) it also uses "1", but ...]
>
>> + if (getenv("GIT_CURL_DEBUG")) {
>> + curl_easy_setopt(curl, CURLOPT_VERBOSE, 1);
>> + curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION, 
>> curl_trace);
>> + } else if (getenv("GIT_CURL_VERBOSE"))
>> + curl_easy_setopt(curl, CURLOPT_VERBOSE, 1);
>> +
OK.
>>
>>   return curl;
>>  }
>>
>
> I would have expected something like:
>
> if (0 < verbosity || getenv("GIT_CURL_VERBOSE")) {
> curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
> if (getenv("GIT_CURL_DEBUG"))
> curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION, curl_trace);
> }
>
No. Thank you. But actually it is not a desirable change
> Hope That Helps.
>
> ATB,
> Ramsay Jones
>
>
>
--
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 v12 1/5] t0040-test-parse-options.sh: fix style issues

2016-04-04 Thread Pranit Bauva

On Mon, Apr 4, 2016 at 2:30 AM, Eric Sunshine  wrote:
> On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva  wrote:
>> Signed-off-by: Pranit Bauva 
>> ---
>> diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
>> @@ -7,7 +7,7 @@ test_description='our own option parser'
>>
>>  . ./test-lib.sh
>>
>> -cat > expect << EOF
>> +cat >expect <>  usage: test-parse-options 
>>  --yes get a boolean
>
> It would be better to use <<\EOF for this one to make it clear that no
> interpolation is desired in the heredoc.
>
>> @@ -156,7 +156,7 @@ test_expect_success 'OPT_MAGNITUDE() 3giga' '
>> check magnitude: 3221225472 -m 3g
>>  '
>>
>> -cat > expect << EOF
>> +cat >expect <
> Ditto: <<\EOF
>
> Same applies to all similar heredocs in subsequent tests where
> interpolation is not desired.
>
>>  boolean: 2
>>  integer: 1729
>>  magnitude: 16384
>> @@ -310,12 +310,12 @@ arg 00: --quux
>>  EOF
>>
>>  test_expect_success 'keep some options as arguments' '
>> -   test-parse-options --quux > output 2> output.err &&
>> +   test-parse-options --quux >output 2>output.err &&
>> test_must_be_empty output.err &&
>> -test_cmp expect output
>> +   test_cmp expect output
>
> Okay, this is a whitespace change (spaces -> tab).
>
>>  '
>> @@ -460,7 +460,7 @@ test_expect_success 'negation of OPT_NONEG flags is not 
>> ambiguous' '
>> -cat >>expect <<'EOF'
>> +cat >>expect <
> This is not a desirable change. This heredoc does not require
> interpolation, so you don't want to turn it into a form which does
> interpolate. For style consistency, therefore, you should change 'EOF'
> to \EOF.
>
>>  list: foo
>>  list: bar
>>  list: baz

Okay I will do the change. I was previously unaware about the use of
'\' before EOF. I googled it now. But I am still confused about its
use in this scenario. Upto what I understood, it is used where you
want to expand a variable, substitute a command, arithmethic
expansion. The use of '\' in the tests I have changed in v12 wrt 11 is
understood by me as you want to remove the use of escape sequences
which is justified. But this seems a bit vague. Is it some convention
in git?
--
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] http.c: implements the GIT_CURL_DEBUG environment variable

2016-04-04 Thread Elia Pinto

2016-04-01 17:03 GMT+02:00 Ramsay Jones :
>
>
> On 01/04/16 11:44, Elia Pinto wrote:
>> Implements the GIT_CURL_DEBUG environment variable to allow a greater
>> degree of detail of GIT_CURL_VERBOSE, in particular the complete
>> transport header and all the data payload exchanged.
>> It might be useful if a particular situation could require a more
>> thorough debugging analysis.
>>
>> Signed-off-by: Elia Pinto 
>> ---
>>  http.c | 97 
>> +-
>>  1 file changed, 96 insertions(+), 1 deletion(-)
>>
>> diff --git a/http.c b/http.c
>> index dfc53c1..079779d 100644
>> --- a/http.c
>> +++ b/http.c
> [snip]
>
>> @@ -532,7 +623,11 @@ static CURL *get_curl_handle(void)
>>   "your curl version is too old (>= 7.19.4)");
>>  #endif
>>
>> - if (getenv("GIT_CURL_VERBOSE"))
>> + if (getenv("GIT_CURL_DEBUG")) {
>> + curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
>> + curl_easy_setopt(result, CURLOPT_DEBUGFUNCTION, curl_trace);
>> + curl_easy_setopt(result, CURLOPT_DEBUGDATA, NULL);
>> + } else if (getenv("GIT_CURL_VERBOSE"))
>>   curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
>>
>>   curl_easy_setopt(result, CURLOPT_USERAGENT,
>>
>
> Again, maybe something like:
>
> if (getenv("GIT_CURL_VERBOSE")) {
> curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
> if (getenv("GIT_CURL_DEBUG"))
> curl_easy_setopt(result, CURLOPT_DEBUGFUNCTION, curl_trace);
> }
>
> Although that does make GIT_CURL_DEBUG subordinate to GIT_CURL_VERBOSE.
> So, that may not be desired ...

Thank you. But actually it is not a desirable change, for me almost, I
prefer that the two definitions are independent. And it is true the
opposite: if it is defined the curl DEBUG flag then it is implicitly
defined the curl VERBOSE flag, because it is a prerequisite of the
DEBUG functionality.

Thanks in any case for the review.

Best
>
> ATB,
> Ramsay Jones
>
>
--
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] doc: Clarify which objects notes can be attached to

Explicitly name the supported object types, and ensure patches cannot be
misinterpreted as non-objects that can have notes attached.

Signed-off-by: Sebastian Schuberth 
---
 Documentation/git-notes.txt | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 8de3499..101e6ba 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -25,7 +25,8 @@ SYNOPSIS
 DESCRIPTION
 ---
 Adds, removes, or reads notes attached to objects, without touching
-the objects themselves.
+the objects themselves.  Supported objects are commits, blobs, trees
+and annotated tags.
 
 By default, notes are saved to and read from `refs/notes/commits`, but
 this default can be overridden.  See the OPTIONS, CONFIGURATION, and
@@ -39,9 +40,9 @@ message stored in the commit object, the notes are indented 
like the
 message, after an unindented line saying "Notes ():" (or
 "Notes:" for `refs/notes/commits`).
 
-Notes can also be added to patches prepared with `git format-patch` by
-using the `--notes` option. Such notes are added as a patch commentary
-after a three dash separator line.
+Notes contents can also be included in patches prepared with
+`git format-patch` by using the `--notes` option. Such notes are added
+as a patch commentary after a three dash separator line.
 
 To change which notes are shown by 'git log', see the
 "notes.displayRef" configuration in linkgit:git-log[1].
-- 
2.8.0.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] doc: clarify that notes can be attached to any type of stored object

On Fri, Apr 1, 2016 at 6:47 PM, Junio C Hamano  wrote:

> DESCRIPTION
> ---
> Adds, removes, or reads notes attached to objects, without touching
> the objects themselves.
>
> This says "notes attached to objects" and "the objects themselves".
> They do not limit the target of attaching a note to "commits".
> So this may be the place to add "  Note that notes can be attached
> to any kind of objects, not limited to commits" or something, if
> we really wanted to.

Done, I'll send a patch shortly. But I wanted to list the supported
object types explicitly, in particular as many guide in the net are
unclear that only annotated tags are object, and lightweight ones are
not.

> Notes can also be added to patches prepared with `git format-patch` by
> using the `--notes` option. Such notes are added as a patch commentary
> after a three dash separator line.
>
> This paragraph _might_ be confusing to new readers.  The "added to"
> sounds as if you are attaching a note to a non-object which is a
> patch.  But this "add" is about "inserting the contents of the note
> attached to the commit being formatted" and corresponds to "can be
> shown by" in the previous paragraph.  We may want to avoid the verb
> "add" when talking about the use of data stored in an existing note
> to somewhere else like this.

Done.

> add::
> Add notes for a given object (defaults to HEAD). Abort if the
>
> And this "Add notes for " should probably be reworded to "Attach
> notes to" to match the first sentence in the description.

After a bit of thinking, I don't believe we should do this. All
subcommand docs start with the verb the subcommand is named after. In
that sense making the "add" docs start with "Attach" would be
inconsistent and probably raise the question why the subcommand is not
called "attach" after all. Also, in the description it says "Adds,
removes, or reads notes attached to objects", so it includes "[...]
removes [...] notes attached to objects", and if you read it like this
the word "attach" is not specific to the "add" subcommand. So I left
this as-is in my patch.

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

[ANNOUNCE] Git for Windows 2.8.1

2016-04-04 Thread Johannes Schindelin

Dear Git users,

It is my pleasure to announce that Git for Windows 2.8.1 is available from:

https://git-for-windows.github.io/

Changes since Git for Windows v2.8.0 (March 29th 2016)

New Features

  ??? Comes with Git v2.8.1.
  ??? The Git for Windows project updated its contributor guidelines to
the Contributor Covenant 1.4.

Bug Fixes

  ??? Git's default editor (vim) is no longer freezing in CMD windows.
  ??? GIT_SSH (and other executable paths that Git wants to spawn) can
now contain spaces.

Filename | SHA-256
 | ---
Git-2.8.1-64-bit.exe | 
5e5283990cc91d1e9bd0858f8411e7d0afb70ce26e23680252fb4869288c7cfb
Git-2.8.1-32-bit.exe | 
17418c2e507243b9c98db161e9e5e8041d958b93ce6078530569b8edaec6b8a4
PortableGit-2.8.1-64-bit.7z.exe | 
dc9d971156cf3b6853bc0c1ad0ca76f1d2c24478cca80036919f12fe46acd64e
PortableGit-2.8.1-32-bit.7z.exe | 
0b6efaaeb4b127edb3a534261b2c9175bd86ee8683dff6e12ccb194e6abb990e
Git-2.8.1-64-bit.tar.bz2 | 
3ebc00b96607174fffb35cf6d96b3b3246aefa504a4bd30375182fea6ab64bde
Git-2.8.1-32-bit.tar.bz2 | 
9e754d83190ba154f012d8eaa7433d29517f7c85fff229d4fb62e6418acf8d41

Ciao,
Johannes

Re: [PATCH] doc: clarify that notes can be attached to any type of stored object

On Fri, Apr 1, 2016 at 5:31 PM, Junio C Hamano  wrote:

> Sebastian Schuberth  writes:
>
>> Signed-off-by: Sebastian Schuberth 
>> ---
>>  Documentation/git-notes.txt | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
>> index 8de3499..5375d98 100644
>> --- a/Documentation/git-notes.txt
>> +++ b/Documentation/git-notes.txt
>> @@ -234,8 +234,9 @@ which operation triggered the update, and the commit 
>> authorship is
>>  determined according to the usual rules (see linkgit:git-commit[1]).
>>  These details may change in the future.
>>
>> -It is also permitted for a notes ref to point directly to a tree
>> -object, in which case the history of the notes can be read with
>> +It is also permitted for a notes ref to point to any other object in
>> +the object store besides commit objects, that is annotated tags, blobs
>> +or trees. For the latter, the history of the notes can be read with
>>  `git log -p -g `.
>
> I do not think this is correct place to patch.  The original is not
> talking about what objects can have notes attached at all.  What it
> explains is this.

Thanks for the explanation, I was indeed misreading this. I'll try to
clarify this section then, too. In order to do so, I think we should
mention how to actually create a  that directly points to a
tree instead of a commit for the history of notes. Would you have an
example how to do that?

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

Re: [PATCH v3 13/16] ref-filter: allow porcelain to translate messages in the output

Karthik Nayak  writes:

> cc'ing Matthieu since this patch was initially written by him.
>
> On Thu, Mar 31, 2016 at 3:28 AM, Junio C Hamano  wrote:
>> Karthik Nayak  writes:
>>
>>> +static struct ref_msg {
>>> + const char *gone;
>>> + const char *ahead;
>>> + const char *behind;
>>> + const char *ahead_behind;
>>> +} msgs = {
>>> + "gone",
>>> + "ahead %d",
>>> + "behind %d",
>>> + "ahead %d, behind %d"
>>> +};
>>> +
>>> +void setup_ref_filter_porcelain_msg(void)
>>> +{
>>> + msgs.gone = _("gone");
>>> + msgs.ahead = _("ahead %d");
>>> + msgs.behind = _("behind %d");
>>> + msgs.ahead_behind = _("ahead %d, behind %d");
>>> +}
>>
>> I do not think this patch is wrong, but I wonder if it would be
>> sufficient to have a single bit in file-scope static variable and
>> flip it in setup_ref_filter_porcelain_msg().  I.e.
>>
>> static int use_porcelain_msg; /* false by default */
>>
>> void setup_ref_filter_porcelain_msg(void)
>> {
>> use_porcelain_msg = 1;
>> }
>>
>> static const char *P_(const char *msg)
>> {
>> return use_porcelain_msg ? _(msg) : msg;
>> }
>>
>> and then mark the message up perhaps like so:
>>
>> -   *s = xstrdup("gone");
>> +   *s = xstrdup(P_("gone"));
>>
>> which may make things much simpler.

... but less evolutive. The non-translatable strings also need to be
cast in stone, while the translatable ones may be subject to future
improvements/tweaks. If they are already duplicated in the code, then
updating one won't change the other, but factoring them means that the
porcelain message can't easily be changed without modifying the plumbing
one.

I'm not sure how important it is in this case, but it was in the case of
setup_unpack_trees_porcelain which I took inspiration from when we
discussed this (actually, in setup_unpack_trees_porcelain, there's isn't
any translation even in porcelain).

Note that this can be worked around later by adding another function like

static const char *get_message(const char *porcelain, const char 
*plumbing)
{
return use_porcelain_msg ? porcelain : plumbing;
}

to be called with get_message(_("this ref was gone"), "gone") or so.

>> We'd need to update Makefile to recognize X_() as another keyword;

(I guess you meant P_, not X_)

>> I'd think something like this should do:
>>
>>  XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
>> ---keyword=_ --keyword=N_ --keyword="Q_:1,2"
>> +--keyword=_ --keyword=N_ --keyword=P_ --keyword="Q_:1,2"

I'm a bit reluctant to modifying the Makefile for something not really
build-related.

> I'm not totally knowledgeable  about how porcelain works, although
> Matthieu did give me a
> brief explanation. I guess it'd better to hear his thoughts on this.

In summary: both would work. No strong opinion from me, but I slightly
prefer the version in the patch (i.e. the one I suggested IIRC) to
Junio's version.

-- 
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: What is an efficient way to get all blobs / trees that have notes attached?

On Fri, Apr 1, 2016 at 2:16 PM, Johan Herland  wrote:

>> 3) Recursively list all blobs / trees (git-ls-tree) and look whether an
>> object's hash is conatined in our table to get its notes.
>>
>> In particular 3) could be expensive for repos with a lot of files as we're
>> looking at all of them just to see whether they have notes attached.
>
> In (3), why would you need to search through _all_ blobs/trees? Would
> it not be cheaper to simply query the object type of each annotated
> object from (2)? I.e. something like:
>
> for notes_ref in $(git for-each-ref refs/notes | cut -c 49-)
> do
> echo "--- $notes_ref ---"
> for annotated_obj in $(git notes --ref=$notes_ref list | cut -c 41-)
> do
> type=$(git cat-file -t "$annotated_obj")
> if test "$type" != "commit"
> then
> echo "$annotated_obj: $type"
> fi
> done
> done

Thanks for the idea. The problem is that I do want to list the notes
by path of the object they belong to. As a blob could potentially
belong to more than one path (copies of files in the repo), I do not
see another way of getting that information other than iterating over
all blobs and checking what path(s) they belong to.

-- 
Sebastian Schuberth
--
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 0/7] t5520: tests for --[no-]autostash option

Mehul Jain  writes:

> -test_rebase_autostash () {
> +test_pull_autostash () {
>   git reset --hard before-rebase &&
>   echo dirty >new_file &&
>   git add new_file &&
> - git pull --rebase --autostash . copy &&
> + git pull $@ . copy &&

Not strictly needed here, but I'd write "$@" (with the double-quotes)
which is the robust way to say "transmit all my arguments without
whitespace interpretation".

I don't mind for this patch since there's no whitespace to interpret,
but some people (sysadmins ;-) ) have the bad habit of writting $@, $*
or "$*" in wrapper scripts and it breaks when you call them with spaces
so it's better to take good habits IHMO.

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

git mv - exclude source / create target directory option

2016-04-04 Thread Boettger, Heiko

Hi,

sometimes I want to reorganize the content of a directory and simply move 
everything into a subdirectory. This seems to be more complicated than it 
should be. Since git mv requires the destination to exist, I need to create the 
target directory first. Unfortunately this results in git mv * to include that 
directory which results in an error since moving a directory into itself is not 
possible. My current workaround is rather long by using extglob:

mkdir -p newfolder/subfolder
shopt -s extglob
git mv !(newfolder) newfolder/subfolder
shopt -u extglob

For my usecase it would be much easier to archive that target by have a 
parameter to simply create the target if it isn't existing.

Best Regards
Heiko Böttger

STORZ Endoskop Produktions GmbH
Niederlassung Schaffhausen
Schneckenackerstr. 1
8200 Schaffhausen
Switzerland

--
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] rev-parse: fix --git-common-dir when executed from subpath of main tree