Re: [PATCH] Fix some sparse warnings

2013-07-15 Thread Jeff King
On Tue, Jul 16, 2013 at 07:57:20AM +0200, Johannes Sixt wrote:

> Am 7/15/2013 19:31, schrieb Ramsay Jones:
> > Sparse issues three "Using plain integer as NULL pointer" warnings.
> > Each warning relates to the use of an '{0}' initialiser expression
> > in the declaration of an 'struct object_info'.
> 
> I question the value of this warning. Initialization with '= {0}' is a
> well-established idiom, and sparse should know about it. Also, plain 0
> *is* a null pointer constant.

I agree with you. It's not a bug, and I think sparse is being overly
picky here; it is missing the forest for the trees in interpreting the
idiom.

Still, it may be worth tweaking in the name of eliminating compiler
noise, since it does not cost us very much to do so (and I believe we
have done so in the past, too).

We could also ask people with sparse to turn off the "use NULL instead
of 0" warning, but I think it _is_ a useful warning elsewhere (even
though it is never a bug, it violates our style guidelines and may be an
indication of a bug). It would be nice if sparse learned to ignore the
warning in this particular idiom, but I am not going to hold my breath
for that.

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


Re: repo consistency under crashes and power failures?

2013-07-15 Thread Johannes Sixt
Am 7/15/2013 19:48, schrieb Greg Troxel:
> Clearly there is the possibility of creating a corrupt repository when
> receiving objects and updating refs, if a crash or power failure causes
> data not to get written to disk but that data is pointed to.  Journaling
> mitigates this, but I'd argue that programs should function safely with
> only the guarantees from POSIX.

Even under POSIX, "guarantees" and "crash/power failure" do not mesh well.
This has been under dispute recently, for example:

http://thread.gmane.org/gmane.comp.standards.posix.austin.general/7456/focus=7487

The best we can achieve with POSIX alone is "to make bad consequences less
likely".

Jonathan already mentioned the knob that allows you to trade performance
for more safety.

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


Re: [PATCH] Fix some sparse warnings

2013-07-15 Thread Johannes Sixt
Am 7/15/2013 19:31, schrieb Ramsay Jones:
> Sparse issues three "Using plain integer as NULL pointer" warnings.
> Each warning relates to the use of an '{0}' initialiser expression
> in the declaration of an 'struct object_info'.

I question the value of this warning. Initialization with '= {0}' is a
well-established idiom, and sparse should know about it. Also, plain 0
*is* a null pointer constant.

-- 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: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

2013-07-15 Thread Mark Levedahl

On 07/15/2013 10:06 PM, Torsten Bögershausen wrote:

On 2013-07-15 21.49, Junio C Hamano wrote:

Mark Levedahl  writes:


In order to limit the adverse effects caused by this implementation,
we provide a new "fast stat" interface, which allows us to use this
only for interactions with the index (i.e. the cached stat data).

Signed-off-by: Ramsay Jones 
---

I've tested this on Cygwin 1.7 on WIndows 7 , comparing to the results
using your prior patch (removing the Cygwin specific lstat entirely)
and get the same results with both, so this seems ok from me.

My comparison point was created by reverting your current patch from
pu, then reapplying your earlier patch on top, so the only difference
was which approach was used to address the stat functions.

Caveats:
1) I don't find any speed improvement of the current patch over the
previous one (the tests actually ran faster with the earlier patch,
though the difference was less than 1%).

Hm, measuring the time for the test suite is one thing,
did you measure the time of "git status" with and without the patch?

(I don't have my test system at hand, so I can test in a few days/weeks)
Timing for 5 rounds of "git status" in the git project. First, with the 
current fast_lstat patches:

/usr/local/src/git>for i in {1..5} ; do time git status >& /dev/null ; done

real0m0.218s
user0m0.000s
sys 0m0.218s

real0m0.187s
user0m0.077s
sys 0m0.109s

real0m0.187s
user0m0.030s
sys 0m0.156s

real0m0.203s
user0m0.031s
sys 0m0.171s

real0m0.187s
user0m0.062s
sys 0m0.124s

Now, with Ramsay's original patch just removing the non-Posix stat 
functions:

/usr/local/src/git>for i in {1..5} ; do time git status >& /dev/null ; done

real0m0.218s
user0m0.046s
sys 0m0.171s

real0m0.187s
user0m0.015s
sys 0m0.171s

real0m0.187s
user0m0.015s
sys 0m0.171s

real0m0.187s
user0m0.047s
sys 0m0.140s

real0m0.187s
user0m0.031s
sys 0m0.156s


I see no difference in the above. (Yes, I checked multiple times that I 
was using different executables).

2) I still question this whole approach, especially having this
non-POSIX compliant mode be the default. Running in this mode breaks
interoperability with Linux, but providing a Linux environment is the
*primary* goal of Cygwin.

Sounds like we are better off without this patch, and instead remove
the "schizophrenic stat"?  I do not have a strong opinion either
way, except that I tend to agree with your point 2) above.

My understanding is that we want both:
Introduction of fast_lstat() as phase 1,
and the removal of the "schizophrenic stat" in compat/cygwin.c
as phase 2. (or do I missunderstand something ?)


And yes, phase 3:
The day we have a both reliable and fast
lstat() in cygwin, we can remove compat/cygwin.[ch]
If you know how to make Cygwin's stat faster while maintaining Posix 
semantics, please post a patch to the Cygwin list, they would *love* it.


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


Re: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

2013-07-15 Thread Mark Levedahl

On 07/15/2013 03:49 PM, Junio C Hamano wrote:

Mark Levedahl  writes:


In order to limit the adverse effects caused by this implementation,
we provide a new "fast stat" interface, which allows us to use this
only for interactions with the index (i.e. the cached stat data).

Signed-off-by: Ramsay Jones 
---

I've tested this on Cygwin 1.7 on WIndows 7 , comparing to the results
using your prior patch (removing the Cygwin specific lstat entirely)
and get the same results with both, so this seems ok from me.

My comparison point was created by reverting your current patch from
pu, then reapplying your earlier patch on top, so the only difference
was which approach was used to address the stat functions.

Caveats:
1) I don't find any speed improvement of the current patch over the
previous one (the tests actually ran faster with the earlier patch,
though the difference was less than 1%).
2) I still question this whole approach, especially having this
non-POSIX compliant mode be the default. Running in this mode breaks
interoperability with Linux, but providing a Linux environment is the
*primary* goal of Cygwin.

Sounds like we are better off without this patch, and instead remove
the "schizophrenic stat"?  I do not have a strong opinion either
way, except that I tend to agree with your point 2) above.

In case my opinion is unclear, I think removal of the schizophrenic stat 
is the right approach. Speed is important, but not at the expense of 
correctness.


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


Re: [PATCH] send-email: improve SSL certificate verification

2013-07-15 Thread brian m. carlson
On Tue, Jul 16, 2013 at 04:33:55AM +0200, Torsten Bögershausen wrote:
> [snip]
> I wasn't sure where to apply the patch, so I manually copy/paste it
> on top of pu:
> commit 6b1ca0f4d443ee8716857b871b0513ae85c9f112
> Merge: bce90ab f351fcf
> 
> Thanks, t9001 passes on Mac OS X 10.6.
> To be sure I didn't messed it up, please see the diff below.
> When it shows up on pu, I can re-test of course.

Yeah, that looks fine.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] send-email: improve SSL certificate verification

2013-07-15 Thread Torsten Bögershausen
[snip]
I wasn't sure where to apply the patch, so I manually copy/paste it
on top of pu:
commit 6b1ca0f4d443ee8716857b871b0513ae85c9f112
Merge: bce90ab f351fcf

Thanks, t9001 passes on Mac OS X 10.6.
To be sure I didn't messed it up, please see the diff below.
When it shows up on pu, I can re-test of course.



diff --git a/git-send-email.perl b/git-send-email.perl
index a9a6661..a965b8e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -28,7 +28,7 @@ use File::Temp qw/ tempdir tempfile /;
 use File::Spec::Functions qw(catfile);
 use Error qw(:try);
 use Git;
-use IO::Socket::SSL qw(SSL_VERIFY_PEER SSL_VERIFY_NONE);
+#use IO::Socket::SSL qw(SSL_VERIFY_PEER SSL_VERIFY_NONE);
 
 Getopt::Long::Configure qw/ pass_through /;
 
@@ -1092,19 +1092,34 @@ sub smtp_auth_maybe {
 # Helper to come up with SSL/TLS certification validation params
 # and warn when doing no verification
 sub ssl_verify_params {
-if ($smtp_ssl_verify == 0) {
-return (SSL_verify_mode => IO::Socket::SSL->SSL_VERIFY_NONE);
+require IO::Socket::SSL;
+eval {
+IO::Socket::SSL->import(qw/SSL_VERIFY_PEER SSL_VERIFY_NONE/);
+};
+if ($@) {
+print STDERR "Not using SSL_VERIFY_PEER due to out-of-date 
IO::Socket::SSL.\n";
+return;
 }
 
-if (! defined $smtp_ssl_cert_path) {
-return (SSL_verify_mode => IO::Socket::SSL->SSL_VERIFY_PEER);
-} elsif (-f $smtp_ssl_cert_path) {
-return (SSL_verify_mode => IO::Socket::SSL->SSL_VERIFY_PEER,
-SSL_ca_file => $smtp_ssl_cert_path);
-} else {
-return (SSL_verify_mode => IO::Socket::SSL->SSL_VERIFY_PEER,
+if (!defined $smtp_ssl_cert_path) {
+$smtp_ssl_cert_path ||= "/etc/ssl/certs";
+}
+
+if (!$smtp_ssl_cert_path) {
+return (SSL_verify_mode => SSL_VERIFY_NONE());
+}
+elsif (-d $smtp_ssl_cert_path) {
+return (SSL_verify_mode => SSL_VERIFY_PEER(),
 SSL_ca_path => $smtp_ssl_cert_path);
 }
+elsif (-f $smtp_ssl_cert_path) {
+return (SSL_verify_mode => SSL_VERIFY_PEER(),
+SSL_ca_file => $smtp_ssl_cert_path);
+}
+else {
+print STDERR "Not using SSL_VERIFY_PEER because the CA path does not 
exist.\n";
+return (SSL_verify_mode => SSL_VERIFY_NONE());
+}
 }
 
 # Returns 1 if the message was sent, and 0 otherwise.
@@ -1229,13 +1244,8 @@ X-Mailer: git-send-email $gitversion
 if ($smtp->code == 220) {
 $smtp = Net::SMTP::SSL->start_SSL($smtp,
   ssl_verify_params())
-or die "STARTTLS failed! ".$smtp->message;
-$smtp_encryption = '';
-# Send EHLO again to receive fresh
-# supported commands
-$smtp->hello($smtp_domain);
-} else {
-die "Server does not support STARTTLS! ".$smtp->message;
+or die "STARTTLS failed! ".$smtp->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: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

2013-07-15 Thread Torsten Bögershausen
On 2013-07-15 21.49, Junio C Hamano wrote:
> Mark Levedahl  writes:
> 
>>> In order to limit the adverse effects caused by this implementation,
>>> we provide a new "fast stat" interface, which allows us to use this
>>> only for interactions with the index (i.e. the cached stat data).
>>>
>>> Signed-off-by: Ramsay Jones 
>>> ---
>>
>> I've tested this on Cygwin 1.7 on WIndows 7 , comparing to the results
>> using your prior patch (removing the Cygwin specific lstat entirely)
>> and get the same results with both, so this seems ok from me.
>>
>> My comparison point was created by reverting your current patch from
>> pu, then reapplying your earlier patch on top, so the only difference
>> was which approach was used to address the stat functions.
>>
>> Caveats:
>> 1) I don't find any speed improvement of the current patch over the
>> previous one (the tests actually ran faster with the earlier patch,
>> though the difference was less than 1%).
Hm, measuring the time for the test suite is one thing,
did you measure the time of "git status" with and without the patch?

(I don't have my test system at hand, so I can test in a few days/weeks)

>> 2) I still question this whole approach, especially having this
>> non-POSIX compliant mode be the default. Running in this mode breaks
>> interoperability with Linux, but providing a Linux environment is the
>> *primary* goal of Cygwin.
> 
> Sounds like we are better off without this patch, and instead remove
> the "schizophrenic stat"?  I do not have a strong opinion either
> way, except that I tend to agree with your point 2) above.

My understanding is that we want both:
Introduction of fast_lstat() as phase 1,
and the removal of the "schizophrenic stat" in compat/cygwin.c
as phase 2. (or do I missunderstand something ?)


And yes, phase 3:
The day we have a both reliable and fast 
lstat() in cygwin, we can remove compat/cygwin.[ch]











--
To unsubscribe from this list: send the line "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] t9200 - Allow cvs version 1.12

2013-07-15 Thread Mark Levedahl

On 07/15/2013 06:06 PM, Junio C Hamano wrote:

Mark Levedahl  writes:


cvs v1.12 does not correctly handle "cvs co -d $DIR", which is shorthand
for "mkdir $DIR, cd $DIR, cvs co, cd -". So, use the latter form.

Hmph, I think I've been using 1.12.13 and without seeing such a
breakage.  Do you mean "exactly v1.12", not "v1.12.x series"?

Hmm, good instincts. Cygwin includes 1.12.13 which is what I used. I 
downloaded the sources, rebuilt, everything works fine, so apparently 
the Cygwin provided cvs binary is corrupt. I apologize for the noise, 
will take this to the Cygwin list.


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


[PATCH] send-email: improve SSL certificate verification

2013-07-15 Thread brian m. carlson
The SSL and TLS code for SMTP is non-trivial, so refactor it into a separate
function for ease of use.  Handle both files and directories as sources for CA
certificates.  Also add handling for older version of IO::Socket::SSL that do
not support the SSL_VERIFY_PEER and SSL_VERIFY_NONE constants; in this case,
print a warning and inform the user of this fact.

Signed-off-by: brian m. carlson 
---

This is completely untested.  I used perl -c, but that's it.

 git-send-email.perl | 50 --
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 095b6fb..11fb2d0 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1083,6 +1083,37 @@ sub smtp_auth_maybe {
return $auth;
 }
 
+sub ssl_verify_params {
+   require IO::Socket::SSL;
+   eval {
+   IO::Socket::SSL->import(qw/SSL_VERIFY_PEER SSL_VERIFY_NONE/);
+   };
+   if ($@) {
+   print STDERR "Not using SSL_VERIFY_PEER due to out-of-date 
IO::Socket::SSL.\n";
+   return;
+   }
+
+   if (!defined $smtp_ssl_cert_path) {
+   $smtp_ssl_cert_path ||= "/etc/ssl/certs";
+   }
+
+   if (!$smtp_ssl_cert_path) {
+   return (SSL_verify_mode => SSL_VERIFY_NONE());
+   }
+   elsif (-d $smtp_ssl_cert_path) {
+   return (SSL_verify_mode => SSL_VERIFY_PEER(),
+   SSL_ca_path => $smtp_ssl_cert_path);
+   }
+   elsif (-f $smtp_ssl_cert_path) {
+   return (SSL_verify_mode => SSL_VERIFY_PEER(),
+   SSL_ca_file => $smtp_ssl_cert_path);
+   }
+   else {
+   print STDERR "Not using SSL_VERIFY_PEER because the CA path 
does not exist.\n";
+   return (SSL_verify_mode => SSL_VERIFY_NONE());
+   }
+}
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.
@@ -1187,7 +1218,8 @@ X-Mailer: git-send-email $gitversion
$smtp_domain ||= maildomain();
$smtp ||= Net::SMTP::SSL->new($smtp_server,
  Hello => $smtp_domain,
- Port => 
$smtp_server_port);
+ Port => $smtp_server_port,
+ ssl_verify_params());
}
else {
require Net::SMTP;
@@ -1203,19 +1235,9 @@ X-Mailer: git-send-email $gitversion
$smtp->command('STARTTLS');
$smtp->response();
if ($smtp->code == 220) {
-   # Attempt to use a ca-certificate by 
default
-   $smtp_ssl_cert_path |= "/etc/ssl/certs";
-   if (-d $smtp_ssl_cert_path) {
-   $smtp = 
Net::SMTP::SSL->start_SSL($smtp,
-   
  SSL_verify_mode => SSL_VERIFY_PEER,
-   
  SSL_ca_path => $smtp_ssl_cert_path)
-   or die "STARTTLS 
failed! ".$smtp->message;
-   } else {
-   print STDERR "warning: Using 
SSL_VERIFY_NONE.  See sendemail.smtpsslcertpath.\n";
-   $smtp = 
Net::SMTP::SSL->start_SSL($smtp,
-   
  SSL_verify_mode => SSL_VERIFY_NONE)
-   or die "STARTTLS 
failed! ".$smtp->message;
-   }
+   $smtp = Net::SMTP::SSL->start_SSL($smtp,
+ 
ssl_verify_params())
+   or die "STARTTLS failed! 
".$smtp->message;
$smtp_encryption = '';
# Send EHLO again to receive fresh
# supported commands
-- 
1.8.3.2.923.g2a18ff8.dirty


-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] git-log.txt: fix typesetting of example "git-log -L" invocation

2013-07-15 Thread Eric Sunshine
All surrounding examples are typeset as monospaced text. Follow suit.

Signed-off-by: Eric Sunshine 
---
 Documentation/git-log.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 2ea79ba..2ee6962 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -153,7 +153,7 @@ Examples
This makes sense only when following a strict policy of merging all
topic branches when staying on a single integration branch.
 
-git log -L '/int main/',/^}/:main.c::
+`git log -L '/int main/',/^}/:main.c`::
 
Shows how the function `main()` in the file 'main.c' evolved
over time.
-- 
1.8.3.3.1016.g4f0baba

--
To unsubscribe from this list: send the line "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/5] config: improve support for http..* settings

2013-07-15 Thread Eric Sunshine
On Mon, Jul 15, 2013 at 5:51 AM, Kyle J. McKay  wrote:
> Improve on the http..* url matching behavior by first
> normalizing the urls before they are compared.
>
> diff --git a/http.c b/http.c
> index 758e5b1..d04386e 100644
> --- a/http.c
> +++ b/http.c
> @@ -169,6 +169,210 @@ static void process_curl_messages(void)
>  }
>  #endif
>
> +static int append_normalized_escapes(struct strbuf *buf,
> +const char *from,
> +size_t from_len,
> +const char *esc_extra,
> +const char *esc_ok)
> +{
> +   /*
> +* Append to strbuf buf characters from string from with length 
> from_len

s/from string from/from string/

> +* while unescaping characters that do not need to be escaped and
> +* escaping characters that do.  The set of characters to escape (the
> +* complement of which is unescaped) starts out as the RFC 3986 unsafe
> +* characters (0x00-0x1F,0x7F-0xFF," <>\"#%{}|\\^`").  If esc_extra
> +* is not NULL, those additional characters will also always be 
> escaped.
> +* If esc_ok is not NULL, those characters will be left escaped if 
> found
> +* that way, but will not be unescaped otherwise (used for 
> delimiters).
> +* If a %-escape sequence is encountered that is not followed by 2
> +* hexadecimal digits, the sequence is invalid and false (0) will be
> +* returned.  Otherwise true (1) will be returned for success.
> +*
> +* Note that all %-escape sequences will be normalized to UPPERCASE
> +* as indicated in RFC 3986.  Unless included in esc_extra or esc_ok
> +* alphanumerics and "-._~" will always be unescaped as per RFC 3986.
> +*/
--
To unsubscribe from this list: send the line "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] t9200 - Allow cvs version 1.12

2013-07-15 Thread Junio C Hamano
Mark Levedahl  writes:

> cvs v1.12 does not correctly handle "cvs co -d $DIR", which is shorthand
> for "mkdir $DIR, cd $DIR, cvs co, cd -". So, use the latter form.

Hmph, I think I've been using 1.12.13 and without seeing such a
breakage.  Do you mean "exactly v1.12", not "v1.12.x series"?

> Also cvs v1.12 does not necessarily match cvs v1.11 in the format of
> CVS/Entries, and this causes a false failure in subtest 14. Eliminate
> checking CVS/Entries for this one test, but keep the test that the
> created file exists and is checked out.

Also I suspect this is not because we are expecting v1.11 format, as
v1.12.13 on my box seems to pass the test.  While your removal of
the "check_entries" step might be a valid workaround for whatever
version is shipped with Cygwin, the above problem description seems
somewhat inaccurate.

> With these changes, all tests in t9200 pass on Cygwin using its default
> cvs version 1.12.
>
> Signed-off-by: Mark Levedahl 
> ---
>  t/t9200-git-cvsexportcommit.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
> index 3fb3368..17cb554 100755
> --- a/t/t9200-git-cvsexportcommit.sh
> +++ b/t/t9200-git-cvsexportcommit.sh
> @@ -28,7 +28,8 @@ rm -rf "$CVSROOT" "$CVSWORK"
>  
>  cvs init &&
>  test -d "$CVSROOT" &&
> -cvs -Q co -d "$CVSWORK" . &&
> +mkdir -p "$CVSWORK" &&
> +(cd "$CVSWORK" && cvs -Q co .) &&
>  echo >empty &&
>  git add empty &&
>  git commit -q -a -m "Initial" 2>/dev/null ||
> @@ -313,7 +314,6 @@ test_expect_success 'commit a file with leading spaces in 
> the name' '
>   git commit -m "Add a file with a leading space" &&
>   id=$(git rev-parse HEAD) &&
>   git cvsexportcommit -w "$CVSWORK" -c $id &&
> - check_entries "$CVSWORK" " 
> space/1.1/|DS/1.1/|attic_gremlin/1.3/|release-notes/1.2/" &&
>   test_cmp "$CVSWORK/ space" " space"
>  
>  '
--
To unsubscribe from this list: send the line "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] howto: Use all-space indentation in ASCII art

2013-07-15 Thread Junio C Hamano
Dirk Wallenstein  writes:

> Keep the sketch aligned independent of the tabstop width used.

Thanks.

This is a source text to be formatted into HTML, isn't it?

In our sources, a HT indents to multiple of 8 columns.  As long as
the output HTML produced from the source can be seen on a terminal
with any tab-width correctly, I do not see any reason to apply this
patch.

Am I missing something???

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


Re: [PATCHv2 3/3] l10n: de.po: switch from pure German to German+English (part 3)

2013-07-15 Thread Thomas Rast
Ralf Thielow  writes:

>  #: builtin/remote.c:187
>  msgid "specifying branches to track makes sense only with fetch mirrors"
>  msgstr ""
> -"Die Angabe von zu folgenden Zweigen kann nur mit dem Anfordern von "
> +"Die Angabe von zu folgenden Branches kann nur mit dem Anfordern von "
>  "Spiegelarchiven verwendet werden."

Spiegelarchiv -> Spiegel-Repository

> @@ -8058,18 +8057,18 @@ msgid_plural ""
>  "Note: Some branches outside the refs/remotes/ hierarchy were not removed;\n"
>  "to delete them, use:"
>  msgstr[0] ""
> -"Hinweis: Ein Zweig außerhalb der /refs/remotes/ Hierachie wurde nicht "
> +"Hinweis: Ein Branch außerhalb der /refs/remotes/ Hierachie wurde nicht "
>  "gelöscht;\n"
>  "um diesen zu löschen, benutzen Sie:"
>  msgstr[1] ""
> -"Hinweis: Einige Zweige außer der /refs/remotes/ Hierarchie wurden nicht "
> +"Hinweis: Einige Branches außer der /refs/remotes/ Hierarchie wurden nicht "
>  "entfernt;\n"
>  "um diese zu entfernen, benutzen Sie:"

Not new, but refs/remotes/ probably shouldn't have a leading slash (at
least that's the convention in English).

>  #: builtin/remote.c:998
>  #, c-format
>  msgid "rebases onto remote %s"
> -msgstr "baut neu auf externen Zweig %s auf"
> +msgstr "baut neu auf Remote-Branch %s auf"

In the glossary you have

rebase   = "rebase" benutzen
rebase   = Rebase

So "neuaufbauen" seems to be out?

>  #: builtin/show-branch.c:675
>  msgid "show commits where no parent comes before its children"
> -msgstr "zeigt Versionen, wo kein Elternteil vor seinem Kind kommt"
> +msgstr "zeigt Commits, wo kein Eltern-Commit vor seinem Kind-Commit kommt"

Not specific to the German versions, but the "where" should really be
"in such a way", e.g. simply "so dass".

> @@ -8871,23 +8869,23 @@ msgid ""
>  "git tag [-a|-s|-u ] [-f] [-m |-F ]  []"
>  msgstr ""
>  "git tag [-a|-s|-u ] [-f] [-m |-F ] "
> -" []"
> +" []"

Not new, but  here should be .

(Yes I know, it can even be another object type.  Let's not confuse the
users.)

> @@ -9735,8 +9731,8 @@ msgstr ""
>  #: git-pull.sh:203
>  msgid "updating an unborn branch with changes added to the index"
>  msgstr ""
> -"Aktualisiere eine ungeborenen Zweig mit Änderungen, die zur Bereitstellung "
> -"hinzugefügt wurden"
> +"Aktualisiere eine ungeborenen Branch mit Änderungen, die zum Commit "
> +"vorgemerkt sind"

Gender mismatch in "eine ungeborenen".

>  #: git-submodule.sh:744
>  #, sh-format
>  msgid "Unable to find current revision in submodule path '$prefix$sm_path'"
>  msgstr ""
> -"Konnte aktuelle Revision in Unterprojekt-Pfad '$prefix$sm_path' nicht 
> finden."
> +"Konnte aktuelle Revision in Submodul-Pfad '$prefix$sm_path' nicht finden."

Revision -> Commit as per the glossary?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 2/3] l10n: de.po: switch from pure German to German+English (part 2)

