Re: [PATCH v12 2/5] test-parse-options: print quiet as integer

2016-04-05 Thread Eric Sunshine
On Tue, Apr 5, 2016 at 11:39 AM, Pranit Bauva  wrote:
> On Mon, Apr 4, 2016 at 3:00 AM, Eric Sunshine  wrote:
>> On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva  wrote:
>>> Current implementation of parse-options.c treats OPT__QUIET() as integer
>>> and not boolean and thus it is more appropriate to print it as integer
>>> to avoid confusion.
>>
>> I can buy this line of reasoning, however, it would be even easier to
>> sell the change if you cited an existing client (a git command) which
>> actually respects multiple quiet levels. Are there any?
>
> I investigated into this. But I was unable to find any git commit
> which actually respects mulitple quiet levels. I first did a 'git grep
> OPT__QUIET' to find the commands which use this. Then I went through
> the documentation which covers it. None of them have any such mention
> of multiple quiet levels. But still I dug further and and went through
> all the individual source files. I followed the corresponding C source
> code for the header file included and also searched there for any
> trace of quiet. But I still didn't find any such use of multiple quiet
> levels. I have found that the commit id 212c0a6f (Duy Ngyuyen; 7 Dec,
> 2013; parse-options: remove OPT__BOOLEAN). Maybe he has something to
> say as to why this was introduced and OPT__QUIET which previously used
> OPT__BOOLEAN, now uses OPT_COUNTUP rather than OPT_BOOL. This commit
> In fact git-repack command has quiet but this is not mentioned in the
> documentation. If someone could include this it in the documentation.
> I would do it but I am not quite familiar with git-repack and haven't
> really used it anytime.

I didn't find any existing Git command recognizing multiple quiet
levels either, however, I think I can still buy the change considering
that there is existing precedent in other Unix commands.
--
To unsubscribe from this list: send the line "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-05 Thread Eric Sunshine
On Tue, Apr 5, 2016 at 6:21 AM, Elia Pinto  wrote:
> 2016-04-01 22:25 GMT+02:00 Eric Sunshine :
>> In addition to review comments by others, why are the new curl_dump()
>> and curl_trace() functions duplicated in both patches rather than
>> factored out to a shared implementation?
>
> It's right. Do you think i can use some existing file or should I
> create a new object file ?

Peff or Junio would be more qualified to answer, but perhaps the
shared implementation could go in http.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 v1] correct blame for files commited with CRLF

2016-04-05 Thread Torsten Bögershausen
On 05.04.16 23:12, Junio C Hamano wrote:
> tbo...@web.de writes:
> 
>> +git config core.autocrlf true &&
>> +mv crlfinrepo tmp &&
>> +git checkout crlfinrepo &&
>> +rm tmp &&
> 
> Why not just "rm -f crlfinrepo" and "git checkout crlfinrepo"?
The intention was to get a new inode number, which marks the file
in the working tree as modified, regardless of mtime.



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


ATTN: BENEFICIARY

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


Can `git grep` have `--Author` option?

2016-04-05 Thread Chen Bin
This is a feature request.

>From time to time, I want to re-use MY code. So git grep with --Author options 
>is really useful. My current git version is 2.6.3, looks it doesn't have this 
>option.

-- 
Best Regards,
Chen Bin

--
Help me, help you
--
To unsubscribe from this list: send the line "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 --exit-code does not honour textconv setting

2016-04-05 Thread Junio C Hamano
Michael J Gruber  writes:

>> The "test" command is used as it does not generate any output on stdout.
>
> "test" is a bit of a red herring here since it will receive commands.
> But your example works even with "true" which ignores its commands and
> produces no output.

Either sounds strange, as they exit without reading their input at
all, so the other side of the pipe may get an write error it has to
handle ;-)

> The description doesn't make it clear whether exit-code refers to
> the actual diff (foo vs. bar) or to the diff after textconv (empty
> vs. empty). In any case, "-b" should not make a difference for
> your example.
>
> diff_flush() in diff.c has this piece of code:
>

It is understandable that "-b" makes difference.  The actual
short-cut happens a bit before the code you quoted.

if (output_format & DIFF_FORMAT_NO_OUTPUT &&
DIFF_OPT_TST(options, EXIT_WITH_STATUS) &&
DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
/*
 * run diff_flush_patch for the exit status. setting
 * options->file to /dev/null should be safe, because we
 * aren't supposed to produce any output anyway.
 */
if (options->close_file)
fclose(options->file);
options->file = fopen("/dev/null", "w");
if (!options->file)
die_errno("Could not open /dev/null");
options->close_file = 1;
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
if (check_pair_status(p))
diff_flush_patch(p, options);
if (options->found_changes)
break;
}
}

When running without producing output but we need the exit status,
unless DIFF_FROM_CONTENTS is set (e.g. "-b" in use), we do not run
the code that would inspect the blob contents that would have
produced the actual patch.  When DIFF_FROM_CONTENTS is set, we need
to dig into the blob contents and the body of the above if() gets
run only to trigger o->found_changes (e.g. in builtin_diff() that
sets ecbdata.found_changesp to point at o->found_changes before
calling into the xdiff machinery), but the output is discarded to
/dev/null.

The textconv filter is an unfortunate beast, in that while some
paths use one while others don't, so it won't be as simple as "-b"
that flips DIFF_FROM_CONTENTS to say "We need to inspect blobs for
ALL FILEPAIRS to see if there is any difference" if we want to keep
the optimization as much as possible.

Without DIFF_FROM_CONTENTS set, any filepair that point at two
different blobs that might end up to be the same after applying
textconv will flip the o->found_changes bit on, and with
EXIT_WITH_STATUS bit on, we have another optimization to skip
checking the remainder of the filepairs.  So if you want to support
textconv with QUICK, you would also need to disable that
optimization by teaching diff_change() to refrain from setting
HAS_CHANGES bit, i.e.

diff --git a/diff.h b/diff.h
index 125447b..f6c0c07 100644
--- a/diff.h
+++ b/diff.h
@@ -69,7 +69,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
diff_options *opt, void *data)
 #define DIFF_OPT_FIND_COPIES_HARDER  (1 <<  6)
 #define DIFF_OPT_FOLLOW_RENAMES  (1 <<  7)
 #define DIFF_OPT_RENAME_EMPTY(1 <<  8)
-/* (1 <<  9) unused */
+#define DIFF_OPT_WITH_TEXTCONV   (1 <<  9)
 #define DIFF_OPT_HAS_CHANGES (1 << 10)
 #define DIFF_OPT_QUICK   (1 << 11)
 #define DIFF_OPT_NO_INDEX(1 << 12)