2013-07-15 Thread Thomas Rast
Ralf Thielow  writes:

>  #: builtin/clone.c:73
>  msgid "create a mirror repository (implies bare)"
> -msgstr "erstellt ein Spiegelarchiv (impliziert bloßes Projektarchiv)"
> +msgstr "erstellt ein Spiegelarchiv (impliziert Bare-Repository)"

Perhaps it's better to just say --bare here?

>  #: builtin/clone.c:93
>  msgid "path to git-upload-pack on the remote"
> -msgstr "Pfad zu \"git-upload-pack\" auf der Gegenseite"
> +msgstr "Pfad zu \"git-upload-pack\" im Remote-Repository"

Here 'remote' does not refer to a repository, but the machine in
general.  I guess "Gegenseite" was okay.

>  #: builtin/commit.c:819
>  msgid "Error building trees"
> -msgstr "Fehler beim Erzeugen der Zweige"
> +msgstr "Fehler beim Erzeugen der Verzeichnisse"

Similar to the one in merge-recursive.c, this is an internal function
that writes trees failing (update_main_cache_tree() in this case).  So
neither old nor new German translations are in fact correct; it is about
tree objects.

>  #: builtin/commit.c:1371
>  msgid "use autosquash formatted message to squash specified commit"
>  msgstr ""
>  "verwendet eine automatisch zusammengesetzte Beschreibung zum Zusammenführen 
> "
> -"der angegebenen Version"
> +"des angegebenen Commits"

Shouldn't the "Zusammenführen" also change to "Mergen"?

> @@ -4503,8 +4501,8 @@ msgid ""
>  "No annotated tags can describe '%s'.\n"
>  "However, there were unannotated tags: try --tags."
>  msgstr ""
> -"Keine annotierten Markierungen können '%s' beschreiben.\n"
> -"Jedoch gab es nicht annotierte Markierungen: versuchen Sie --tags."
> +"Keine annotierten Tags können '%s' beschreiben.\n"
> +"Jedoch gab es nicht annotierte Tags: versuchen Sie --tags."

You could add a clarifying dash to make it easier to read:

  Jedoch gab es nicht-annotierte Tags: versuchen Sie --tags.

(My German teacher probably hates me now.)

> @@ -4572,7 +4570,7 @@ msgstr "Kennzeichen"
>  #: builtin/describe.c:417
>  msgid "append  on dirty working tree (default: \"-dirty\")"
>  msgstr ""
> -"fügt  bei geändertem Arbeitsbaum hinzu (Standard: \"-dirty\")"
> +"fügt  bei geändertem Arbeitsverzeichnis hinzu (Standard: 
> \"-dirty\")"

Here you have dirty -> geändertes Arbeitsverzeichnis ("modified
worktree"), but in other places I saw clean (relating to worktree) ->
sauber.

How about always talking about an (un)modified worktree, i.e.,
geändertem/unverändertem Arbeitsbaum?

>  #: builtin/fetch.c:64
>  msgid "path to upload pack on remote end"
> -msgstr "Pfad des Programms zum Hochladen von Paketen auf der Gegenseite"
> +msgstr "Pfad des Programms zum Hochladen von Paketen im Remote-Repository"

Similar to the message in clone.c, this does not refer to a
repo-specific path and should probably remain "auf der Gegenseite".

>  #: builtin/fetch.c:1023
>  msgid "Fetching a group and specifying refspecs does not make sense"
>  msgstr ""
>  "Das Abholen einer Gruppe von externen Archiven kann nicht mit der Angabe\n"
> -"von Referenzspezifikationen verwendet werden."
> +"von Refspecs verwendet werden."

You missed an instance of "externe Archive" here.

>  #: builtin/fsck.c:618
>  msgid "make index objects head nodes"
> -msgstr "erzeugt Kopfknoten der Bereitstellungsobjekte"
> +msgstr "erzeugt Kopfknoten der Staging-Area-Objekte"
> 
>  #: builtin/fsck.c:619
>  msgid "make reflogs head nodes (default)"
> -msgstr "erzeugt Kopfknoten des Referenzprotokolls (Standard)"
> +msgstr "erzeugt Kopfknoten des Reflogs (Standard)"

Not a new problem, but none of these (including English) is particularly
enlightening without context.  The message refers to fsck starting an
unreachability and verification walk at a list of objects.  This option
(--cache) adds all blobs in the index to that list.

So perhaps simply

  prüfe Objekte in der Staging Area

and then similarly change all the "head nodes" talk to an application of
"prüfen".

>  #: builtin/grep.c:905
>  msgid "both --cached and trees are given."
> -msgstr "Die Option --cached kann nicht mit Zweigen verwendet werden."
> +msgstr "Die Option --cached kann nicht mit Verzeichnissen verwendet werden."

"Trees" here refers to a tree object.

>  #: builtin/init-db.c:102
>  #, c-format
>  msgid "cannot symlink '%s' '%s'"
> -msgstr "kann '%s' nicht mit '%s' symbolisch verknüpfen"
> +msgstr "kann '%s' nicht mit '%s' symbolisch verweisen"

Is "verweisen" transitive?  Perhaps

  Kann symbolischen Verweis '%s' auf '%s' nicht erstellen

>  #: builtin/log.c:1138
>  msgid "don't include a patch matching a commit upstream"
>  msgstr ""
> -"schließt keine Patches ein, die einer Version im Übernahmezweig entsprechen"
> +"schließt keine Patches ein, die einem Commit im Upstream-Branch entsprechen"

Perhaps it doesn't matter, but: the use of "upstream" here does not
refer to the upstream config, but the argument to format-patch.  Which
might be the upstream, but the user is free to specify e.g. HEAD~1
instead.

>  #: builtin/log.c:1484
>  msgid "git cherry [-v] [ [ []]]"
> -msgstr "git cherry [-v] [<Übernahmezweig>

Re: [PATCHv2 0/3] Switch German translation to G+E

2013-07-15 Thread Thomas Rast
Ralf Thielow  writes:

> Ralf Thielow (3):
>   l10n: de.po: switch from pure German to German+English (part 1)
>   l10n: de.po: switch from pure German to German+English (part 2)
>   l10n: de.po: switch from pure German to German+English (part 3)

Thanks a lot, and sorry I was so slow.  I'll send out the comments to
the individual patches in a second.

After that I think I'll be happy with whatever you revise it to.  Thanks
so much for your work (while I did most of the complaining).  Let it be
on public record that I will invite you for dinner and/or copious
amounts of beer next time you come near Zurich!


My favorite "resolved" ambiguity/clunky translation:

> -msgstr "stellt den angegebenen Eintrag zur Eintragung bereit"

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv2 1/3] l10n: de.po: switch from pure German to German+English (part 1)

2013-07-15 Thread Thomas Rast
Ralf Thielow  writes:

>  #: merge-recursive.c:268
>  msgid "error building trees"
> -msgstr "Fehler beim Erstellen der Bäume"
> +msgstr "Fehler beim Erstellen der Verzeichnisse"

This should remain "Bäume" or possibly '"Tree"-Objekte', as it refers to
a failure within write_tree_from_memory().

>  #: merge-recursive.c:1815
>  #, c-format
>  msgid "merging of trees %s and %s failed"
> -msgstr "Zusammenführen der Bäume %s und %s fehlgeschlagen"
> +msgstr "Zusammenführen der Verzeichnisse %s und %s fehlgeschlagen"

Similarly; here merge_trees() is failing.

>  #: pathspec.c:99
>  #, c-format
>  msgid "'%s' is beyond a symbolic link"
> -msgstr "'%s' ist über einem symbolischen Link"
> +msgstr "'%s' ist hinter einem symbolischen Verweis"

Good fix for a pretty bad preimage :-)

>  #: remote.c:1787
>  msgid "  (use \"git push\" to publish your local commits)\n"
> -msgstr "  (benutzen Sie \"git push\" um lokalen Versionen herauszubringen)\n"
> +msgstr "  (benutzen Sie \"git push\" um lokale Commits herauszubringen)\n"

I don't see this in the glossary.  "Publizieren" is a perfectly good
german word, can't we use this?  "Herausbringen" allows some other
interpretations that "publizieren" doesn't.

>  #: sequencer.c:1119
>  msgid "Can't revert as initial commit"
> -msgstr "Rücknahme-Version kann nicht initial sein."
> +msgstr "Revert-Commit kann nicht initial sein."

Not a new problem, but how about

  Kann nicht als allerersten Commit einen Revert ausführen.

(This should then go into the glossary for "initial commit".)

>  #: sequencer.c:1120
>  msgid "Can't cherry-pick into empty head"
> -msgstr "Kann \"cherry-pick\" nicht in einem leeren Zweig ausführen."
> +msgstr "Kann \"cherry-pick\" nicht in einem leeren Branch ausführen."

Could reuse the same formulation as above; the symptom is the same,
though not the English message.

> @@ -1494,7 +1496,7 @@ msgstr "  (alle Konflikte behoben: führen Sie \"git 
> commit\" aus)"
>  #: wt-status.c:970
>  #, c-format
>  msgid "You are currently reverting commit %s."
> -msgstr "Sie nehmen gerade Version '%s' zurück."
> +msgstr "Sie nehmen gerade Commit '%s' zurück."

"Revert" is partly untranslated in other messages.  Any strong opinions?
If you would like to consistently leave it untranslated, this message
could be

  Sie sind gerade an einem Revert von Commit '%s'.

>  #: builtin/apply.c:3850
>  #, c-format
>  msgid "corrupt patch for subproject %s"
> -msgstr "fehlerhafter Patch für Unterprojekt %s"
> +msgstr "fehlerhafter Patch für Submodul %s"

We could change this on the English side too, though the patch format is
frozen and calls them "subprojects" (sigh).

> @@ -2330,12 +2331,12 @@ msgstr "erwartet keinen Kontext"
>  #: builtin/apply.c:4406
>  msgid "leave the rejected hunks in corresponding *.rej files"
>  msgstr ""
> -"hinterlässt zurückgewiesene Patch-Bereiche in den entsprechenden *.rej "
> +"hinterlässt zurückgewiesene Patch-Blöcke in den entsprechenden *.rej "
>  "Dateien"

Not a new problem, but "in entsprechenden *.rej" (dropping the article)
would be more accurate, wouldn't it?

>  #: builtin/blame.c:2369
>  msgid "Ignore whitespace differences"
> -msgstr "Ignoriert Unterschiede in Leerzeichen"
> +msgstr "Ignoriert Unterschiede in Whitespaces"

  Ignoriert Unterschiede in Whitespace

(dropping the final s)?

I saw that you matched the German gender of the English words to the
gender that your favorite translation has in German, which I think is
fine.  But whitespace is not countable (at least in my ears), and I
think that should be inherited into the pseudo-German usage.

>  #: builtin/branch.c:585
>  #, c-format
>  msgid "(no branch, bisect started on %s)"
> -msgstr "(kein Zweig, Neuaufbau begonnen bei %s)"
> +msgstr "(kein Branch, Rebase begonnen bei %s)"

You turned bisect into rebase (not new, but still).

>  #: builtin/checkout.c:1053
>  msgid "new unparented branch"
> -msgstr "neuer Zweig ohne Elternversion"
> +msgstr "neuer Branch ohne Elternversion"

Elternversions should become Eltern-Commit according to the glossary.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/46] "struct pathspec" conversion and :(glob) and :(icase)

2013-07-15 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> Compared to the last round [1] this series mainly fixes comments and
> commit messages suggested by Eric and Junio. It also fixes a conflict
> with cb/log-follow-with-combined (in master) and introduces :(icase)
> mentioned in the last round.
>
> [1] http://thread.gmane.org/gmane.comp.version-control.git/226892
>
> Nguyễn Thái Ngọc Duy (46):
>   clean: remove unused variable "seen"
>   ...
>   parse_pathspec: accept :(icase)path syntax

This was a good exercise for git-imerge.  I merged it just below the
"2.0 migration topic" that is kept outside the 'next' first by hand,
and then used imerge.  Interested parties may try to do this, after
fetching from me:


$ git checkout cb5d2fc
$ git branch nd/magic-pathspec 93d93537
$ git imerge start --first-parent --name bogo nd/magic-pathspec

... imerge thinks and then will stop, asking you to resolve
... and commit, repeatedly.  Do the next two steps until it
... stops.

$ edit
$ git commit -a --no-edit && git imerge continue

... and then
$ git imerge finish

The imerge program was a bit under-help (yet), and also it was a bit
cumbersome at times having to resolve trivial and obvious two
conflicts in the same file (e.g. the series adds description for
GIT_*_PATHSPECS environment variables to Documentation/git.txt in
two separate patches, and the resolution is obvious for a single
merge, but imerge wants me to resolve them in two steps), but in the
end, after "imerge finish", it produced identical result as my hand
merge, so at least it seems to be doing something right ;-)

A few things I noticed:

 - It was not immediately clear from "git imerge -h" text what the
   simplest "hello world" workflow was.

 - It was not clear what it wanted for --name parameter (it becomes
   the name of the temporary and final branch that stores the
   result).

 - The final step "imerge finish" gave me this ugliness:

 Merge commit 93d9353... into commit cb5d2fc7

   Perhaps you can at least use the initial branch name
   "nd/magic-pathspec" I gave you, and use "git fmt-merge-msg"?

A bigger test will be to try merging jn/add-2.0-u-A-sans-pathspec
and jc/add-2.0-ignore-removal topics on top of the merge result
(2857023 in today's pushout).  These two merges, especially the
latter one, have to involve some "evil merges", so it would be
interesting to see how well it fares.

In any case, thanks for a fun tool.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[ANNOUNCE] Git v1.8.3.3

2013-07-15 Thread Junio C Hamano
The latest maintenance release Git v1.8.3.3 is now available at
the usual places.

The release tarballs are found at:

http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

417cb12660446702bffc5c2c83cbb6e7f1e60c79  git-1.8.3.3.tar.gz
c6104064c1276b2405a281e104fc54ff86f7299d  git-htmldocs-1.8.3.3.tar.gz
07361cfd38b8c57207b9a5f8bf0c4456b7229b52  git-manpages-1.8.3.3.tar.gz

The following public repositories all have a copy of the v1.8.3.3
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 = https://code.google.com/p/git-core/
  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

Also, http://www.kernel.org/pub/software/scm/git/ has copies of the
release tarballs.

Git v1.8.3.3 Release Notes
==

Fixes since v1.8.3.2


 * "git apply" parsed patches that add new files, generated by programs
   other than Git, incorrectly.  This is an old breakage in v1.7.11.

 * Older cURL wanted piece of memory we call it with to be stable, but
   we updated the auth material after handing it to a call.

 * "git pull" into nothing trashed "local changes" that were in the
   index.

 * Many "git submodule" operations did not work on a submodule at a
   path whose name is not in ASCII.

 * "cherry-pick" had a small leak in its error codepath.

 * Logic used by git-send-email to suppress cc mishandled names like
   "A U. Thor" , where the human readable part
   needs to be quoted (the user input may not have the double quotes
   around the name, and comparison was done between quoted and
   unquoted strings).  It also mishandled names that need RFC2047
   quoting.

 * "gitweb" forgot to clear a global variable $search_regexp upon each
   request, mistakenly carrying over the previous search to a new one
   when used as a persistent CGI.

 * The wildmatch engine did not honor WM_CASEFOLD option correctly.

 * "git log -c --follow $path" segfaulted upon hitting the commit that
   renamed the $path being followed.

 * When a reflog notation is used for implicit "current branch",
   e.g. "git log @{u}", we did not say which branch and worse said
   "branch ''" in the error messages.

 * Mac OS X does not like to write(2) more than INT_MAX number of
   bytes; work it around by chopping write(2) into smaller pieces.

 * Newer MacOS X encourages the programs to compile and link with
   their CommonCrypto, not with OpenSSL.

Also contains various minor documentation updates.



Changes since v1.8.3.2 are as follows:

Andrew Pimlott (2):
  lib-rebase: document exec_ in FAKE_LINES
  t7500: fix flipped actual/expect

Anthony Ramine (1):
  wildmatch: properly fold case everywhere

Brandon Casey (1):
  http.c: don't rewrite the user:passwd string multiple times

Charles McGarvey (1):
  gitweb: fix problem causing erroneous project list

Chris Rorvick (1):
  git.txt: remove stale comment regarding GIT_WORK_TREE

Clemens Buchacher (1):
  fix segfault with git log -c --follow

David Aguilar (4):
  Makefile: fix default regex settings on Darwin
  Makefile: add support for Apple CommonCrypto facility
  cache.h: eliminate SHA-1 deprecation warnings on Mac OS X
  imap-send: eliminate HMAC deprecation warnings on Mac OS X

Dmitry Marakasov (1):
  contrib/git-subtree: Use /bin/sh interpreter instead of /bin/bash

Felipe Contreras (4):
  read-cache: fix wrong 'the_index' usage
  read-cache: trivial style cleanups
  sequencer: remove useless indentation
  sequencer: avoid leaking message buffer when refusing to create an empty 
commit

Filipe Cabecinhas (1):
  compate/clipped-write.c: large write(2) fails on Mac OS X/XNU

Fredrik Gustafsson (1):
  handle multibyte characters in name

Jeff King (1):
  pull: update unborn branch tip after index

John Keeping (1):
  git-config: update doc for --get with multiple values

Junio C Hamano (6):
  deprecate core.statinfo at Git 2.0 boundary
  t5551: do not use unportable sed '\+'
  Documentation/diff-index: mention two modes of operation
  Start preparing for 1.8.3.3
  Update draft release notes to 1.8.3.3
  Git 1.8.3.3

Michael S. Tsirkin (9):
  t/send-email.sh: add test for suppress-cc=self
  send-email: fix suppress-cc=self on cccmd
  t/send-email: test suppress-cc=self on cccmd
  send-email: make --suppress-cc=self sanitize input
  t/send-email: add test with quoted sender
  t/send-email: test suppress-cc=self with non-ascii
  test-send-email: test for pre-sanitized self name
  send-email: add test for duplicate utf8 name
  send-email: sanitize author when writing From line

Ramkumar Ramachandra (6):
  sha1_name: fix error

[PATCH] t9200 - Allow cvs version 1.12

2013-07-15 Thread Mark Levedahl
cvs v1.12 does not correctly handle "cvs co -d $DIR", which is shorthand
for "mkdir $DIR, cd $DIR, cvs co, cd -". So, use the latter form.

Also cvs v1.12 does not necessarily match cvs v1.11 in the format of
CVS/Entries, and this causes a false failure in subtest 14. Eliminate
checking CVS/Entries for this one test, but keep the test that the
created file exists and is checked out.

With these changes, all tests in t9200 pass on Cygwin using its default
cvs version 1.12.

Signed-off-by: Mark Levedahl 
---
 t/t9200-git-cvsexportcommit.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
index 3fb3368..17cb554 100755
--- a/t/t9200-git-cvsexportcommit.sh
+++ b/t/t9200-git-cvsexportcommit.sh
@@ -28,7 +28,8 @@ rm -rf "$CVSROOT" "$CVSWORK"
 
 cvs init &&
 test -d "$CVSROOT" &&
-cvs -Q co -d "$CVSWORK" . &&
+mkdir -p "$CVSWORK" &&
+(cd "$CVSWORK" && cvs -Q co .) &&
 echo >empty &&
 git add empty &&
 git commit -q -a -m "Initial" 2>/dev/null ||
@@ -313,7 +314,6 @@ test_expect_success 'commit a file with leading spaces in 
the name' '
git commit -m "Add a file with a leading space" &&
id=$(git rev-parse HEAD) &&
git cvsexportcommit -w "$CVSWORK" -c $id &&
-   check_entries "$CVSWORK" " 
space/1.1/|DS/1.1/|attic_gremlin/1.3/|release-notes/1.2/" &&
test_cmp "$CVSWORK/ space" " space"
 
 '
-- 
1.8.3.2.0.63

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


Re: [PATCH 7/7] push: document --lockref

2013-07-15 Thread Johannes Sixt
Am 14.07.2013 22:59, schrieb Johannes Sixt:
> ... I wonder how Junio's last
> example with push.default=simple can work today:
> 
>$ git pull --rebase  # not a merge
>$ git push
> 
> because it is not a fast-forward.

*blush* I was mostly asleep and and totally off the rails when I wrote
this nonsense.

-- Hannes

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


Re: [PATCH 7/7] push: document --lockref

2013-07-15 Thread Johannes Sixt
Am 15.07.2013 05:50, schrieb Junio C Hamano:
> ... or also with your "--lockref is default"
>   $ git push origin +master
> 
>   ... rejected due to stale expectation
> $ git fetch
> 
> You just have updated the lockref base, so if you did, without doing
> anything else, 
> 
>   $ git push origin +master
> 
> then you will lose the updated contents.
> 
> The conclusion?  It does not make sense to make "lockref" the
> default.

Point taken.

> So it will not happen; "lockref" will not be on by default, even if
> it is made independent of "must fast-forward".

OK.

-- 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: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

2013-07-15 Thread Junio C Hamano
Mark Levedahl  writes:

>> In order to limit the adverse effects caused by this implementation,
>> we provide a new "fast stat" interface, which allows us to use this
>> only for interactions with the index (i.e. the cached stat data).
>>
>> Signed-off-by: Ramsay Jones 
>> ---
>
> I've tested this on Cygwin 1.7 on WIndows 7 , comparing to the results
> using your prior patch (removing the Cygwin specific lstat entirely)
> and get the same results with both, so this seems ok from me.
>
> My comparison point was created by reverting your current patch from
> pu, then reapplying your earlier patch on top, so the only difference
> was which approach was used to address the stat functions.
>
> Caveats:
> 1) I don't find any speed improvement of the current patch over the
> previous one (the tests actually ran faster with the earlier patch,
> though the difference was less than 1%).
> 2) I still question this whole approach, especially having this
> non-POSIX compliant mode be the default. Running in this mode breaks
> interoperability with Linux, but providing a Linux environment is the
> *primary* goal of Cygwin.

Sounds like we are better off without this patch, and instead remove
the "schizophrenic stat"?  I do not have a strong opinion either
way, except that I tend to agree with your point 2) above.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jul 2013, #03; Tue, 9)

2013-07-15 Thread Junio C Hamano
Ramsay Jones  writes:

>> [Stalled]
>> 
>> * rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits
>>  - ### DONTMERGE: needs better explanation on what config they need
>>  - pack-refs.c: Add missing call to git_config()
>>  - show-ref.c: Add missing call to git_config()
>> 
>>  The changes themselves are probably good, but it is unclear what
>>  basic setting needs to be read for which exact operation.
>> 
>>  Waiting for clarification.
>>  $gmane/228294
>
> Sorry, still on my TODO list. (Having said that, I'm no longer sure
> that these patches do the right thing! ;-)

Then let's discard them without prejudice.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jul 2013, #05; Fri, 12)

2013-07-15 Thread Junio C Hamano
Matthieu Moy  writes:

> Junio C Hamano  writes:
>
>> * bp/mediawiki-preview (2013-07-08) 7 commits
>>   (merged to 'next' on 2013-07-12 at 870890a)
>>  + git-remote-mediawiki: add preview subcommand into git mw
>>  + git-remote-mediawiki: add git-mw command
>>  + git-remote-mediawiki: factoring code between git-remote-mediawiki and 
>> Git::Mediawiki
>>  + git-remote-mediawiki: update tests to run with the new bin-wrapper
>>  + git-remote-mediawiki: add a git bin-wrapper for developement
>>  + wrap-for-bin: make bin-wrappers chainable
>>  + git-remote-mediawiki: introduction of Git::Mediawiki.pm
>>
>>  Add a command to allow previewing the contents locally before
>>  pushing it out, when working with a MediaWiki remote.
>>
>>  I personally do not think this belongs to Git.  If you are working
>>  on a set of AsciiDoc source files, you sure do want to locally
>>  format to preview what you will be pushing out, and if you are
>>  working on a set of C or Java source files, you do want to test it
>>  before pushing it out, too.  That kind of thing belongs to your
>>  build script, not to your SCM.
>
> There's one big difference: when you use AsciiDoc/C/Java/... your build
> system works locally. "git mw preview" uses the remote wiki to do the
> rendering.
>
> It doesn't do so with the remote-helper interface, but uses
> the same remote and same configuration as the remote-helper, and it
> shares some code with it. It seems logical to let it leave next to the
> remote-helper (but clearly in contrib/, not in the core Git).

Hmph, I do not see it fundamentally different from using distcc to
compile not locally.

Your build infrastructure can (ab)use the configuration mechanism
for Git to store necessary information to find out what workers are
willing to help your compilation.  The mediawiki-preview program is
talking over MediaWiki protocol, and it may reuse the same
credential information stored in the config to talk to the same
MediaWiki installation in connect_maybe() and sending the updated
page contents to ask it to render, but I do not see a fundamental
reason why the worker has to be the same MediaWiki installation as
your "repository".

So, no, I do not see a "big" difference there.

That does not mean I'd retract what I already said in the previous
issue of "What's cooking", i.e.

>> But I'll let it pass, as this is only a contrib/ thing.

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


Re: [PATCH v2 00/46] "struct pathspec" conversion and :(glob) and :(icase)

2013-07-15 Thread Junio C Hamano
Thanks for a reroll.  I am slowly reading it through ;-).
--
To unsubscribe from this list: send the line "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/6] diff: allow --patch & cie to override -s/--no-patch

2013-07-15 Thread Jonathan Nieder
Matthieu Moy wrote:
> Jonathan Nieder  writes:
>> Matthieu Moy wrote:

>>> All options that trigger a patch output now override --no-patch.
>>>
>>> The case of --binary is particular as the name may suggest that it turns
>>
>> Usage nit: this should say "is unusual" 
>
> I don't get it. The point is not that --binary is unusual, but that it
> is a particular case that deserves extra attention.

Ah, so you mean: "The case of --binary deserves extra attention
because ...".

"is particular" would be an unusual expression, meaning something like
"is made of particles".  It's a weird case in English usage where a
word commonly appears attached to a noun ("This particular case") but
cannot be used as the RHS of "is" ("This case is particular").

[...]
>> A couple of other test ideas:
>>
>>  - "git diff-files --patch --no-patch"
>>  - "git diff-files -s --patch-with-stat"
>
> I'd rather avoid having a too long list here, or we'll end-up testing
> all combinations of options ...

Sure.  The point of "--patch --no-patch" is to test that ordering is
respected.  The point of "--no-patch --patch-with-stat" is so we
remember that there are options other than --patch that should
override --no-patch, for example if this code is ever converted to
parse_options some day.

> I'll send a reroll tomorrow.

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


Re: [PATCH v3 3/6] diff: allow --patch & cie to override -s/--no-patch

2013-07-15 Thread Matthieu Moy
Jonathan Nieder  writes:

> Matthieu Moy wrote:
>
>> All options that trigger a patch output now override --no-patch.
>>
>> The case of --binary is particular as the name may suggest that it turns
>
> Usage nit: this should say "is unusual" 

I don't get it. The point is not that --binary is unusual, but that it
is a particular case that deserves extra attention.

> or "In the case of --binary in particular, the name may suggest ...".

Not really. I'd use "in particular" if --binary was a sub-case of the
others, but here I'm precisely saying that it may not be.

>> --- a/t/t4000-diff-format.sh
>> +++ b/t/t4000-diff-format.sh
>> @@ -71,4 +71,9 @@ test_expect_success 'git diff-files --no-patch as synonym 
>> for -s' '
>>  test_must_be_empty err
>>  '
>>  
>> +test_expect_success 'git diff-files --no-patch --patch shows the patch' '
>> +git diff-files --no-patch --patch >diff-np-output 2>err &&
>> +compare_diff_patch expected actual
>
> Shouldn't that be "compare_diff_patch expected diff-np-output"?

Oops, right.

> A couple of other test ideas:
>
>  - "git diff-files --patch --no-patch"
>  - "git diff-files -s --patch-with-stat"

I'd rather avoid having a too long list here, or we'll end-up testing
all combinations of options ...

I'll send a reroll tomorrow.

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


Re: [PATCH v3 6/6] Documentation/git-log.txt: capitalize section names

2013-07-15 Thread Jonathan Nieder
Matthieu Moy wrote:

> This is the convention in other files and even at the beginning of git-log.txt

The docs aren't so consistent on this, but I agree that it makes sense
to at least be consistent within the generated git-log.html. :)

Generally the series looks very good.  Thanks for taking this on.
--
To unsubscribe from this list: send the line "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] Add the NO_SENTINEL build variable

2013-07-15 Thread Jonathan Nieder
Ramsay Jones wrote:

> One of the three gcc compilers that I use does not understand the
> sentinel function attribute. (so, it spews 108 warning messages)

Do you know what version of gcc introduced the sentinel attribute?
Would it make sense for the ifdef in git-compat-util.h to be keyed on 
__GNUC__ and __GNUC_MINOR__ instead of a new makefile flag?

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


Re: [PATCH v3 3/6] diff: allow --patch & cie to override -s/--no-patch

2013-07-15 Thread Jonathan Nieder
Matthieu Moy wrote:

> All options that trigger a patch output now override --no-patch.
>
> The case of --binary is particular as the name may suggest that it turns

Usage nit: this should say "is unusual" or "In the case of --binary in
particular, the name may suggest ...".

> a normal patch into a binary patch, but it actually already enables patch
> output when normally disabled (e.g. "git log --binary" displays a patch),
> hence it makes sense that "git show --no-patch --binary" display the
> binary patch.
[...]
> --- a/t/t4000-diff-format.sh
> +++ b/t/t4000-diff-format.sh
> @@ -71,4 +71,9 @@ test_expect_success 'git diff-files --no-patch as synonym 
> for -s' '
>   test_must_be_empty err
>  '
>  
> +test_expect_success 'git diff-files --no-patch --patch shows the patch' '
> + git diff-files --no-patch --patch >diff-np-output 2>err &&
> + compare_diff_patch expected actual

Shouldn't that be "compare_diff_patch expected diff-np-output"?

A couple of other test ideas:

 - "git diff-files --patch --no-patch"
 - "git diff-files -s --patch-with-stat"

Aside from that, the patch looks good.

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


Re: [PATCH v3 1/6] t4000-diff-format.sh: modernize style

2013-07-15 Thread Jonathan Nieder
Matthieu Moy wrote:

> Signed-off-by: Matthieu Moy 
> ---
>  t/t4000-diff-format.sh | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)

This test script can use more cleanup, but as preparation for later
patches in this series the above is enough. :)  If I forget to do more
cleanup as a followup, feel free to kick me.

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


[PATCH v3 5/6] Documentation: move description of -s, --no-patch to diff-options.txt

2013-07-15 Thread Matthieu Moy
Technically, "-s, --no-patch" is implemented in diff.c ("git diff
--no-patch" is essentially useless, but valid). From the user point of
view, this allows the documentation to show up in "git show --help",
which is one of the most useful use of the option.

While we're there, add a sentence explaining why the option can be
useful.

Signed-off-by: Matthieu Moy 
---
 Documentation/diff-options.txt | 5 +
 Documentation/rev-list-options.txt | 4 
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 87e92d6..bbed2cd 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -26,6 +26,11 @@ ifndef::git-format-patch[]
{git-diff? This is the default.}
 endif::git-format-patch[]
 
+-s::
+--no-patch::
+   Suppress diff output. Useful for commands like `git show` that
+   show the patch by default, or to cancel the effect of `--patch`.
+
 -U::
 --unified=::
Generate diffs with  lines of context instead of
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index c128a85..e632e85 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -849,8 +849,4 @@ options may be given. See linkgit:git-diff-files[1] for 
more options.
 -t::
 
Show the tree objects in the diff output. This implies '-r'.
-
--s::
---no-patch::
-   Suppress diff output.
 endif::git-rev-list[]
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH v3 2/6] diff: allow --no-patch as synonym for -s

2013-07-15 Thread Matthieu Moy
This follows the usual convention of having a --no-foo option to negate
--foo.

Signed-off-by: Matthieu Moy 
---
 Documentation/rev-list-options.txt |  1 +
 diff.c |  2 +-
 t/t4000-diff-format.sh | 12 
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index e157ec3..c128a85 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -851,5 +851,6 @@ options may be given. See linkgit:git-diff-files[1] for 
more options.
Show the tree objects in the diff output. This implies '-r'.
 
 -s::
+--no-patch::
Suppress diff output.
 endif::git-rev-list[]
diff --git a/diff.c b/diff.c
index 6578690..6bd821d 100644
--- a/diff.c
+++ b/diff.c
@@ -3551,7 +3551,7 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
options->output_format |= DIFF_FORMAT_NAME;
else if (!strcmp(arg, "--name-status"))
options->output_format |= DIFF_FORMAT_NAME_STATUS;
-   else if (!strcmp(arg, "-s"))
+   else if (!strcmp(arg, "-s") || !strcmp(arg, "--no-patch"))
options->output_format |= DIFF_FORMAT_NO_OUTPUT;
else if (!prefixcmp(arg, "--stat"))
/* --stat, --stat-width, --stat-name-width, or --stat-count */
diff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
index 2b5dffc..d702575 100755
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -59,4 +59,16 @@ test_expect_success 'validate git diff-files -p output.' '
compare_diff_patch expected actual
 '
 
+test_expect_success 'git diff-files -s after editing work tree' '
+   git diff-files -s >diff-s-output 2>err &&
+   test_must_be_empty diff-s-output &&
+   test_must_be_empty err
+'
+
+test_expect_success 'git diff-files --no-patch as synonym for -s' '
+   git diff-files --no-patch >diff-np-output 2>err &&
+   test_must_be_empty diff-np-output &&
+   test_must_be_empty err
+'
+
 test_done
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH v3 6/6] Documentation/git-log.txt: capitalize section names

2013-07-15 Thread Matthieu Moy
This is the convention in other files and even at the beginning of git-log.txt

Signed-off-by: Matthieu Moy 
---
 Documentation/git-log.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 2ea79ba..2eda5e4 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -97,7 +97,7 @@ include::rev-list-options.txt[]
 
 include::pretty-formats.txt[]
 
-Common diff options
+COMMON DIFF OPTIONS
 ---
 
 :git-log: 1
@@ -105,7 +105,7 @@ include::diff-options.txt[]
 
 include::diff-generate-patch.txt[]
 
-Examples
+EXAMPLES
 
 `git log --no-merges`::
 
@@ -161,12 +161,12 @@ git log -L '/int main/',/^}/:main.c::
 `git log -3`::
Limits the number of commits to show to 3.
 
-Discussion
+DISCUSSION
 --
 
 include::i18n.txt[]
 
-Configuration
+CONFIGURATION
 -
 
 See linkgit:git-config[1] for core variables and linkgit:git-diff[1]
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH v3 4/6] Documentation/git-show.txt: include common diff options, like git-log.txt

2013-07-15 Thread Matthieu Moy
Signed-off-by: Matthieu Moy 
---
 Documentation/git-show.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
index ae4edcc..4e617e6 100644
--- a/Documentation/git-show.txt
+++ b/Documentation/git-show.txt
@@ -45,6 +45,15 @@ include::pretty-options.txt[]
 include::pretty-formats.txt[]
 
 
+COMMON DIFF OPTIONS
+---
+
+:git-log: 1
+include::diff-options.txt[]
+
+include::diff-generate-patch.txt[]
+
+
 EXAMPLES
 
 
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH v3 3/6] diff: allow --patch & cie to override -s/--no-patch

2013-07-15 Thread Matthieu Moy
All options that trigger a patch output now override --no-patch.

The case of --binary is particular as the name may suggest that it turns
a normal patch into a binary patch, but it actually already enables patch
output when normally disabled (e.g. "git log --binary" displays a patch),
hence it makes sense that "git show --no-patch --binary" display the
binary patch.

Signed-off-by: Matthieu Moy 
---
 diff.c | 28 +---
 t/t4000-diff-format.sh |  5 +
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/diff.c b/diff.c
index 6bd821d..66ab714 100644
--- a/diff.c
+++ b/diff.c
@@ -3508,6 +3508,11 @@ static int parse_submodule_opt(struct diff_options 
*options, const char *value)
return 1;
 }
 
+static void enable_patch_output(int *fmt) {
+   *fmt &= ~DIFF_FORMAT_NO_OUTPUT;
+   *fmt |= DIFF_FORMAT_PATCH;
+}
+
 int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 {
const char *arg = av[0];
@@ -3515,15 +3520,15 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
int argcount;
 
/* Output format options */
-   if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch"))
-   options->output_format |= DIFF_FORMAT_PATCH;
-   else if (opt_arg(arg, 'U', "unified", &options->context))
-   options->output_format |= DIFF_FORMAT_PATCH;
+   if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch")
+   || opt_arg(arg, 'U', "unified", &options->context))
+   enable_patch_output(&options->output_format);
else if (!strcmp(arg, "--raw"))
options->output_format |= DIFF_FORMAT_RAW;
-   else if (!strcmp(arg, "--patch-with-raw"))
-   options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW;
-   else if (!strcmp(arg, "--numstat"))
+   else if (!strcmp(arg, "--patch-with-raw")) {
+   enable_patch_output(&options->output_format);
+   options->output_format |= DIFF_FORMAT_RAW;
+   } else if (!strcmp(arg, "--numstat"))
options->output_format |= DIFF_FORMAT_NUMSTAT;
else if (!strcmp(arg, "--shortstat"))
options->output_format |= DIFF_FORMAT_SHORTSTAT;
@@ -3545,9 +3550,10 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
options->output_format |= DIFF_FORMAT_CHECKDIFF;
else if (!strcmp(arg, "--summary"))
options->output_format |= DIFF_FORMAT_SUMMARY;
-   else if (!strcmp(arg, "--patch-with-stat"))
-   options->output_format |= DIFF_FORMAT_PATCH | 
DIFF_FORMAT_DIFFSTAT;
-   else if (!strcmp(arg, "--name-only"))
+   else if (!strcmp(arg, "--patch-with-stat")) {
+   enable_patch_output(&options->output_format);
+   options->output_format |= DIFF_FORMAT_DIFFSTAT;
+   } else if (!strcmp(arg, "--name-only"))
options->output_format |= DIFF_FORMAT_NAME;
else if (!strcmp(arg, "--name-status"))
options->output_format |= DIFF_FORMAT_NAME_STATUS;
@@ -3624,7 +3630,7 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
 
/* flags options */
else if (!strcmp(arg, "--binary")) {
-   options->output_format |= DIFF_FORMAT_PATCH;
+   enable_patch_output(&options->output_format);
DIFF_OPT_SET(options, BINARY);
}
else if (!strcmp(arg, "--full-index"))
diff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
index d702575..4fce12f 100755
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -71,4 +71,9 @@ test_expect_success 'git diff-files --no-patch as synonym for 
-s' '
test_must_be_empty err
 '
 
+test_expect_success 'git diff-files --no-patch --patch shows the patch' '
+   git diff-files --no-patch --patch >diff-np-output 2>err &&
+   compare_diff_patch expected actual
+'
+
 test_done
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH v3 0/6] Make "git show -s" easier to discover for users

2013-07-15 Thread Matthieu Moy
Compared to v2, I just added tests. Strongly inspired from Jonathan's,
but there's one more, and I chose the "modern" indentation style
(hence a clean-up patch before, to avoid mixed-style in the same file).

Matthieu Moy (6):
  t4000-diff-format.sh: modernize style
  diff: allow --no-patch as synonym for -s
  diff: allow --patch & cie to override -s/--no-patch
  Documentation/git-show.txt: include common diff options, like
git-log.txt
  Documentation: move description of -s, --no-patch to diff-options.txt
  Documentation/git-log.txt: capitalize section names

 Documentation/diff-options.txt |  5 +
 Documentation/git-log.txt  |  8 
 Documentation/git-show.txt |  9 +
 Documentation/rev-list-options.txt |  3 ---
 diff.c | 30 ++
 t/t4000-diff-format.sh | 35 ++-
 6 files changed, 62 insertions(+), 28 deletions(-)

-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH v3 1/6] t4000-diff-format.sh: modernize style

2013-07-15 Thread Matthieu Moy
Signed-off-by: Matthieu Moy 
---
 t/t4000-diff-format.sh | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
index 6ddd469..2b5dffc 100755
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -15,17 +15,17 @@ line 3'
 cat path0 >path1
 chmod +x path1
 
-test_expect_success \
-'update-index --add two files with and without +x.' \
-'git update-index --add path0 path1'
+test_expect_success 'update-index --add two files with and without +x.' '
+   git update-index --add path0 path1
+'
 
 mv path0 path0-
 sed -e 's/line/Line/' path0
 chmod +x path0
 rm -f path1
-test_expect_success \
-'git diff-files -p after editing work tree.' \
-'git diff-files -p >current'
+test_expect_success 'git diff-files -p after editing work tree.' '
+   git diff-files -p >actual
+'
 
 # that's as far as it comes
 if [ "$(git config --get core.filemode)" = false ]
@@ -55,8 +55,8 @@ deleted file mode 100755
 -line 3
 EOF
 
-test_expect_success \
-'validate git diff-files -p output.' \
-'compare_diff_patch current expected'
+test_expect_success 'validate git diff-files -p output.' '
+   compare_diff_patch expected actual
+'
 
 test_done
-- 
1.8.3.1.495.g13f33cf.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


[RFC/PATCH] Add the NO_SENTINEL build variable

2013-07-15 Thread Ramsay Jones

Signed-off-by: Ramsay Jones 
---

Hi Jeff,

One of the three gcc compilers that I use does not understand the
sentinel function attribute. (so, it spews 108 warning messages)

Is this, or something like it, too ugly for you to squash into
your patch? :-D

ATB,
Ramsay Jones


 Makefile  | 6 ++
 argv-array.h  | 2 +-
 builtin/revert.c  | 4 ++--
 exec_cmd.h| 2 +-
 git-compat-util.h | 6 ++
 run-command.h | 2 +-
 6 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 9c0da06..63b539c 100644