diff --git a/diff.c b/diff.c
index 4dfe660..0016ad2 100644
--- a/diff.c
+++ b/diff.c
@@ -5018,6 +5018,11 @@ void diff_change(struct diff_options *options,
two->dirty_submodule = new_dirty_submodule;
p = diff_queue(_queued_diff, one, two);

+   if (either path one or path two has textconv defined) {
+   DIFF_OPT_SET(options, DIFF_WITH_TEXTCONV);
+   return;
+   }
+
if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
return;

And then in order to keep the optimization, add this to the above
codepath:

diff --git a/diff.c b/diff.c
index 4dfe660..7b318cc 100644
--- a/diff.c
+++ b/diff.c
@@ -4598,6 +4598,7 @@ void diff_flush(struct diff_options *options)
int i, output_format = options->output_format;
int separator = 0;
int dirstat_by_line = 0;
+   int textconv_hack;

/*
 * Order: raw, stat, summary, patch
@@ -4652,9 +4653,17 @@ void diff_flush(struct diff_options *options)
separator++;
}

+   /*
+* If there is any path that needs textconv and we haven't
 

Re: Triangular workflows and some anecdotes from the trenches

2016-04-05 Thread Junio C Hamano
Chris Packham  writes:

> We ran into something at $dayjob the other day. The actual problem was
> a developer ended up amending a commit that had already been pushed.
> It happens occasionally and is usually recoverable with a simple
> rebase and is generally a learning experience. In this particular case
> however things were a bit more complicated.

Developer ending up amending is not an issue per-se, unless the
result is pushed back to the public.

However, it would be nicer if "git commit --amend", "git rebase",
"git branch -f", and "git checkout -B" notices the situation and
warns about what would happen.

>   git config alias.amend '!git merge-base --is-ancestor HEAD
> @{upstream} || git commit --amend'
>
> I'm just wondering if something more official can be added to git
> commit --amend (and probably git rebase).

A bigger problem may be how you make sure everybody sets up
@{upstream} correctly.  You may fork your own copy of a branch from
the target branch, start working on it, further fork other branches
on your work to experiment different approaches, with the intention
to later use the best one to update your first fork.

At which point, the @{upstream} of the secondary branches are your
own first branch, not the public one--which is not a problem per-se,
because your first branch (whose @{upstream} is the remote one) is
not yet public and you should be allowed to freely update it to
polish it by rewriting.  But then after you push out your first
branch as an interim snapshot to the public, you no longer want to
rewrite the commits reachable from it.  So (to put it mildly) it
would be quite complex to get all the corner cases right, and the
definition of "right" would probably depend on the exact workflow.

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


reply

2016-04-05 Thread Manuelle Duszynski


Good day, friend. I'm a Banker with HSBC here in Malaysia.I have an interesting
business deal for you that will be of immense benefit to both of us.Although 
this
may be hard for you to believe,we stand to gain $7million in a matter of  days.
Please grant me the benefit of doubt and hear me out. I need you to signify your
interest by replying to this mail.

--
To unsubscribe from this list: send the line "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] diff: run arguments through precompose_argv

2016-04-05 Thread Junio C Hamano
Torsten Bögershausen  writes:

> On 05.04.16 21:15, Johannes Sixt wrote:
>> Am 05.04.2016 um 19:09 schrieb Junio C Hamano:
 Thanks-to: Torsten Bögershausen 
>> 
>> I sense NFD disease: The combining diaresis should combine with the o, not 
>> the g. Here is a correct line to copy-and-paste if you like:
>> 
>> Thanks-to: Torsten Bögershausen 
>> 
>> -- Hannes
>
> Good eyes.
>
> And thanks to Alexander for doing this patch

Yup.  It is funny that I wrote:

Thanks.  The log message talks about "such a file path", but
precompose_argv() applies indiscriminately on any command line
arguments, so things like -G would also get the same
treatment, which I think is what most users would want.

but "git log --grep=" with your name would not have found the one
that would have resulted from the original e-mail message applied.
--
To unsubscribe from this list: send the line "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] send-email: more meaningful Message-ID

2016-04-05 Thread Eric Wong
Junio C Hamano  wrote:
> Eric Wong  writes:
> 
> > Using a mmddHHMMSS date representation is more meaningful to
> > humans, especially when used for lookups on NNTP servers or linking
> > to archive sites via Message-ID (e.g. mid.gmane.org or
> > mid.mail-archive.com).  This timestamp format more easily gives a
> > reader of the URL itself a rough date of a linked message compared
> > to having them calculate the seconds since the Unix epoch.
> >
> > Furthermore, having the MUA name in the Message-ID seems to be a
> > rare oddity I haven't noticed outside of git-send-email.  We
> > already have an optional X-Mailer header field to advertise for
> > us, so extending the Message-ID by 15 characters can make for
> > unpleasant Message-ID-based URLs to archive sites.
> >
> > Signed-off-by: Eric Wong 
> > ---
> 
> Sounds like a sensible goal.  Just a few comments.
> 
>  - Is it safe to assume that we always can use POSIX::strftime(), or
>do we need some fallback?  I am guessing that this is safe, as
>POSIX has been part of the core modules for a long time, and the
>script does "use 5.008" upfront.

I'm hoping so :)  And none of the format specifiers used here
should be subject to locale-dependent weirdness, at least.

+Cc both Johannes for Windows knowledge.

>  - It is my understanding that, as "use" is a compilation-time
>thing, hiding it inside a block does not help reducing the
>start-up overhead (people can use "require" if they want to do a
>lazy loading and optionally a fallback).  Is my Perl5 outdated?
>Otherwise, let's have it near the beginning of the script, close
>to where we use Term::ReadLine and others.

You're correct, I'll move the "use" to the top in v2.

I could call "require" and call the sub as "POSIX::strftime",
but this code path is likely enough that any startup time
improvement for uncommon cases wouldn't be worth it.

Will wait a bit for strftime portability comments before v2.

> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -949,7 +949,8 @@ my ($message_id_stamp, $message_id_serial);
> >  sub make_message_id {
> > my $uniq;
> > if (!defined $message_id_stamp) {
> > -   $message_id_stamp = sprintf("%s-%s", time, $$);
> > +   use POSIX qw/strftime/;
> > +   $message_id_stamp = strftime("%Y%m%d%H%M%S.$$", gmtime(time));
> > $message_id_serial = 0;
> > }
> > $message_id_serial++;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1] correct blame for files commited with CRLF

2016-04-05 Thread Junio C Hamano
tbo...@web.de writes:

> + git config core.autocrlf true &&
> + mv crlfinrepo tmp &&
> + git checkout crlfinrepo &&
> + rm tmp &&

Why not just "rm -f crlfinrepo" and "git checkout crlfinrepo"?

> + git blame crlfinrepo >actual &&
> + grep "A U Thor" actual
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] send-email: more meaningful Message-ID

2016-04-05 Thread Junio C Hamano
Eric Wong  writes:

> Using a mmddHHMMSS date representation is more meaningful to
> humans, especially when used for lookups on NNTP servers or linking
> to archive sites via Message-ID (e.g. mid.gmane.org or
> mid.mail-archive.com).  This timestamp format more easily gives a
> reader of the URL itself a rough date of a linked message compared
> to having them calculate the seconds since the Unix epoch.
>
> Furthermore, having the MUA name in the Message-ID seems to be a
> rare oddity I haven't noticed outside of git-send-email.  We
> already have an optional X-Mailer header field to advertise for
> us, so extending the Message-ID by 15 characters can make for
> unpleasant Message-ID-based URLs to archive sites.
>
> Signed-off-by: Eric Wong 
> ---

Sounds like a sensible goal.  Just a few comments.

 - Is it safe to assume that we always can use POSIX::strftime(), or
   do we need some fallback?  I am guessing that this is safe, as
   POSIX has been part of the core modules for a long time, and the
   script does "use 5.008" upfront.

 - It is my understanding that, as "use" is a compilation-time
   thing, hiding it inside a block does not help reducing the
   start-up overhead (people can use "require" if they want to do a
   lazy loading and optionally a fallback).  Is my Perl5 outdated?
   Otherwise, let's have it near the beginning of the script, close
   to where we use Term::ReadLine and others.

Thanks.

>  git-send-email.perl | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index d356901..23141e7 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -949,7 +949,8 @@ my ($message_id_stamp, $message_id_serial);
>  sub make_message_id {
>   my $uniq;
>   if (!defined $message_id_stamp) {
> - $message_id_stamp = sprintf("%s-%s", time, $$);
> + use POSIX qw/strftime/;
> + $message_id_stamp = strftime("%Y%m%d%H%M%S.$$", gmtime(time));
>   $message_id_serial = 0;
>   }
>   $message_id_serial++;
> @@ -964,7 +965,7 @@ sub make_message_id {
>   require Sys::Hostname;
>   $du_part = 'user@' . Sys::Hostname::hostname();
>   }
> - my $message_id_template = "<%s-git-send-email-%s>";
> + my $message_id_template = "<%s-%s>";
>   $message_id = sprintf($message_id_template, $uniq, $du_part);
>   #print "new message id = $message_id\n"; # Was useful for debugging
>  }
--
To unsubscribe from this list: send the line "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: Is OS X still supported?

2016-04-05 Thread Michał Staruch
Thanks for the information that binary builds are availably on
SourceForge faster than on git-scm. I can see the v2.8.1 for OS X was
uploaded few hours ago to the SF, so my main problem (lack of security
fixes in git for OS X) is solved.

The automation process should be probably reviewed, though - because
all the other folks around the world using git-scm (not the SF) to
download OS X builds are still stuck at v2.6.4. Ideally git-scm would
point to the new Mac version within single minutes since the release
(or even seconds) - not hours, days, or weeks.

>From my point of view SourceForge vs GitHub is kinda implementation
detail. I'd go with GitHub as it's more convenient to use and supported
HTTPS since the beginning. And then SF had really bad idea with pushing
malware (see https://sourceforge.net/p/forge/site-support/7414/). But
as long as git-scm will be getting binaries on time most folks won't
really care about details of delivery process.


On Tue, Apr 5, 2016 at 6:43 PM, Tim Harper  wrote:
> It is still supported. I'm not sure why git-scm is pointing to the wrong 
> version. There's been some discussion to upload to github instead, which I'm 
> for, but SourceForge publishing is already automated.
>
>> On Apr 5, 2016, at 10:38, Junio C Hamano  wrote:
>>
>> Michał Staruch  writes:
>>
>>> I'd like to ask if OS X is still supported platform for git. Sources
>>> and Windows build were updated to version 2.8.1, while OS X build
>>> stopped at 2.6.4, staying vulnerable to CVE-2016-2315 and
>>> CVE-2016-2324.
>>
>> Thanks for asking.
>>
>> Tim Harper (CC'ed) helps the OSX users by supplying the OSX
>> installer.
>>
>> I think git-scm.com attempts to show the latest OSX installer from
>> https://sourceforge.net/projects/git-osx-installer/.
>>
>> It's funny that that
>>
>>  https://sourceforge.net/projects/git-osx-installer/files/
>>
>> does list 2.7.1 that is newer than 2.6.4, but the quick download
>> link on that page points at 2.6.2; there is something screwy
>> happening at sourceforge.  I am not sure how git-scm.com chooses to
>> claim that 2.6.4 is the latest.  There seems to be an issue open on
>> this.
>>
>>https://github.com/git/git-scm.com/issues/715
>>
>> As I do not do binary packaging for individual platforms, I cannot
>> be of more help than what this message says; sorry about that.
>>
>> Next time please send any message that is related to Git to either
>> git@vger.kernel.org mailing list (public) or if you want to
>> privately discuss security related issues that are not yet known to
>> the public, then to git-secur...@googlegroups.com [*1*].  There are
>> at least three reasons to do so:
>>
>> - A message that is addressed only to gits...@pobox.com and not one
>>   of these lists are often eaten by spam filters and will not be
>>   seen by me.
>>
>> - I am not an expert on everything that is related to Git (this
>>   topic is a good example), and people more qualified to answer are
>>   on these lists.
>>
>> - I suspect that you are not the only Git user on OSX, so there
>>   must be more people wondering the same thing as you are, so
>>   asking git@vger.kernel.org would help other OSX users.
>>
>> I almost added "Cc: git@vger.kernel.org" myself on this response,
>> but I didn't because there might be a reason for you to hide your
>> e-mail address from the public (some people are weird that way, and
>> you might be one of them but I couldn't tell because I do not know
>> you).  If you do not mind helping other OSX users, I am fine if you
>> CC'ed your response to this message to git@vger.kernel.org while
>> quoting everything I wrote here.
>>
>> Thanks.
>>
>>
>> [Footnote]
>>
>> *1* Both of these two lists accept messages from non-subscribers,
>> i.e.  you can send messages to them without subscribing to them, and
>> you'll be kept in the loop in the discussion by CC'ing the original
>> poster.
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v1] correct blame for files commited with CRLF

2016-04-05 Thread Junio C Hamano
tbo...@web.de writes:

>  This fix is completely independent of the rest of the series,
>  so break out 6/7 from tb/safe-crlf-output.

Sounds sensible.  It is somewhat sad and strange that we need to
rely on what is in the index to show the current working tree state,
but this makes the things more consistent.

Will queue.  Thanks.

> builtin/blame.c   |  1 +
>  t/t8003-blame-corner-cases.sh | 14 ++
>  2 files changed, 15 insertions(+)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index e982fb8..21f42b0 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2307,6 +2307,7 @@ static struct commit *fake_working_tree_commit(struct 
> diff_options *opt,
>   unsigned mode;
>   struct strbuf msg = STRBUF_INIT;
>  
> + read_cache();
>   time();
>   commit = alloc_commit_node();
>   commit->object.parsed = 1;
> diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
> index 6568429..a9b266f 100755
> --- a/t/t8003-blame-corner-cases.sh
> +++ b/t/t8003-blame-corner-cases.sh
> @@ -212,4 +212,18 @@ test_expect_success 'blame file with CRLF attributes 
> text' '
>   grep "A U Thor" actual
>  '
>  
> +test_expect_success 'blame file with CRLF core.autocrlf=true' '
> + git config core.autocrlf false &&
> + printf "testcase\r\n" >crlfinrepo &&
> + >.gitattributes &&
> + git add crlfinrepo &&
> + git commit -m "add crlfinrepo" &&
> + git config core.autocrlf true &&
> + mv crlfinrepo tmp &&
> + git checkout crlfinrepo &&
> + rm tmp &&
> + git blame crlfinrepo >actual &&
> + grep "A U Thor" actual
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] api-trace.txt: fix typo

2016-04-05 Thread Junio C Hamano
Eric Sunshine  writes:

> On Tue, Apr 5, 2016 at 6:05 AM, Elia Pinto  wrote:
>> The correct api is trace_printf_key
>>
>> Signed-off-by: Elia Pinto 
>> ---
>> diff --git a/Documentation/technical/api-trace.txt 
>> b/Documentation/technical/api-trace.txt
>> @@ -28,7 +28,7 @@ static struct trace_key trace_foo = TRACE_KEY_INIT(FOO);
>>  static void trace_print_foo(const char *message)
>>  {
>> -   trace_print_key(_foo, message);
>> +   trace_printf_key(_foo, message);
>>  }
>
> Since you're touching this already, I wonder if it would make sense to
> rewrite this example to avoid the dangerous sending of an arbitrary
> string (which might contain %) to a printf-like function. Like this,
> for example:
>
> trace_printf_key(_foo, "%s", message);

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


Condominium Owners List

2016-04-05 Thread Phoebe Calkins


Hi,

Would you be interested in reaching out to "Condominium Owners List"? with 
opt-in verified email addresses from the USA.

We also have contacts of Real Estate Investors, Home Owners, Mortgage Holders, 
Mortgage with Home, Investors List, New Movers List, Golfers Email List, High 
Net-worth Individuals List, Women's List, and many more...

Each record in the list contains Contact Name (First, Middle and Last Name), 
Mailing Address, List type and Opt-in email address.
 
Please let me know your targeted criteria, so that I can help you out to drive your sales effort in the right direction.
 
Thanks & Regards,

Phoebe Calkins


---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

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


Fsck message override issue

2016-04-05 Thread Seren Thompson
I'm not having luck with overriding the fsck message types (zeroPaddedFilemode 
specifically). I've tried adding
 [fsck]
zeroPaddedFilemode = ignore
to both global and local configs, but it seems to have no effect:

  $ git --version
  git version 2.8.0
  $ git config --get fetch.fsckobjects
  true
  $ git config --get fsck.zeroPaddedFilemode
  ignore
  $ git pull
  remote: Counting objects: 14777, done.
  remote: Compressing objects: 100% (5495/5495), done.
  error: object 2b7227859263b6aabcc28355b0b994995b7148b6: zeroPaddedFilemode: 
contains zero-padded file modes
  fatal: Error in object
  fatal: index-pack failed

Is this expected behavior and I'm doing something wrong? Thanks.

-Seren


--
To unsubscribe from this list: send the line "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] send-email: more meaningful Message-ID

2016-04-05 Thread Eric Wong
Using a mmddHHMMSS date representation is more meaningful to
humans, especially when used for lookups on NNTP servers or linking
to archive sites via Message-ID (e.g. mid.gmane.org or
mid.mail-archive.com).  This timestamp format more easily gives a
reader of the URL itself a rough date of a linked message compared
to having them calculate the seconds since the Unix epoch.

Furthermore, having the MUA name in the Message-ID seems to be a
rare oddity I haven't noticed outside of git-send-email.  We
already have an optional X-Mailer header field to advertise for
us, so extending the Message-ID by 15 characters can make for
unpleasant Message-ID-based URLs to archive sites.

Signed-off-by: Eric Wong 
---
 git-send-email.perl | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index d356901..23141e7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -949,7 +949,8 @@ my ($message_id_stamp, $message_id_serial);
 sub make_message_id {
my $uniq;
if (!defined $message_id_stamp) {
-   $message_id_stamp = sprintf("%s-%s", time, $$);
+   use POSIX qw/strftime/;
+   $message_id_stamp = strftime("%Y%m%d%H%M%S.$$", gmtime(time));
$message_id_serial = 0;
}
$message_id_serial++;
@@ -964,7 +965,7 @@ sub make_message_id {
require Sys::Hostname;
$du_part = 'user@' . Sys::Hostname::hostname();
}
-   my $message_id_template = "<%s-git-send-email-%s>";
+   my $message_id_template = "<%s-%s>";
$message_id = sprintf($message_id_template, $uniq, $du_part);
#print "new message id = $message_id\n"; # Was useful for debugging
 }
-- 
EW

--
To unsubscribe from this list: send the line "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] diff: run arguments through precompose_argv

2016-04-05 Thread Torsten Bögershausen
On 05.04.16 21:15, Johannes Sixt wrote:
> Am 05.04.2016 um 19:09 schrieb Junio C Hamano:
>>> Thanks-to: Torsten Bögershausen 
> 
> I sense NFD disease: The combining diaresis should combine with the o, not 
> the g. Here is a correct line to copy-and-paste if you like:
> 
> Thanks-to: Torsten Bögershausen 
> 
> -- Hannes

Good eyes.

And thanks to Alexander for doing this patch


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


[PATCH v1] correct blame for files commited with CRLF

2016-04-05 Thread tboegi
From: Torsten Bögershausen 

git blame reports lines as not "Not Committed Yet" when they have
CRLF in the index, CRLF in the worktree and core.autocrlf is true.

Since commit c48053 "new safer autocrlf handling", files that have CRLF
in the index are not normalized at commit when core.autocrl is set.

Add a call to read_cache() early in fake_working_tree_commit(),
before calling convert_to_git().

Signed-off-by: Torsten Bögershausen 
---
 This fix is completely independent of the rest of the series,
 so break out 6/7 from tb/safe-crlf-output.
 The rest of the series will be send in a couple of days, some
 rework is needed.
builtin/blame.c   |  1 +
 t/t8003-blame-corner-cases.sh | 14 ++
 2 files changed, 15 insertions(+)

diff --git a/builtin/blame.c b/builtin/blame.c
index e982fb8..21f42b0 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2307,6 +2307,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
unsigned mode;
struct strbuf msg = STRBUF_INIT;
 
+   read_cache();
time();
commit = alloc_commit_node();
commit->object.parsed = 1;
diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index 6568429..a9b266f 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -212,4 +212,18 @@ test_expect_success 'blame file with CRLF attributes text' 
'
grep "A U Thor" actual
 '
 
+test_expect_success 'blame file with CRLF core.autocrlf=true' '
+   git config core.autocrlf false &&
+   printf "testcase\r\n" >crlfinrepo &&
+   >.gitattributes &&
+   git add crlfinrepo &&
+   git commit -m "add crlfinrepo" &&
+   git config core.autocrlf true &&
+   mv crlfinrepo tmp &&
+   git checkout crlfinrepo &&
+   rm tmp &&
+   git blame crlfinrepo >actual &&
+   grep "A U Thor" actual
+'
+
 test_done
-- 
2.8.0.rc2.2.g1a4d45a.dirty

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


Re: [PATCH] diff: run arguments through precompose_argv

2016-04-05 Thread Johannes Sixt

Am 05.04.2016 um 19:09 schrieb Junio C Hamano:

Thanks-to: Torsten Bögershausen 


I sense NFD disease: The combining diaresis should combine with the o, 
not the g. Here is a correct line to copy-and-paste if you like:


Thanks-to: Torsten Bögershausen 

-- Hannes

--
To unsubscribe from this list: send the line "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: Timestamp of zero in reflog considered invalid

2016-04-05 Thread Andreas Schwab
Junio C Hamano  writes:

> Checking the value against ULONG_MAX and errno==ERANGE would be an
> improvement.  It may be debatable if we should silently ignore an
> entry with an invalid timestamp, but that is a separate issue.
>
>  refs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/refs.c b/refs.c
> index 4e15f60..ff24184 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3701,7 +3701,8 @@ static int show_one_reflog_ent(struct strbuf *sb, 
> each_reflog_ent_fn fn, void *c
>   get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' ||
>   !(email_end = strchr(sb->buf + 82, '>')) ||
>   email_end[1] != ' ' ||
> - !(timestamp = strtoul(email_end + 2, , 10)) ||
> + ((timestamp = strtoul(email_end + 2, , 10)) == ULONG_MAX &&
> +  errno == ERANGE) ||

You need to set errno = 0 before calling strtoul, to distinguish the
valid return of ULONG_MAX (which would keep errno intact) and a real
overflow.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "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] api-trace.txt: fix typo

2016-04-05 Thread Eric Sunshine
On Tue, Apr 5, 2016 at 6:05 AM, Elia Pinto  wrote:
> The correct api is trace_printf_key
>
> Signed-off-by: Elia Pinto 
> ---
> diff --git a/Documentation/technical/api-trace.txt 
> b/Documentation/technical/api-trace.txt
> @@ -28,7 +28,7 @@ static struct trace_key trace_foo = TRACE_KEY_INIT(FOO);
>  static void trace_print_foo(const char *message)
>  {
> -   trace_print_key(_foo, message);
> +   trace_printf_key(_foo, message);
>  }

Since you're touching this already, I wonder if it would make sense to
rewrite this example to avoid the dangerous sending of an arbitrary
string (which might contain %) to a printf-like function. Like this,
for example:

trace_printf_key(_foo, "%s", 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] diff: run arguments through precompose_argv

2016-04-05 Thread Junio C Hamano
Alexander Rinass  writes:

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

Thanks.  The log message talks about "such a file path", but
precompose_argv() applies indiscriminately on any command line
arguments, so things like -G would also get the same
treatment, which I think is what most users would want).

Will queue.

> Signed-off-by: Alexander Rinass 
> Thanks-to: Torsten Bö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 

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

2016-04-05 Thread Santiago Torres
Sorry, forgot to add, this is a follow on to [1].

Thanks,
-Santiago.

[1]:
http://git.661346.n2.nabble.com/PATCH-v4-0-6-tag-move-PGP-verification-code-to-tag-c-td7652451.html
On Tue, Apr 05, 2016 at 12:07:23PM -0400, santi...@nyu.edu wrote:
> From: Santiago Torres 
> 
> v5 (this): 
> Added helpful feedback by Eric
> 
>  * Reordering of the patches, to avoid temporal inclusion of a regression
>  * Fix typos here and there.
>  * Review commit messages, as some weren't representative of what the patches
>were doing anymore.
>  * Updated t7030 to include Peff's suggestion, and added a helped-by line here
>as it was mostly Peff's code.
>  * Updated the error-handling/printing issues that were introduced when.
>libifying the verify_tag function.
>
> v4:
> 
> Thanks Eric, Jeff, 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
> 
> 
> Santiago Torres (6):
>   builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface
>   t7030-verify-tag: Adds validation for multiple tags
>   builtin/verify-tag: change variable name for readability
>   builtin/verify-tag: replace name argument with sha1
>   builtin/verify-tag: move verification code to tag.c
>   tag: use gpg_verify_function in tag -v call
> 
>  builtin/tag.c |  8 +--
>  builtin/verify-tag.c  | 65 
> +--
>  gpg-interface.c   |  2 ++
>  t/t7030-verify-tag.sh | 12 ++
>  tag.c | 48 +
>  tag.h |  1 +
>  6 files changed, 75 insertions(+), 61 deletions(-)
> 
> -- 
> 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: Feature request: config option for default git commit -v

2016-04-05 Thread Jacek Wielemborek
W dniu 05.04.2016 o 16:47, Pranit Bauva pisze:
> On Tue, Apr 5, 2016 at 8:08 PM, Jacek Wielemborek  wrote:
>> Hello,
>>
>> I'm asking for this one because there's quite a lot of interest
>> (including me) in this feature and there is no convenient walkaround:
>>
>> https://stackoverflow.com/questions/5875275/git-commit-v-by-default
>>
>> Cheers,
>> d33tah
> 
> This is currently under progress. I am the one who is working on it.
> One of the patches is currently on the pu branch. I am still polishing
> it to include some more stuff. You can track its status by reading the
> git.git messages by the git maintainer. The latest revision of the
> patch is at http://thread.gmane.org/gmane.comp.version-control.git/288820
> 
> Thanks,
> Pranit Bauva

Awesome, thanks for the quick answer! I let the StackOverflow folks know.



signature.asc
Description: OpenPGP digital signature


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

2016-04-05 Thread santiago
From: Santiago Torres 

v5 (this): 
Added helpful feedback by Eric

 * Reordering of the patches, to avoid temporal inclusion of a regression
 * Fix typos here and there.
 * Review commit messages, as some weren't representative of what the patches
   were doing anymore.
 * Updated t7030 to include Peff's suggestion, and added a helped-by line here
   as it was mostly Peff's code.
 * Updated the error-handling/printing issues that were introduced when.
   libifying the verify_tag function.
   
v4:

Thanks Eric, Jeff, 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


Santiago Torres (6):
  builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface
  t7030-verify-tag: Adds validation for multiple tags
  builtin/verify-tag: change variable name for readability
  builtin/verify-tag: replace name argument with sha1
  builtin/verify-tag: move verification code to tag.c
  tag: use gpg_verify_function in tag -v call

 builtin/tag.c |  8 +--
 builtin/verify-tag.c  | 65 +--
 gpg-interface.c   |  2 ++
 t/t7030-verify-tag.sh | 12 ++
 tag.c | 48 +
 tag.h |  1 +
 6 files changed, 75 insertions(+), 61 deletions(-)

-- 
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 v5 1/6] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface

2016-04-05 Thread santiago
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 
---
 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 v5 2/6] t7030-verify-tag: Adds validation for multiple tags

2016-04-05 Thread santiago
From: Santiago Torres 

The verify-tag command supports multiple tag names as an argument.
However, existing tests only test for invocation with a single tag, so
we add a test invoking with multiple tags.

Helped-by: Jeff King 
Signed-off-by: Santiago Torres 
---
 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..c01621a 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 "^.GNUPG" expect.stderr &&
+   git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
+   grep "^.GNUPG" 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 v5 5/6] builtin/verify-tag: move verification code to tag.c

2016-04-05 Thread santiago
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
module. We rename the verify_tag() function to gpg_verify_tag() to avoid
conflicts with the mktag.c function.

Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 50 +-
 tag.c| 48 
 tag.h|  1 +
 3 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 7a7c376..e9a2005 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,54 +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 payload_size;
-   int ret;
-
-   memset(, 0, sizeof(sigc));
-
-   payload_size = parse_signature(buf, size);
-
-   if (size == payload_size) {
-   if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, payload_size);
-   return error("no signature found");
-   }
-
-   ret = check_signature(buf, payload_size, buf + payload_size,
-   size - payload_size, );
-   print_signature_buffer(, flags);
-
-   signature_check_clear();
-   return ret;
-}
-
-static int verify_tag(const unsigned char *sha1, unsigned flags)
-{
-   enum object_type type;
-   char *buf;
-   char *hex_sha1;
-   unsigned long size;
-   int ret;
-
-   hex_sha1 = sha1_to_hex(sha1);
-   type = sha1_object_info(sha1, NULL);
-   if (type != OBJ_TAG)
-   return error("%s: cannot verify a non-tag object of type %s.",
-   hex_sha1, typename(type));
-
-   buf = read_sha1_file(sha1, , );
-   if (!buf)
-   return error("%s: unable to read file.", hex_sha1);
-
-   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);
@@ -103,7 +55,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
had_error = 1;
continue;
}
-   if (verify_tag(sha1, flags))
+   if (gpg_verify_tag(sha1, flags))
had_error = 1;
}
return had_error;
diff --git a/tag.c b/tag.c
index d72f742..3f7669f 100644
--- a/tag.c
+++ b/tag.c
@@ -6,6 +6,54 @@
 
 const char *tag_type = "tag";
 
+static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
+{
+   struct signature_check sigc;
+   int payload_size;
+   int ret;
+
+   memset(, 0, sizeof(sigc));
+
+   payload_size = parse_signature(buf, size);
+
+   if (size == payload_size) {
+   if (flags & GPG_VERIFY_VERBOSE)
+   write_in_full(1, buf, payload_size);
+   return error("no signature found");
+   }
+
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, );
+   print_signature_buffer(, flags);
+
+   signature_check_clear();
+   return ret;
+}
+
+int gpg_verify_tag(const unsigned char *sha1, unsigned flags)
+{
+   enum object_type type;
+   char *buf;
+   char *hex_sha1;
+   unsigned long size;
+   int ret;
+
+   hex_sha1 = sha1_to_hex(sha1);
+   type = sha1_object_info(sha1, NULL);
+   if (type != OBJ_TAG)
+   return error("%s: cannot verify a non-tag object of type %s.",
+   hex_sha1, typename(type));
+
+   buf = read_sha1_file(sha1, , );
+   if (!buf)
+   return error("%s: unable to read file.", hex_sha1);
+
+   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..cb643b9 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 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 v5 4/6] builtin/verify-tag: replace name argument with sha1

2016-04-05 Thread santiago
From: Santiago Torres 

This change is meant to prepare verify_tag for libification. Many
existing modules/commands already do the refname to sha1 resolution, so
should avoid resolving the refname twice. To avoid breaking
builtin/verify-tag, we move the refname resolution outside of the
verify_tag() call.

Signed-off-by: Santiago Torres 
---
 builtin/verify-tag.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 1ca9a05..7a7c376 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -42,25 +42,23 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-static int verify_tag(const char *name, unsigned flags)
+static int verify_tag(const unsigned char *sha1, unsigned flags)
 {
enum object_type type;
-   unsigned char sha1[20];
char *buf;
+   char *hex_sha1;
unsigned long size;
int ret;
 
-   if (get_sha1(name, sha1))
-   return error("tag '%s' not found.", name);
-
+   hex_sha1 = sha1_to_hex(sha1);
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));
+   hex_sha1, typename(type));
 
buf = read_sha1_file(sha1, , );
if (!buf)
-   return error("%s: unable to read file.", name);
+   return error("%s: unable to read file.", hex_sha1);
 
ret = run_gpg_verify(buf, size, flags);
 
@@ -80,6 +78,8 @@ 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 char *name;
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),
@@ -96,8 +96,15 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
if (verbose)
flags |= GPG_VERIFY_VERBOSE;
 
-   while (i < argc)
-   if (verify_tag(argv[i++], flags))
+   while (i < argc) {
+   name = argv[i++];
+   if (get_sha1(name, sha1)) {
+   error("tag '%s' not found.", name);
had_error = 1;
+   continue;
+   }
+   if (verify_tag(sha1, flags))
+   had_error = 1;
+   }
return had_error;
 }
-- 
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 v5 6/6] tag: use gpg_verify_function in tag -v call

2016-04-05 Thread santiago
From: Santiago Torres 

Instead of running the verify-tag plumbing command, we use the
gpg_verify_tag() function within the verify_tag function to avoid doing
an additional fork call.

Signed-off-by: Santiago Torres 
---
 builtin/tag.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 1705c94..398c892 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(sha1, 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 v5 3/6] builtin/verify-tag: change variable name for readability

2016-04-05 Thread santiago
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 
---
 builtin/verify-tag.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 77f070a..1ca9a05 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -21,20 +21,21 @@ static const char * const verify_tag_usage[] = {
 static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
 {
struct signature_check sigc;
-   int len;
+   int payload_size;
int ret;
 
memset(, 0, sizeof(sigc));
 
-   len = parse_signature(buf, size);
+   payload_size = parse_signature(buf, size);
 
-   if (size == len) {
+   if (size == payload_size) {
if (flags & GPG_VERIFY_VERBOSE)
-   write_in_full(1, buf, len);
+   write_in_full(1, buf, payload_size);
return error("no signature found");
}
 
-   ret = check_signature(buf, len, buf + len, size - len, );
+   ret = check_signature(buf, payload_size, buf + payload_size,
+   size - payload_size, );
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 v12 5/5] commit: add a commit.verbose config variable

2016-04-05 Thread Pranit Bauva
On Tue, Apr 5, 2016 at 4:59 AM, Eric Sunshine  wrote:
> 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.
>
I hadn't thought of this before. I also believe using for-loops will
make it more clear, crisp and will avoid the effort of going through
the whole patch to find out if some test is missing.

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

I will include it this series only as it will be a bit easier for me
to keep a track of the previous reviews.
--
To unsubscribe from this list: send the line "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 4/5] t7507-commit-verbose: improve test coverage by testing number of diffs

2016-04-05 Thread Pranit Bauva
On Mon, Apr 4, 2016 at 5:32 AM, Eric Sunshine  wrote:
> On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva  wrote:
>> Make the fake "editor" store output of grep in a file so that we can
>> see how many diffs were contained in the message and use them in
>> individual tests where ever it is required. Also use write_script()
>> to create the fake "editor".
>>
>> A subsequent commit will introduce scenarios where it is important to be
>> able to exactly determine how many diffs were present.
>
> These two sentences are backward. The "subsequent commit" bit is
> justification for why you are making the "editor" store the output,
> thus it belongs with the first paragraph. The bit about write_script()
> is just a minor aside which can go in its own paragraph.
>
> I think it's also important to explain that you're changing the
> behavior of write_script() so that it always succeeds, regardless of
> whether grep found diff headers or not, and to give the reason for
> making this change ("so that you don't have to use 'test_must_fail'
> for cases when no diff headers are expected and can instead easily use
> 'test_line_count = 0'").
>
> The patch itself looks fine and is easy enough to read.

I will include some more explanation as you suggest.

>
>> Helped-by: Eric Sunshine 
>> Signed-off-by: Pranit Bauva 
>> ---
>> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
>> index 2ddf28c..0f28a86 100755
>> --- a/t/t7507-commit-verbose.sh
>> +++ b/t/t7507-commit-verbose.sh
>> @@ -3,11 +3,10 @@
>>  test_description='verbose commit template'
>>  . ./test-lib.sh
>>
>> -cat >check-for-diff <> -#!$SHELL_PATH
>> -exec grep '^diff --git' "\$1"
>> +write_script "check-for-diff" <<\EOF &&
>> +grep '^diff --git' "$1" >out
>> +exit 0
>>  EOF
>> -chmod +x check-for-diff
>>  test_set_editor "$PWD/check-for-diff"
>>
>>  cat >message <<'EOF'
>> @@ -23,7 +22,8 @@ test_expect_success 'setup' '
>>  '
>>
>>  test_expect_success 'initial commit shows verbose diff' '
>> -   git commit --amend -v
>> +   git commit --amend -v &&
>> +   test_line_count = 1 out
>>  '
>>
>>  test_expect_success 'second commit' '
>> @@ -39,13 +39,15 @@ check_message() {
>>
>>  test_expect_success 'verbose diff is stripped out' '
>> git commit --amend -v &&
>> -   check_message message
>> +   check_message message &&
>> +   test_line_count = 1 out
>>  '
>>
>>  test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '
>> git config diff.mnemonicprefix true &&
>> git commit --amend -v &&
>> -   check_message message
>> +   check_message message &&
>> +   test_line_count = 1 out
>>  '
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Timestamp of zero in reflog considered invalid

2016-04-05 Thread Junio C Hamano
Erik Bray  writes:

> I tracked the issue to refs/files-backend.c in show_one_reflog_ent :
>
> https://github.com/git/git/blob/11529ecec914d2f0d7575e6d443c2d5a6ff75424/refs/files-backend.c#L2923
>
> in which
>
> !(timestamp = strtoul(email_end + 2, , 10)) ||
>
> implies an invalid reflog entry.  Why should 0 be treated as an
> invalid timestamp (even if it's unlikely outside of corner cases)?

Thanks for a report.

I think this dates back to 883d60fa (Sanitize for_each_reflog_ent(),
2007-01-08):

commit 883d60fa97c6397450fb129634054e0a6101baac
Author: Johannes Schindelin 
Date:   Mon Jan 8 01:59:54 2007 +0100

Sanitize for_each_reflog_ent()

It used to ignore the return value of the helper function; now, it
expects it to return 0, and stops iteration upon non-zero return
values; this value is then passed on as the return value of
for_each_reflog_ent().

Further, it makes no sense to force the parsing upon the helper
functions; for_each_reflog_ent() now calls the helper function with
old and new sha1, the email, the timestamp & timezone, and the message.

Signed-off-by: Johannes Schindelin 
Signed-off-by: Junio C Hamano 

I do not see a corresponding "timestamp must be non-zero" check in
the lines removed by that commit.  I suspect that there was some
confusion as to how strtoul() signals an error condition when the
commit was written, or something.  I do not think existing
implementations of Git supports timestamps before the epoch (the
timestamp on the common object headers are read into unsigned long
variables and the code is not prepared to see anything negative
there), but if anything the code should be detecting errors from
strtoul() properly, i.e. if a timestamp is way long into the future
and does not fit in "unsigned long", we should detect it.

Checking the value against ULONG_MAX and errno==ERANGE would be an
improvement.  It may be debatable if we should silently ignore an
entry with an invalid timestamp, but that is a separate issue.

 refs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index 4e15f60..ff24184 100644
--- a/refs.c
+++ b/refs.c
@@ -3701,7 +3701,8 @@ static int show_one_reflog_ent(struct strbuf *sb, 
each_reflog_ent_fn fn, void *c
get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' ||
!(email_end = strchr(sb->buf + 82, '>')) ||
email_end[1] != ' ' ||
-   !(timestamp = strtoul(email_end + 2, , 10)) ||
+   ((timestamp = strtoul(email_end + 2, , 10)) == ULONG_MAX &&
+errno == ERANGE) ||
!message || message[0] != ' ' ||
(message[1] != '+' && message[1] != '-') ||
!isdigit(message[2]) || !isdigit(message[3]) ||
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v12 3/5] parse-options.c: make OPTION_COUNTUP respect "unspecified" values

2016-04-05 Thread Pranit Bauva
On Mon, Apr 4, 2016 at 4:40 AM, Eric Sunshine  wrote:
> On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva  wrote:
>> The reason to make it respect "unspecified" values is to give the
>> ability to differentiate whether `--option` or `--no-option` was
>> specified at all. "unspecified" values should be in the form of negative
>> values. If initial value is set to negative and `--option` specified
>> then it will reflect the number of occurrences (counting done from 0),
>> if `--no-option` is specified then it will reflect 0 and if nothing at
>> all is given then it will retain its negative value.
>
> Thanks, this rewrite does a better job of explaining the aim of the
> change and how a client can take advantage of it. However, with my
> "first-time reader" hat on, I still have some trouble digesting it as
> a coherent whole. I wonder if a rewrite like this would help?
>
> OPT_COUNTUP() merely increments the counter upon --option, and
> resets it to 0 upon --no-option, which means that there is no
> "unspecified" value with which a client can initialize the
> counter to determine whether or not --[no-]option was seen at
> all.
>
> Make OPT_COUNTUP() treat any negative number as an "unspecified"
> value to address this shortcoming. In particular, if a client
> initializes the counter to -1, then if it is still -1 after
> parse_options(), then neither --option nor --no-option was seen;
> if it is 0, then --no-option was seen last, and if it is 1 or
> greater, than --option was seen last.
>
>> This change will not affect existing users of COUNTUP, because they all
>> use the initial value of 0 (or more).
>
> "This change does not affect behavior of existing clients of..."
>

Sure I could do the change.

>> Note that builtin/clean.c initializes the variable used with
>> OPT__FORCE (which uses OPT_COUNTUP()) to a negative value, but it is set
>> to either 0 or 1 by reading the configuration before the code calls
>> parse_options(), i.e. as far as parse_options() is concerned, the
>> initial value of the variable is not negative.
>>
>> To test this behavior "verbose" is set to "unspecified" while quiet is
>> set to 0 which will test the new behavior with all sets of values.
>
> I think you need to mention here that you're talking about 
> test-parse-options.c
> (and indirectly t0040) since it's otherwise too easy for the reader to
> think that this paragraph is a continuation of the discussion about
> OPT_COUNTUP()'s new behavior and how it won't impact existing tests,
> rather than a new topic of its own (testing the behavior). Maybe say
> something like this:
>
> To test the new behavior, augment the initial "verbose" setting
> of test-parse-options.c to be unspecified...
>
> and then go on to say that, because "quiet" is still initialized to 0,
> you have complete coverage. An alternative would be to split off the
> new testing into its own patch, which would make this patch (which is
> the real meat of the change) less noisy.

I do like including test-parse-options.c in commit message. My main
motive behind including the test with this patch was to show the
"first-time" reader how to use the changes introduced in this patch.
This would also set a complete picture of this commit. And I kind of
believe it is much effort for the reader to find the commit whose
parent will be this (if it exists at all, as the reader won't know
about it) which will give a kind of demonstration to utilizing this
change.

>
> I actually expected you to add new functionality to
> test-parse-options.c specifically to test OPT_COUNTUP() directly
> rather than indirectly through --verbose and --quiet, however, I think
> I can be sold on the approach you took for a couple reasons. First,
> you have a genuine use-case for an "unspecified" --verbose value, so
> changing test-parse-options.c's --verbose to start at -1 tests what
> you ultimately care about. Second, since you retained 0-initialization
> of --quiet, that case of OPT_COUNTUP() is still being tested.
>
> What I find a bit disturbing about this approach is that you are
> getting "full coverage" only because of current *implementation*, not
> because you are explicitly testing for *expected* behavior. That is,
> you get that coverage only while both OPT__VERBOSE() and OPT__QUIET()
> are built atop OPT_COUNTUP(); if OPT__QUIET() is ever rewritten so it
> no longer uses OPT_COUNTUP(), then the tests silently lose full
> coverage. However, I may be over-worrying about the situation...

The main reason as I mentioned above was to give a demonstration of
how to utilize the change introduced.

>
>> Signed-off-by: Pranit Bauva 
>> ---
>> diff --git a/Documentation/technical/api-parse-options.txt 
>> b/Documentation/technical/api-parse-options.txt
>> @@ -144,8 +144,12 @@ There are some macros to easily define options:
>>  `OPT_COUNTUP(short, long, _var, 

Re: [PATCH v12 2/5] test-parse-options: print quiet as integer

2016-04-05 Thread Pranit Bauva
[+cc:Duy Nguyen, Jonathan Nieder]

On Mon, Apr 4, 2016 at 3:00 AM, Eric Sunshine  wrote:
> On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva  wrote:
>> Current implementation of parse-options.c treats OPT__QUIET() as integer
>> and not boolean and thus it is more appropriate to print it as integer
>> to avoid confusion.
>
> I can buy this line of reasoning, however, it would be even easier to
> sell the change if you cited an existing client (a git command) which
> actually respects multiple quiet levels. Are there any?

I investigated into this. But I was unable to find any git commit
which actually respects mulitple quiet levels. I first did a 'git grep
OPT__QUIET' to find the commands which use this. Then I went through
the documentation which covers it. None of them have any such mention
of multiple quiet levels. But still I dug further and and went through
all the individual source files. I followed the corresponding C source
code for the header file included and also searched there for any
trace of quiet. But I still didn't find any such use of multiple quiet
levels. I have found that the commit id 212c0a6f (Duy Ngyuyen; 7 Dec,
2013; parse-options: remove OPT__BOOLEAN). Maybe he has something to
say as to why this was introduced and OPT__QUIET which previously used
OPT__BOOLEAN, now uses OPT_COUNTUP rather than OPT_BOOL. This commit
In fact git-repack command has quiet but this is not mentioned in the
documentation. If someone could include this it in the documentation.
I would do it but I am not quite familiar with git-repack and haven't
really used it anytime.

> More importantly, though, this change implies that you should also add
> tests to ensure that the quiet level is indeed incremented with each
> --quiet, just as "-vv" and "--verbose --verbose" are already tested.
> You might be able to include such new tests directly in this patch as
> long as the commit message is clear about it, or add them in a
> separate patch.

I will include the tests for multiple level of quiet in the patch.

> By the way, I don't see any tests to ensure that --no-verbose and
> --no-quiet reset those respective values to 0. A separate patch which
> adds such tests would be nice (unless such tests already exist and I
> merely missed them).
There are no tests to ensure that --no-verbose and --no-quiet reset
those respective values to 0 just before this patch. But I have
covered the --no-verbose tests in the upcoming patch 3/5. I will
include the tests for --no-quiet in this patch.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Feature request: config option for default git commit -v

2016-04-05 Thread Pranit Bauva
On Tue, Apr 5, 2016 at 8:08 PM, Jacek Wielemborek  wrote:
> Hello,
>
> I'm asking for this one because there's quite a lot of interest
> (including me) in this feature and there is no convenient walkaround:
>
> https://stackoverflow.com/questions/5875275/git-commit-v-by-default
>
> Cheers,
> d33tah

This is currently under progress. I am the one who is working on it.
One of the patches is currently on the pu branch. I am still polishing
it to include some more stuff. You can track its status by reading the
git.git messages by the git maintainer. The latest revision of the
patch is at http://thread.gmane.org/gmane.comp.version-control.git/288820

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


Feature request: config option for default git commit -v

2016-04-05 Thread Jacek Wielemborek
Hello,

I'm asking for this one because there's quite a lot of interest
(including me) in this feature and there is no convenient walkaround:

https://stackoverflow.com/questions/5875275/git-commit-v-by-default

Cheers,
d33tah



signature.asc
Description: OpenPGP digital signature


Re: git diff --exit-code does not honour textconv setting

2016-04-05 Thread Michael J Gruber
Georg Pichler venit, vidit, dixit 20.03.2016 13:43:
> Hi,
> 
> I realized that "git diff --exit-code" does not honour textconv settings.
> Maybe this behaviour is desired. It can be partially circumvented by using 
> the "-b" flag if one does not care about whitespace changes.
> To reproduce this, create an empty repository and run the following commands:
> 
> (I was using git version 2.7.3)
> 
> $ git config --add diff.void.textconv test
> $ echo "foo diff=void" >.gitattributes
> $ echo foo >foo
> $ git add . && git commit -m "Init"
> [master (root-commit) 70c39d9] Init
> 2 files changed, 2 insertions(+)
> create mode 100644 .gitattributes
> create mode 100644 foo
> $ echo bar >foo
> $ git status
> On branch master
> Changes not staged for commit:
> (use "git add ..." to update what will be committed)
> (use "git checkout -- ..." to discard changes in working directory)
> 
> modified: foo
> 
> no changes added to commit (use "git add" and/or "git commit -a")
> $ git diff
> $ git diff --exit-code
> [exits with 1, no output]
> $ git diff --exit-code -b
> [exits with 0, no output]
> 
> The "test" command is used as it does not generate any output on stdout.

"test" is a bit of a red herring here since it will receive commands.
But your example works even with "true" which ignores its commands and
produces no output.

> I would expect "git diff --exit-code" to return with exit code 0. If this is 
> not desired, it should be clearly stated in the man page,
> that "--exit-code" does not honour the textconv setting, except if "-b" is 
> given. Currently this is not clear:
> 
>--exit-code
>Make the program exit with codes similar to diff(1). That is, it 
> exits
>with 1 if there were differences and 0 means no differences.
> 
> Best,
> Georg Pichler

The description doesn't make it clear whether exit-code refers to the
actual diff (foo vs. bar) or to the diff after textconv (empty vs.
empty). In any case, "-b" should not make a difference for your example.


diff_flush() in diff.c has this piece of code:

/*
 * Report the content-level differences with HAS_CHANGES;
 * diff_addremove/diff_change does not set the bit when
 * DIFF_FROM_CONTENTS is in effect (e.g. with -w).
 */
if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
if (options->found_changes)
DIFF_OPT_SET(options, HAS_CHANGES);
else
DIFF_OPT_CLR(options, HAS_CHANGES);
}