--- a/Makefile
+++ b/Makefile
@@ -224,6 +224,9 @@ all::
 # Define NO_NORETURN if using buggy versions of gcc 4.6+ and profile feedback,
 # as the compiler can crash (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49299)
 #
+# Define NO_SENTINEL if you have a compiler which does not understand the
+# sentinel function attribute.
+#
 # Define USE_NSEC below if you want git to care about sub-second file mtimes
 # and ctimes. Note that you need recent glibc (at least 2.2.4) for this, and
 # it will BREAK YOUR LOCAL DIFFS! show-diff and anything using it will likely
@@ -1232,6 +1235,9 @@ endif
 ifdef NO_NORETURN
BASIC_CFLAGS += -DNO_NORETURN
 endif
+ifdef NO_SENTINEL
+   BASIC_CFLAGS += -DNO_SENTINEL
+endif
 ifdef NO_NSEC
BASIC_CFLAGS += -DNO_NSEC
 endif
diff --git a/argv-array.h b/argv-array.h
index e805748..31bc492 100644
--- a/argv-array.h
+++ b/argv-array.h
@@ -15,7 +15,7 @@ void argv_array_init(struct argv_array *);
 void argv_array_push(struct argv_array *, const char *);
 __attribute__((format (printf,2,3)))
 void argv_array_pushf(struct argv_array *, const char *fmt, ...);
-__attribute__((sentinel))
+SENTINEL(0)
 void argv_array_pushl(struct argv_array *, ...);
 void argv_array_pop(struct argv_array *);
 void argv_array_clear(struct argv_array *);
diff --git a/builtin/revert.c b/builtin/revert.c
index b8b5174..6aedc18 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -54,7 +54,7 @@ static int option_parse_x(const struct option *opt,
return 0;
 }
 
-__attribute__((sentinel))
+SENTINEL(0)
 static void verify_opt_compatible(const char *me, const char *base_opt, ...)
 {
const char *this_opt;
@@ -71,7 +71,7 @@ static void verify_opt_compatible(const char *me, const char 
*base_opt, ...)
die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
 }
 
-__attribute__((sentinel))
+SENTINEL(0)
 static void verify_opt_mutually_compatible(const char *me, ...)
 {
const char *opt1, *opt2 = NULL;
diff --git a/exec_cmd.h b/exec_cmd.h
index 307b55c..75c0a82 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -7,7 +7,7 @@ extern const char *git_exec_path(void);
 extern void setup_path(void);
 extern const char **prepare_git_cmd(const char **argv);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
-__attribute__((sentinel))
+SENTINEL(0)
 extern int execl_git_cmd(const char *cmd, ...);
 extern const char *system_path(const char *path);
 
diff --git a/git-compat-util.h b/git-compat-util.h
index 9f1eaca..e846e01 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -300,6 +300,12 @@ extern char *gitbasename(char *);
 #endif
 #endif
 
+#if defined(__GNUC__) && !defined(NO_SENTINEL)
+#define SENTINEL(n) __attribute__((sentinel(n)))
+#else
+#define SENTINEL(n)
+#endif
+
 #include "compat/bswap.h"
 
 #ifdef USE_WILDMATCH
diff --git a/run-command.h b/run-command.h
index 0a47679..8e75671 100644
--- a/run-command.h
+++ b/run-command.h
@@ -46,7 +46,7 @@ int finish_command(struct child_process *);
 int run_command(struct child_process *);
 
 extern char *find_hook(const char *name);
-__attribute__((sentinel))
+SENTINEL(0)
 extern int run_hook(const char *index_file, const char *name, ...);
 
 #define RUN_COMMAND_NO_STDIN 1
-- 
1.8.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


[PATCH] Fix some sparse warnings

2013-07-15 Thread Ramsay Jones

Sparse issues three "Using plain integer as NULL pointer" warnings.
Each warning relates to the use of an '{0}' initialiser expression
in the declaration of an 'struct object_info'. The first field of
this structure has pointer type. Thus, in order to suppress these
warnings, we replace the initialiser expression with '{NULL}'.

Signed-off-by: Ramsay Jones 
---

Hi Jeff,

If you need to re-roll the patches in your 'jk/in-pack-size-measurement'
branch, could you please squash this (or something like it) into the
patches equivalent to commit 7c07385d ("zero-initialize object_info structs",
07-07-2013) [sha1_file.c and streaming.c] and commit 778d263a ("cat-file: add
--batch-disk-sizes option", 07-07-2013) [builtin/cat-file.c].

Thanks!

ATB,
Ramsay Jones


 builtin/cat-file.c | 2 +-
 sha1_file.c| 2 +-
 streaming.c| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index bf12883..860576e 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -135,7 +135,7 @@ static int batch_one_object(const char *obj_name, int 
print_contents)
if (print_contents == BATCH)
contents = read_sha1_file(sha1, &type, &size);
else if (print_contents == BATCH_DISK_SIZES) {
-   struct object_info oi = {0};
+   struct object_info oi = {NULL};
oi.disk_sizep = &size;
type = sha1_object_info_extended(sha1, &oi);
}
diff --git a/sha1_file.c b/sha1_file.c
index 4c2365f..e4ab0a4 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2440,7 +2440,7 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi)
 
 int sha1_object_info(const unsigned char *sha1, unsigned long *sizep)
 {
-   struct object_info oi = {0};
+   struct object_info oi = {NULL};
 
oi.sizep = sizep;
return sha1_object_info_extended(sha1, &oi);
diff --git a/streaming.c b/streaming.c
index cac282f..5710065 100644
--- a/streaming.c
+++ b/streaming.c
@@ -135,7 +135,7 @@ struct git_istream *open_istream(const unsigned char *sha1,
 struct stream_filter *filter)
 {
struct git_istream *st;
-   struct object_info oi = {0};
+   struct object_info oi = {NULL};
const unsigned char *real = lookup_replace_object(sha1);
enum input_source src = istream_source(real, type, &oi);
 
-- 
1.8.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: What's cooking in git.git (Jul 2013, #03; Tue, 9)

2013-07-15 Thread Ramsay Jones
Junio C Hamano wrote:
[ ... ]
> * rr/send-email-ssl-verify (2013-07-06) 6 commits
>  - SQUASH??? update to support SSL_ca_file as well as SSL_ca_path
>  - SQUASH??? send-email: cover both smtps and starttls cases
>  - fixup! send-email: squelch warning from Net::SMTP::SSL
>  - SQUASH??? send-email giving default value to ssl-cert-path with ||= 
> assignment
>  - send-email: introduce sendemail.smtpsslcertpath
>  - send-email: squelch warning from Net::SMTP::SSL
> 
>  The issue seems a lot deeper than the initial attempt and needs
>  somebody to sit down and sort out to get the version dependencies
>  and lazy loading right.

This causes test failures for me, since send-email can't load the
IO/Socket/SSL.pm module. (on Linux, cygwin and MinGW!)

[ ... ]

> --
> [Stalled]
> 
> * rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits
>  - ### DONTMERGE: needs better explanation on what config they need
>  - pack-refs.c: Add missing call to git_config()
>  - show-ref.c: Add missing call to git_config()
> 
>  The changes themselves are probably good, but it is unclear what
>  basic setting needs to be read for which exact operation.
> 
>  Waiting for clarification.
>  $gmane/228294

Sorry, still on my TODO list. (Having said that, I'm no longer sure
that these patches do the right thing! ;-)

ATB,
Ramsay Jones


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


Re: repo consistency under crashes and power failures?

2013-07-15 Thread Jonathan Nieder
Greg Troxel wrote:

> Alternatively, is there somewhere a first-principles analysis vs POSIX
> specs (such as fsyncing object files before updating refs to point to
> them, which I realize has performance negatives)?

You might be interested in the 'core.fsyncobjectfiles' setting.
git-config(1) has details.

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


repo consistency under crashes and power failures?

2013-07-15 Thread Greg Troxel

Clearly there is the possibility of creating a corrupt repository when
receiving objects and updating refs, if a crash or power failure causes
data not to get written to disk but that data is pointed to.  Journaling
mitigates this, but I'd argue that programs should function safely with
only the guarantees from POSIX.

I am curious if anyone has actual experiences to share, either

  a report of corruption after a crash (where corruption means that
  either 1) git fsck reports worse than dangling objects or 2) some ref
  did not either point to the old place or the new place)

  experiments intended to provoke corruption, like dropping power during
  pushes, or forced panics in the kernel due to timers, etc.

Alternatively, is there somewhere a first-principles analysis vs POSIX
specs (such as fsyncing object files before updating refs to point to
them, which I realize has performance negatives)?

(I have not done experiments, but have observed no corruption.)

Thanks,
Greg


pgpgbG9bqc3bd.pgp
Description: PGP signature


Re: [PATCH v2 1/5] diff: allow --no-patch as synonym for -s

2013-07-15 Thread Jonathan Nieder
Matthieu Moy wrote:

> --- a/diff.c
> +++ b/diff.c
> @@ -3551,7 +3551,7 @@ int diff_opt_parse(struct diff_options *options, const 
> char **av, int ac)
>   options->output_format |= DIFF_FORMAT_NAME;
>   else if (!strcmp(arg, "--name-status"))
>   options->output_format |= DIFF_FORMAT_NAME_STATUS;
> - else if (!strcmp(arg, "-s"))
> + else if (!strcmp(arg, "-s") || !strcmp(arg, "--no-patch"))
>   options->output_format |= DIFF_FORMAT_NO_OUTPUT;

Very nice idea.

Tests?  E.g.:

diff --git i/t/t4000-diff-format.sh w/t/t4000-diff-format.sh
index 6ddd469..0fa7380 100755
--- i/t/t4000-diff-format.sh
+++ w/t/t4000-diff-format.sh
@@ -59,4 +59,18 @@ test_expect_success \
 'validate git diff-files -p output.' \
 'compare_diff_patch current expected'
 
+test_expect_success \
+'git diff-files -s after editing work tree' \
+'>empty &&
+git diff-files -s >diff-s-output 2>err &&
+test_cmp empty diff-s-output &&
+test_cmp empty err'
+
+test_expect_success \
+'git diff-files --no-patch as synonym for -s' \
+'>empty &&
+git diff-files --no-patch >diff-np-output 2>err &&
+test_cmp empty diff-np-output &&
+test_cmp empty err'
+
 test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/5] diff: allow --patch & cie to override -s/--no-patch

2013-07-15 Thread Matthieu Moy
All options that trigger a patch output now override --no-patch.

The case of --binary is particular as the name may suggest that it turns
a normal patch into a binary patch, but it actually already enables patch
output when normally disabled (e.g. "git log --binary" displays a patch),
hence it makes sense that "git show --no-patch --binary" display the
binary patch.

Signed-off-by: Matthieu Moy 
---
This is the one which changed.

 diff.c | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/diff.c b/diff.c
index 6bd821d..66ab714 100644
--- a/diff.c
+++ b/diff.c
@@ -3508,6 +3508,11 @@ static int parse_submodule_opt(struct diff_options 
*options, const char *value)
return 1;
 }
 
+static void enable_patch_output(int *fmt) {
+   *fmt &= ~DIFF_FORMAT_NO_OUTPUT;
+   *fmt |= DIFF_FORMAT_PATCH;
+}
+
 int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 {
const char *arg = av[0];
@@ -3515,15 +3520,15 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
int argcount;
 
/* Output format options */
-   if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch"))
-   options->output_format |= DIFF_FORMAT_PATCH;
-   else if (opt_arg(arg, 'U', "unified", &options->context))
-   options->output_format |= DIFF_FORMAT_PATCH;
+   if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch")
+   || opt_arg(arg, 'U', "unified", &options->context))
+   enable_patch_output(&options->output_format);
else if (!strcmp(arg, "--raw"))
options->output_format |= DIFF_FORMAT_RAW;
-   else if (!strcmp(arg, "--patch-with-raw"))
-   options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW;
-   else if (!strcmp(arg, "--numstat"))
+   else if (!strcmp(arg, "--patch-with-raw")) {
+   enable_patch_output(&options->output_format);
+   options->output_format |= DIFF_FORMAT_RAW;
+   } else if (!strcmp(arg, "--numstat"))
options->output_format |= DIFF_FORMAT_NUMSTAT;
else if (!strcmp(arg, "--shortstat"))
options->output_format |= DIFF_FORMAT_SHORTSTAT;
@@ -3545,9 +3550,10 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
options->output_format |= DIFF_FORMAT_CHECKDIFF;
else if (!strcmp(arg, "--summary"))
options->output_format |= DIFF_FORMAT_SUMMARY;
-   else if (!strcmp(arg, "--patch-with-stat"))
-   options->output_format |= DIFF_FORMAT_PATCH | 
DIFF_FORMAT_DIFFSTAT;
-   else if (!strcmp(arg, "--name-only"))
+   else if (!strcmp(arg, "--patch-with-stat")) {
+   enable_patch_output(&options->output_format);
+   options->output_format |= DIFF_FORMAT_DIFFSTAT;
+   } else if (!strcmp(arg, "--name-only"))
options->output_format |= DIFF_FORMAT_NAME;
else if (!strcmp(arg, "--name-status"))
options->output_format |= DIFF_FORMAT_NAME_STATUS;
@@ -3624,7 +3630,7 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
 
/* flags options */
else if (!strcmp(arg, "--binary")) {
-   options->output_format |= DIFF_FORMAT_PATCH;
+   enable_patch_output(&options->output_format);
DIFF_OPT_SET(options, BINARY);
}
else if (!strcmp(arg, "--full-index"))
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH v2 1/5] diff: allow --no-patch as synonym for -s

2013-07-15 Thread Matthieu Moy
This follows the usual convention of having a --no-foo option to negate
--foo.

Signed-off-by: Matthieu Moy 
---
 Documentation/rev-list-options.txt | 1 +
 diff.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index e157ec3..c128a85 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -851,5 +851,6 @@ options may be given. See linkgit:git-diff-files[1] for 
more options.
Show the tree objects in the diff output. This implies '-r'.
 
 -s::
+--no-patch::
Suppress diff output.
 endif::git-rev-list[]
diff --git a/diff.c b/diff.c
index 6578690..6bd821d 100644
--- a/diff.c
+++ b/diff.c
@@ -3551,7 +3551,7 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
options->output_format |= DIFF_FORMAT_NAME;
else if (!strcmp(arg, "--name-status"))
options->output_format |= DIFF_FORMAT_NAME_STATUS;
-   else if (!strcmp(arg, "-s"))
+   else if (!strcmp(arg, "-s") || !strcmp(arg, "--no-patch"))
options->output_format |= DIFF_FORMAT_NO_OUTPUT;
else if (!prefixcmp(arg, "--stat"))
/* --stat, --stat-width, --stat-name-width, or --stat-count */
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH v2 5/5] Documentation/git-log.txt: capitalize section names

2013-07-15 Thread Matthieu Moy
This is the convention in other files and even at the beginning of git-log.txt

Signed-off-by: Matthieu Moy 
---
 Documentation/git-log.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 2ea79ba..2eda5e4 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -97,7 +97,7 @@ include::rev-list-options.txt[]
 
 include::pretty-formats.txt[]
 
-Common diff options
+COMMON DIFF OPTIONS
 ---
 
 :git-log: 1
@@ -105,7 +105,7 @@ include::diff-options.txt[]
 
 include::diff-generate-patch.txt[]
 
-Examples
+EXAMPLES
 
 `git log --no-merges`::
 
@@ -161,12 +161,12 @@ git log -L '/int main/',/^}/:main.c::
 `git log -3`::
Limits the number of commits to show to 3.
 
-Discussion
+DISCUSSION
 --
 
 include::i18n.txt[]
 
-Configuration
+CONFIGURATION
 -
 
 See linkgit:git-config[1] for core variables and linkgit:git-diff[1]
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH v2 3/5] Documentation/git-show.txt: include common diff options, like git-log.txt

2013-07-15 Thread Matthieu Moy
Signed-off-by: Matthieu Moy 
---
 Documentation/git-show.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
index ae4edcc..4e617e6 100644
--- a/Documentation/git-show.txt
+++ b/Documentation/git-show.txt
@@ -45,6 +45,15 @@ include::pretty-options.txt[]
 include::pretty-formats.txt[]
 
 
+COMMON DIFF OPTIONS
+---
+
+:git-log: 1
+include::diff-options.txt[]
+
+include::diff-generate-patch.txt[]
+
+
 EXAMPLES
 
 
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH v2 0/5] Make "git show -s" easier to discover for users

2013-07-15 Thread Matthieu Moy
This fixes the issue found by Junio where "git log --no-patch -u" was
showing the patch, but not "git log --no-patch -U8". Other patches are
unmodified.

Matthieu Moy (5):
  diff: allow --no-patch as synonym for -s
  diff: allow --patch & cie to override -s/--no-patch
  Documentation/git-show.txt: include common diff options, like
git-log.txt
  Documentation: move description of -s, --no-patch to diff-options.txt
  Documentation/git-log.txt: capitalize section names

 Documentation/diff-options.txt |  5 +
 Documentation/git-log.txt  |  8 
 Documentation/git-show.txt |  9 +
 Documentation/rev-list-options.txt |  3 ---
 diff.c | 30 ++
 5 files changed, 36 insertions(+), 19 deletions(-)

-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH v2 4/5] Documentation: move description of -s, --no-patch to diff-options.txt

2013-07-15 Thread Matthieu Moy
Technically, "-s, --no-patch" is implemented in diff.c ("git diff
--no-patch" is essentially useless, but valid). From the user point of
view, this allows the documentation to show up in "git show --help",
which is one of the most useful use of the option.

While we're there, add a sentence explaining why the option can be
useful.

Signed-off-by: Matthieu Moy 
---
 Documentation/diff-options.txt | 5 +
 Documentation/rev-list-options.txt | 4 
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 87e92d6..bbed2cd 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -26,6 +26,11 @@ ifndef::git-format-patch[]
{git-diff? This is the default.}
 endif::git-format-patch[]
 
+-s::
+--no-patch::
+   Suppress diff output. Useful for commands like `git show` that
+   show the patch by default, or to cancel the effect of `--patch`.
+
 -U::
 --unified=::
Generate diffs with  lines of context instead of
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index c128a85..e632e85 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -849,8 +849,4 @@ options may be given. See linkgit:git-diff-files[1] for 
more options.
 -t::
 
Show the tree objects in the diff output. This implies '-r'.
-
--s::
---no-patch::
-   Suppress diff output.
 endif::git-rev-list[]
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH] howto: Use all-space indentation in ASCII art

2013-07-15 Thread Dirk Wallenstein
Keep the sketch aligned independent of the tabstop width used.

Signed-off-by: Dirk Wallenstein 
---
 Documentation/howto/revert-a-faulty-merge.txt | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/howto/revert-a-faulty-merge.txt 
b/Documentation/howto/revert-a-faulty-merge.txt
index 075418e..4b75bfc 100644
--- a/Documentation/howto/revert-a-faulty-merge.txt
+++ b/Documentation/howto/revert-a-faulty-merge.txt
@@ -30,7 +30,7 @@ The history immediately after the "revert of the merge" would 
look like
 this:
 
  ---o---o---o---M---x---x---W
-  /
+   /
---A---B
 
 where A and B are on the side development that was not so good, M is the
@@ -47,7 +47,7 @@ After the developers of the side branch fix their mistakes, 
the history
 may look like this:
 
  ---o---o---o---M---x---x---W---x
-  /
+   /
---A---B---C---D
 
 where C and D are to fix what was broken in A and B, and you may already
@@ -81,7 +81,7 @@ In such a situation, you would want to first revert the 
previous revert,
 which would make the history look like this:
 
  ---o---o---o---M---x---x---W---x---Y
-  /
+   /
---A---B---C---D
 
 where Y is the revert of W.  Such a "revert of the revert" can be done
@@ -93,14 +93,14 @@ This history would (ignoring possible conflicts between 
what W and W..Y
 changed) be equivalent to not having W nor Y at all in the history:
 
  ---o---o---o---M---x---x---x
-  /
+   /
---A---B---C---D
 
 and merging the side branch again will not have conflict arising from an
 earlier revert and revert of the revert.
 
  ---o---o---o---M---x---x---x---*
-  /   /
+   /   /
---A---B---C---D
 
 Of course the changes made in C and D still can conflict with what was
@@ -111,13 +111,13 @@ faulty A and B, and redone the changes on top of the 
updated mainline
 after the revert, the history would have looked like this:
 
  ---o---o---o---M---x---x---W---x---x
-  / \
+   / \
---A---B   A'--B'--C'
 
 If you reverted the revert in such a case as in the previous example:
 
  ---o---o---o---M---x---x---W---x---x---Y---*
-  / \ /
+   / \ /
---A---B   A'--B'--C'
 
 where Y is the revert of W, A' and B' are rerolled A and B, and there may
@@ -129,7 +129,7 @@ lot of overlapping changes that result in conflicts.  So do 
not do "revert
 of revert" blindly without thinking..
 
  ---o---o---o---M---x---x---W---x---x
-  / \
+   / \
---A---B   A'--B'--C'
 
 In the history with rebased side branch, W (and M) are behind the merge
-- 
1.8.3.2.50.g531c8dd

--
To unsubscribe from this list: send the line "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 4/6] Documentation: Update manpage for pre-commit hook

2013-07-15 Thread Junio C Hamano
Richard Hartmann  writes:

> Verbatim copy of 4b8234b2693af634a77ea059331d1658e070f6d7 in original
> patch series from 2013-06-10.

As Jonathan said, this is not a commit log message.

I've applied up to 3/6 with fixups, but will stop here for now.

>
> Signed-off-by: Richard Hartmann 
> ---
>  Documentation/githooks.txt |3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index b9003fe..1276730 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -80,7 +80,8 @@ causes the 'git commit' to abort.
>  
>  The default 'pre-commit' hook, when enabled, catches introduction
>  of lines with trailing whitespaces and aborts the commit when
> -such a line is found.
> +such a line is found. It will also prevent addition of non-ASCII
> +file names.
>  
>  All the 'git commit' hooks are invoked with the environment
>  variable `GIT_EDITOR=:` if the command will not bring up an editor
--
To unsubscribe from this list: send the line "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/6] templates: Use heredoc in pre-commit hook

2013-07-15 Thread Junio C Hamano
Richard Hartmann  writes:

> On Sun, Jul 14, 2013 at 9:20 PM, Junio C Hamano  wrote:
>
>> Shells on modern distros and platforms have "echo" built-in, so this
>> patch replaces series of writes internal to the shell with a fork to
>> cat with heredoc (which often is implemented with a temporary file).
>
> True; fwiw, I replaced my one single echo with heredoc as you
> suggested I do that. I don't mind undoing that, or I can drop it from
> this series altogether.

The _real_ reason you wanted to do this change in the context of
this series is to make it easier to reword the messages and also
have the messages span the full width of the source line, to match
the expected output better, isn't it?  Git is not _only_ about
performance, so even if using "cat 

Re: [PATCH 2/5] diff: allow --patch to override -s/--no-patch

2013-07-15 Thread Junio C Hamano
Matthieu Moy  writes:

> Junio C Hamano  writes:
>
>> I am wondering if the difference after this patch between "-p" and
>> "-U8" is deliberate, or just an accident coming from the way the
>> original was written in ee1e5412 (git diff: support "-U" and
>> "--unified" options properly, 2006-05-13).
>
> No, it isn't. I just didn't notice the -U case.
>
>> If the original were written in this way:
>>
>>  if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch") 
>> ||
>> opt_arg(arg, 'U', "unified", &options->context))
>>  options->output_format |= DIFF_FORMAT_PATCH;
>
> Yes, this seems to be a better way.
>
> There are other cases like --patch-with-raw, I'll send a reroll.

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


Re: [PATCH] show-ref: make --head always show the HEAD ref

2013-07-15 Thread Junio C Hamano
Doug Bell  writes:

> diff --git a/builtin/show-ref.c b/builtin/show-ref.c
> index 4a0310d..4b069e7 100644
> --- a/builtin/show-ref.c
> +++ b/builtin/show-ref.c
> @@ -31,6 +31,9 @@ static int show_ref(const char *refname, const unsigned 
> char *sha1, int flag, vo
>   const char *hex;
>   unsigned char peeled[20];
>  
> + if (show_head && !strncmp(refname, "HEAD\0", 5))
> + goto match;
> +
>   if (tags_only || heads_only) {
>   int match;

I think !strcmp(refname, "HEAD") is better up there.

Also we would want some basic test, which so far seems to be lacking
for the command.

How about this squashed in?

 builtin/show-ref.c   |   2 +-
 t/t1403-show-ref.txt | 167 +++
 2 files changed, 168 insertions(+), 1 deletion(-)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 497ad66..099c2a4 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -31,7 +31,7 @@ static int show_ref(const char *refname, const unsigned char 
*sha1, int flag, vo
const char *hex;
unsigned char peeled[20];
 
-   if (show_head && !strncmp(refname, "HEAD\0", 5))
+   if (show_head && !strcmp(refname, "HEAD"))
goto match;
 
if (tags_only || heads_only) {
diff --git a/t/t1403-show-ref.txt b/t/t1403-show-ref.txt
new file mode 100755
index 000..3e500ed
--- /dev/null
+++ b/t/t1403-show-ref.txt
@@ -0,0 +1,167 @@
+#!/bin/sh
+
+test_description='show-ref'
+. ./test-lib.sh
+
+test_expect_success setup '
+   test_commit A &&
+   git tag -f -a -m "annotated A" A &&
+   git checkout -b side &&
+   test_commit B &&
+   git tag -f -a -m "annotated B" B &&
+   git checkout master &&
+   test_commit C &&
+   git branch B A^0
+'
+
+test_expect_success 'show-ref' '
+   echo $(git rev-parse refs/tags/A) refs/tags/A >expect &&
+
+   git show-ref A >actual &&
+   test_cmp expect actual &&
+
+   git show-ref tags/A >actual &&
+   test_cmp expect actual &&
+
+   git show-ref refs/tags/A >actual &&
+   test_cmp expect actual &&
+
+   >expect &&
+
+   test_must_fail git show-ref D >actual
+   test_cmp expect actual
+'
+
+test_expect_success 'show-ref -q' '
+   >expect &&
+
+   git show-ref -q A >actual &&
+   test_cmp expect actual &&
+
+   git show-ref -q tags/A >actual &&
+   test_cmp expect actual &&
+
+   git show-ref -q refs/tags/A >actual &&
+   test_cmp expect actual &&
+
+   test_must_fail git show-ref -q D >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'show-ref --verify' '
+   echo $(git rev-parse refs/tags/A) refs/tags/A >expect &&
+
+   git show-ref --verify refs/tags/A >actual &&
+   test_cmp expect actual &&
+
+   >expect &&
+
+   test_must_fail git show-ref --verify A >actual &&
+   test_cmp expect actual &&
+
+   test_must_fail git show-ref --verify tags/A >actual &&
+   test_cmp expect actual &&
+
+   test_must_fail git show-ref --verify D >actual
+   test_cmp expect actual
+'
+
+test_expect_success 'show-ref --verify -q' '
+   >expect &&
+
+   git show-ref --verify -q refs/tags/A >actual &&
+   test_cmp expect actual &&
+
+   test_must_fail git show-ref --verify -q A >actual &&
+   test_cmp expect actual &&
+
+   test_must_fail git show-ref --verify -q tags/A >actual &&
+   test_cmp expect actual &&
+
+   test_must_fail git show-ref --verify -q D >actual
+   test_cmp expect actual
+'
+
+test_expect_success 'show-ref -d' '
+   {
+   echo $(git rev-parse refs/tags/A) refs/tags/A &&
+   echo $(git rev-parse refs/tags/A^0) "refs/tags/A^{}"
+   echo $(git rev-parse refs/tags/C) refs/tags/C
+   } >expect &&
+   git show-ref -d A C >actual &&
+   test_cmp expect actual &&
+
+   git show-ref -d tags/A tags/C >actual &&
+   test_cmp expect actual &&
+
+   git show-ref -d refs/tags/A refs/tags/C >actual &&
+   test_cmp expect actual &&
+
+   echo $(git rev-parse refs/heads/master) refs/heads/master >expect &&
+   git show-ref -d master >actual &&
+   test_cmp expect actual &&
+
+   git show-ref -d heads/master >actual &&
+   test_cmp expect actual &&
+
+   git show-ref -d refs/heads/master >actual &&
+   test_cmp expect actual
+
+   git show-ref -d --verify refs/heads/master >actual &&
+   test_cmp expect actual
+
+   >expect &&
+
+   test_must_fail git show-ref -d --verify master >actual &&
+   test_cmp expect actual &&
+
+   test_must_fail git show-ref -d --verify heads/master >actual &&
+   test_cmp expect actual
+
+'
+
+test_expect_success 'show-ref --heads, --tags, --head, pattern' '
+   for branch in B master side
+   do
+   echo $(git rev-parse refs/heads/$branch) refs/heads/$branch
+   done >expect.heads &&
+   git show-ref --heads >actual &&
+   

Re: [PATCH] mailmap: Testing the single letter name case.

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


Re: [PATCH] http.c: fix parsing of http.sslCertPasswordProtected variable

2013-07-15 Thread Junio C Hamano
"Kyle J. McKay"  writes:

> That works fine for GIT_SSL_CERT_PASSWORD_PROTECTED and
> GIT_CURL_VERBOSE, but it's a little bit awkward for GIT_SSL_NO_VERIFY
> and GIT_CURL_FTP_NO_EPSV since they have "_NO_" in their names.
>
> If the user wants to override a "http.sslVerify=false" then
> "GIT_SSL_NO_VERIFY=false" is needed rather than "GIT_SSL_VERIFY=true".
>
> We could:
>
> 1) Introduce GIT_SSL_VERIFY and GIT_CURL_FTP_EPSV and say if they are
> set the "_NO_" version is ignored.
>
> 2) Go ahead with "GIT_SSL_NO_VERIFY=false" to force http.sslVerify
> back to true (and similarly for EPSV).
>
> 3) Just leave GIT_SSL_NO_VERIFY and GIT_CURL_FTP_NO_EPSV alone.
>
> 4) Do something else, ideas?
>
> Comments?

The usual way we have done this kind of thing so far is:

 (1) Add GIT_SSL_VERIFY and GIT_CURL_FTP_EPSV support, that is
 boolean.  If that is set, it takes precedence to
 GIT_SSL_NO_VERIFY and GIT_CURL_FTP_NO_EPSV.  If not,
 GIT_SSL_NO_VERIFY and GIT_CURL_FTP_NO_EPSV are used just like
 before (setting it to any value will decline VERIFY or EPSV).

 (2) Issue a warning to tell users to use GIT_SSL_VERIFY or
 GIT_CURL_FTP_EPSV instead, when GIT_SSL_NO_VERIFY and
 GIT_CURL_FTP_NO_EPSV are used.

 (3) Later at a large version bump, remove support for *_NO_*
 variants.

We can stop at (1) and not do (2) and (3), if the resulting code
after (1) is not too cumbersome to maintain.  If we do (2) then we
must follow it through with (3), and if we plan to do (3) then we
must precede it with (2).

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


Default expectation of --lockref

2013-07-15 Thread Junio C Hamano

> I think the use of "reverse tracking" is way overrated.  It is
> probably the only default value that we could use, if the user is
> too lazy not to specify it, but I do not think it is particularly a
> sensible or safe default.
>
> The following does not discuss "should --lockref automatically
> disable the 'must fast-forward' check?".  The problem highlighted is
> the same, regardless of the answer to that question.
>
> After rebasing beyond what is already published, you try the
> "lockref" push, e.g. (we assume you work on master and push back to
> update master at your origin):
>
>   $ git fetch
>   $ git rebase -i @{u}~4 ;# rebase beyond what is there
>   $ git push ;# of course this will not fast-forward
>   $ git push --lockref
>
> If somebody else pushed while you are working on the rebase, the
> last step (one of the above push) will fail due to stale
> expectation.  What now?
>
> The user would want to keep the updated tip, so the first thing that
> happens will always be
>
>   $ git fetch
>   $ git log ..@{u} ;# what will we be losing?
>
> The right thing to do at this point is to rebase your 'master' again
> on top of @{u}
>
>   $ git rebase -i @{u}
>
> before attempting to push back again.  If you do that, then you can
> do another "lockref" push.
>
> But the thing is, a novice who does not know what he is doing will
> likely to do this:
>
>   $ git push --lockref
>
>   ... rejected due to stale expectation
>   $ git fetch
>
> You just have updated the lockref base, so if you did, without doing
> anything else, 
>
>   $ git push --lockref
>
> then you will lose the updated contents.

We _might_ be able to use the reflog on refs/remotes/origin/master
to come up with a better default expectation.

We are pushing an updated master, and the commit at the tip of the
branch has a committer timestamp.  refs/remotes/origin/master should
at least have two reflog entries for it at this point.  The latest
one is our latest "git fetch" after the previous lockref push failed
(and we see somebody else updated the master at the origin).  One
before is the one we based our judgement that the rebased result can
replace it.  They both have timestamps for reflog updates.

So we _could_ use refs/remotes/origin/master@{$timestamp} where
the $timestamp is the committer timestamp of the tip of 'master'
we are pushing to replace the 'master' branch at 'origin'.

I do not particularly like this approach, though.  I do not
particulary like the "look at the tracking branch of what we are
updating" in the first place myself, because it requires you to have
such a tracking branch, but now with this, we will also require you
to have a reflog on such a tracking branch, too, which is even
worse.  And it is making it too complex and obscure, even though I
think the semantics would make sense.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/7] push: document --lockref

2013-07-15 Thread Junio C Hamano
Jonathan Nieder  writes:

> Now suppose my relay has some downtime.  That's fine --- I can still
> maintain the mirror by running the same commands on another machine.
> But when the old relay comes back up, "push --lockref" will fail and
> "pu" and "next" in my mirror are not updated any more.
>
> That is why I said that --force is more appropriate than --lockref
> for this application.

Sure.
--
To unsubscribe from this list: send the line "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] Revert "git-clone.txt: remove the restriction on pushing from a shallow clone"

2013-07-15 Thread Junio C Hamano
This reverts commit dacd2bcc414e0b7aac36aaa400da0a743c4741cc.

"It fails reliably without corrupting the receiving repository when
it should fail" may be better than the situation before the receiving
end was hardened recently, but the fact that sometimes the push does
not go through still remains.  It is better to advice the users that
they cannot push from a shallow repository as a limitation before
they decide to use (or not to use) a shallow clone.
---

 * Unfortunately I thought the documentation update was a
   no-brainer and already applied it, so here is a revert.

 Documentation/git-clone.txt | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 85769b8..450f158 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -182,13 +182,11 @@ objects from the source repository into a pack in the 
cloned repository.
 --depth ::
Create a 'shallow' clone with a history truncated to the
specified number of revisions.  A shallow repository has a
-   number of limitations (you cannot clone or fetch from it, nor
-   push into it), but is adequate if you are only interested in
-   the recent history of a large project with a long history.
-+
-Pushing from a shallow clone should be avoided if the git version on
-the receiver end is older than v1.7.10, or any other git
-implementation that does not perform connectivity check.
+   number of limitations (you cannot clone or fetch from
+   it, nor push from nor into it), but is adequate if you
+   are only interested in the recent history of a large project
+   with a long history, and would want to send in fixes
+   as patches.
 
 --[no-]single-branch::
Clone only the history leading to the tip of a single branch,
-- 
1.8.3.2-941-gda9c3c8

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


Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone

2013-07-15 Thread Junio C Hamano
Duy Nguyen  writes:

> On Mon, Jul 15, 2013 at 8:01 AM, Duy Nguyen  wrote:
>>> Also, the sender may have cloned from the receiver (fully) and then
>>> fetched a different history shallowly from elsewhere.  The receiver
>>> may have no commit on that history, including the shallow-bottom.
>>>
>>
>> Hmm.. right. And the receiver needs to setup proper graft to seal the
>> shallow bottom. So it's really not safe to do pushing from a shallow
>> repo without 52fed6e
>
> Because this makes pushing from a shallow repo fall into "mostly
> works" category, I withdraw this patch.

Yeah, "reliably fails when it shouldn't" may be safer than before
that patch, but the fact that you cannot push from a shallow one
still remains, so we probably shouldn't tell the users to use it.


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


Re: [PATCH 2/5] diff: allow --patch to override -s/--no-patch

2013-07-15 Thread Matthieu Moy
Junio C Hamano  writes:

> I am wondering if the difference after this patch between "-p" and
> "-U8" is deliberate, or just an accident coming from the way the
> original was written in ee1e5412 (git diff: support "-U" and
> "--unified" options properly, 2006-05-13).

No, it isn't. I just didn't notice the -U case.

> If the original were written in this way:
>
>   if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch") 
> ||
> opt_arg(arg, 'U', "unified", &options->context))
>   options->output_format |= DIFF_FORMAT_PATCH;

Yes, this seems to be a better way.

There are other cases like --patch-with-raw, I'll send a reroll.

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


[PATCH] do_one_ref(): save and restore value of current_ref

2013-07-15 Thread Michael Haggerty
If do_one_ref() is called recursively, then the inner call should not
permanently overwrite the value stored in current_ref by the outer
call.  Aside from the tiny optimization loss, peel_ref() expects the
value of current_ref not to change across a call to peel_entry().  But
in the presence of replace references that assumption could be
violated by a recursive call to do_one_ref:

do_for_each_entry()
  do_one_ref()
builtin/describe.c:get_name()
  peel_ref()
peel_entry()
  peel_object ()
deref_tag_noverify()
  parse_object()
lookup_replace_object()
  do_lookup_replace_object()
prepare_replace_object()
  do_for_each_ref()
do_for_each_entry()
  do_for_each_entry_in_dir()
do_one_ref()

The inner call to do_one_ref() was unconditionally setting current_ref
to NULL when it was done, causing peel_ref() to perform an invalid
memory access.

So change do_one_ref() to save the old value of current_ref before
overwriting it, and restore the old value afterward rather than
setting it to NULL.

Reported by: Mantas Mikulėnas 

Signed-off-by: Michael Haggerty 
---
 refs.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index 4302206..222baf2 100644
--- a/refs.c
+++ b/refs.c
@@ -634,7 +634,9 @@ struct ref_entry_cb {
 static int do_one_ref(struct ref_entry *entry, void *cb_data)
 {
struct ref_entry_cb *data = cb_data;
+   struct ref_entry *old_current_ref;
int retval;
+
if (prefixcmp(entry->name, data->base))
return 0;
 
@@ -642,10 +644,12 @@ static int do_one_ref(struct ref_entry *entry, void 
*cb_data)
  !ref_resolves_to_object(entry))
return 0;
 
+   /* Store the old value, in case this is a recursive call: */
+   old_current_ref = current_ref;
current_ref = entry;
retval = data->fn(entry->name + data->trim, entry->u.value.sha1,
  entry->flag, data->cb_data);
-   current_ref = NULL;
+   current_ref = old_current_ref;
return retval;
 }
 
-- 
1.8.3.2

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


Re: [PATCH v2 9/9] mailmap: style fixes

2013-07-15 Thread Junio C Hamano
Thanks, the whole series looks sensible.  Will replace and queue.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 5/9] mailmap: debug: fix out-of-order fprintf() arguments

2013-07-15 Thread Junio C Hamano
Eric Sunshine  writes:

> Resolve segmentation fault due to arguments passed in wrong order.
>
> Signed-off-by: Eric Sunshine 

Thanks.  Shows that the debugging support hasn't been in use for
quite a long time X-<.

> ---
>  mailmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mailmap.c b/mailmap.c
> index a7e92db..0516354 100644
> --- a/mailmap.c
> +++ b/mailmap.c
> @@ -309,7 +309,7 @@ int map_user(struct string_list *map,
>   struct mailmap_entry *me;
>  
>   debug_mm("map_user: map '%.*s' <%.*s>\n",
> -  *name, *namelen, *emaillen, *email);
> +  *namelen, *name, *emaillen, *email);
>  
>   item = lookup_prefix(map, *email, *emaillen);
>   if (item != NULL) {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/5] Documentation/git-show.txt: include common diff options, like git-log.txt

2013-07-15 Thread Junio C Hamano
Junio C Hamano  writes:

> Matthieu Moy  writes:
>
>> Signed-off-by: Matthieu Moy 
>> ---
>>  Documentation/git-show.txt | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
>> index ae4edcc..4e617e6 100644
>> --- a/Documentation/git-show.txt
>> +++ b/Documentation/git-show.txt
>> @@ -45,6 +45,15 @@ include::pretty-options.txt[]
>>  include::pretty-formats.txt[]
>>  
>>  
>> +COMMON DIFF OPTIONS
>> +---
>> +
>> +:git-log: 1
>> +include::diff-options.txt[]
>> +
>> +include::diff-generate-patch.txt[]
>> +
>> +
>>  EXAMPLES
>>  
>
> This is a good start; the output should match what you would get for
> git-log(1) with the above.
>
> But we would need to say, unlike "log" whose default is not to show
> any patch, "show" that works on a commit defaults to "--cc"
> somewhere.
>
> Other than that the whole series looks sensible to me.  Thanks.

Actually, we say "log message and textual diff" with "merge commit
... diff-tree --cc", so please scratch the above comment (I think we
should however stop saying "in a special format" there---when the
combined diff was invented, the format may have been novelty, but it
no longer is).

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


Re: [PATCH 2/5] diff: allow --patch to override -s/--no-patch

2013-07-15 Thread Junio C Hamano
Matthieu Moy  writes:

> Signed-off-by: Matthieu Moy 
> ---
>  diff.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 6bd821d..66a6877 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3515,9 +3515,10 @@ int diff_opt_parse(struct diff_options *options, const 
> char **av, int ac)
>   int argcount;
>  
>   /* Output format options */
> - if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch"))
> + if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, 
> "--patch")) {
> + options->output_format &= ~DIFF_FORMAT_NO_OUTPUT;
>   options->output_format |= DIFF_FORMAT_PATCH;
> - else if (opt_arg(arg, 'U', "unified", &options->context))
> + } else if (opt_arg(arg, 'U', "unified", &options->context))
>   options->output_format |= DIFF_FORMAT_PATCH;
>   else if (!strcmp(arg, "--raw"))
>   options->output_format |= DIFF_FORMAT_RAW;

I am wondering if the difference after this patch between "-p" and
"-U8" is deliberate, or just an accident coming from the way the
original was written in ee1e5412 (git diff: support "-U" and
"--unified" options properly, 2006-05-13).

If the original were written in this way:

if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch") 
||
opt_arg(arg, 'U', "unified", &options->context))
options->output_format |= DIFF_FORMAT_PATCH;

I suspect you would have given the additional

options->output_format &= ~DIFF_FORMAT_NO_OUTPUT;

to the whole thing, without singling out "-U8".  Or am I missing
some good reason why "--no-patch -U8" should not produce a patch,
while "--no-patch -p" and "-U8" should both should?

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


Re: [PATCH 3/5] Documentation/git-show.txt: include common diff options, like git-log.txt

2013-07-15 Thread Junio C Hamano
Matthieu Moy  writes:

> Signed-off-by: Matthieu Moy 
> ---
>  Documentation/git-show.txt | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
> index ae4edcc..4e617e6 100644
> --- a/Documentation/git-show.txt
> +++ b/Documentation/git-show.txt
> @@ -45,6 +45,15 @@ include::pretty-options.txt[]
>  include::pretty-formats.txt[]
>  
>  
> +COMMON DIFF OPTIONS
> +---
> +
> +:git-log: 1
> +include::diff-options.txt[]
> +
> +include::diff-generate-patch.txt[]
> +
> +
>  EXAMPLES
>  

This is a good start; the output should match what you would get for
git-log(1) with the above.

But we would need to say, unlike "log" whose default is not to show
any patch, "show" that works on a commit defaults to "--cc"
somewhere.

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


Re: [PATCH] commit: Fix a memory leak in determine_author_info