So it's clear that depending on "-b" (or "-w") or not, it's taking
shortcuts or looking at the textconved diff but I'm not sure where's the
proper place to fix that.

Michael
--
To unsubscribe from this list: send the line "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: [RFC/PATCH] git.spec: fix changelog dates

2016-04-05 Thread Remi Galan Alfonso
Michael J Gruber  wrote:
> Remi Galan Alfonso venit, vidit, dixit 05.04.2016 14:28:
> > Michael J Gruber  wrote:
> >> A few changelog entries have inconsistent dates, which rpmlint reports
> >> as errors.
> >>
> >> Fix them based on these assumptions:
> >> - It's easier to mistype a number than a weekday abbreviation.
> >> - changelog date must be before git commit date
> >> - The mistyped date is just a few days off.
> >>
> >> Signed-off-by: Michael J Gruber 
> >> ---
> >> I dunno if this is worthwhile, but rpmlint is the first thing we tell
> >> packagers and reviewers to check.
> >>
> >>  git.spec.in | 8 
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/git.spec.in b/git.spec.in
> >> index bfd1cfb..eb581a3 100644
> >> --- a/git.spec.in
> >> +++ b/git.spec.in
> >> @@ -229,7 +229,7 @@ rm -rf $RPM_BUILD_ROOT
> >>  * Sat Jan 30 2010 Junio C Hamano 
> >>  - We don't ship Python bits until a real foreign scm interface comes.
> >>  
> >> -* Mon Feb 04 2009 David J. Mellor 
> >> +* Mon Feb 02 2009 David J. Mellor 
> >>  - fixed broken git help -w after renaming the git-core package to git.
> >>  
> >>  * Fri Sep 12 2008 Quy Tonthat 
> >> @@ -262,7 +262,7 @@ rm -rf $RPM_BUILD_ROOT
> >>  * Thu Jun 21 2007 Shawn O. Pearce 
> >>  - Added documentation files for git-gui
> >>  
> >> -* Tue May 13 2007 Quy Tonthat 
> >> +* Sun May 13 2007 Quy Tonthat 
> >
> > It is inconsistent with what you said in the commit message ("It's
> > easier to mistype a number than a weekday abbreviation.").
> >
> > Following that logic, it should be:
> > * Tue May 15 2007 Quy Tonthat 
> >
> > (or 08, I didn't check the condition "changelog date must be before
> > git commit date")
> 
> I did.
> 
> The thing is that you can't always fulfill all of these 3 conditions.
> "Easier" doesn't mean that the other case is impossible, just less likely.

Ah alright. Makes sense then, it is much harder to mistype 13 from 08
than from 15.

Sorry for the noise.

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


Re: [RFC/PATCH] git.spec: fix changelog dates

2016-04-05 Thread Michael J Gruber
Remi Galan Alfonso venit, vidit, dixit 05.04.2016 14:28:
> Michael J Gruber  wrote:
>> A few changelog entries have inconsistent dates, which rpmlint reports
>> as errors.
>>
>> Fix them based on these assumptions:
>> - It's easier to mistype a number than a weekday abbreviation.
>> - changelog date must be before git commit date
>> - The mistyped date is just a few days off.
>>
>> Signed-off-by: Michael J Gruber 
>> ---
>> I dunno if this is worthwhile, but rpmlint is the first thing we tell
>> packagers and reviewers to check.
>>
>>  git.spec.in | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/git.spec.in b/git.spec.in
>> index bfd1cfb..eb581a3 100644
>> --- a/git.spec.in
>> +++ b/git.spec.in
>> @@ -229,7 +229,7 @@ rm -rf $RPM_BUILD_ROOT
>>  * Sat Jan 30 2010 Junio C Hamano 
>>  - We don't ship Python bits until a real foreign scm interface comes.
>>  
>> -* Mon Feb 04 2009 David J. Mellor 
>> +* Mon Feb 02 2009 David J. Mellor 
>>  - fixed broken git help -w after renaming the git-core package to git.
>>  
>>  * Fri Sep 12 2008 Quy Tonthat 
>> @@ -262,7 +262,7 @@ rm -rf $RPM_BUILD_ROOT
>>  * Thu Jun 21 2007 Shawn O. Pearce 
>>  - Added documentation files for git-gui
>>  
>> -* Tue May 13 2007 Quy Tonthat 
>> +* Sun May 13 2007 Quy Tonthat 
> 
> It is inconsistent with what you said in the commit message ("It's
> easier to mistype a number than a weekday abbreviation.").
> 
> Following that logic, it should be:
>   * Tue May 15 2007 Quy Tonthat 
> 
> (or 08, I didn't check the condition "changelog date must be before
> git commit date")

I did.

The thing is that you can't always fulfill all of these 3 conditions.
"Easier" doesn't mean that the other case is impossible, just less likely.

> 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


Re: support block comments in gitconfig

2016-04-05 Thread stefan.naewe
Am 05.04.2016 um 04:19 schrieb 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

Maybe not what you're looking for, but couldn't you use
the include functionality of gitconfig:

[include]
  path = ~/.another.gitconfig

??

That way you can comment out a lot of configuration with one '#'.

Just my €0.02

Stefan
-- 

/dev/random says: Air conditioned environment - Do not open Windows.
python -c "print 
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
 
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF


Re: [RFC/PATCH] git.spec: fix changelog dates

2016-04-05 Thread Remi Galan Alfonso
Michael J Gruber  wrote:
> A few changelog entries have inconsistent dates, which rpmlint reports
> as errors.
> 
> Fix them based on these assumptions:
> - It's easier to mistype a number than a weekday abbreviation.
> - changelog date must be before git commit date
> - The mistyped date is just a few days off.
> 
> Signed-off-by: Michael J Gruber 
> ---
> I dunno if this is worthwhile, but rpmlint is the first thing we tell
> packagers and reviewers to check.
> 
>  git.spec.in | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/git.spec.in b/git.spec.in
> index bfd1cfb..eb581a3 100644
> --- a/git.spec.in
> +++ b/git.spec.in
> @@ -229,7 +229,7 @@ rm -rf $RPM_BUILD_ROOT
>  * Sat Jan 30 2010 Junio C Hamano 
>  - We don't ship Python bits until a real foreign scm interface comes.
>  
> -* Mon Feb 04 2009 David J. Mellor 
> +* Mon Feb 02 2009 David J. Mellor 
>  - fixed broken git help -w after renaming the git-core package to git.
>  
>  * Fri Sep 12 2008 Quy Tonthat 
> @@ -262,7 +262,7 @@ rm -rf $RPM_BUILD_ROOT
>  * Thu Jun 21 2007 Shawn O. Pearce 
>  - Added documentation files for git-gui
>  
> -* Tue May 13 2007 Quy Tonthat 
> +* Sun May 13 2007 Quy Tonthat 

It is inconsistent with what you said in the commit message ("It's
easier to mistype a number than a weekday abbreviation.").

Following that logic, it should be:
* Tue May 15 2007 Quy Tonthat 

(or 08, I didn't check the condition "changelog date must be before
git commit date")

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


[RFC/PATCH] git.spec: fix changelog dates

2016-04-05 Thread Michael J Gruber
A few changelog entries have inconsistent dates, which rpmlint reports
as errors.

Fix them based on these assumptions:
- It's easier to mistype a number than a weekday abbreviation.
- changelog date must be before git commit date
- The mistyped date is just a few days off.

Signed-off-by: Michael J Gruber 
---
I dunno if this is worthwhile, but rpmlint is the first thing we tell
packagers and reviewers to check.

 git.spec.in | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/git.spec.in b/git.spec.in
index bfd1cfb..eb581a3 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -229,7 +229,7 @@ rm -rf $RPM_BUILD_ROOT
 * Sat Jan 30 2010 Junio C Hamano 
 - We don't ship Python bits until a real foreign scm interface comes.
 
-* Mon Feb 04 2009 David J. Mellor 
+* Mon Feb 02 2009 David J. Mellor 
 - fixed broken git help -w after renaming the git-core package to git.
 
 * Fri Sep 12 2008 Quy Tonthat 
@@ -262,7 +262,7 @@ rm -rf $RPM_BUILD_ROOT
 * Thu Jun 21 2007 Shawn O. Pearce 
 - Added documentation files for git-gui
 
-* Tue May 13 2007 Quy Tonthat 
+* Sun May 13 2007 Quy Tonthat 
 - Added lib files for git-gui
 - Added Documentation/technical (As needed by Git Users Manual)
 
@@ -272,7 +272,7 @@ rm -rf $RPM_BUILD_ROOT
 * Tue Mar 27 2007 Eygene Ryabinkin 
 - Added the git-p4 package: Perforce import stuff.
 
-* Mon Feb 13 2007 Nicolas Pitre 
+* Mon Feb 12 2007 Nicolas Pitre 
 - Update core package description (Git isn't as stupid as it used to be)
 
 * Mon Feb 12 2007 Junio C Hamano 
@@ -326,5 +326,5 @@ rm -rf $RPM_BUILD_ROOT
 * Thu Jul 14 2005 Eric Biederman 
 - Add the man pages, and the --without docs build option
 
-* Wed Jul 7 2005 Chris Wright 
+* Wed Jul 6 2005 Chris Wright 
 - initial git spec file
-- 
2.8.1.120.g4a400c2

--
To unsubscribe from this list: send the line "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: support block comments in gitconfig

2016-04-05 Thread Johannes Schindelin
Hi Timothee,

On Mon, 4 Apr 2016, Timothee Cour wrote:

> 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

You could have such block comments. If you implemented that feature.

Having said that, this syntax is distinctly *not* the INI syntax we tried
to imitate. Plus, there are Git implementations *other* than core Git in
the meantime, and they would need to be taught about this
backwards-incompatible syntax, too.

And then we still could not turn on that feature by default because there
are setups out there where different users use different versions of Git.

If you are prepared to push this feature forward, you can make it happen.
In that case, you will need to spend a bit of effort and definitely
exercise patience because you would likely get this feature accepted only
into 2.9.0 (or 2.10.0 or 3.0.0...), i.e. a major version bump.

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


Timestamp of zero in reflog considered invalid

2016-04-05 Thread Erik Bray
Hi all,

I found this issue through a test suite for a Python git interface,
which during the tests at some point sets

GIT_COMMITTER_DATE=1970-01-01T00:00:00

To reproduce the issue:

$ git init
$ echo foo > testfile
$ git add testfile
$ git commit -m "test"
$ echo bar >> testfile
$ export GIT_COMMITTER_DATE=1970-01-01T00:00:00
$ git stash save
$ git stash apply
refs/stash@{0} is not a valid reference

At this point one can see:

$ git rev-parse --symbolic --verify 'refs/stash@{0}'
fatal: Log for refs/stash is empty.

Expected:

$ git rev-parse --symbolic --verify 'refs/stash@{0}'
refs/stash@{0}

I tracked the issue to refs/files-backend.c in show_one_reflog_ent :

https://github.com/git/git/blob/11529ecec914d2f0d7575e6d443c2d5a6ff75424/refs/files-backend.c#L2923

in which

!(timestamp = strtoul(email_end + 2, , 10)) ||

implies an invalid reflog entry.  Why should 0 be treated as an
invalid timestamp (even if it's unlikely outside of corner cases)?

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


Triangular workflows and some anecdotes from the trenches

2016-04-05 Thread Chris Packham
Hi,

We ran into something at $dayjob the other day. The actual problem was
a developer ended up amending a commit that had already been pushed.
It happens occasionally and is usually recoverable with a simple
rebase and is generally a learning experience. In this particular case
however things were a bit more complicated.

We had (attempted to) setup a triangular workflow. The developers
would fetch from a server that was closer to them but push to the
central server that was at the other end of a WAN link. Our build
system would update the local server after a successful build for all
configurations. The problem was instead of setting
branch..pushdefault we were setting remote.origin.pushurl. So
now the warning in git-remote(1) comes back to bite us and a lot of
head scratching ensues.

It appears that the triangular workflow support is under-documented
(mentioned in a couple of release notes and gitrevisions). I'm not
suggesting we would have done the right thing if the documentation
existed but we would have had a chance. Once we get our setup sorted
I'll try and send an update for gitworkflows.txt (unless someone else
beats me to it). There are a few blog post around the Internet that I
might be able to draw upon.

The subject of preventing modifying published history has come up on
this list before. And in-fact it's relatively simple to implement as
an alias

  git config alias.amend '!git merge-base --is-ancestor HEAD
@{upstream} || git commit --amend'

I'm just wondering if something more official can be added to git
commit --amend (and probably git rebase).

Finally I was wondering if there is any way of detecting if
remote.*.pushurl and remote.*.url point to the same repo, although I'm
not sure when you'd do such verification.

Thanks,
Chris
--
To unsubscribe from this list: send the line "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] completion: complete --cherry-mark for git log

2016-04-05 Thread Michael J Gruber
Signed-off-by: Michael J Gruber 
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e3918c8..d87cf4d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1458,6 +1458,7 @@ _git_log ()
--relative-date --date=
--pretty= --format= --oneline
--show-signature
+   --cherry-mark
--cherry-pick
--graph
--decorate --decorate=
-- 
2.8.1.120.g4a400c2

--
To unsubscribe from this list: send the line "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: [ANNOUNCE] Git v2.8.1 (and call-for-help to "make rpmbuild" users)

2016-04-05 Thread Elia Pinto
2016-04-03 21:21 GMT+02:00 Junio C Hamano :
> If you do not build RPM binary packages from our pristine source,
> you can safely ignore this release and stop reading this message.
>
> Now that the audience of this message has been limited to a narrow
> target, before I make an announcement, here is a call-for-help to
> you.
>
> Git v2.8 removed README file and added a corresponding README.md
> file.  The change however did not adjust git.spec.in that still
> referred to README, causing "make rpmbuild" to fail.  The breakage
> was not noticed by anybody who tested v2.8.0-rc0 and later release
> candidates, and ended up in the final v2.8 release, and we saw a
> handful of bug reports on the list after the release happened.
>
> This maintenance release is to correct this bug for those who run
> "make rpmbuild". It has no other changes.
>
> It is clear that nobody who relies on being able to "make rpmbuild"
> ever tried any of the 5 release candidate snapshots that happened
> during Feb 26-Mar 28.  We had a whole month and nobody noticed?
>
> This incident clearly shows that something needs to happen, if
> people want "make rpmbuild" to keep working.  Even though this
> maintenance release may fix this single bug, breakages similar to it
> that only affect "make rpmbuild" users are guaranteed to appear in
> future releases, unless those who can prevent them from happening
> start helping to test at least release candidate snapshots.
>
> It is even more preferrable if they can test the tip of 'next'
> branch regularly, in order to prevent such breakages from hitting
> the 'master' branch to be included in the next release, which is
> what the other parts of the system aims at.
>
> The other obvious option is for us to stop pretending that "make
> rpmbuild" does anything useful to do and drop the build target and
> the unmaintained git.spec.in file on which nobody in the active
> development community keeps eyes.  I do not mean this as a threat
> "help us or else"; there is a precedent.  We used to ship our own
> debian/rules and friends for those who wanted to debbuild from the
> source, but the Debian packagers wanted to have their own proper
> ones and ours ended up confusing the users, and we made the world
> a better place by removing our copy.  If "make rpmbuild" people want
> us to take this route, that is also OK for us.
>
> So that's the call for help.  Now to the announcement.

How old contributor to rpm5.org (as devzero2000) I have been following
these issues in the past.
Unfortunately, to my knowledge, no one has ever come to a convergence
of views between the distros
that use rpm on the various differences.
Keeping for a project a spec file that then no distro uses (and i
think also a local sysadmin)
i do not think it is worth the effort.

http://lists.rpm.org/pipermail/rpm-maint/2008-June/002185.html

IMHO

Best

>
> The latest maintenance release Git v2.8.1 is now available at
> the usual places.
>
> The tarballs are found at:
>
> https://www.kernel.org/pub/software/scm/git/
>
> The following public repositories all have a copy of the 'v2.8.1'
> tag and the 'maint' branch that the tag points at:
>
>   url = https://kernel.googlesource.com/pub/scm/git/git
>   url = git://repo.or.cz/alt-git.git
>   url = git://git.sourceforge.jp/gitroot/git-core/git.git
>   url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
>   url = https://github.com/gitster/git
>
> 
>
> Git v2.8.1 Release Notes
> 
>
> Fixes since v2.8
> 
>
>  * "make rpmbuild" target was broken as its input, git.spec.in, was
>not updated to match a file it describes that has been renamed
>recently.  This has been fixed.
>
> 
>
> Changes since v2.8.0 are as follows:
>
> Junio C Hamano (1):
>   Git 2.8.1
>
> Matthieu Moy (1):
>   git.spec.in: use README.md, not README
>
> --
> To unsubscribe from this list: send the line "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 1/2] imap-send.c: implements the GIT_CURL_DEBUG environment variable

2016-04-05 Thread Elia Pinto
2016-04-01 22:25 GMT+02:00 Eric Sunshine :
> On Fri, Apr 1, 2016 at 6:44 AM, 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.
>
> In addition to review comments by others, why are the new curl_dump()
> and curl_trace() functions duplicated in both patches rather than
> factored out to a shared implementation?
It's right. Do you think i can use some existing file or should I
create a new object file ?

Thank you very much
>
>> Signed-off-by: Elia Pinto 
>> ---
>> diff --git a/imap-send.c b/imap-send.c
>> @@ -1395,6 +1395,96 @@ static int append_msgs_to_imap(struct 
>> imap_server_conf *server,
>>  }
>>
>>  #ifdef USE_CURL_FOR_IMAP_SEND
>> +
>> +static
>> +void curl_dump(const char *text,
>> + FILE * stream, unsigned char *ptr, size_t size, char nohex)
>> +{
>> +   size_t i;
>> +   size_t c;
>> +
>> +   unsigned int width = 0x10;
>> +
>> +   if (nohex)
>> +   /* without the hex output, we can fit more on screen */
>> +   width = 0x40;
>> +
>> +   fprintf(stream, "%s, %10.10ld bytes (0x%8.8lx)\n",
>> +   text, (long)size, (long)size);
>> +
>> +   for (i = 0; i < size; i += width) {
>> +
>> +   fprintf(stream, "%4.4lx: ", (long)i);
>> +
>> +   if (!nohex) {
>> +   /* hex not disabled, show it */
>> +   for (c = 0; c < width; c++)
>> +   if (i + c < size)
>> +   fprintf(stream, "%02x ", ptr[i + c]);
>> +   else
>> +   fputs("   ", stream);
>> +   }
>> +
>> +   for (c = 0; (c < width) && (i + c < size); c++) {
>> +   /* check for 0D0A; if found, skip past and start a 
>> new line of output */
>> +   if (nohex && (i + c + 1 < size) && ptr[i + c] == 0x0D
>> +   && ptr[i + c + 1] == 0x0A) {
>> +   i += (c + 2 - width);
>> +   break;
>> +   }
>> +   fprintf(stream, "%c",
>> +   (ptr[i + c] >= 0x20)
>> +   && (ptr[i + c] < 0x80) ? ptr[i + c] : '.');
>> +   /* check again for 0D0A, to avoid an extra \n if 
>> it's at width */
>> +   if (nohex && (i + c + 2 < size)
>> +   && ptr[i + c + 1] == 0x0D
>> +   && ptr[i + c + 2] == 0x0A) {
>> +   i += (c + 3 - width);
>> +   break;
>> +   }
>> +   }
>> +   fputc('\n', stream);/* newline */
>> +   }
>> +   fflush(stream);
>> +}
>> +
>> +static
>> +int curl_trace(CURL * handle, curl_infotype type,
>> +char *data, size_t size, void *userp)
>> +{
>> +   const char *text;
>> +   (void)handle;   /* prevent compiler warning */
>> +
>> +   switch (type) {
>> +   case CURLINFO_TEXT:
>> +   fprintf(stderr, "== Info: %s", data);
>> +   default:/* in case a new one is introduced to shock 
>> us */
>> +   return 0;
>> +
>> +   case CURLINFO_HEADER_OUT:
>> +   text = "=> Send header";
>> +   break;
>> +   case CURLINFO_DATA_OUT:
>> +   text = "=> Send data";
>> +   break;
>> +   case CURLINFO_SSL_DATA_OUT:
>> +   text = "=> Send SSL data";
>> +   break;
>> +   case CURLINFO_HEADER_IN:
>> +   text = "<= Recv header";
>> +   break;
>> +   case CURLINFO_DATA_IN:
>> +   text = "<= Recv data";
>> +   break;
>> +   case CURLINFO_SSL_DATA_IN:
>> +   text = "<= Recv SSL data";
>> +   break;
>> +   }
>> +
>> +   curl_dump(text, stderr, (unsigned char *)data, size, 1);
>> +   return 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] api-trace.txt: fix typo

2016-04-05 Thread Elia Pinto
The correct api is trace_printf_key

Signed-off-by: Elia Pinto 
---
 Documentation/technical/api-trace.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/technical/api-trace.txt 
b/Documentation/technical/api-trace.txt
index 389ae16..45a0ecd 100644
--- a/Documentation/technical/api-trace.txt
+++ b/Documentation/technical/api-trace.txt
@@ -28,7 +28,7 @@ static struct trace_key trace_foo = TRACE_KEY_INIT(FOO);
 
 static void trace_print_foo(const char *message)
 {
-   trace_print_key(_foo, message);
+   trace_printf_key(_foo, message);
 }
 
 +
-- 
2.8.0.270.g9d4de1f.dirty

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


Re: [ANNOUNCE] Git v2.8.1 (and call-for-help to "make rpmbuild" users)

2016-04-05 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 03.04.2016 21:21:
> If you do not build RPM binary packages from our pristine source,
> you can safely ignore this release and stop reading this message.
> 
> Now that the audience of this message has been limited to a narrow
> target, before I make an announcement, here is a call-for-help to
> you.
> 
> Git v2.8 removed README file and added a corresponding README.md
> file.  The change however did not adjust git.spec.in that still
> referred to README, causing "make rpmbuild" to fail.  The breakage
> was not noticed by anybody who tested v2.8.0-rc0 and later release
> candidates, and ended up in the final v2.8 release, and we saw a
> handful of bug reports on the list after the release happened.
> 
> This maintenance release is to correct this bug for those who run
> "make rpmbuild". It has no other changes.
> 
> It is clear that nobody who relies on being able to "make rpmbuild"
> ever tried any of the 5 release candidate snapshots that happened
> during Feb 26-Mar 28.  We had a whole month and nobody noticed?
> 
> This incident clearly shows that something needs to happen, if
> people want "make rpmbuild" to keep working.  Even though this
> maintenance release may fix this single bug, breakages similar to it
> that only affect "make rpmbuild" users are guaranteed to appear in
> future releases, unless those who can prevent them from happening
> start helping to test at least release candidate snapshots.

In RedHat/Fedora land, we use our own spec files, "make" and "make
install" into a special purpose tree and package that.

E.g. in fedora, the packager noticed the README file name change in
2.8.0, adjusted the corresponding "%doc" line, and that was basically
all besides updating the sources.

As a fedora user, I either use rpms targetting fedora (possibly patched
and rebuilt by myself), or I build from sources (--prefix=$HOME). With
"generic" rpms you never know which guidelines they follow or where they
install to. Also, naming schemes for dependencies may differ between
RedHat land, SuSe land and other rpm based distros.

So, just like in the deb case, I'm wondering if there's a use case for
make rpmbuild in git.git.

Michael
--
To unsubscribe from this list: send the line "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

2016-04-05 Thread Perry Hutchison
Jeff King  wrote:
> > +test_expand --pretty=fuller
> > +test_expand --pretty=fuller
>
> Duplicated fuller?

Just brush it off :)





[Those too young to get the joke can
 look up "fuller brush" in Wikipedia.]
--
To unsubscribe from this list: send the line "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 3/4] format-patch: introduce --base=auto option

2016-04-05 Thread Junio C Hamano
Ye Xiaolong  writes:

>   ---1---o---A
>  /\ /
>  ---O  X
>  \/ \
>   ---2---o---o---B
>
> For this criss-cross merges, as neither merge base(like 1) is better
> than the other(both 1 and 2 are 'best' merge bases), I think it should
> be fine to pick a random one as base commit(Or you prefer to show all of
> them?) and I'll add this part of discusstion into documentation.

I think you should error out; it would give you blatantly wrong to
pick either one at random.

Suppose A is where your remote-tracking branch is, and B is where
the user started working on her serie.  We are sending patches built
on top of B (not depicted) with "format-patch --base=A B..".

If you picked '1' as the base, you'll include commits on the ===
stretch as prerequisite patches (think of any non-merge --o-- in the
picture to consist of multiple commits), but you won't be showing
what the merge 'M' between '1' and '2' did to the tree of '1' to
arrive at the resulting tree of 'M'.

   ---1---o---A
  /\ /
  ---O  X
  \/ \
   ---2---M===o===B...your.patches.here...

If you picked '2' as the base, you'll include commits on the ===
stretch as prerequisite patches, but again you won't be showing what
the merge 'M' between '1' and '2' did to the tree of '2' to arrive
at the resulting tree of 'M'..

   ---1---o---A
  /\ /
  ---O  X
  \/ \
   ---2===M===o===B...your.patches.here...

So either case, you cannot rebuild the tree of B by going from the
base and piling on patches in such a case, because you won't have
"patch" for merge 'M'.
--
To unsubscribe from this list: send the line "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] Makefile: fix misdirected redirections

2016-04-05 Thread Junio C Hamano
In general "echo 2>&1 $msg" to redirect a possible error message
that comes from 'echo' itself into the same standard output stream
$msg is getting written to does not make any sense; it is not like
we are expecting to see any errors out of 'echo' in these statements,
and even if it were the case, there is no reason to prevent the
error messages from being sent to the standard error stream.

These are clearly meant to send the argument given to echo to the
standard error stream as error messages.  Correctly redirect by
saying "send what is written to the standard output to the standard
error", i.e. "1>&2" aka ">&2".

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

 * There are two more uses of 2>&1 in the Makefile, and they are
   correct.

   - The one in dep_check one is used as

   dep_check = $(shell ... 2>&1; echo $$?)

 and it is an attempt to make sure that anything sent to the
 standard error is not leaked out of the shell substitution.  It
 does want to send whatever is written to the standard error
 instead written to the standard output.

   - The other is "cmp ... >/dev/null 2>&1", which is a typical way
 to squelch both the standard output and the standard error for
 any command.

 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 8700db8..296f175 100644
--- a/Makefile
+++ b/Makefile
@@ -2211,10 +2211,10 @@ sparse: $(SP_OBJ)
 check: common-cmds.h
@if sparse; \
then \
-   echo 2>&1 "Use 'make sparse' instead"; \
+   echo >&2 "Use 'make sparse' instead"; \
$(MAKE) --no-print-directory sparse; \
else \
-   echo 2>&1 "Did you mean 'make test'?"; \
+   echo >&2 "Did you mean 'make test'?"; \
exit 1; \
fi
 
--
To unsubscribe from this list: send the line "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] Makefile: stop pretending to support rpmbuild

2016-04-05 Thread Junio C Hamano
Dennis Kaarsemaker  writes:

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

OK, while we wait for other people to raise their opinions, here is
to prepare for the removal, if we decide to follow through.

-- >8 --
Nobody in the active development community seems to watch breakages
in the rpmbuild target.  As most major RPM based distros use their
own specfile when packaging us, they aren't looking after us as
their pristine upstream tree, either.  At this point, it is turning
to be a disservice to the users to pretend that our tree natively
supports "make rpmbuild" target when we do not properly maintain it.

Signed-off-by: Junio C Hamano 
---
 Makefile|  17 +---
 git.spec.in | 330 
 2 files changed, 5 insertions(+), 342 deletions(-)

diff --git a/Makefile b/Makefile
index 2742a69..23182bc 100644
--- a/Makefile
+++ b/Makefile
@@ -443,7 +443,6 @@ DIFF = diff
 TAR = tar
 FIND = find
 INSTALL = install
-RPMBUILD = rpmbuild
 TCL_PATH = tclsh
 TCLTK_PATH = wish
 XGETTEXT = xgettext
@@ -2396,31 +2395,25 @@ quick-install-html:
 
 ### Maintainer's dist rules
 
-git.spec: git.spec.in GIT-VERSION-FILE
-   sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+
-   mv $@+ $@
-
 GIT_TARNAME = git-$(GIT_VERSION)
 dist: git.spec git-archive$(X) configure
./git-archive --format=tar \
--prefix=$(GIT_TARNAME)/ HEAD^{tree} > $(GIT_TARNAME).tar
@mkdir -p $(GIT_TARNAME)
-   @cp git.spec configure $(GIT_TARNAME)
+   @cp configure $(GIT_TARNAME)
@echo $(GIT_VERSION) > $(GIT_TARNAME)/version
@$(MAKE) -C git-gui TARDIR=../$(GIT_TARNAME)/git-gui dist-version
$(TAR) rf $(GIT_TARNAME).tar \
-   $(GIT_TARNAME)/git.spec \
$(GIT_TARNAME)/configure \
$(GIT_TARNAME)/version \
$(GIT_TARNAME)/git-gui/version
@$(RM) -r $(GIT_TARNAME)
gzip -f -9 $(GIT_TARNAME).tar
 
-rpm: dist
-   $(RPMBUILD) \
-   --define "_source_filedigest_algorithm md5" \
-   --define "_binary_filedigest_algorithm md5" \
-   -ta $(GIT_TARNAME).tar.gz
+rpm::
+   @echo >&2 "Use distro packaged sources to run rpmbuild"
+   @false
+.PHONY: rpm
 
 htmldocs = git-htmldocs-$(GIT_VERSION)
 manpages = git-manpages-$(GIT_VERSION)
diff --git a/git.spec.in b/git.spec.in
deleted file mode 100644
index d61d537..000
--- a/git.spec.in
+++ /dev/null
@@ -1,330 +0,0 @@
-# Pass --without docs to rpmbuild if you don't want the documentation
-
-Name:  git
-Version:   @@VERSION@@
-Release:   1%{?dist}
-Summary:   Core git tools
-License:   GPL
-Group: Development/Tools
-URL:   http://kernel.org/pub/software/scm/git/
-Source:http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz
-BuildRequires: zlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, 
gettext  %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
-BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
-
-Requires:  perl-Git = %{version}-%{release}
-Requires:  zlib >= 1.2, rsync, less, openssh-clients, expat
-Provides:  git-core = %{version}-%{release}
-Obsoletes: git-core <= 1.5.4.2
-Obsoletes: git-p4
-
-%description
-Git is a fast, scalable, distributed revision control system with an
-unusually rich command set that provides both high-level operations
-and full access to internals.
-
-The git rpm installs the core tools with minimal dependencies.  To
-install all git packages, including tools for integrating with other
-SCMs, install the git-all meta-package.
-
-%package all
-Summary:   Meta-package to pull in all git tools
-Group: Development/Tools
-Requires:  git = %{version}-%{release}
-Requires:  git-svn = %{version}-%{release}
-Requires:  

Re: [PATCH v3 3/4] format-patch: introduce --base=auto option

2016-04-05 Thread Ye Xiaolong
On Fri, Apr 01, 2016 at 09:06:20AM -0700, Junio C Hamano wrote:
>Ye Xiaolong  writes:
>
>> On Thu, Mar 31, 2016 at 10:43:48AM -0700, Junio C Hamano wrote:
>>>Xiaolong Ye  writes:
>>>
 Introduce --base=auto to record the base commit info automatically, the 
 base_commit
 will be the merge base of tip commit of the upstream branch and 
 revision-range
 specified in cmdline.
>>>
>>>This line is probably a bit too long.
>>
>> How about simplifying it to "the base_commit is the merge base of upstream 
>> and
>> specified revision-range."?
>
>What I meant was not that profound.  I just wanted you to wrap your
>lines a bit shorter so that quoting in the discussion thread like
>this would not make the result overlong to fit on a 80-column
>terminal ;-)

Emm, get your point now, I'll shorten the lines to fit in 80-column.

>
 +  base = base_list->item;
 +  free_commit_list(base_list);
>>>
>>>What should happen when there are multiple merge bases?  The code
>>>picks one at random and ignores the remainder, if I am reading this
>>>correctly.
>>
>> If there is more than one merge base, commits in base_list should
>> be sorted by date, if I am understanding it correctly, so
>> base_list->item should be the lastest merge base commit, it should
>> be enough for us to used as base commit.
>
>By definition, when there are multiple merge bases, there is no
>latest one among them.
>
>When the history involves criss-cross merges, there can be more than
>one 'best' common ancestor for two commits.  For example, with this
>topology (note that X is not a commit; it merely denotes crossing of
>two lines):
>
>   ---1---o---A
>  /\ /
>  ---O  X
>  \/ \
>   ---2---o---o---B
>
>both '1' and '2' are merge-bases of 'A' and 'B'.  And the timestamps
>on one (be it committer or author timestamp) being later than those
>of the other do not make it any more suitable than the other one.
>

For this criss-cross merges, as neither merge base(like 1) is better
than the other(both 1 and 2 are 'best' merge bases), I think it should
be fine to pick a random one as base commit(Or you prefer to show all of
them?) and I'll add this part of discusstion into documentation.

Thanks,
Xiaolong.
--
To unsubscribe from this list: send the line "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

2016-04-05 Thread Junio C Hamano
Jeff King  writes:

> 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 --pretty=fuller
>> +test_expand --pretty=fuller
> ...
> Duplicated fuller?

Thanks.  Here is a replacement.

-- >8 --
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 | 105 +++
 1 file changed, 105 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..e01a8f6
--- /dev/null
+++ b/t/t4213-log-tabexpand.sh
@@ -0,0 +1,105 @@
+#!/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'
+
+# usage: count_expand $indent $numSP $numHT @format_args
+count_expand ()
+{
+   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
+
+   # The remainder of the command line is "git show -s" options
+   case " $* " in
+   *' --pretty=short '*)
+   line=$title ;;
+   *)
+   line=$body ;;
+   esac
+
+   # Prefix the output with the command line arguments, and
+   # replace SP with a dot both in the expecte and actual output
+   # so that test_cmp would show the differene together with the
+   # breakage in a way easier to consume by the debugging user.
+   {
+   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=raw
+test_expand --pretty=email
+
+test_done
-- 
2.8.1-253-gd0f4798


--
To unsubscribe from this list: send the line "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

2016-04-05 Thread Junio C Hamano
Jeff King  writes:

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

I think the reason why I started doing the same is because some
shells can be configured to lose the comment-introducer-ness of "#"
in interactive mode, and I wanted to make sure that many things I
write can be tried out by others more easily by copy-and-paste to
their interactive session.

--
To unsubscribe from this list: send the line "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] branch: fix shortening of non-remote symrefs

2016-04-05 Thread Karthik Nayak
Hello,

On Sun, Apr 3, 2016 at 9:44 AM, Jeff King  wrote:
> On Sun, Apr 03, 2016 at 02:54:22PM +1200, Phil Sainty wrote:
>
>> Given the following symbolic reference:
>>
>> $ git symbolic-ref refs/heads/m refs/heads/master
>>
>>
>> Correct in 2.6.6:
>>
>> $ PATH=~/git/git-2.6.6:$PATH git branch
>>   m -> master
>> * master
>>
>>
>> Wrong in 2.7.0:
>>
>> $ PATH=~/git/git-2.7.0:$PATH git branch
>>   m -> m
>> * master

Thanks for reporting this.

>
> Thanks for an easy test case. Though we don't officially support
> arbitrary symrefs in the ref namespace, they do mostly work. And
> certainly the current output is nonsense, and it worked before. This
> bisects to aedcb7d (branch.c: use 'ref-filter' APIs, 2015-09-23).
>
> The fix is below. Karthik, I didn't look at all how this interacts with
> your work to convert branch to ref-filter for printing. I imagine it
> drops this code completely, but we should make sure that ref-filter gets
> this case right. I almost didn't prepare this patch at all, but I
> suspect we may want it for "maint", while the full conversion would wait
> for "master".
>

It's dropped in my latest series. I should be able to replicate what you've done
here onto ref-filter.c. Since I'm re-rolling my patches, I'll add this
one along too.

> -- >8 --
> Subject: branch: fix shortening of non-remote symrefs
>
> Commit aedcb7d (branch.c: use 'ref-filter' APIs, 2015-09-23)
> adjusted the symref-printing code to look like this:
>
> if (item->symref) {
> skip_prefix(item->symref, "refs/remotes/", );
> strbuf_addf(, " -> %s", desc);
> }
>
> This has three bugs in it:
>
>   1. It always skips past "refs/remotes/", instead of
>  skipping past the prefix associated with the branch we
>  are showing (so commonly we see "refs/remotes/" for the
>  refs/remotes/origin/HEAD symref, but the previous code
>  would skip "refs/heads/" when showing a symref it found
>  in refs/heads/.
>
>   2. If skip_prefix() does not match, it leaves "desc"
>  untouched, and we show whatever happened to be in it
>  (which is the refname from a call to skip_prefix()
>  earlier in the function).
>
>   3. If we do match with skip_prefix(), we stomp on the
>  "desc" variable, which is later passed to
>  add_verbose_info(). We probably want to retain the
>  original refname there (though it likely doesn't matter
>  in practice, since after all, one points to the other).
>
> The fix to match the original code is fairly easy: record
> the prefix to strip based on item->kind, and use it here.
> However, since we already have a local variable named "prefix",
> let's give the two prefixes verbose names so we don't
> confuse them.
>
> Signed-off-by: Jeff King 
> ---
> The test makes sure we restored the v2.6.x behavior, namely that
> cross-prefix symrefs will not be shortened at all. It might be nice to
> show:
>
>   ref-to-remote -> remotes/origin/branch-one
>
> or something, but that should be separate from the fix (and I don't
> overly care either way, so I probably won't work on it).
>
>  builtin/branch.c | 19 ---
>  t/t3203-branch-output.sh | 12 
>  2 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 7b45b6b..f6c23bf 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -393,22 +393,25 @@ static void format_and_print_ref_item(struct 
> ref_array_item *item, int maxwidth,
> int current = 0;
> int color;
> struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
> -   const char *prefix = "";
> +   const char *prefix_to_show = "";
> +   const char *prefix_to_skip = NULL;
> const char *desc = item->refname;
> char *to_free = NULL;
>
> switch (item->kind) {
> case FILTER_REFS_BRANCHES:
> -   skip_prefix(desc, "refs/heads/", );
> +   prefix_to_skip = "refs/heads/";
> +   skip_prefix(desc, prefix_to_skip, );
> if (!filter->detached && !strcmp(desc, head))
> current = 1;
> else
> color = BRANCH_COLOR_LOCAL;
> break;
> case FILTER_REFS_REMOTES:
> -   skip_prefix(desc, "refs/remotes/", );
> +   prefix_to_skip = "refs/remotes/";
> +   skip_prefix(desc, prefix_to_skip, );
> color = BRANCH_COLOR_REMOTE;
> -   prefix = remote_prefix;
> +   prefix_to_show = remote_prefix;
> break;
> case FILTER_REFS_DETACHED_HEAD:
> desc = to_free = get_head_description();
> @@ -425,7 +428,7 @@ static void format_and_print_ref_item(struct 
> ref_array_item *item, int maxwidth,
> color = BRANCH_COLOR_CURRENT;
> }
>
> -   strbuf_addf(, "%s%s", prefix, desc);
> +   strbuf_addf(, "%s%s", prefix_to_show, desc);