2013-07-15 Thread Stefan Beller
On 07/15/2013 04:42 PM, Junio C Hamano wrote:
> Stefan Beller  writes:
> 
>> The date variable is assigned new memory via xmemdupz and 2 lines later
>> it is assigned new memory again via xmalloc, but the first assignment
>> is never freed nor used.
>> ---
>>  builtin/commit.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index 790e5ab..00da83c 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -534,7 +534,6 @@ static void determine_author_info(struct strbuf 
>> *author_ident)
>>  (lb - strlen(" ") -
>>   (a + strlen("\nauthor ";
>>  email = xmemdupz(lb + strlen("<"), rb - (lb + strlen("<")));
>> -date = xmemdupz(rb + strlen("> "), eol - (rb + strlen("> ")));
>>  len = eol - (rb + strlen("> "));
>>  date = xmalloc(len + 2);
>>  *date = '@';
> 
> Makes sense. I'd assume this is signed-off?
> 

As I realised I did not sign it,
I resend it with some other findings a few hours later,
so you'll find it there as well.

See Message-ID
<1373837749-14402-2-git-send-email-stefanbel...@googlemail.com>
[PATCH 2/4] commit: Fix a memory leak in determine_author_info

--
To unsubscribe from this list: send the line "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] commit: Fix a memory leak in determine_author_info

2013-07-15 Thread Junio C Hamano
Stefan Beller  writes:

> The date variable is assigned new memory via xmemdupz and 2 lines later
> it is assigned new memory again via xmalloc, but the first assignment
> is never freed nor used.
> ---
>  builtin/commit.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 790e5ab..00da83c 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -534,7 +534,6 @@ static void determine_author_info(struct strbuf 
> *author_ident)
>   (lb - strlen(" ") -
>(a + strlen("\nauthor ";
>   email = xmemdupz(lb + strlen("<"), rb - (lb + strlen("<")));
> - date = xmemdupz(rb + strlen("> "), eol - (rb + strlen("> ")));
>   len = eol - (rb + strlen("> "));
>   date = xmalloc(len + 2);
>   *date = '@';

Makes sense. I'd assume this is signed-off?
--
To unsubscribe from this list: send the line "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] Use compat/regex on Cygwin

2013-07-15 Thread Mark Levedahl
Cygwin's regex library does not pass git's tests, so don't use it. This
fixes failures in t4018 and t4034.

Signed-off-by: Mark Levedahl 
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index 7ac541e..6901597 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -165,6 +165,7 @@ ifeq ($(uname_O),Cygwin)
NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
+   NO_REGEX = UnfortunatelyYes
# There are conflicting reports about this.
# On some boxes NO_MMAP is needed, and not so elsewhere.
# Try commenting this out if you suspect MMAP is more efficient
-- 
1.8.3.2.0.13

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


Re: [PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

2013-07-15 Thread Junio C Hamano
Matthieu Moy  writes:

> Jonathan Nieder  writes:
>
>> (3)
>>  # An example hook ...
>>  #
>>  # Warning: this script is no longer actively maintained.  Consider
>>  # switching to ...
>>
>> I prefer (2), which makes it clear to the reader that it is dangerous
>> to keep using the script (since no one is actively chasing down bugs)
>> while also making it clear why a potentially buggy script with a good
>> natural successor is still in contrib for now.  What do you think?
>
> I don't think it is dangerous to keep using the old script. If you look
> at its history, it's pretty stable these day. I think it has known bugs
> in new revision detections that are fixed by git-multimail, but nothing
> really blocking IMHO.
>
> There are two good reasons to use it: 1) you already use it, and you're
> too lazy to change (e.g. because it's packaged by Debian and is already
> there on your server), and 2) you don't have Python on your server.

Well said.

> I think the notice still deserve the "***NOTICE***" or whatever makes it
> visible enough to distinguish it from the traditional licence &
> non-warranty header, but I don't think we should kill the old script too
> early.

True.  I personally felt that Jonathan's (1) read the most natural
(i.e. showing no strong preference, just let the users decide).

--
To unsubscribe from this list: send the line "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: Segfault in `git describe`

2013-07-15 Thread Mantas Mikulėnas
On Mon, Jul 15, 2013 at 4:03 PM, Michael Haggerty  wrote:
> On 07/13/2013 03:27 PM, Mantas Mikulėnas wrote:
>> I have a clone of linux.git with various stuff added to it (remotes for
>> 'stable' and 'next', a bunch of local tags, and historical repositories
>> imported using `git replace`).
>>
>> Yesterday, I noticed that `git describe`, built from git.git master
>> (v1.8.3.2-804-g0da7a53, gcc 4.8) would simply crash when run in that
>> repository, with the following backtrace:
>>
>>> Program terminated with signal 11, Segmentation fault.
>>> #0  0x004c39dc in hashcpy (sha_src=0x1c >> bounds>,
>>> sha_dst=0x7fffc0b4d610 "\242\271\301\366 
>>> \201&\346\337l\002B\214P\037\210ShX\022")
>>> at cache.h:694
>>> 694  memcpy(sha_dst, sha_src, 20);
>>> (gdb) bt
>>> #0  0x004c39dc in hashcpy (sha_src=0x1c >> bounds>,
>>> sha_dst=0x7fffc0b4d610 "\242\271\301\366 
>>> \201&\346\337l\002B\214P\037\210ShX\022")
>>> at cache.h:694
>>> #1  peel_ref (refname=refname@entry=0x1fe2d10 "refs/tags/next-20130607",
>>> sha1=sha1@entry=0x7fffc0b4d610 "\242\271\301\366 
>>> \201&\346\337l\002B\214P\037\210ShX\022") at refs.c:1586
>>> #2  0x00424194 in get_name (path=0x1fe2d10 
>>> "refs/tags/next-20130607",
>>> sha1=0x1fe2ce8 
>>> "\222V\356\276S5\tk\231Hi\264\r=\336\315\302\225\347\257\300N\376\327\064@\237ZDq[T\246\312\033T\260\314\362\025refs/tags/next-20130607",
>>>  flag=,
>>> cb_data=) at builtin/describe.c:156
>>> #3  0x004c1c21 in do_one_ref (entry=0x1fe2ce0, 
>>> cb_data=0x7fffc0b4d7c0)
>>> at refs.c:646
>>> #4  0x004c318d in do_for_each_entry_in_dir (dir=0x1fe1728,
>>> offset=, fn=0x4c1bc0 , 
>>> cb_data=0x7fffc0b4d7c0)
>>> at refs.c:672
>>> #5  0x004c33d1 in do_for_each_entry_in_dirs (dir1=0x1fdf4d8, 
>>> dir2=0x1fd6318,
>>> cb_data=0x7fffc0b4d7c0, fn=0x4c1bc0 ) at refs.c:716
>>> #6  0x004c33d1 in do_for_each_entry_in_dirs (dir1=0x1fdf1f8, 
>>> dir2=0x1fd62d8,
>>> cb_data=0x7fffc0b4d7c0, fn=0x4c1bc0 ) at refs.c:716
>>> #7  0x004c3540 in do_for_each_entry (refs=refs@entry=0x7a2800 
>>> ,
>>> base=base@entry=0x509cc6 "", cb_data=cb_data@entry=0x7fffc0b4d7c0,
>>> fn=0x4c1bc0 ) at refs.c:1689
>>> #8  0x004c3ff8 in do_for_each_ref (cb_data=cb_data@entry=0x0, 
>>> flags=1, trim=0,
>>> fn=fn@entry=0x424120 , base=0x509cc6 "", refs=0x7a2800 
>>> )
>>> at refs.c:1724
>>> #9  for_each_rawref (fn=fn@entry=0x424120 , 
>>> cb_data=cb_data@entry=0x0)
>>> at refs.c:1873
>>> #10 0x00424f5b in cmd_describe (argc=0, argv=0x7fffc0b4ddc0, 
>>> prefix=0x0)
>>> at builtin/describe.c:466
>>> #11 0x0040596d in run_builtin (argv=0x7fffc0b4ddc0, argc=1,
>>> p=0x760b40 ) at git.c:291
>>> #12 handle_internal_command (argc=1, argv=0x7fffc0b4ddc0) at git.c:453
>>> #13 0x00404d6e in run_argv (argv=0x7fffc0b4dc78, 
>>> argcp=0x7fffc0b4dc5c)
>>> at git.c:499
>>> #14 main (argc=1, av=) at git.c:575
>>> (gdb)
>>
>> According to `git bisect`, the first bad commit is:
>>
>> commit 9a489f3c17d6c974b18c47cf406404ca2a721c87
>> Author: Michael Haggerty 
>> Date:   Mon Apr 22 21:52:22 2013 +0200
>>
>> refs: extract a function peel_entry()
>>
>> The crash happens only in repositories that have at least one replaced
>> object in the branch's history. Running `git --no-replace-objects
>> describe` avoids the crash.
>>
>> The crash happens only if there are any tags under .git/refs/tags/ that
>> do not exist in .git/packed-refs, or if I remove all "peeled" lines from
>> .git/packed-refs (including the '#' line; /^[#^]/d).
>>
>> A quick way to reproduce this with git.git master is:
>>
>> git tag -f test-tag HEAD~10
>> git replace -f HEAD $(git --no-replace-objects cat-file commit HEAD \
>>   | sed 's/@/@test/' | git hash-object --stdin -t commit -w)
>> ./git describe
>
> Thanks for the bug report.
>
> I think the cause of this bug is that peel_entry() is causing a nested
> call to do_for_each_entry() to look up the replace reference, which
> resets current_ref to NULL between the test and the dereference of
> current_ref in peel_ref().
>
> Unfortunately, I cannot reproduce the failure by following your recipe
> (though I didn't have a lot of time yet for this).  I suppose that my
> repo starts out in a slightly different state than yours and therefore I
> don't get the same results.  If you could find a recipe to reproduce the
> problem, starting either with an empty repo, or perhaps a fresh clone of
> git.git, and double-check that you don't have any unusual config options
> that might be affecting things, that would be very helpful.

Hmm, yes, just creating a new tag doesn't break in another
freshly-cloned repo, either.

However,

> …or if I remove all "peeled" lines from .git/packed-refs (including the '#' 
> line; /^[#^]/d).

still works for reproducing the crash. When packed-refs does not have
any peeled refs, older git versions do it manually (I assume for
compatibility with 

[PATCH 4/5] Documentation: move description of -s, --no-patch to diff-options.txt

2013-07-15 Thread Matthieu Moy
Technically, "-s, --no-patch" is implemented in diff.c ("git diff
--no-patch" is essentially useless, but valid). From the user point of
view, this allows the documentation to show up in "git show --help",
which is one of the most useful use of the option.

While we're there, add a sentence explaining why the option can be
useful.

Signed-off-by: Matthieu Moy 
---
It might make sense to surround this with ifndef::git-diff[], but
since "git diff --no-patch" actually works, I've left it
unconditional to be technically correct.

 Documentation/diff-options.txt | 5 +
 Documentation/rev-list-options.txt | 4 
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 87e92d6..bbed2cd 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -26,6 +26,11 @@ ifndef::git-format-patch[]
{git-diff? This is the default.}
 endif::git-format-patch[]
 
+-s::
+--no-patch::
+   Suppress diff output. Useful for commands like `git show` that
+   show the patch by default, or to cancel the effect of `--patch`.
+
 -U::
 --unified=::
Generate diffs with  lines of context instead of
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index c128a85..e632e85 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -849,8 +849,4 @@ options may be given. See linkgit:git-diff-files[1] for 
more options.
 -t::
 
Show the tree objects in the diff output. This implies '-r'.
-
--s::
---no-patch::
-   Suppress diff output.
 endif::git-rev-list[]
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH 0/5] Make "git show -s" easier to discover for users

2013-07-15 Thread Matthieu Moy
> Stefan Beller  writes:
>
>> However I sometimes also get:
>> sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53
>> Fri Jul 12 10:49:34 2013 -0700
>>
>> diff --git a/Documentation/RelNotes/1.8.4.txt
>> b/Documentation/RelNotes/1.8.4.txt
>> index 0e50df8..4250e5a 100644
>> --- a/Documentation/RelNotes/1.8.4.txt
>> +++ b/Documentation/RelNotes/1.8.4.txt
>
> "git show" will show the diff by default. For merge commits, it shows
> the --cc diff which is often empty, hence the behavior you see.
>
> You want to use "git show -s", which suppresses the patch output.

... and this "git show -s" is extraordinarily hard to discover, as it
is only documented in "git log --help". Google has been my friend
here, but we should really improve that.

This patch series does essentially two things:

* Add a --no-patch synonym for -s. I'm actually wondering why the
  option wasn't called this way from the beginning.

* Reorganize the doc so that "git show" actually mentions it.

While we're there, there's a reformatting patch, and one to make "git
log --no-patch --patch" actually show the patch.

(60aa9cf8f3, Documentation: document show -s, Tue Nov 9 11:12:48 2010
actually acknowledged that the documentation wasn't complete and that
something else was needed, this should be it)

Matthieu Moy (5):
  diff: allow --no-patch as synonym for -s
  diff: allow --patch to override -s/--no-patch
  Documentation/git-show.txt: include common diff options, like
git-log.txt
  Documentation: move description of -s, --no-patch to diff-options.txt
  Documentation/git-log.txt: capitalize section names

 Documentation/diff-options.txt | 5 +
 Documentation/git-log.txt  | 8 
 Documentation/git-show.txt | 9 +
 Documentation/rev-list-options.txt | 3 ---
 diff.c | 7 ---
 5 files changed, 22 insertions(+), 10 deletions(-)

-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH 5/5] Documentation/git-log.txt: capitalize section names

2013-07-15 Thread Matthieu Moy
This is the convention in other files and even at the beginning of git-log.txt

Signed-off-by: Matthieu Moy 
---
 Documentation/git-log.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 2ea79ba..2eda5e4 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -97,7 +97,7 @@ include::rev-list-options.txt[]
 
 include::pretty-formats.txt[]
 
-Common diff options
+COMMON DIFF OPTIONS
 ---
 
 :git-log: 1
@@ -105,7 +105,7 @@ include::diff-options.txt[]
 
 include::diff-generate-patch.txt[]
 
-Examples
+EXAMPLES
 
 `git log --no-merges`::
 
@@ -161,12 +161,12 @@ git log -L '/int main/',/^}/:main.c::
 `git log -3`::
Limits the number of commits to show to 3.
 
-Discussion
+DISCUSSION
 --
 
 include::i18n.txt[]
 
-Configuration
+CONFIGURATION
 -
 
 See linkgit:git-config[1] for core variables and linkgit:git-diff[1]
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH 2/5] diff: allow --patch to override -s/--no-patch

2013-07-15 Thread Matthieu Moy
Signed-off-by: Matthieu Moy 
---
 diff.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 6bd821d..66a6877 100644
--- a/diff.c
+++ b/diff.c
@@ -3515,9 +3515,10 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
int argcount;
 
/* Output format options */
-   if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch"))
+   if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, 
"--patch")) {
+   options->output_format &= ~DIFF_FORMAT_NO_OUTPUT;
options->output_format |= DIFF_FORMAT_PATCH;
-   else if (opt_arg(arg, 'U', "unified", &options->context))
+   } else if (opt_arg(arg, 'U', "unified", &options->context))
options->output_format |= DIFF_FORMAT_PATCH;
else if (!strcmp(arg, "--raw"))
options->output_format |= DIFF_FORMAT_RAW;
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH 1/5] diff: allow --no-patch as synonym for -s

2013-07-15 Thread Matthieu Moy
This follows the usual convention of having a --no-foo option to negate
--foo.

Signed-off-by: Matthieu Moy 
---
 Documentation/rev-list-options.txt | 1 +
 diff.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index e157ec3..c128a85 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -851,5 +851,6 @@ options may be given. See linkgit:git-diff-files[1] for 
more options.
Show the tree objects in the diff output. This implies '-r'.
 
 -s::
+--no-patch::
Suppress diff output.
 endif::git-rev-list[]
diff --git a/diff.c b/diff.c
index 6578690..6bd821d 100644
--- a/diff.c
+++ b/diff.c
@@ -3551,7 +3551,7 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
options->output_format |= DIFF_FORMAT_NAME;
else if (!strcmp(arg, "--name-status"))
options->output_format |= DIFF_FORMAT_NAME_STATUS;
-   else if (!strcmp(arg, "-s"))
+   else if (!strcmp(arg, "-s") || !strcmp(arg, "--no-patch"))
options->output_format |= DIFF_FORMAT_NO_OUTPUT;
else if (!prefixcmp(arg, "--stat"))
/* --stat, --stat-width, --stat-name-width, or --stat-count */
-- 
1.8.3.1.495.g13f33cf.dirty

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


[PATCH 3/5] Documentation/git-show.txt: include common diff options, like git-log.txt

2013-07-15 Thread Matthieu Moy
Signed-off-by: Matthieu Moy 
---
 Documentation/git-show.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
index ae4edcc..4e617e6 100644
--- a/Documentation/git-show.txt
+++ b/Documentation/git-show.txt
@@ -45,6 +45,15 @@ include::pretty-options.txt[]
 include::pretty-formats.txt[]
 
 
+COMMON DIFF OPTIONS
+---
+
+:git-log: 1
+include::diff-options.txt[]
+
+include::diff-generate-patch.txt[]
+
+
 EXAMPLES
 
 
-- 
1.8.3.1.495.g13f33cf.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: Segfault in `git describe`

2013-07-15 Thread Michael Haggerty
On 07/13/2013 03:27 PM, Mantas Mikulėnas wrote:
> I have a clone of linux.git with various stuff added to it (remotes for
> 'stable' and 'next', a bunch of local tags, and historical repositories
> imported using `git replace`).
> 
> Yesterday, I noticed that `git describe`, built from git.git master
> (v1.8.3.2-804-g0da7a53, gcc 4.8) would simply crash when run in that
> repository, with the following backtrace:
> 
>> Program terminated with signal 11, Segmentation fault.
>> #0  0x004c39dc in hashcpy (sha_src=0x1c > bounds>, 
>> sha_dst=0x7fffc0b4d610 "\242\271\301\366 
>> \201&\346\337l\002B\214P\037\210ShX\022")
>> at cache.h:694
>> 694  memcpy(sha_dst, sha_src, 20);
>> (gdb) bt
>> #0  0x004c39dc in hashcpy (sha_src=0x1c > bounds>, 
>> sha_dst=0x7fffc0b4d610 "\242\271\301\366 
>> \201&\346\337l\002B\214P\037\210ShX\022")
>> at cache.h:694
>> #1  peel_ref (refname=refname@entry=0x1fe2d10 "refs/tags/next-20130607", 
>> sha1=sha1@entry=0x7fffc0b4d610 "\242\271\301\366 
>> \201&\346\337l\002B\214P\037\210ShX\022") at refs.c:1586
>> #2  0x00424194 in get_name (path=0x1fe2d10 
>> "refs/tags/next-20130607", 
>> sha1=0x1fe2ce8 
>> "\222V\356\276S5\tk\231Hi\264\r=\336\315\302\225\347\257\300N\376\327\064@\237ZDq[T\246\312\033T\260\314\362\025refs/tags/next-20130607",
>>  flag=, 
>> cb_data=) at builtin/describe.c:156
>> #3  0x004c1c21 in do_one_ref (entry=0x1fe2ce0, 
>> cb_data=0x7fffc0b4d7c0)
>> at refs.c:646
>> #4  0x004c318d in do_for_each_entry_in_dir (dir=0x1fe1728, 
>> offset=, fn=0x4c1bc0 , cb_data=0x7fffc0b4d7c0)
>> at refs.c:672
>> #5  0x004c33d1 in do_for_each_entry_in_dirs (dir1=0x1fdf4d8, 
>> dir2=0x1fd6318, 
>> cb_data=0x7fffc0b4d7c0, fn=0x4c1bc0 ) at refs.c:716
>> #6  0x004c33d1 in do_for_each_entry_in_dirs (dir1=0x1fdf1f8, 
>> dir2=0x1fd62d8, 
>> cb_data=0x7fffc0b4d7c0, fn=0x4c1bc0 ) at refs.c:716
>> #7  0x004c3540 in do_for_each_entry (refs=refs@entry=0x7a2800 
>> , 
>> base=base@entry=0x509cc6 "", cb_data=cb_data@entry=0x7fffc0b4d7c0, 
>> fn=0x4c1bc0 ) at refs.c:1689
>> #8  0x004c3ff8 in do_for_each_ref (cb_data=cb_data@entry=0x0, 
>> flags=1, trim=0, 
>> fn=fn@entry=0x424120 , base=0x509cc6 "", refs=0x7a2800 
>> )
>> at refs.c:1724
>> #9  for_each_rawref (fn=fn@entry=0x424120 , 
>> cb_data=cb_data@entry=0x0)
>> at refs.c:1873
>> #10 0x00424f5b in cmd_describe (argc=0, argv=0x7fffc0b4ddc0, 
>> prefix=0x0)
>> at builtin/describe.c:466
>> #11 0x0040596d in run_builtin (argv=0x7fffc0b4ddc0, argc=1, 
>> p=0x760b40 ) at git.c:291
>> #12 handle_internal_command (argc=1, argv=0x7fffc0b4ddc0) at git.c:453
>> #13 0x00404d6e in run_argv (argv=0x7fffc0b4dc78, 
>> argcp=0x7fffc0b4dc5c)
>> at git.c:499
>> #14 main (argc=1, av=) at git.c:575
>> (gdb) 
> 
> According to `git bisect`, the first bad commit is:
> 
> commit 9a489f3c17d6c974b18c47cf406404ca2a721c87
> Author: Michael Haggerty 
> Date:   Mon Apr 22 21:52:22 2013 +0200
> 
> refs: extract a function peel_entry()
> 
> The crash happens only in repositories that have at least one replaced
> object in the branch's history. Running `git --no-replace-objects
> describe` avoids the crash.
> 
> The crash happens only if there are any tags under .git/refs/tags/ that
> do not exist in .git/packed-refs, or if I remove all "peeled" lines from
> .git/packed-refs (including the '#' line; /^[#^]/d).
> 
> A quick way to reproduce this with git.git master is:
> 
> git tag -f test-tag HEAD~10
> git replace -f HEAD $(git --no-replace-objects cat-file commit HEAD \
>   | sed 's/@/@test/' | git hash-object --stdin -t commit -w)
> ./git describe

Thanks for the bug report.

I think the cause of this bug is that peel_entry() is causing a nested
call to do_for_each_entry() to look up the replace reference, which
resets current_ref to NULL between the test and the dereference of
current_ref in peel_ref().

Unfortunately, I cannot reproduce the failure by following your recipe
(though I didn't have a lot of time yet for this).  I suppose that my
repo starts out in a slightly different state than yours and therefore I
don't get the same results.  If you could find a recipe to reproduce the
problem, starting either with an empty repo, or perhaps a fresh clone of
git.git, and double-check that you don't have any unusual config options
that might be affecting things, that would be very helpful.

I might have more time to look at this tonight.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Missing capabilities in Documentation/technical/protocol-capbilities.txt

2013-07-15 Thread Duy Nguyen
I noticed that "quiet" and "agent" capabilities were missing in
protocol-capabilitities.txt. I have a rough idea what they do, but I
think it's best to be documented by the authors. Maybe you have some
time to make a patch?
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Bug in "git show"?

2013-07-15 Thread Stefan Beller
On 07/15/2013 02:12 PM, Matthieu Moy wrote:
> Stefan Beller  writes:
> 
>> However I sometimes also get:
>> sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53
>> Fri Jul 12 10:49:34 2013 -0700
>>
>> diff --git a/Documentation/RelNotes/1.8.4.txt
>> b/Documentation/RelNotes/1.8.4.txt
>> index 0e50df8..4250e5a 100644
>> --- a/Documentation/RelNotes/1.8.4.txt
>> +++ b/Documentation/RelNotes/1.8.4.txt
> 
> "git show" will show the diff by default. For merge commits, it shows
> the --cc diff which is often empty, hence the behavior you see.
> 
> You want to use "git show -s", which suppresses the patch output.
> 

Thanks, that's exactly what I am looking for.
Stefan



signature.asc
Description: OpenPGP digital signature


Re: Bug in "git show"?

2013-07-15 Thread Matthieu Moy
Stefan Beller  writes:

> However I sometimes also get:
> sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53
> Fri Jul 12 10:49:34 2013 -0700
>
> diff --git a/Documentation/RelNotes/1.8.4.txt
> b/Documentation/RelNotes/1.8.4.txt
> index 0e50df8..4250e5a 100644
> --- a/Documentation/RelNotes/1.8.4.txt
> +++ b/Documentation/RelNotes/1.8.4.txt

"git show" will show the diff by default. For merge commits, it shows
the --cc diff which is often empty, hence the behavior you see.

You want to use "git show -s", which suppresses the patch output.

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


Bug in "git show"?

2013-07-15 Thread Stefan Beller
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hello,

so I wanted to write a script using some git commands,
but the output of the git commands is not as expected.
I am using

git --version
git version 1.8.3.2.804.g0da7a53
(current origin/master at git://github.com/gitster/git.git)

The command I am trying to use is
git --no-pager show --format="%ad" 

Expected output:
sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53^
Fri Jul 12 12:04:19 2013 -0700

sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53^^
Fri Jul 12 12:04:17 2013 -0700

sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53^^^
Fri Jul 12 12:04:16 2013 -0700

sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53
Fri Jul 12 12:04:14 2013 -0700

sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53^
Fri Jul 12 12:04:12 2013 -0700

sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53^^
Fri Jul 12 12:04:10 2013 -0700

sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53^^^
Fri Jul 12 12:04:09 2013 -0700

sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53
Fri Jul 12 12:04:07 2013 -0700

However I sometimes also get:
sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53
Fri Jul 12 10:49:34 2013 -0700

diff --git a/Documentation/RelNotes/1.8.4.txt
b/Documentation/RelNotes/1.8.4.txt
index 0e50df8..4250e5a 100644
- --- a/Documentation/RelNotes/1.8.4.txt
+++ b/Documentation/RelNotes/1.8.4.txt
@@ -79,6 +79,13 @@ Foreign interfaces, subsystems and ports.

 UI, Workflows & Features

+ * "gitweb" learned to optionally place extra links that point at the
+   levels higher than the Gitweb pages themselves in the breadcrumbs,
+   so that it can be used as part of a larger installation.
+
+ * "git log --format=" now honors i18n.logoutputencoding configuration
+   variable.
+
  * The "push.default=simple" mode of "git push" has been updated to
behave like "current" without requiring a remote tracking
information, when you push to a remote that is different from where


sb@sb:~/OSS/git$ git show --format="%ad" 0da7a53
Fri Jul 12 12:04:07 2013 -0700

diff --cc t/t5505-remote.sh
index ee5d65d,8d0f3e3..8f6e392
- --- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@@ -61,25 -62,25 +61,25 @@@ test_expect_success C_LOCALE_OUTPUT 're
  '

  test_expect_success 'add another remote' '
 -(
 -  cd test &&
 -  git remote add -f second ../two &&
 -  tokens_match "origin second" "$(git remote)" &&
 -  check_tracking_branch second master side another &&
 -  git for-each-ref "--format=%(refname)" refs/remotes |
 -  sed -e "/^refs\/remotes\/origin\//d" \
 -  -e "/^refs\/remotes\/second\//d" >actual &&
 -  >expect &&




So at some commits, also the diff is shown, which should not happen
if you're using --format="%ad" to my understanding.

So far it seems to be deterministic here. (Each commit either always
behaves correctly or incorrectly). Initially I suspected it being
different for merges or no-merges, but I gut the unexpected behavior
for both merge and non-merge commits.

Can somebody confirm this behavior or has any idea, whether I am doing
something wrong here?

Thanks,
Stefan
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQIcBAEBAgAGBQJR49xHAAoJEJQCPTzLflhqiFcP/0S5M71urjn1Fh5Cz2950wBl
cs6+r9wKF5366wv3Ombfh0KrOPQCE7Yv1GWk2r2L008BWrz0wgnsl1xnMnAT00dH
1WzmupWExxwXHAueDisq2qL4dubFrnVxKWvpwxBd3JBbsWNXeTGl5o6dl69zaXdx
JHzIHvw1/vrxoCLaflitjcQezuFITOVVoNsYawK12gznjxujONm0sej5TBFUw784
K5KTJNJqxzUf9+Z+88hg2oif7kJlugTIqtH5sRMVwXrkpc12f+HcwROg0srE5ITc
8WpK0s7xgZokUCohMhUXlLAOYJwAZju+K1LExkrQ9T32oP4iAKsJpqcRLAAX6Ig7
OeaWUQ2WX2CfYDExjuV6h+FXAU0qT87iv3PgeZWAnmgDQPUwFboIxuF1Nrpq/FOY
Ioe2YOsFOdhmuDjCHEfu0aVNOeejHS8LEkC1IUI/+PzDlEJC/b17SwEdrD4aVCPj
RZhz2zI8ZxYHP9ITvMDs1VQRP6jMSAwEtAWg7ac7ypETToOIAxRp2j9Rrjayt8A5
/TZ++wMh7G4Tm/D9+iutQnqzE8E8eiT6i7LgAcDSA3g6oq7/hUzupKU2lc+znO2J
wxiwdqYBHWtw1Jc8eaZAJC/NMfxbRzOSCx4lJbO+tpXP0pR/OPIg2f2wUXcAh3zH
JC9GdEUVIwN34j2ueoZ8
=G6Oc
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 12/19] read-cache: read index-v5

2013-07-15 Thread Duy Nguyen
A little bit more..

On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer  wrote:
> +static void ce_queue_push(struct cache_entry **head,
> +struct cache_entry **tail,
> +struct cache_entry *ce)
> +{
> +   if (!*head) {
> +   *head = *tail = ce;
> +   (*tail)->next = NULL;
> +   return;
> +   }
> +
> +   (*tail)->next = ce;
> +   ce->next = NULL;
> +   *tail = (*tail)->next;

No no no. "next" is for name-hash.c don't "reuse" it here. And I don't
think you really need to maintain a linked list of cache_entries by
the way. More on read_entries comments below..

> +}
> +
> +static struct cache_entry *ce_queue_pop(struct cache_entry **head)
> +{
> +   struct cache_entry *ce;
> +
> +   ce = *head;
> +   *head = (*head)->next;
> +   return ce;
> +}

Same as ce_queue_push.

> +static int read_entries(struct index_state *istate, struct directory_entry 
> **de,
> +   unsigned int *entry_offset, void **mmap,
> +   unsigned long mmap_size, unsigned int *nr,
> +   unsigned int *foffsetblock)
> +{
> +   struct cache_entry *head = NULL, *tail = NULL;
> +   struct conflict_entry *conflict_queue;
> +   struct cache_entry *ce;
> +   int i;
> +
> +   conflict_queue = NULL;
> +   if (read_conflicts(&conflict_queue, *de, mmap, mmap_size) < 0)
> +   return -1;
> +   for (i = 0; i < (*de)->de_nfiles; i++) {
> +   if (read_entry(&ce,
> +  *de,
> +  entry_offset,
> +  mmap,
> +  mmap_size,
> +  foffsetblock) < 0)
> +   return -1;
> +   ce_queue_push(&head, &tail, ce);
> +   *foffsetblock += 4;
> +
> +   /*
> +* Add the conflicted entries at the end of the index file
> +* to the in memory format
> +*/
> +   if (conflict_queue &&
> +   (conflict_queue->entries->flags & CONFLICT_CONFLICTED) != 
> 0 &&
> +   !cache_name_compare(conflict_queue->name, 
> conflict_queue->namelen,
> +   ce->name, ce_namelen(ce))) {
> +   struct conflict_part *cp;
> +   cp = conflict_queue->entries;
> +   cp = cp->next;
> +   while (cp) {
> +   ce = convert_conflict_part(cp,
> +   conflict_queue->name,
> +   conflict_queue->namelen);
> +   ce_queue_push(&head, &tail, ce);
> +   conflict_part_head_remove(&cp);
> +   }
> +   conflict_entry_head_remove(&conflict_queue);
> +   }

I start to wonder if separating staged entries is a good idea. It
seems to make the code more complicated. The good point about conflict
section at the end of the file is you can just truncate() it out.
Another way is putting staged entries in fileentries, sorted
alphabetically then by stage number, and a flag indicating if the
entry is valid. When you remove resolve an entry, just set the flag to
invalid (partial write), so that read code will skip it.

I think this approach is reasonably cheap (unless there are a lot of
conflicts) and it simplifies this piece of code. truncate() may be
overrated anyway. In my experience, I "git add " as soon as I
resolve  (so that "git diff" shrinks). One entry at a time, one
index write at a time. I don't think I ever resolve everything then
"git add -u .", which is where truncate() shines because staged
entries are removed all at once. We should optimize for one file
resolution at a time, imo.

> +   }
> +
> +   *de = (*de)->next;
> +
> +   while (head) {
> +   if (*de != NULL
> +   && strcmp(head->name, (*de)->pathname) > 0) {
> +   read_entries(istate,
> +de,
> +entry_offset,
> +mmap,
> +mmap_size,
> +nr,
> +foffsetblock);
> +   } else {
> +   ce = ce_queue_pop(&head);
> +   set_index_entry(istate, *nr, ce);
> +   (*nr)++;
> +   ce->next = NULL;
> +   }

In this loop, you do some sort of merge sort combining a list of
sorted files and a list of sorted directories (which will be expanded
to bunches of files by the recursive read_entries). strcmp() does two
things. Assume we're in directory 'a', it makes sure that subdirector

[PATCH v5 2/5] http.c: fix parsing of GIT_SSL_CERT_PASSWORD_PROTECTED variable

2013-07-15 Thread Kyle J. McKay
The existing code triggers whenever GIT_SSL_CERT_PASSWORD_PROTECTED
is defined.  Setting GIT_SSL_CERT_PASSWORD_PROTECTED to a false
value could not be used to override the http.sslCertPasswordProtected
setting once it had been turned on.

Signed-off-by: Kyle J. McKay 
---
 http.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/http.c b/http.c
index e65661e..9542a59 100644
--- a/http.c
+++ b/http.c
@@ -403,11 +403,10 @@ void http_init(struct remote *remote, const char *url, 
int proactive_auth)
curl_ftp_no_epsv = 1;
 
if (url) {
+   int pwdreq = git_env_bool("GIT_SSL_CERT_PASSWORD_PROTECTED", 
-1);
credential_from_url(&http_auth, url);
-   if (!ssl_cert_password_required &&
-   getenv("GIT_SSL_CERT_PASSWORD_PROTECTED") &&
-   !prefixcmp(url, "https://";))
-   ssl_cert_password_required = 1;
+   if (pwdreq != -1 && !prefixcmp(url, "https://";))
+   ssl_cert_password_required = pwdreq;
}
 
 #ifndef NO_CURL_EASY_DUPHANDLE
-- 
1.8.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


[PATCH v5 3/5] config: add support for http..* settings

2013-07-15 Thread Kyle J. McKay
The credentials configuration values already support url-specific
configuration items in the form credential..*.  This patch
adds similar support for http configuration values.

The  value is considered a match to a url if the  value
is either an exact match or a prefix of the url which ends on a
path component boundary ('/').  So "https://example.com/test"; will
match "https://example.com/test"; and "https://example.com/test/too";
but not "https://example.com/testextra";.

Longer matches take precedence over shorter matches with
environment variable settings taking precedence over all.

With this configuration:

[http]
useragent = other-agent
noEPSV = true
[http "https://example.com";]
useragent = example-agent
sslVerify = false
[http "https://example.com/path";]
useragent = path-agent

The "https://other.example.com/"; url will have useragent
"other-agent" and sslVerify will be on.

The "https://example.com/"; url will have useragent
"example-agent" and sslVerify will be off.

The "https://example.com/path/sub"; url will have useragent
"path-agent" and sslVerify will be off.

All three of the examples will have noEPSV enabled.

Signed-off-by: Kyle J. McKay 
---
 Documentation/config.txt |  15 +
 http.c   | 169 ++-
 2 files changed, 167 insertions(+), 17 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 81856dd..ceced99 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1531,6 +1531,21 @@ http.useragent::
of common USER_AGENT strings (but not including those like git/1.7.1).
Can be overridden by the 'GIT_HTTP_USER_AGENT' environment variable.
 
+http..*::
+   Any of the http.* options above can be applied selectively to some urls.
+   For example "http.https://example.com.useragent"; would set the user
+   agent only for https connections to example.com.  The  value
+   matches a url if it is an exact match or a prefix of the url matching
+   at a "/" boundary.  Longer  matches take precedence over shorter
+   ones with the environment variable settings taking precedence over all.
+   Note that  must match the url passed to git exactly (other than
+   possibly being a prefix).  This means any user, password and/or port
+   setting that appears in a url as well as any %XX escapes that are
+   present must also appear in  to have a successful match.  The urls
+   that are matched against are those given directly to git commands.  In
+   other words, use exactly the same url that was passed to git (possibly
+   shortened) for the  value of the config setting.
+
 i18n.commitEncoding::
Character encoding the commit messages are stored in; Git itself
does not care per se, but this information is necessary e.g. when
diff --git a/http.c b/http.c
index 9542a59..758e5b1 100644
--- a/http.c
+++ b/http.c
@@ -30,6 +30,34 @@ static CURL *curl_default;
 
 char curl_errorstr[CURL_ERROR_SIZE];
 
+enum http_option_type {
+   OPT_POST_BUFFER,
+   OPT_MIN_SESSIONS,
+   OPT_SSL_VERIFY,
+   OPT_SSL_TRY,
+   OPT_SSL_CERT,
+   OPT_SSL_CAINFO,
+   OPT_LOW_SPEED,
+   OPT_LOW_TIME,
+   OPT_NO_EPSV,
+   OPT_HTTP_PROXY,
+   OPT_COOKIE_FILE,
+   OPT_USER_AGENT,
+   OPT_PASSWD_REQ,
+#ifdef USE_CURL_MULTI
+   OPT_MAX_REQUESTS,
+#endif
+#if LIBCURL_VERSION_NUM >= 0x070903
+   OPT_SSL_KEY,
+#endif
+#if LIBCURL_VERSION_NUM >= 0x070908
+   OPT_SSL_CAPATH,
+#endif
+   OPT_MAX
+};
+
+static size_t http_option_max_matched_len[OPT_MAX];
+
 static int curl_ssl_verify = -1;
 static int curl_ssl_try;
 static const char *ssl_cert;
@@ -141,33 +169,121 @@ static void process_curl_messages(void)
 }
 #endif
 
+static size_t http_options_url_match_prefix(const char *url,
+   const char *url_prefix,
+   size_t url_prefix_len)
+{
+   /*
+* url_prefix matches url if url_prefix is an exact match for url or it
+* is a prefix of url and the match ends on a path component boundary.
+* Both url and url_prefix are considered to have an implicit '/' on the
+* end for matching purposes if they do not already.
+*
+* url must be NUL terminated.  url_prefix_len is the length of
+* url_prefix which need not be NUL terminated.
+*
+* The return value is the length of the match in characters (excluding
+* any final '/') or 0 for no match.  Passing "/" as url_prefix will
+* always cause 0 to be returned.
+*/
+   size_t url_len;
+   if (url_prefix_len && url_prefix[url_prefix_len - 1] == '/')
+   url_prefix_len--;
+   if (!url_prefix_len || strncmp(url, url_prefix, url_prefix_len))
+   return 0;
+   url_len = strlen(url);
+   if ((url_len == url_

[PATCH v5 4/5] config: improve support for http..* settings

2013-07-15 Thread Kyle J. McKay
Improve on the http..* url matching behavior by first
normalizing the urls before they are compared.

With this change, for example, the following configuration
section:

[http "https://example.com/path";]
useragent = example-agent
sslVerify = false

will properly match a "HTTPS://example.COM/p%61th" url which
is equivalent.

The normalization rules are based on RFC 3986 and should
result in any two equivalent urls being a match.

Signed-off-by: Kyle J. McKay 
---
 Documentation/config.txt |  19 ++--
 http.c   | 222 ++-
 2 files changed, 229 insertions(+), 12 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ceced99..98eee23 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1535,16 +1535,15 @@ http..*::
Any of the http.* options above can be applied selectively to some urls.
For example "http.https://example.com.useragent"; would set the user
agent only for https connections to example.com.  The  value
-   matches a url if it is an exact match or a prefix of the url matching
-   at a "/" boundary.  Longer  matches take precedence over shorter
-   ones with the environment variable settings taking precedence over all.
-   Note that  must match the url passed to git exactly (other than
-   possibly being a prefix).  This means any user, password and/or port
-   setting that appears in a url as well as any %XX escapes that are
-   present must also appear in  to have a successful match.  The urls
-   that are matched against are those given directly to git commands.  In
-   other words, use exactly the same url that was passed to git (possibly
-   shortened) for the  value of the config setting.
+   matches a url if it is an exact match or if it is a prefix of the url
+   matching at a "/" boundary.  Longer  matches take precedence over
+   shorter ones with the environment variable settings taking precedence
+   over all.  The urls are normalized before testing for a match.  Note,
+   however, that any user, password and/or port setting that appears in a
+   url must also match that part of  to have a successful match.  The
+   urls that are matched against are those given directly to git commands.
+   This means any urls visited as a result of a redirection do not
+   participate in matching.
 
 i18n.commitEncoding::
Character encoding the commit messages are stored in; Git itself
diff --git a/http.c b/http.c
index 758e5b1..d04386e 100644
--- a/http.c
+++ b/http.c
@@ -169,6 +169,210 @@ static void process_curl_messages(void)
 }
 #endif
 
+#define URL_ALPHA "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+#define URL_DIGIT "0123456789"
+#define URL_HEXDIGIT URL_DIGIT "ABCDEFabcdef"
+#define URL_ALPHADIGIT URL_ALPHA URL_DIGIT
+#define URL_SCHEME_CHARS URL_ALPHADIGIT "+.-"
+#define URL_HOST_CHARS URL_ALPHADIGIT ".-[:]" /* IPv6 literals need [:] */
+#define URL_UNSAFE_CHARS " <>\"%{}|\\^`" /* plus 0x00-0x1F,0x7F-0xFF */
+#define URL_GEN_RESERVED ":/?#[]@"
+#define URL_SUB_RESERVED "!$&'()*+,;="
+#define URL_RESERVED URL_GEN_RESERVED URL_SUB_RESERVED /* only allowed delims 
*/
+
+static int hex_digit_value(int ch)
+{
+   /*
+* Returns the hexadecimal digit value (0x00..0x0F) of character ch.
+* If ch is not a hexadecimal digit, the return value is undefined.
+*/
+
+   if ('0' <= ch && ch <= '9')
+   return ch - '0';
+   return toupper(ch) - 'A' + 10;
+}
+
+static int append_normalized_escapes(struct strbuf *buf,
+const char *from,
+size_t from_len,
+const char *esc_extra,
+const char *esc_ok)
+{
+   /*
+* Append to strbuf buf characters from string from with length from_len
+* while unescaping characters that do not need to be escaped and
+* escaping characters that do.  The set of characters to escape (the
+* complement of which is unescaped) starts out as the RFC 3986 unsafe
+* characters (0x00-0x1F,0x7F-0xFF," <>\"#%{}|\\^`").  If esc_extra
+* is not NULL, those additional characters will also always be escaped.
+* If esc_ok is not NULL, those characters will be left escaped if found
+* that way, but will not be unescaped otherwise (used for delimiters).
+* If a %-escape sequence is encountered that is not followed by 2
+* hexadecimal digits, the sequence is invalid and false (0) will be
+* returned.  Otherwise true (1) will be returned for success.
+*
+* Note that all %-escape sequences will be normalized to UPPERCASE
+* as indicated in RFC 3986.  Unless included in esc_extra or esc_ok
+* alphanumerics and "-._~" will always be unescaped as per RFC 3986.

[PATCH v5 5/5] tests: add new test for the url_normalize function

2013-07-15 Thread Kyle J. McKay
In order to perform sane URL matching for http..* options,
http.c normalizes URLs before performing matches.

A new test-url-normalize test program is introduced along with
a new t5200-url-normalize.sh script to run the tests.

Since the url_normalize function currently lives in http.c this
test will be skipped if NO_CURL is defined since http.c is skipped
in that case.

Signed-off-by: Kyle J. McKay 
---
 .gitignore   |   1 +
 Makefile |   5 +++
 t/t5200-url-normalize.sh | 109 +++
 test-url-normalize.c |  62 +++
 4 files changed, 177 insertions(+)
 create mode 100755 t/t5200-url-normalize.sh
 create mode 100644 test-url-normalize.c

diff --git a/.gitignore b/.gitignore
index efa8db0..b60a9df 100644
--- a/.gitignore
+++ b/.gitignore
@@ -201,6 +201,7 @@
 /test-string-list
 /test-subprocess
 /test-svn-fe
+/test-url-normalize
 /test-wildmatch
 /common-cmds.h
 *.tar.gz
diff --git a/Makefile b/Makefile
index 0600eb4..195c62a 100644
--- a/Makefile
+++ b/Makefile
@@ -580,6 +580,7 @@ TEST_PROGRAMS_NEED_X += test-sigchain
 TEST_PROGRAMS_NEED_X += test-string-list
 TEST_PROGRAMS_NEED_X += test-subprocess
 TEST_PROGRAMS_NEED_X += test-svn-fe
+TEST_PROGRAMS_NEED_X += test-url-normalize
 TEST_PROGRAMS_NEED_X += test-wildmatch
 
 TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))
@@ -2278,6 +2279,10 @@ test-parse-options$X: parse-options.o parse-options-cb.o
 
 test-svn-fe$X: vcs-svn/lib.a
 
+test-url-normalize$X: test-url-normalize.o GIT-LDFLAGS $(GITLIBS)
+   $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
+   $(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
+
 .PRECIOUS: $(TEST_OBJS)
 
 test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)
diff --git a/t/t5200-url-normalize.sh b/t/t5200-url-normalize.sh
new file mode 100755
index 000..40ebcd3
--- /dev/null
+++ b/t/t5200-url-normalize.sh
@@ -0,0 +1,109 @@
+#!/bin/sh
+
+test_description='url normalization'
+. ./test-lib.sh
+
+if test -n "$NO_CURL"; then
+   skip_all='skipping test, git built without http support'
+   test_done
+fi
+
+# Note that only file: URLs should be allowed without a host
+
+test_expect_success 'url scheme' '
+   ! test-url-normalize "" &&
+   ! test-url-normalize "_" &&
+   ! test-url-normalize "scheme" &&
+   ! test-url-normalize "scheme:" &&
+   ! test-url-normalize "scheme:/" &&
+   ! test-url-normalize "scheme://" &&
+   ! test-url-normalize "file" &&
+   ! test-url-normalize "file:" &&
+   ! test-url-normalize "file:/" &&
+   test-url-normalize "file://" &&
+   ! test-url-normalize "://acme.co" &&
+   ! test-url-normalize "x_test://acme.co" &&
+   ! test-url-normalize "schem%6e://" &&
+   test-url-normalize "x-Test+v1.0://acme.co" &&
+   test "$(test-url-normalize -p "AbCdeF://x.Y")" = "abcdef://x.y/"
+'
+
+test_expect_success 'url authority' '
+   ! test-url-normalize "scheme://user:pass@" &&
+   ! test-url-normalize "scheme://?" &&
+   ! test-url-normalize "scheme://#" &&
+   ! test-url-normalize "scheme:///" &&
+   ! test-url-normalize "scheme://:" &&
+   ! test-url-normalize "scheme://:555" &&
+   test-url-normalize "file://user:pass@" &&
+   test-url-normalize "file://?" &&
+   test-url-normalize "file://#" &&
+   test-url-normalize "file:///" &&
+   test-url-normalize "file://:" &&
+   test-url-normalize "file://:555" &&
+   test-url-normalize "scheme://user:pass@host" &&
+   test-url-normalize "scheme://@host" &&
+   test-url-normalize "scheme://%00@host" &&
+   ! test-url-normalize "scheme://%%@host" &&
+   ! test-url-normalize "scheme://host_" &&
+   test-url-normalize "scheme://user:pass@host/" &&
+   test-url-normalize "scheme://@host/" &&
+   test-url-normalize "scheme://host/" &&
+   test-url-normalize "scheme://host?x" &&
+   test-url-normalize "scheme://host#x" &&
+   test-url-normalize "scheme://host/@" &&
+   test-url-normalize "scheme://host?@x" &&
+   test-url-normalize "scheme://host#@x" &&
+   test-url-normalize "scheme://[::1]" &&
+   test-url-normalize "scheme://[::1]/" &&
+   ! test-url-normalize "scheme://hos%41/" &&
+   test-url-normalize "scheme://[invalid:/" &&
+   test-url-normalize "scheme://invalid:]/" &&
+   ! test-url-normalize "scheme://invalid:[/" &&
+   ! test-url-normalize "scheme://invalid:["
+'
+
+test_expect_success 'url port' '
+   test-url-normalize "xyz://q...@some.host:" &&
+   test-url-normalize "xyz://q...@some.host:456" &&
+   test-url-normalize "xyz://q...@some.host:0" &&
+   test-url-normalize "xyz://q...@some.host:9" &&
+   test-url-normalize "http://q...@some.host:80"; &&
+   test-url-normalize "https://q...@some.host:443";
+   ! test-url-normalize "http://q@:8008"; &&
+   ! test-url-normalize "http://:8080"; &&
+ 

Re: [PATCH v2 08/19] grep.c: Use index api

2013-07-15 Thread Thomas Gummerer
Duy Nguyen  writes:

> On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer  
> wrote:
>> +static int grep_cache(struct cache_entry *ce, void *cb_data)
>>  {
>> -   int hit = 0;
>> -   int nr;
>> -   read_cache();
>> +   struct grep_opts *opts = cb_data;
>>
>> -   for (nr = 0; nr < active_nr; nr++) {
>> -   struct cache_entry *ce = active_cache[nr];
>> -   if (!S_ISREG(ce->ce_mode))
>> -   continue;
>> -   if (!match_pathspec_depth(pathspec, ce->name, 
>> ce_namelen(ce), 0, NULL))
>> -   continue;
>> -   /*
>> -* If CE_VALID is on, we assume worktree file and its cache 
>> entry
>> -* are identical, even if worktree file has been modified, 
>> so use
>> -* cache version instead
>> -*/
>> -   if (cached || (ce->ce_flags & CE_VALID) || 
>> ce_skip_worktree(ce)) {
>> -   if (ce_stage(ce))
>> -   continue;
>> -   hit |= grep_sha1(opt, ce->sha1, ce->name, 0, 
>> ce->name);
>> -   }
>> -   else
>> -   hit |= grep_file(opt, ce->name);
>> -   if (ce_stage(ce)) {
>> -   do {
>> -   nr++;
>> -   } while (nr < active_nr &&
>> -!strcmp(ce->name, active_cache[nr]->name));
>> -   nr--; /* compensate for loop control */
>> -   }
>> -   if (hit && opt->status_only)
>> -   break;
>> -   }
>> -   return hit;
>> +   if (!S_ISREG(ce->ce_mode))
>> +   return 0;
>> +   if (!match_pathspec_depth(opts->pathspec, ce->name, ce_namelen(ce), 
>> 0, NULL))
>> +   return 0;
>
> You do a match_pathspec_depth here..
>
>> @@ -895,10 +887,21 @@ int cmd_grep(int argc, const char **argv, const char 
>> *prefix)
>> } else if (0 <= opt_exclude) {
>> die(_("--[no-]exclude-standard cannot be used for tracked 
>> contents."));
>> } else if (!list.nr) {
>> +   struct grep_opts opts;
>> +   struct filter_opts *filter_opts = 
>> xmalloc(sizeof(*filter_opts));
>> +
>> if (!cached)
>> setup_work_tree();
>>
>> -   hit = grep_cache(&opt, &pathspec, cached);
>> +   memset(filter_opts, 0, sizeof(*filter_opts));
>> +   filter_opts->pathspec = &pathspec;
>> +   opts.opt = &opt;
>> +   opts.pathspec = &pathspec;
>> +   opts.cached = cached;
>> +   opts.hit = 0;
>> +   read_cache_filtered(filter_opts);
>> +   for_each_cache_entry(grep_cache, &opts);
>
> And here again inside for_each_cache_entry. In the worst case that
> could turn into 2 expensive fnmatch instead of one. Is this conversion
> worth it? Note that match_pathspec is just a deprecated version of
> match_pathspec_depth. They basically do the same thing.

Right, the match_pathspec_depth should in builtin/grep.c should be
removed, it's unnecessary when using for_each_index_entry.  Thanks for
spotting it.  Other than that I still think the change makes sense.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 0/5] config: add support for http..* settings

2013-07-15 Thread Kyle J. McKay
This patch series adds support for url-specific http.* settings.

It has been suggested that a preparatory patch to address the way the
http.sslCertPasswordProtected variable is handled be included.

I'm not sure if the intent was to make that a separate patch or include it
in a patch series together with the http..* settings updates.  I have,
however, included it here this time along with a second patch that does the
same thing for the GIT_SSL_CERT_PASSWORD_PROTECTED environment variable.

There is talk about possibly altering GIT_SSL_NO_VERIFY, GIT_CURL_FTP_NO_EPSV
and GIT_CURL_VERBOSE to behave similarly.  No patch is included here for that.

The http..* settings support addresses various feedback.  To better
support matching URLs that are equivalent but spelled differently, a
url_normalize function has been added.  Currently this patch leaves it in
http.c as http_options_url_normalize as I am unclear whether it should go into
url.{h,c} at this time since only http.c uses.

Since the url_normalize function's behavior is non-trivial, it is presented as
a separate patch on top of the basic http..* settings support.  A new test
for it has also been included as the final patch.  I am unclear on the proper
number for this test, but have gone ahead and put it with the other http tests
since this patch series places the url_normalize function into http.c.

Junio C Hamano (1):
  http.c: fix parsing of http.sslCertPasswordProtected variable

Kyle J. McKay (4):
  http.c: fix parsing of GIT_SSL_CERT_PASSWORD_PROTECTED variable
  config: add support for http..* settings
  config: improve support for http..* settings
  tests: add new test for the url_normalize function

 .gitignore   |   1 +
 Documentation/config.txt |  14 ++
 Makefile |   5 +
 http.c   | 397 ---
 t/t5200-url-normalize.sh | 109 +
 test-url-normalize.c |  62 
 6 files changed, 565 insertions(+), 23 deletions(-)
 create mode 100755 t/t5200-url-normalize.sh
 create mode 100644 test-url-normalize.c

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


[PATCH v5 1/5] http.c: fix parsing of http.sslCertPasswordProtected variable

2013-07-15 Thread Kyle J. McKay
From: Junio C Hamano 

The existing code triggers only when the configuration variable is
set to true.  Once the variable is set to true in a more generic
configuration file (e.g. ~/.gitconfig), it cannot be overriden to
false in the repository specific one (e.g. .git/config).

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

diff --git a/http.c b/http.c
index 2d086ae..e65661e 100644
--- a/http.c
+++ b/http.c
@@ -160,8 +160,7 @@ static int http_options(const char *var, const char *value, 
void *cb)
if (!strcmp("http.sslcainfo", var))
return git_config_string(&ssl_cainfo, var, value);
if (!strcmp("http.sslcertpasswordprotected", var)) {
-   if (git_config_bool(var, value))
-   ssl_cert_password_required = 1;
+   ssl_cert_password_required = git_config_bool(var, value);
return 0;
}
if (!strcmp("http.ssltry", var)) {
-- 
1.8.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 v3] config: add support for http..* settings

2013-07-15 Thread Kyle J. McKay
(I'm attempting to combine the various separate email replies into a  
single response here, please forgive me if I mangle something up.)




On Jul 14, 2013, at 22:12, Jeff King wrote:

On Sun, Jul 14, 2013 at 09:02:19PM -0700, Junio C Hamano wrote:


Or proceed with what's there right now (there are a few pending
updates from reviewers) and then, as Junio says above, adjust it  
later

if needed?


I have been assuming that "strictly textual match" will be a subset
of the matching semantics Aaron and Peff suggested.  That is, if we
include your version in the upcoming release, the user writes the
http.. configuration so that the entries match
what they want them to match, the enhanced URL matcher Aaron and
Peff suggested will still make them match.

Am I mistaken?  Will there be some  that will not match
with the same URL literally?


I think we need to decide now, because the two schemes are not
compatible, and switching will break setups. Yes, the matcher that  
Aaron

and I suggest is a strict superset (i.e., we will not stop matching
things that used to match), which is good. But we would not be able to
implement "longest prefix wins" overriding anymore, which would change
the meaning of cases like:

 [http "https://example.com";] foo = 1
 [http] foo = 2

(under Kyle's scheme it is "1", and under ours "2"). We can probably
come up with some clever rules for overriding a broken-down URL that
would stay backwards compatible. E.g., longest-prefix-match if there  
are

no wildcarded components, and last-one-wins if there are. But that is
not a rule I would want readers to have to puzzle out in the
documentation.

So I think we are much better off to decide the semantics now.


Yes.  Consider these two commands:


git config http.foo = 2
git config http.https://example.com/.foo = 1


I am proposing that this means https://example.com has foo set to 1  
(assuming there are no other http*.foo configurations).


The other proposal probably means that, but it might not.

Given this sequence:


git config http.foo = 2
git config http.https://example.com/.foo = 1



or this sequence:


git config http.https://example.com/.foo = 1
git config http.foo = 2


what actually happens when using the other proposal will depend on  
whether or not the user has previously configured any other "http.*"  
setting or any other "http.https://example.com/.*"; setting since doing  
so would have established such a section in the config file and it  
will affect the order the directives are processed since they will be  
added to the existing section rather than creating a new same-named  
section at the end of the file.


For this reason the order the above two "git config" commands are  
given cannot be relied upon to determine what setting http.foo will  
have when a https://example.com/ url is accessed.


Since git config does not have a "git config --placing-after-section  
section-name http.foo 2" option it seems to me that the only way to be  
sure what you would end up with using this method is to examine the  
created config file (git config -l would be sufficient although  
probably harder to read than viewing the config file itself).


For an individual .git/config file I don't expect this to be an  
issue.  However for the --global config file, I believe quite some  
time could go by between setting one http.* option and another so it  
seems quite likely to me that the user may not remember or have ever  
been aware of what order the various [http*] sections are currently in.


This is why I originally proposed longest-match-wins semantics, but  
please read on.




On Jul 14, 2013, at 22:06, Jeff King wrote:
I admit that these are unlikely to come up in practice, but I am  
worried

that there is some room for mischief here. For example:

 https://example.com%2ftricky.host/repo.git


This is actually an invalid URL.  Host names may not contain '%'  
characters and can only be followed by optionally ':port' and then one  
of '/', '?', or '#'.  A URL parser would actually die when it sees the  
'%' there as there's no way to match that to the URL grammar rules.   
(It wouldn't actually be taken as part of the host name even though it  
may appear to be.)



If we canonicalize that into:

 https://example.com/tricky.host/repo.git


I don't think this will be a concern since that is an invalid  
normalization.  Perhaps there is another example that is also  
concerning that needs to be addressed?



One of the things that gets encoded are the delimiting characters.  
So if

I have the URL:

 https://foo%3a...@example.com

you would "canonicalize" it into:

 https://foo:b...@example.com

But those are two different URLs entirely; the first has the username
"foo:bar", and the second has the username "foo" and the password  
"bar".


That would be an incorrect normalization according to RFC 3986.  '@',  
'/' and ':' must be escaped in the user:password portion and decoding  
them there is verboten (prohibited).  And delim

Re: [PATCH v2 00/19] Index-v5

2013-07-15 Thread Duy Nguyen
On Mon, Jul 15, 2013 at 4:30 PM, Thomas Gummerer  wrote:
> Duy Nguyen  writes:
>
>> On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer  
>> wrote:
>>>  t/perf/p0003-index.sh|   59 +
>>>  t/t2104-update-index-skip-worktree.sh|1 +
>>
>> For such a big code addition, the test part seems modest. Speaking
>> from my experience, I rarely run perf tests and "make test" does not
>> activate v5 code at all. A few more tests would be nice. The good news
>> is I changed default index version to 5 and ran "make test", all
>> passed.
>
> I was using the test suite with index version 5 as default index version
> for testing of the new index file format.   I think that's the best way
> to test the index, as it covers all aspects.

We need other people to run the test suite with v5 to catch bugs that
only appear on other platforms. Perhaps an env variable to allow the
test suite to override the binary's default index version and a new
make target "test-v5" maybe.

> Maybe we should add a test
> that covers the basic functionality, just to make sure nothing obvious
> is broken when running the test suite with index-v2?  Something like
> this maybe:

Yep. v5 specfic test cases can cover some corner cases that the rest
of the test suite does not care. Haven't read your t1600 though.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/19] Index-v5

2013-07-15 Thread Thomas Gummerer
Duy Nguyen  writes:

> On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer  
> wrote:
>>  t/perf/p0003-index.sh|   59 +
>>  t/t2104-update-index-skip-worktree.sh|1 +
>
> For such a big code addition, the test part seems modest. Speaking
> from my experience, I rarely run perf tests and "make test" does not
> activate v5 code at all. A few more tests would be nice. The good news
> is I changed default index version to 5 and ran "make test", all
> passed.

I was using the test suite with index version 5 as default index version
for testing of the new index file format.   I think that's the best way
to test the index, as it covers all aspects.  Maybe we should add a test
that covers the basic functionality, just to make sure nothing obvious
is broken when running the test suite with index-v2?  Something like
this maybe:

--->8---

>From c476b521c94f1a9b0b4fcfe92d63321442d79c9a Mon Sep 17 00:00:00 2001
From: Thomas Gummerer 
Date: Mon, 15 Jul 2013 11:21:06 +0200
Subject: [PATCH] t1600: add basic test for index-v5

Add a test that checks the index-v5 file format when running the
test-suite with index-v2 as default index format.  When making changes
to the index, the test suite still should be run with both index v2 and
index v5 as default index format, for better coverage of all aspects of
the index.

Signed-off-by: Thomas Gummerer 
---
 t/t1600-index-v5.sh | 133 
 1 file changed, 133 insertions(+)
 create mode 100755 t/t1600-index-v5.sh

diff --git a/t/t1600-index-v5.sh b/t/t1600-index-v5.sh
new file mode 100755
index 000..528c17e
--- /dev/null
+++ b/t/t1600-index-v5.sh
@@ -0,0 +1,133 @@
+#!/bin/sh
+#
+# Copyright (c) 2013 Thomas Gummerer
+
+test_description='Test basic functionaltiy of index-v5.
+
+This test just covers the basics, to make sure normal runs of the test
+suite cover this version of the index file format too.  For better
+testing of the index-v5 format, the default index version should be
+changed to 5 and the test suite should be re-run'
+
+. ./test-lib.sh
+
+check_resolve_undo () {
+   msg=$1
+   shift
+   while case $# in
+   0)  break ;;
+   1|2|3)  die "Bug in check-resolve-undo test" ;;
+   esac
+   do
+   path=$1
+   shift
+   for stage in 1 2 3
+   do
+   sha1=$1
+   shift
+   case "$sha1" in
+   '') continue ;;
+   esac
+   sha1=$(git rev-parse --verify "$sha1")
+   printf "100644 %s %s\t%s\n" $sha1 $stage $path
+   done
+   done >"$msg.expect" &&
+   git ls-files --resolve-undo >"$msg.actual" &&
+   test_cmp "$msg.expect" "$msg.actual"
+}
+
+prime_resolve_undo () {
+   git reset --hard &&
+   git checkout second^0 &&
+   test_tick &&
+   test_must_fail git merge third^0 &&
+   echo merge does not leave anything &&
+   check_resolve_undo empty &&
+   echo different >fi/le &&
+   git add fi/le &&
+   echo resolving records &&
+   check_resolve_undo recorded fi/le initial:fi/le second:fi/le third:fi/le
+}
+
+test_expect_success 'setup' '
+   git update-index --index-version=5 &&
+   echo file1 >file1 &&
+   echo file2 >file2 &&
+   mkdir -p top/sub &&
+   echo x >top/x &&
+   echo xy >top/xy &&
+   echo y >top/y &&
+   echo yx >top/yx &&
+   echo sub1 >top/sub/sub1 &&
+   git add . &&
+   git commit -m "initial import"
+'
+
+test_expect_success 'ls-files shows all files' '
+   cat >expected <<-EOF &&
+   100644 e2129701f1a4d54dc44f03c93bca0a2aec7c5449 0   file1
+   100644 6c493ff740f9380390d5c9ddef4af18697ac9375 0   file2
+   100644 48df0cb83fee5d667537343f60a6057a63dd3c9b 0   top/sub/sub1
+   100644 587be6b4c3f93f93c489c0111bba5596147a26cb 0   top/x
+   100644 5aad9376af82d7b98a34f95fb0f298a162f52656 0   top/xy
+   100644 975fbec8256d3e8a3797e7a3611380f27c49f4ac 0   top/y
+   100644 ba1575927fa5b1f4bce72ad0c349566f1b02508e 0   top/yx
+   EOF
+   git ls-files --stage >actual &&
+   test_cmp expected actual
+'
+
+test_expect_success 'ls-files with pathspec in subdir' '
+   cd top/sub &&
+   cat >expected <<-EOF &&
+   ../x
+   ../xy
+   EOF
+   git ls-files --error-unmatch ../x* >actual &&
+   test_cmp expected actual &&
+   cd ../..
+'
+
+test_expect_success 'read-tree HEAD establishes cache-tree' '
+   git read-tree HEAD &&
+   cat >expected <<-EOF &&
+   84e73410ea7864ccada24d897462e8ce1e1b872b  (7 entries, 1 subtrees)
+   602482536bd3852e8ac2977ed1a9913a8c244aa0 top/ (5 entries, 1 subtrees)
+   20bb0109200f37a7e19283b4abc4a672be3f0adb top/sub/ (1 entries, 0 
subtrees)
+   EOF
+   test-dump-cache-tree >actual &&
+   test_cmp expected a

[PATCH 2/4] gitweb: vertically centre contents of page footer

2013-07-15 Thread Tony Finch
Signed-off-by: Tony Finch 
---
 gitweb/static/gitweb.css | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index a869be1..3b4d833 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -68,12 +68,13 @@ div.page_path {
 }
 
 div.page_footer {
-   height: 17px;
+   height: 22px;
padding: 4px 8px;
background-color: #d9d8d1;
 }
 
 div.page_footer_text {
+   line-height: 22px;
float: left;
color: #55;
font-style: italic;
-- 
1.8.3.1.605.g85318f5

--
To unsubscribe from this list: send the line "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 4/4] gitweb: make search help link less ugly

2013-07-15 Thread Tony Finch
The search help link was a superscript question mark right next to
a drop-down menu, which looks misaligned and is a cramped and
awkward click target. Remove the superscript tags and add some
spacing to fix these nits. Add a title attribute to provide an
explanatory mouseover.

Signed-off-by: Tony Finch 
---
 gitweb/gitweb.perl | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c029b98..874c948 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4029,9 +4029,9 @@ sub print_search_form {
  $cgi->input({-name=>"a", -value=>"search", -type=>"hidden"}) . 
"\n" .
  $cgi->input({-name=>"h", -value=>$search_hash, -type=>"hidden"}) 
. "\n" .
  $cgi->popup_menu(-name => 'st', -default => 'commit',
-  -values => ['commit', 'grep', 'author', 
'committer', 'pickaxe']) .
- $cgi->sup($cgi->a({-href => href(action=>"search_help")}, "?")) .
- " search:\n",
+  -values => ['commit', 'grep', 'author', 
'committer', 'pickaxe']) .
+ " " . $cgi->a({-href => href(action=>"search_help"),
+-title => "search help" }, "?") . " search:\n",
  $cgi->textfield(-name => "s", -value => $searchtext, -override => 
1) . "\n" .
  "" .
  $cgi->checkbox(-name => 'sr', -value => 1, -label => 're',
-- 
1.8.3.1.605.g85318f5

--
To unsubscribe from this list: send the line "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 3/4] gitweb: omit the repository owner when it is unset

2013-07-15 Thread Tony Finch
On the repository summary page, leave the whole owner line out if
the repo does not have an owner, rather than displaying a labelled
empty field..

Signed-off-by: Tony Finch 
---
 gitweb/gitweb.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8d69ada..c029b98 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6463,7 +6463,7 @@ sub git_summary {
print " \n";
print "\n" .
  "description" . 
esc_html($descr) . "\n";
-unless ($omit_owner) {
+if ($owner and not $omit_owner) {
print  "owner" . 
esc_html($owner) . "\n";
 }
if (defined $cd{'rfc2822'}) {
-- 
1.8.3.1.605.g85318f5

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


  1   2   >