Re: [PATCH v2 2/2] send-email: introduce sendemail.smtpsslcertpath

[Junio C Hamano]

John Keeping  writes:

> I'd rather have '$smtp_ssl_cert_path ne ""' in the first if condition
> (instead of the '-d $smtp_ssl_cert_path') ...

I agree.  The signal for "no certs" should be an explicit "nonsense"
value like an empty string, not just a string that does not name an
expected filesystem object.  Otherwise people can misspell paths and
disable the validation by accident.

> Perhaps a complete solution could allow CA files as well.

Yes, that would be a good idea.  Care to roll into a "fixup!" patch
against [2/2]?

>   if (defined $smtp_ssl_cert_path) {
>   if ($smtp_ssl_cert_path eq "") {
>   return (SSL_verify_mode => SSL_VERIFY_NONE);
>   } elsif (-f $smtp_ssl_cert_path) {
>   return (SSL_verify_mode => SSL_VERIFY_PEER,
>   SSL_ca_file => $smtp_ssl_cert_path);
>   } else {
>   return (SSL_verify_mode => SSL_VERIFY_PEER,
>   SSL_ca_path => $smtp_ssl_cert_path);
>   }
>   } else {
>   return (SSL_verify_mode => SSL_VERIFY_PEER);
>   }

Two things that worry me a bit are:

 (1) At the end user UI level, it may look nice to accept some form
 of --no-option-name to say "I have been using SSL against my
 server with handrolled cert, and I want to keep using the
 verify-none option"; "--ssl-cert-path=" looks somewhat ugly.
 The same goes for [sendemail] ssl_cert_path = "" config.

 (2) How loudly does the new code barf when no configuration is done
 (i.e. we just pass SSL_VERIFY_PEER and let the system default
 CA path to take effect) and the server cert does not validate?
 The warning that triggered this thread, if we had the
 configuration mechanism we have been designing together, would
 have been a good reminder for the user to use it, but would we
 give a similar (less noisy is fine, as long as it is clear)
 diagnostic 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

[PATCH 2/2] git-svn: allow git-svn fetching to work using serf

[Kyle McKay]

When attempting to git-svn fetch files from an svn https?: url using
the serf library (the only choice starting with svn 1.8) the following
errors can occur:

Temp file with moniker 'svn_delta' already in use at Git.pm line 1250
Temp file with moniker 'git_blob' already in use at Git.pm line 1250

David Rothenberger  has determined the cause to
be that ra_serf does not drive the delta editor in a depth-first
manner [...]. Instead, the calls come in this order:

1. open_root
2. open_directory
3. add_file
4. apply_textdelta
5. add_file
6. apply_textdelta

This change causes a new temp file moniker to be generated if the
one that would otherwise have been used is currently locked.

Signed-off-by: Kyle J. McKay 
---
perl/Git/SVN/Fetcher.pm | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm
index bd17418..10edb27 100644
--- a/perl/Git/SVN/Fetcher.pm
+++ b/perl/Git/SVN/Fetcher.pm
@@ -315,11 +315,13 @@ sub change_file_prop {
sub apply_textdelta {
my ($self, $fb, $exp) = @_;
return undef if $self->is_path_ignored($fb->{path});
-   my $fh = $::_repository->temp_acquire('svn_delta');
+   my $suffix = 0;
+	++$suffix while $::_repository->temp_is_locked("svn_delta_${$}_ 
$suffix");

+   my $fh = $::_repository->temp_acquire("svn_delta_${$}_$suffix");
# $fh gets auto-closed() by SVN::TxDelta::apply(),
# (but $base does not,) so dup() it for reading in close_file
open my $dup, '<&', $fh or croak $!;
-   my $base = $::_repository->temp_acquire('git_blob');
+   my $base = $::_repository->temp_acquire("git_blob_${$}_$suffix");

if ($fb->{blob}) {
my ($base_is_link, $size);
--
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: git-svn "Temp file with moniker 'svn_delta' already in use" and skelta mode

[Kyle McKay]

On Jul 5, 2013, at 16:14, David Rothenberger wrote:

On 7/5/2013 1:48 PM, David Rothenberger wrote:

I recently upgraded my Subversion server to 1.8.0 and started
receiving the following error from "git svn fetch":

Temp file with moniker 'svn_delta' already in use at /usr/lib/perl5/ 
vendor_perl/5.10/Git.pm line 1024.


This occurs only when using an http:// URL; svn:// URLs work fine.


I have created a patch (separate emails) that seems to work.  However,  
if the server being fetched against is not configured well for use  
with serf then "Error retrieving REPORT (54): Connection reset by peer  
Git/SVN/Ra.pm line 282" errors can occur and/or the fetch can be  
horribly slow compared to using neon.  If the "Connection reset" error  
occurs, the fetch can be restarted to get more revisions until it dies  
again.


Kyle

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

[PATCH 0/2] allow git-svn fetching to work using serf

[Kyle McKay]

This patch allows git-svn to fetch successfully using the
serf library when given an https?: url to fetch from.

Unfortunately some svn servers do not seem to be configured
well for use with the serf library.  This can cause fetching
to take longer compared to the neon library or actually
cause timeouts during the fetch.  When timeouts occur
git-svn can be safely restarted to fetch more revisions.

A new temp_is_locked function has been added to Git.pm
to facilitate using the minimal number of temp files
possible when using serf.

The problem that occurs when running git-svn fetch using
the serf library is that the previously used temp file
is not always unlocked before the next temp file needs
to be used.

To work around this problem, a new temp name is used
if the temp name that would otherwise be chosen is
currently locked.

This patch may not be all that is required, but at least
it is a starting point.

Daniel Shahaf has suggested also setting "servers:global:http-bulk- 
updates=on".


Kyle J. McKay (2):
 Git.pm: add new temp_is_locked function
 git-svn: allow git-svn fetching to work using serf

perl/Git.pm | 17 -
perl/Git/SVN/Fetcher.pm |  6 --
2 files changed, 20 insertions(+), 3 deletions(-)

--
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 1/2] Git.pm: add new temp_is_locked function

[Kyle McKay]

The temp_is_locked function can be used to determine whether
or not a given name previously passed to temp_acquire is
currently locked.

Signed-off-by: Kyle J. McKay 
---
perl/Git.pm | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 7a252ef..5416b5a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -61,7 +61,7 @@ require Exporter;
remote_refs prompt
get_tz_offset
credential credential_read credential_write
-temp_acquire temp_release temp_reset temp_path);
+temp_acquire temp_is_locked temp_release temp_reset  
temp_path);



=head1 DESCRIPTION
@@ -1206,6 +1206,21 @@ sub temp_acquire {
$temp_fd;
}

+=item temp_is_locked ( NAME )
+
+Returns true if the file mapped to C is currently locked.
+
+If true is returned, an attempt to C the same C  
will

+throw an error.
+
+=cut
+
+sub temp_is_locked {
+   my ($self, $name) = _maybe_self(@_);
+   my $temp_fd = \$TEMP_FILEMAP{$name};
+	defined $$temp_fd && $$temp_fd->opened && $TEMP_FILES{$$temp_fd} 
{locked};

+}
+
=item temp_release ( NAME )

=item temp_release ( FILEHANDLE )
--
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: git-svn "Temp file with moniker 'svn_delta' already in use" and skelta mode

[Daniel Shahaf]

On Sat, Jul 06, 2013 at 02:04:07AM +, Daniel Shahaf wrote:
> On Sat, Jul 06, 2013 at 02:34:23AM +0200, Branko Čibej wrote:
> > http://subversion.apache.org/docs/release-notes/1.7.html#svnrdump
> > In other words, this is a limitation of the Serf-based backend that has
> > been around since Subversion 1.4. I'm aware that it isn't documented as
> > well as it should be, but the bulk-mode workaround exists in part as a
> > workaround for that, effectively disabling the more efficient HTTPv2
> > protocol.
> 
> Is it possible to set "SVNAllowBulkUpdates Prefer" on a per-client basis?

Actually, I'm approaching this from the wrong direction.  We have an
'http-bulk-updates' config knob, so an immediate workaround is for git-svn
to install --config-option=servers:global:http-bulk-updates=on in their client
context.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-svn "Temp file with moniker 'svn_delta' already in use" and skelta mode

[Daniel Shahaf]

On Sat, Jul 06, 2013 at 02:34:23AM +0200, Branko Čibej wrote:
> http://subversion.apache.org/docs/release-notes/1.7.html#svnrdump
> In other words, this is a limitation of the Serf-based backend that has
> been around since Subversion 1.4. I'm aware that it isn't documented as
> well as it should be, but the bulk-mode workaround exists in part as a
> workaround for that, effectively disabling the more efficient HTTPv2
> protocol.

Is it possible to set "SVNAllowBulkUpdates Prefer" on a per-client basis?

That would require two things: (1) for git-svn to identify itself as such via
the User-Agent string, (2) for httpd to support making the SVNAllowBulkUpdates
directive conditional on a request header.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-svn "Temp file with moniker 'svn_delta' already in use" and skelta mode

[David Rothenberger]

On 7/5/2013 6:01 PM, Kyle McKay wrote:
> On Jul 5, 2013, at 16:07, David Rothenberger wrote:
>> On 7/5/2013 3:58 PM, Kyle McKay wrote:
>>> On Jul 5, 2013, at 13:48, David Rothenberger wrote:
 I recently upgraded my Subversion server to 1.8.0 and started
 receiving the following error from "git svn fetch":

 Temp file with moniker 'svn_delta' already in use at
 /usr/lib/perl5/vendor_perl/5.10/Git.pm line 1024.

 This occurs only when using an http:// URL; svn:// URLs work fine.
>>> [snip]
 The client is Cygwin: svn version 1.8.0 and git version
 1.8.3.2.
>>>
>>> The subversion 1.8 release has removed the neon library, all svn client
>>> http access now always goes through the serf library.  If you put
>>> "http-library = serf" in the "[global]" section of the
>>> "~/.subversion/servers" file you will get the 'svn_delta' error with
>>> git-svn when running earlier versions of the svn client as well.
>>
>> That was not my experience. I did try this with the 1.7 perl bindings
>> and libraries and with "http-library = serf" in by servers and it worked
>> fine. I confirmed that serf was being used with a Wireshark trace.
> 
> I had it always fail right away from my home ISP (kinda slow
> connection), but when running on a host with quite a different internet
> connection it would sometimes run for a while before generating the
> error (maybe a couple hundred revisions fetched) for some sources, but
> it would *always* eventually fail.
> 
> For this reason I also believe the problem is timing sensitive.
> 
> Try doing "git svn clone --quiet
> http://dev.heuristiclab.com/svn/hl/core"; with "~/.subversion/servers"
> section "[global]" including "http_library = serf".  I find this one
> always seems to fail right away for me with git-svn over serf but will
> clone fine over neon (if you can stand to wait long enough).

Yes, you're right. That one fails for me too with 1.7, although I note
that the server itself is running 1.5. Still, the reason is the same --
a non-depth-first drive of the delta editor from serf.

-- 
David Rothenberger    daver...@acm.org

"If Diet Coke did not exist it would have been necessary to invent it."
-- Karl Lehenbauer
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-svn "Temp file with moniker 'svn_delta' already in use" and skelta mode

[Kyle McKay]

On Jul 5, 2013, at 16:07, David Rothenberger wrote:

On 7/5/2013 3:58 PM, Kyle McKay wrote:

On Jul 5, 2013, at 13:48, David Rothenberger wrote:

I recently upgraded my Subversion server to 1.8.0 and started
receiving the following error from "git svn fetch":

Temp file with moniker 'svn_delta' already in use at
/usr/lib/perl5/vendor_perl/5.10/Git.pm line 1024.

This occurs only when using an http:// URL; svn:// URLs work fine.

[snip]

The client is Cygwin: svn version 1.8.0 and git version
1.8.3.2.


The subversion 1.8 release has removed the neon library, all svn  
client

http access now always goes through the serf library.  If you put
"http-library = serf" in the "[global]" section of the
"~/.subversion/servers" file you will get the 'svn_delta' error with
git-svn when running earlier versions of the svn client as well.


That was not my experience. I did try this with the 1.7 perl bindings
and libraries and with "http-library = serf" in by servers and it  
worked

fine. I confirmed that serf was being used with a Wireshark trace.


I had it always fail right away from my home ISP (kinda slow  
connection), but when running on a host with quite a different  
internet connection it would sometimes run for a while before  
generating the error (maybe a couple hundred revisions fetched) for  
some sources, but it would *always* eventually fail.


For this reason I also believe the problem is timing sensitive.

Try doing "git svn clone --quiet http://dev.heuristiclab.com/svn/hl/ 
core" with "~/.subversion/servers" section "[global]" including  
"http_library = serf".  I find this one always seems to fail right  
away for me with git-svn over serf but will clone fine over neon (if  
you can stand to wait long enough).


I have been looking into using serf for git-svn clones ever since I  
discovered how very much faster snvrdump works on an http url over  
serf as compared to neon.


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

Re: git-svn "Temp file with moniker 'svn_delta' already in use" and skelta mode

[David Rothenberger]

On 7/5/2013 3:58 PM, Kyle McKay wrote:
> On Jul 5, 2013, at 13:48, David Rothenberger wrote:
>> I recently upgraded my Subversion server to 1.8.0 and started
>> receiving the following error from "git svn fetch":
>>
>> Temp file with moniker 'svn_delta' already in use at
>> /usr/lib/perl5/vendor_perl/5.10/Git.pm line 1024.
>>
>> This occurs only when using an http:// URL; svn:// URLs work fine.
> [snip]
>> The client is Cygwin: svn version 1.8.0 and git version
>> 1.8.3.2.
> 
> The subversion 1.8 release has removed the neon library, all svn client
> http access now always goes through the serf library.  If you put
> "http-library = serf" in the "[global]" section of the
> "~/.subversion/servers" file you will get the 'svn_delta' error with
> git-svn when running earlier versions of the svn client as well.

That was not my experience. I did try this with the 1.7 perl bindings
and libraries and with "http-library = serf" in by servers and it worked
fine. I confirmed that serf was being used with a Wireshark trace.

-- 
David Rothenberger    daver...@acm.org

To downgrade the human mind is bad theology.
-- C. K. Chesterton
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-svn "Temp file with moniker 'svn_delta' already in use" and skelta mode

[David Rothenberger]

On 7/5/2013 1:48 PM, David Rothenberger wrote:
> I recently upgraded my Subversion server to 1.8.0 and started
> receiving the following error from "git svn fetch":
> 
> Temp file with moniker 'svn_delta' already in use at 
> /usr/lib/perl5/vendor_perl/5.10/Git.pm line 1024.
> 
> This occurs only when using an http:// URL; svn:// URLs work fine.

I traced git-svn and discovered that the error is due to a known
problem in the SVN APIs. ra_serf does not drive the delta editor in
a depth-first manner as required by the API [1]. Instead, the calls
come in this order:

 1. open_root
 2. open_directory
 3. add_file
 4. apply_textdelta
 5. add_file
 6. apply_textdelta

This is a known issue [2] and one that the Subversion folks have
elected not to fix [3].

[1]
http://subversion.apache.org/docs/api/latest/structsvn__delta__editor__t.html#details
[2] http://subversion.tigris.org/issues/show_bug.cgi?id=2932
[3] http://subversion.tigris.org/issues/show_bug.cgi?id=3831

-- 
David Rothenberger    daver...@acm.org

management, n.:
The art of getting other people to do all the 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: git-svn "Temp file with moniker 'svn_delta' already in use" and skelta mode

[Kyle McKay]

On Jul 5, 2013, at 13:48, David Rothenberger wrote:

I recently upgraded my Subversion server to 1.8.0 and started
receiving the following error from "git svn fetch":

Temp file with moniker 'svn_delta' already in use at /usr/lib/perl5/ 
vendor_perl/5.10/Git.pm line 1024.


This occurs only when using an http:// URL; svn:// URLs work fine.

[snip]

The client is Cygwin: svn version 1.8.0 and git version
1.8.3.2.


The subversion 1.8 release has removed the neon library, all svn  
client http access now always goes through the serf library.  If you  
put "http-library = serf" in the "[global]" section of the  
"~/.subversion/servers" file you will get the 'svn_delta' error with  
git-svn when running earlier versions of the svn client as well.



I initially reported this to the subversion users mailing list, but
was (not so politely) told to report this to the git-svn authors.
I'm not so sure, since the problem goes away simply by downgrading
the subversion-perl bindings back to 1.7.


That changes the default http client access back to neon which doesn't  
exhibit the problem.  Adding "http-library = serf" as described above  
will make the problem reappear with the 1.7 subversion-perl bindings  
(and the 1.6 as well).


Kyle

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

git-svn "Temp file with moniker 'svn_delta' already in use" and skelta mode

[David Rothenberger]

I recently upgraded my Subversion server to 1.8.0 and started
receiving the following error from "git svn fetch":

Temp file with moniker 'svn_delta' already in use at 
/usr/lib/perl5/vendor_perl/5.10/Git.pm line 1024.

This occurs only when using an http:// URL; svn:// URLs work fine.

I think this is related to skelta mode [1] with serf. When I add
"SVNAllowBulkUpdates Prefer" to mod_dav configuration, the error
goes away. (Similarly, if I add "http-bulk-updates = yes" to
~/.subversion/servers.)

The server is running WanDisco's 1.8.0 distribution on 64-bit Debian
7.0. The client is Cygwin: svn version 1.8.0 and git version
1.8.3.2.

I initially reported this to the subversion users mailing list, but
was (not so politely) told to report this to the git-svn authors.
I'm not so sure, since the problem goes away simply by downgrading
the subversion-perl bindings back to 1.7.

[1] http://subversion.apache.org/docs/release-notes/1.8.html#serf-skelta-default

-- 
David Rothenberger    daver...@acm.org

Teutonic:
Not enough gin.

--
To unsubscribe from this list: send the line "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 2/2] send-email: introduce sendemail.smtpsslcertpath

[brian m. carlson]

On Fri, Jul 05, 2013 at 10:20:11AM -0700, Junio C Hamano wrote:
> +# Helper to come up with SSL/TLS certification validation params
> +# and warn when doing no verification
> +sub ssl_verify_params {
> + use IO::Socket::SSL qw(SSL_VERIFY_PEER SSL_VERIFY_NONE);

You might as well put this at the top of the file, because all use
statements happen at compile time anyway, regardless of their location.
If you want to lazy-load this, you need to do:

require IO::Socket::SSL;
IO::Socket::SSL->import(qw(SSL_VERIFY_PEER SSL_VERIFY_NONE));

which is equivalent to "use" except that it happens at runtime.

Otherwise, it 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: git gui replaces amend message when prepare-commit-msg hook is used

[Antoine Pelisse]

On Thu, Jul 4, 2013 at 2:46 PM, Orgad Shaneh  wrote:
> On Thu, Jul 4, 2013 at 3:42 PM, Antoine Pelisse  wrote:
 Your problem is that your hook script is not checking $2 so it is
 overwriting the message even when you do not want to do so.
>>>
>>> No, it isn't. Not by git-gui at least. Check /tmp/hook.log with the
>>> hook I provided...
>>
>> So what you mean is that the hook is not executed with the correct 
>> parameters?
>> Could you please provide the output of the /tmp/hook.log file (I can't
>> reproduce right now) ?
>
> It only runs for "New commit" (possibly with merge or squash as
> arguments). In my case the log is:
> .git/PREPARE_COMMIT_MSG
> .git/PREPARE_COMMIT_MSG
> .git/PREPARE_COMMIT_MSG
> ...
>
> Not running the hook for amend is another problem.
>
> What I referred to was that handling the hook's result is done without
> checking if the state has changed while it was running, like Fredrik
> has already pointed out.

Yep, I've had a quick look at that, and clearly the problem seems to
come from git-gui.
I have absolutely no knowledge of TCL so I won't be able to help.
Indeed, from what I've seen it looks like prepare-commit-message hook
is not called in amend case.

I cc'ed Pat Thoyts as he's the maintainer of git-gui.

Sorry for not being able to help any further on this one.

Antoine,
--
To unsubscribe from this list: send the line "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 v10 3/5] t4205, t6006, t7102: make functions better readable

[Junio C Hamano]

Junio C Hamano  writes:

> Alexey Shumkin  writes:
>
>> -msg=$(printf "modify 2nd file (ge\303\244ndert)")
>> +msg="modify 2nd file (ge\303\244ndert)"
>>  if test -n "$1"
>>  then
>> -msg=$(echo $msg | iconv -f utf-8 -t $1)
>> +print "$msg" | iconv -f utf-8 -t "$1"
>> +else
>> +print "$msg"
>>  fi
>> -echo $msg
>>  }
>
> I think I'll do s/print/&f/ before queuing this.

Also, I'll change the $msg constant to have \n at the end, as we are
losing it by bypassing "echo".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git p4 clone not processing branches properly

[Matthieu Brucher]

>> I can try. Indeed, at this revision, the two other branches do not yet
>> exist. But @all will get everything? Last time, I only got head
>> (IIRC).
>
> Our P4 server has a limitation on the number of lines returned by "p4
> changes" command, so sometimes I have to use @change_start,@change_stop
> instead of @all. You might want to use this range limitation to test
> git-p4 by limiting to a small number of changelists that allows you to
> check if at least one branch is correctly detected.

I didn't know about this. I wanted to start the cloning at some point
in the past, that's why I used the @123456789 notation.

>>> Also, by using that command it means that the following depot paths must
>>> exist:
>>> //Depot/Project/Branch/Main
>>> //Depot/Project/Releases/2013
>>> //Depot/Project/Branch/Feature1
>>
>> Yes, they indeed do.
>
> In this case the problem should not be in branchList configuration.
>
>>> I've never used the --use-client-spec, so I'm not sure if that will not
>>> break the branch detection code.
>>
>> I need to do that because if I don't, the depot is clobbed with
>> binaries. Or perhaps if I put some .gitignore stuff, I might not do
>> this?
>
> Keep using it, at least for now. If everything else fails we can look at
> this again.

OK, I'll send a mail on Monday (forgot it was the week end tomorrow...)

Cheers,

Matthieu
--
Information System Engineer, Ph.D.
Blog: http://matt.eifelle.com
LinkedIn: http://www.linkedin.com/in/matthieubrucher
Music band: http://liliejay.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

Re: [PATCH v2 2/2] send-email: introduce sendemail.smtpsslcertpath

[John Keeping]

On Fri, Jul 05, 2013 at 11:30:11AM -0700, Junio C Hamano wrote:
> John Keeping  writes:
> 
> > On Fri, Jul 05, 2013 at 10:20:11AM -0700, Junio C Hamano wrote:
> >> "brian m. carlson"  writes:
> >> 
> >> > You've covered the STARTTLS case, but not the SSL one right above it.
> >> > Someone using smtps on port 465 will still see the warning.  You can
> >> > pass SSL_verify_mode to Net::SMTP::SSL->new just like you pass it to
> >> > start_SSL.
> >> 
> >> OK, will a fix-up look like this on top of 1/2 and 2/2?
> >
> > According to IO::Socket::SSL [1], if neither SSL_ca_file nor SSL_ca_path
> > is specified then builtin defaults will be used, so I wonder if we
> > should pass SSL_VERIFY_PEER regardless (possibly with a switch for
> > SSL_VERIFY_NONE if people really need that).
> >
> > [1] http://search.cpan.org/~sullr/IO-Socket-SSL-1.951/lib/IO/Socket/SSL.pm
> 
> Interesting.  That frees us from saying "we assume /etc/ssl/cacerts
> is the default location, and let the users override it".
> 
> To help those "I do not want verification because I know my server
> does not present valid certificate, I know my server is internal and
> trustable, and I do not bother to fix it" people, we can let them
> specify an empty string (or any non-directory) as the CACertPath,
> and structure the code like so?
> 
> if (defined $smtp_ssl_cert_path && -d $smtp_ssl_cert_path) {
> return (SSL_verify_mode => SSL_VERIFY_PEER,
> SSL_ca_path => $smtp_ssl_cert_path);
> } elsif (defined $smtp_ssl_cert_path) {
> return (SSL_verify_mode => SSL_VERIFY_NONE);
> } else {
> return (SSL_verify_mode => SSL_VERIFY_PEER);
> }

I'd rather have '$smtp_ssl_cert_path ne ""' in the first if condition
(instead of the '-d $smtp_ssl_cert_path') but that seems reasonable and
agrees with my reading of the documentation.

Perhaps a complete solution could allow CA files as well:

if (defined $smtp_ssl_cert_path) {
if ($smtp_ssl_cert_path eq "") {
return (SSL_verify_mode => SSL_VERIFY_NONE);
} elsif (-f $smtp_ssl_cert_path) {
return (SSL_verify_mode => SSL_VERIFY_PEER,
SSL_ca_file => $smtp_ssl_cert_path);
} else {
return (SSL_verify_mode => SSL_VERIFY_PEER,
SSL_ca_path => $smtp_ssl_cert_path);
}
} else {
return (SSL_verify_mode => SSL_VERIFY_PEER);
}
--
To unsubscribe from this list: send the line "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 v10 3/5] t4205, t6006, t7102: make functions better readable

[Junio C Hamano]

Alexey Shumkin  writes:

> - msg=$(printf "modify 2nd file (ge\303\244ndert)")
> + msg="modify 2nd file (ge\303\244ndert)"
>   if test -n "$1"
>   then
> - msg=$(echo $msg | iconv -f utf-8 -t $1)
> + print "$msg" | iconv -f utf-8 -t "$1"
> + else
> + print "$msg"
>   fi
> - echo $msg
>  }

I think I'll do s/print/&f/ before queuing this.

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

Re: git p4 clone not processing branches properly

[Vitor Antunes]

On Fri, Jul 5, 2013 at 7:11 PM, Matthieu Brucher
 wrote:
>> Hi Matthieu,
>>
>> Could you please try using //Depot/Projectall instead of selecting a
>> specific revision?
>
> I can try. Indeed, at this revision, the two other branches do not yet
> exist. But @all will get everything? Last time, I only got head
> (IIRC).

Our P4 server has a limitation on the number of lines returned by "p4
changes" command, so sometimes I have to use @change_start,@change_stop
instead of @all. You might want to use this range limitation to test
git-p4 by limiting to a small number of changelists that allows you to
check if at least one branch is correctly detected.

>> Also, by using that command it means that the following depot paths must
>> exist:
>> //Depot/Project/Branch/Main
>> //Depot/Project/Releases/2013
>> //Depot/Project/Branch/Feature1
>
> Yes, they indeed do.

In this case the problem should not be in branchList configuration.

>> I've never used the --use-client-spec, so I'm not sure if that will not
>> break the branch detection code.
>
> I need to do that because if I don't, the depot is clobbed with
> binaries. Or perhaps if I put some .gitignore stuff, I might not do
> this?

Keep using it, at least for now. If everything else fails we can look at
this again.

Cheers,
Vitor
--
To unsubscribe from this list: send the line "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 2/2] send-email: introduce sendemail.smtpsslcertpath

[Junio C Hamano]

John Keeping  writes:

> On Fri, Jul 05, 2013 at 10:20:11AM -0700, Junio C Hamano wrote:
>> "brian m. carlson"  writes:
>> 
>> > You've covered the STARTTLS case, but not the SSL one right above it.
>> > Someone using smtps on port 465 will still see the warning.  You can
>> > pass SSL_verify_mode to Net::SMTP::SSL->new just like you pass it to
>> > start_SSL.
>> 
>> OK, will a fix-up look like this on top of 1/2 and 2/2?
>
> According to IO::Socket::SSL [1], if neither SSL_ca_file nor SSL_ca_path
> is specified then builtin defaults will be used, so I wonder if we
> should pass SSL_VERIFY_PEER regardless (possibly with a switch for
> SSL_VERIFY_NONE if people really need that).
>
> [1] http://search.cpan.org/~sullr/IO-Socket-SSL-1.951/lib/IO/Socket/SSL.pm

Interesting.  That frees us from saying "we assume /etc/ssl/cacerts
is the default location, and let the users override it".

To help those "I do not want verification because I know my server
does not present valid certificate, I know my server is internal and
trustable, and I do not bother to fix it" people, we can let them
specify an empty string (or any non-directory) as the CACertPath,
and structure the code like so?

if (defined $smtp_ssl_cert_path && -d $smtp_ssl_cert_path) {
return (SSL_verify_mode => SSL_VERIFY_PEER,
SSL_ca_path => $smtp_ssl_cert_path);
} elsif (defined $smtp_ssl_cert_path) {
return (SSL_verify_mode => SSL_VERIFY_NONE);
} else {
return (SSL_verify_mode => SSL_VERIFY_PEER);
}

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

Re: git p4 clone not processing branches properly

[Matthieu Brucher]

> Hi Matthieu,
>
> Could you please try using //Depot/Projectall instead of selecting a
> specific revision?

I can try. Indeed, at this revision, the two other branches do not yet
exist. But @all will get everything? Last time, I only got head
(IIRC).

> Also, by using that command it means that the following depot paths must
> exist:
> //Depot/Project/Branch/Main
> //Depot/Project/Releases/2013
> //Depot/Project/Branch/Feature1

Yes, they indeed do.

> I've never used the --use-client-spec, so I'm not sure if that will not
> break the branch detection code.

I need to do that because if I don't, the depot is clobbed with
binaries. Or perhaps if I put some .gitignore stuff, I might not do
this?

> Cheers,
> Vitor

Thanks for the tips, I will try tomorrow.

Cheers,
--
Information System Engineer, Ph.D.
Blog: http://matt.eifelle.com
LinkedIn: http://www.linkedin.com/in/matthieubrucher
Music band: http://liliejay.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

Re: git p4 clone not processing branches properly

[Vitor Antunes]

Matthieu Brucher  gmail.com> writes:
> 
> Hi,
> 
> I'm trying to convert a Perforce repository to git, knowing that:
> - I use client specs to remove a bunch of folders containing binaires
> (several GiB)
> - branch mappings may not be properly set, and I can't change them
> 
> Now, the branches are layout like this:
> - Branch/Main <- master
> - Branch/Feature1
> - ...
> - Releases/2013
> - ...
> I would like to have these branches and releases branches inside by
> cloned git repository, but this doesn't work. I keep on getting each
> file with Project/Branch/Main as well as Project/Branch/Feature1 and
> all others in my master branch.
> I tried to add branchLists like this:
> branchList = Branch/Main:Releases/2013
> branchList = Releases/2013:Branch/Feature1
> but it doesn't change a thing with the following command:
>  git p4 clone --verbose --use-client-spec --detect-branches
> //Depot/Project  specificrevision
> 
> I can see that branches are detected from the Perforce server, but
> none are actually detected for this specific project:
> p4-git branches: []
> initial parents: {}
> 
> Can someone give a pointer to a tutorial or something for a complex
> case like this?

Hi Matthieu,

Could you please try using //Depot/Projectall instead of selecting a
specific revision? 
Also, by using that command it means that the following depot paths must
exist:
//Depot/Project/Branch/Main
//Depot/Project/Releases/2013
//Depot/Project/Branch/Feature1

I've never used the --use-client-spec, so I'm not sure if that will not
break the branch detection code.

Cheers,
Vitor

P.S. - Please keep me in CC because I'm not subscribed to the mailing
list.


--
To unsubscribe from this list: send the line "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 2/2] send-email: introduce sendemail.smtpsslcertpath

[John Keeping]

On Fri, Jul 05, 2013 at 10:20:11AM -0700, Junio C Hamano wrote:
> "brian m. carlson"  writes:
> 
> > You've covered the STARTTLS case, but not the SSL one right above it.
> > Someone using smtps on port 465 will still see the warning.  You can
> > pass SSL_verify_mode to Net::SMTP::SSL->new just like you pass it to
> > start_SSL.
> 
> OK, will a fix-up look like this on top of 1/2 and 2/2?

According to IO::Socket::SSL [1], if neither SSL_ca_file nor SSL_ca_path
is specified then builtin defaults will be used, so I wonder if we
should pass SSL_VERIFY_PEER regardless (possibly with a switch for
SSL_VERIFY_NONE if people really need that).

[1] http://search.cpan.org/~sullr/IO-Socket-SSL-1.951/lib/IO/Socket/SSL.pm

>  git-send-email.perl | 39 +++
>  1 file changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 52028ba..3b80340 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1093,6 +1093,25 @@ sub smtp_auth_maybe {
>   return $auth;
>  }
>  
> +# Helper to come up with SSL/TLS certification validation params
> +# and warn when doing no verification
> +sub ssl_verify_params {
> + use IO::Socket::SSL qw(SSL_VERIFY_PEER SSL_VERIFY_NONE);
> +
> + if (!defined $smtp_ssl_cert_path) {
> + $smtp_ssl_cert_path = "/etc/ssl/certs";
> + }
> +
> + if (-d $smtp_ssl_cert_path) {
> + return (SSL_verify_mode => SSL_VERIFY_PEER,
> + SSL_ca_path => $smtp_ssl_cert_path);
> + } else {
> + print STDERR "warning: Using SSL_VERIFY_NONE.  " .
> + "See sendemail.smtpsslcertpath.\n";
> + return (SSL_verify_mode => SSL_VERIFY_NONE);
> + }
> +}
> +
--
To unsubscribe from this list: send the line "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 2/2] send-email: introduce sendemail.smtpsslcertpath

[Junio C Hamano]

"brian m. carlson"  writes:

> You've covered the STARTTLS case, but not the SSL one right above it.
> Someone using smtps on port 465 will still see the warning.  You can
> pass SSL_verify_mode to Net::SMTP::SSL->new just like you pass it to
> start_SSL.

OK, will a fix-up look like this on top of 1/2 and 2/2?

 git-send-email.perl | 39 +++
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 52028ba..3b80340 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1093,6 +1093,25 @@ sub smtp_auth_maybe {
return $auth;
 }
 
+# Helper to come up with SSL/TLS certification validation params
+# and warn when doing no verification
+sub ssl_verify_params {
+   use IO::Socket::SSL qw(SSL_VERIFY_PEER SSL_VERIFY_NONE);
+
+   if (!defined $smtp_ssl_cert_path) {
+   $smtp_ssl_cert_path = "/etc/ssl/certs";
+   }
+
+   if (-d $smtp_ssl_cert_path) {
+   return (SSL_verify_mode => SSL_VERIFY_PEER,
+   SSL_ca_path => $smtp_ssl_cert_path);
+   } else {
+   print STDERR "warning: Using SSL_VERIFY_NONE.  " .
+   "See sendemail.smtpsslcertpath.\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.
@@ -1195,12 +1214,11 @@ sub send_message {
if ($smtp_encryption eq 'ssl') {
$smtp_server_port ||= 465; # ssmtp
require Net::SMTP::SSL;
-   use IO::Socket::SSL qw(SSL_VERIFY_NONE);
$smtp_domain ||= maildomain();
$smtp ||= Net::SMTP::SSL->new($smtp_server,
  Hello => $smtp_domain,
  Port => $smtp_server_port,
- SSL_verify_mode => 
SSL_VERIFY_NONE);
+ ssl_verify_params());
}
else {
require Net::SMTP;
@@ -1210,23 +1228,12 @@ sub send_message {
 Debug => $debug_net_smtp);
if ($smtp_encryption eq 'tls' && $smtp) {
require Net::SMTP::SSL;
-   use IO::Socket::SSL qw(SSL_VERIFY_PEER 
SSL_VERIFY_NONE);
$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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Issue

[Ms. Janet Gohoho]

My dear,I am Ms Janet Gohoho; I got your email address from inter-net. I
have issues I will like to discuss with you either by email or phone.
Thanks as I do hope to receive your reply as soon as possible. Ms Janet
Gohoho

--
To unsubscribe from this list: send the line "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] git svn geotrust certificate problem

[Ilya Holinov]

I have svn repository on https singed with GeoTrust issued certificate.
Every time i try to access this repository i have message :

$ git svn rebase
Error validating server certificate for 'https://svn.egspace.ru:443':
 - The certificate is not issued by a trusted authority. Use the
   fingerprint to validate the certificate manually!
Certificate information:
 - Hostname: *.egspace.ru
 - Valid: from Apr 28 01:38:17 2013 GMT until Apr 30 12:00:40 2014 GMT
 - Issuer: GeoTrust, Inc., US
 - Fingerprint: b2:8d:f8:3b:7c:d2:a2:36:e2:1d:c3:5c:56:ec:87:6f:22:3e:4b:a8
Certificate problem.
(R)eject, accept (t)emporarily or accept (p)ermanently? p
Authentication realm:  VisualSVN Server
Username: holinov
Password for 'holinov':

Even if i choose permanently every next attempt to access in i have
same issue. And this happens on svn rebase on every commit. I mean if
i have 10 commits in local repository i will be asked about cert and
user login\passwor for every one of them (and that's is verry
annoying).
But if i use TortoiseSVN i have no problem with checking that cert.

P.S.: I'm using Windows 8 x64.
P.P.S: I like git very much but in this case it makes me impossible to
work in this way.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git p4 clone not processing branches properly

[Matthieu Brucher]

Hi,

I'm trying to convert a Perforce repository to git, knowing that:
- I use client specs to remove a bunch of folders containing binaires
(several GiB)
- branch mappings may not be properly set, and I can't change them

Now, the branches are layout like this:
- Branch/Main <- master
- Branch/Feature1
- ...
- Releases/2013
- ...
I would like to have these branches and releases branches inside by
cloned git repository, but this doesn't work. I keep on getting each
file with Project/Branch/Main as well as Project/Branch/Feature1 and
all others in my master branch.
I tried to add branchLists like this:
branchList = Branch/Main:Releases/2013
branchList = Releases/2013:Branch/Feature1
but it doesn't change a thing with the following command:
 git p4 clone --verbose --use-client-spec --detect-branches
//Depot/Project@specificrevision

I can see that branches are detected from the Perforce server, but
none are actually detected for this specific project:
p4-git branches: []
initial parents: {}

Can someone give a pointer to a tutorial or something for a complex
case like this?

Regards,

Matthieu Brucher
--
Information System Engineer, Ph.D.
Blog: http://matt.eifelle.com
LinkedIn: http://www.linkedin.com/in/matthieubrucher
Music band: http://liliejay.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

Re: [PATCH v2 2/2] send-email: introduce sendemail.smtpsslcertpath

[brian m. carlson]

On Fri, Jul 05, 2013 at 06:23:58PM +0530, Ramkumar Ramachandra wrote:
> Thanks.  On a related note, how do I find out about all these things?  I tried
> 
>   $ perldoc Net::SMTP::SSL
> 
> but it was completely useless.  The only reason I got this far is
> because you literally told me what to do.  Do I have to resort to
> reading the sources?  If so, how do I bring up the relevant functions
> quickly? (I'm looking for something like the elisp M-x find-function).

Since Net::SMTP::SSL inherits from IO::Socket::SSL, you can do "perldoc
IO::Socket::SSL", which is significantly more helpful.  If you're
looking for the source of a module, use "perldoc -lm Net::SMTP::SSL";
that will print the .pm file for the module.

-- 
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 v2 2/2] send-email: introduce sendemail.smtpsslcertpath

[Ramkumar Ramachandra]

brian m. carlson wrote:
> You've covered the STARTTLS case, but not the SSL one right above it.
> Someone using smtps on port 465 will still see the warning.  You can
> pass SSL_verify_mode to Net::SMTP::SSL->new just like you pass it to
> start_SSL.

Thanks.  On a related note, how do I find out about all these things?  I tried

  $ perldoc Net::SMTP::SSL

but it was completely useless.  The only reason I got this far is
because you literally told me what to do.  Do I have to resort to
reading the sources?  If so, how do I bring up the relevant functions
quickly? (I'm looking for something like the elisp M-x find-function).
--
To unsubscribe from this list: send the line "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 1/2] commit: reject invalid UTF-8 codepoints

[Peter Krefting]

brian m. carlson:


+   /* U+FFFE and U+ are guaranteed non-characters. */
+   if ((codepoint & 0x1e) == 0xfffe)
+   return bad_offset;


I missed this the first time around: All Unicode characters whose 
lower 16-bits are FFFE or  are non-characters, so you can re-write 
that to:


  /* U+xxFFFE and U+xx are guaranteed non-characters. */
  if ((codepoint & 0xfffe) == 0xfffe)
   return bad_offset;

Also, the range U+FDD0--U+FDEF are also non-characters, if you wish to 
be really pedantic.


$ grep '^[0-9A-F].*
FDD1
FDD2
FDD3
FDD4
FDD5
FDD6
FDD7
FDD8
FDD9
FDDA
FDDB
FDDC
FDDD
FDDE
FDDF
FDE0
FDE1
FDE2
FDE3
FDE4
FDE5
FDE6
FDE7
FDE8
FDE9
FDEA
FDEB
FDEC
FDED
FDEE
FDEF
FFFE

1FFFE   
1   
2FFFE   
2   
3FFFE   
3   
4FFFE   
4   
5FFFE   
5   
6FFFE   
6   
7FFFE   
7   
8FFFE   
8   
9FFFE   
9   
AFFFE   
A   
BFFFE   
B   
CFFFE   
C   
DFFFE   
D   
EFFFE   
E   
E   
F   
10FFFE  
10  

--
\\// Peter - http://www.softwolves.pp.se/
--
To unsubscribe from this list: send the line "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 2/2] send-email: introduce sendemail.smtpsslcertpath

[brian m. carlson]

On Fri, Jul 05, 2013 at 05:35:47PM +0530, Ramkumar Ramachandra wrote:
> @@ -1193,13 +1197,23 @@ X-Mailer: git-send-email $gitversion
>Debug => $debug_net_smtp);
>   if ($smtp_encryption eq 'tls' && $smtp) {
>   require Net::SMTP::SSL;
> - use IO::Socket::SSL qw(SSL_VERIFY_NONE);
> + use IO::Socket::SSL qw(SSL_VERIFY_PEER 
> SSL_VERIFY_NONE);
>   $smtp->command('STARTTLS');
>   $smtp->response();
>   if ($smtp->code == 220) {
> - $smtp = Net::SMTP::SSL->start_SSL($smtp,
> -   
> SSL_verify_mode => SSL_VERIFY_NONE)
> - or die "STARTTLS failed! 
> ".$smtp->message;
> + # 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;
> + }

You've covered the STARTTLS case, but not the SSL one right above it.
Someone using smtps on port 465 will still see the warning.  You can
pass SSL_verify_mode to Net::SMTP::SSL->new just like you pass it to
start_SSL.

-- 
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 v2 2/2] send-email: introduce sendemail.smtpsslcertpath

[Ramkumar Ramachandra]

Eric Sunshine wrote:
>> +   $smtp_ssl_cert_path |= 
>> "/etc/ssl/certs";
>
> You're going to want to use logical ||= here. Bitwise |= on a string
> does not do what you expect[1]:
>
>   my $s = '/usr/local/etc/ssl/certs';
>   $s |= '/etc/ssl/certs';
>   print $s, "\n";
>
> Outputs: /uws/wts/ssl/certs

Thanks; much appreciated.  My Perl is quite weak.
--
To unsubscribe from this list: send the line "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 2/2] send-email: introduce sendemail.smtpsslcertpath

[Eric Sunshine]

On Fri, Jul 5, 2013 at 8:05 AM, Ramkumar Ramachandra  wrote:
> Use the ca-certificates in /etc/ssl/certs by default (that's where most
> distributions put it).  SSL_VERIFY_NONE is now the fallback mode.
>
> Signed-off-by: Ramkumar Ramachandra 
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 758100d..026bcbc 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1193,13 +1197,23 @@ X-Mailer: git-send-email $gitversion
>  Debug => $debug_net_smtp);
> if ($smtp_encryption eq 'tls' && $smtp) {
> require Net::SMTP::SSL;
> -   use IO::Socket::SSL qw(SSL_VERIFY_NONE);
> +   use IO::Socket::SSL qw(SSL_VERIFY_PEER 
> SSL_VERIFY_NONE);
> $smtp->command('STARTTLS');
> $smtp->response();
> if ($smtp->code == 220) {
> -   $smtp = 
> Net::SMTP::SSL->start_SSL($smtp,
> - 
> SSL_verify_mode => SSL_VERIFY_NONE)
> -   or die "STARTTLS failed! 
> ".$smtp->message;
> +   # Attempt to use a ca-certificate by 
> default
> +   $smtp_ssl_cert_path |= 
> "/etc/ssl/certs";

You're going to want to use logical ||= here. Bitwise |= on a string
does not do what you expect[1]:

  my $s = '/usr/local/etc/ssl/certs';
  $s |= '/etc/ssl/certs';
  print $s, "\n";

Outputs: /uws/wts/ssl/certs

[1]: http://perldoc.perl.org/perlop.html#Bitwise-String-Operators

> +   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_encryption = '';
> # Send EHLO again to receive fresh
> # supported commands
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/2] Squelch warning from send-email

[Ramkumar Ramachandra]

Hi,

Since nobody stepped up to write it, I wrote [2/2] myself.  It will be
tested when I send out this series.

Thanks.

Ramkumar Ramachandra (2):
  send-email: squelch warning from Net::SMTP::SSL
  send-email: introduce sendemail.smtpsslcertpath

 Documentation/config.txt |  3 +++
 Documentation/git-send-email.txt |  4 
 git-send-email.perl  | 20 ++--
 3 files changed, 25 insertions(+), 2 deletions(-)

-- 
1.8.3.2.722.g3244e19.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/2] send-email: squelch warning from Net::SMTP::SSL

[Ramkumar Ramachandra]

Due to a recent change in the Net::SMTP::SSL module, send-email emits
the following ugly warning everytime a email is sent via SSL:

***
 Using the default of SSL_verify_mode of SSL_VERIFY_NONE for client
 is deprecated! Please set SSL_verify_mode to SSL_VERIFY_PEER
 together with SSL_ca_file|SSL_ca_path for verification.
 If you really don't want to verify the certificate and keep the
 connection open to Man-In-The-Middle attacks please set
 SSL_verify_mode explicitly to SSL_VERIFY_NONE in your application.
***

Fix this by explicitly specifying SSL_verify_mode => SSL_VERIFY_NONE in
Net::SMTP::SSL->start_SSL().

Helped-by: brian m. carlson 
Signed-off-by: Ramkumar Ramachandra 
---
 git-send-email.perl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index ecbf56f..758100d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1193,10 +1193,12 @@ X-Mailer: git-send-email $gitversion
 Debug => $debug_net_smtp);
if ($smtp_encryption eq 'tls' && $smtp) {
require Net::SMTP::SSL;
+   use IO::Socket::SSL qw(SSL_VERIFY_NONE);
$smtp->command('STARTTLS');
$smtp->response();
if ($smtp->code == 220) {
-   $smtp = Net::SMTP::SSL->start_SSL($smtp)
+   $smtp = Net::SMTP::SSL->start_SSL($smtp,
+ 
SSL_verify_mode => SSL_VERIFY_NONE)
or die "STARTTLS failed! 
".$smtp->message;
$smtp_encryption = '';
# Send EHLO again to receive fresh
-- 
1.8.3.2.723.gad7967b.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 2/2] send-email: introduce sendemail.smtpsslcertpath

[Ramkumar Ramachandra]

Use the ca-certificates in /etc/ssl/certs by default (that's where most
distributions put it).  SSL_VERIFY_NONE is now the fallback mode.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/config.txt |  3 +++
 Documentation/git-send-email.txt |  4 
 git-send-email.perl  | 22 ++
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index b4d4887..b216a63 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2048,6 +2048,9 @@ sendemail.smtpencryption::
 sendemail.smtpssl::
Deprecated alias for 'sendemail.smtpencryption = ssl'.
 
+sendemail.smtpsslcertpath::
+   Path to ca-certificates.  Defaults to `/etc/ssl/certs`.
+
 sendemail..*::
Identity-specific versions of the 'sendemail.*' parameters
found below, taking precedence over those when the this
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 40a9a9a..605f263 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -198,6 +198,10 @@ must be used for each option.
 --smtp-ssl::
Legacy alias for '--smtp-encryption ssl'.
 
+--smtp-ssl-cert-path::
+   Path to ca-certificates.  Defaults to `/etc/ssl/certs`, or
+   'sendemail.smtpsslcertpath'.
+
 --smtp-user=::
Username for SMTP-AUTH. Default is the value of 'sendemail.smtpuser';
if a username is not specified (with '--smtp-user' or 
'sendemail.smtpuser'),
diff --git a/git-send-email.perl b/git-send-email.perl
index 758100d..026bcbc 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -69,6 +69,8 @@ git send-email [options] 
 --smtp-pass   * Password for SMTP-AUTH; not necessary.
 --smtp-encryption * tls or ssl; anything else disables.
 --smtp-ssl * Deprecated. Use '--smtp-encryption ssl'.
+--smtp-ssl-cert-path  * Path to ca-certificates.  Defaults to
+ /etc/ssl/certs.
 --smtp-domain * The domain name sent to HELO/EHLO 
handshake
 --smtp-debug<0|1>  * Disable, enable Net::SMTP debug.
 
@@ -195,6 +197,7 @@ my ($thread, $chain_reply_to, $suppress_from, 
$signed_off_by_cc);
 my ($to_cmd, $cc_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption);
+my ($smtp_ssl_cert_path);
 my ($identity, $aliasfiletype, @alias_files, $smtp_domain);
 my ($validate, $confirm);
 my (@suppress_cc);
@@ -220,6 +223,7 @@ my %config_settings = (
 "smtpserveroption" => \@smtp_server_options,
 "smtpuser" => \$smtp_authuser,
 "smtppass" => \$smtp_authpass,
+"smtpsslcertpath" => \$smtp_ssl_cert_path,
 "smtpdomain" => \$smtp_domain,
 "to" => \@initial_to,
 "tocmd" => \$to_cmd,
@@ -1193,13 +1197,23 @@ X-Mailer: git-send-email $gitversion
 Debug => $debug_net_smtp);
if ($smtp_encryption eq 'tls' && $smtp) {
require Net::SMTP::SSL;
-   use IO::Socket::SSL qw(SSL_VERIFY_NONE);
+   use IO::Socket::SSL qw(SSL_VERIFY_PEER 
SSL_VERIFY_NONE);
$smtp->command('STARTTLS');
$smtp->response();
if ($smtp->code == 220) {
-   $smtp = Net::SMTP::SSL->start_SSL($smtp,
- 
SSL_verify_mode => SSL_VERIFY_NONE)
-   or die "STARTTLS failed! 
".$smtp->message;
+   # 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_encryption = '';
   

[PATCH v10 4/5] t6006 (rev-list-format): add tests for "%b" and "%s" for the case i18n.commitEncoding is not set

[Alexey Shumkin]

In de6029a (pretty: Add failing tests: --format output should honor
logOutputEncoding, 2013-06-26) 'complex-subject' test was changed.
Revert it back, because that change actually removed tests for "%b" and
"%s" with i18n.commitEncoding set.
Also, add two more tests for mentioned above "%b" and "%s" to test
encoding conversions with no i18n.commitEncoding set.

Signed-off-by: Alexey Shumkin 
Suggested-by: Johannes Sixt 
---
 t/t6006-rev-list-format.sh | 41 -
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index b32405a..e51d0f0 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -218,12 +218,7 @@ test_expect_success 'setup complex body' '
git config i18n.commitencoding iso8859-1 &&
echo change2 >foo && git commit -a -F commit-msg &&
head3=$(git rev-parse --verify HEAD) &&
-   head3_short=$(git rev-parse --short $head3) &&
-   # unset commit encoding config
-   # otherwise %e does not print encoding value
-   # and following test fails
-   git config --unset i18n.commitEncoding
-
+   head3_short=$(git rev-parse --short $head3)
 '
 
 test_format complex-encoding %e expected.iso8859-1
+'
+
+test_format complex-body %b expect commit $head3 &&
echo >>expect fooQbar &&
-- 
1.8.3.2.16.gb1f0d63

--
To unsubscribe from this list: send the line "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 v10 3/5] t4205, t6006, t7102: make functions better readable

[Alexey Shumkin]

Function 'test_format' has become harder to read after its
change in de6029a2 (pretty: Add failing tests: --format output
should honor logOutputEncoding, 2013-06-26). Simplify it by
moving its "should we expect it to fail?" parameter to the end.

Note, current code does not use this last parameter as far as there
are no tests expected to fail. We can keep that for future use.

Also, reformat comments

Signed-off-by: Alexey Shumkin 
Improved-by: Johannes Sixt 
---
 t/t4205-log-pretty-formats.sh |  3 ++-
 t/t6006-rev-list-format.sh| 28 
 t/t7102-reset.sh  | 10 ++
 3 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index ef9770a..2933c63 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -8,7 +8,8 @@ test_description='Test pretty formats'
 . ./test-lib.sh
 
 commit_msg () {
-   # String "initial. initial" partly in German (translated with Google 
Translate),
+   # String "initial. initial" partly in German
+   # (translated with Google Translate),
# encoded in UTF-8, used as a commit log message below.
msg=$(printf "initial. anf\303\244nglich")
if test -n "$1"
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 4751d22..b32405a 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -9,8 +9,9 @@ test_description='git rev-list --pretty=format test'
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
 test_tick
-# String "added" in German (translated with Google Translate), encoded in 
UTF-8,
-# used as a commit log message below.
+# String "added" in German
+# (translated with Google Translate),
+# encoded in UTF-8, used as a commit log message below.
 added=$(printf "added (hinzugef\303\274gt) foo")
 added_iso88591=$(echo "$added" | iconv -f utf-8 -t iso8859-1)
 # same but "changed"
@@ -35,26 +36,13 @@ test_expect_success 'setup' '
git config --unset i18n.commitEncoding
 '
 
-# usage: test_format [failure] name format_string expect.$1
-   name="format $1"
-   command="git rev-list --pretty=format:'$2' master >output.$1 &&
-   test_cmp expect.$1 output.$1"
-   if test $must_fail -eq 1
-   then
-   test_expect_failure "$name" "$command"
-   else
-   test_expect_success "$name" "$command"
-   fi
+   test_expect_${3:-success} "format $1" "
+   git rev-list --pretty=format:'$2' master >output.$1 &&
+   test_cmp expect.$1 output.$1
+   "
 }
 
 # Feed to --format to provide predictable colored sequences.
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 2ef96e9..535e609 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -10,14 +10,16 @@ Documented tests for git reset'
 . ./test-lib.sh
 
 commit_msg () {
-   # String "modify 2nd file (changed)" partly in German(translated with 
Google Translate),
+   # String "modify 2nd file (changed)" partly in German
+   # (translated with Google Translate),
# encoded in UTF-8, used as a commit log message below.
-   msg=$(printf "modify 2nd file (ge\303\244ndert)")
+   msg="modify 2nd file (ge\303\244ndert)"
if test -n "$1"
then
-   msg=$(echo $msg | iconv -f utf-8 -t $1)
+   print "$msg" | iconv -f utf-8 -t "$1"
+   else
+   print "$msg"
fi
-   echo $msg
 }
 
 test_expect_success 'creating initial files and commits' '
-- 
1.8.3.2.16.gb1f0d63

--
To unsubscribe from this list: send the line "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 v10 5/5] t4205 (log-pretty-formats): avoid using `sed`

[Alexey Shumkin]

For testing truncated log messages 'commit_msg' function uses `sed` to
cut a message. On various platforms `sed` behaves differently and
results of its work depend on locales installed. So, avoid using `sed`.
Use predefined expected outputs instead of calculated ones.

Signed-off-by: Alexey Shumkin 
Signed-off-by: Junio C Hamano 
---
 t/t4205-log-pretty-formats.sh | 26 ++
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 2933c63..73fd236 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -7,25 +7,19 @@
 test_description='Test pretty formats'
 . ./test-lib.sh
 
+sample_utf8_part=$(printf "f\303\244ng")
+
 commit_msg () {
# String "initial. initial" partly in German
# (translated with Google Translate),
# encoded in UTF-8, used as a commit log message below.
-   msg=$(printf "initial. anf\303\244nglich")
+   msg="initial. an${sample_utf8_part}lich"
if test -n "$1"
then
-   msg=$(echo $msg | iconv -f utf-8 -t $1)
-   fi
-   if test -n "$2" -a -n "$3"
-   then
-   # cut string, replace cut part with two dots
-   # $2 - chars count from the beginning of the string
-   # $3 - "trailing" chars
-   # LC_ALL is set to make `sed` interpret "." as a UTF-8 char not 
a byte
-   # as it does with C locale
-   msg=$(echo $msg | LC_ALL=en_US.UTF-8 sed -e 
"s/^\(.\{$2\}\)$3/\1../")
+   print $msg | iconv -f utf-8 -t "$1"
+   else
+   print $msg
fi
-   echo $msg
 }
 
 test_expect_success 'set up basic repos' '
@@ -193,7 +187,7 @@ test_expect_success 'left alignment formatting with trunc' '
 message ..
 message ..
 add bar  Z
-$(commit_msg "" "8" "..*$")
+initial...
 EOF
test_cmp expected actual
 '
@@ -206,7 +200,7 @@ test_expect_success 'left alignment formatting with ltrunc' 
'
 ..sage two
 ..sage one
 add bar  Z
-$(commit_msg "" "0" ".\{11\}")
+..${sample_utf8_part}lich
 EOF
test_cmp expected actual
 '
@@ -219,7 +213,7 @@ test_expect_success 'left alignment formatting with mtrunc' 
'
 mess.. two
 mess.. one
 add bar  Z
-$(commit_msg "" "4" ".\{11\}")
+init..lich
 EOF
test_cmp expected actual
 '
@@ -311,7 +305,7 @@ test_expect_success 'left/right alignment formatting with 
stealing' '
 short long  long long
 message ..   A U Thor
 add bar  A U Thor
-$(commit_msg "" "8" "..*$")   A U Thor
+initial...   A U Thor
 EOF
test_cmp expected actual
 '
-- 
1.8.3.2.16.gb1f0d63

--
To unsubscribe from this list: send the line "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 v10 0/5] Incremental updates against 'next' branch

[Alexey Shumkin]

This patches series includes following changes against v9
1. [PATCH v10 1/5] t4041, t4205, t6006, t7102: use iso8859-1 rather than 
iso-8859-1
reworded. reasons of renaming explained "here and now" but not only
redirects to an older commit which did the same.
2. [PATCH v10 2/5] t4205 (log-pretty-formats): revert back single quotes
little change to commit message (added "(log-pretty-formats)" after
"t4205")
3. [PATCH v10 3/5] t4205, t6006, t7102: make functions better readable
reworded. comments reformatted. function 'test_format' refactored.
4. [PATCH v10 4/5] t6006 (rev-list-format): add tests for "%b" and "%s" for the 
case i18n.commitEncoding is not set
reworded. Now whole file is in utf-8. Tested output messages are
converted from utf-8 to iso8859-1 "on the fly" and written fo files.
5. [PATCH v10 5/5] t4205 (log-pretty-formats): avoid using `sed`
little change to commit message (added "(log-pretty-formats)" after
"t4205")

And references to "de6029a2d7734a93a9e27b9c4471862a47dd8123" commit in
all patch messages replaced with "de6029a (pretty: Add failing
tests: --format output should honor logOutputEncoding, 2013-06-26)"

P.S. This patch series is an incremental updates on top of (7c375214 t4205:
replace .\+ with ..* in sed commands, 2013-07-01) as far as v7 patches
were applied to the 'next' branch but there were more improvements made
in v8.

Alexey Shumkin (5):
  t4041, t4205, t6006, t7102: use iso8859-1 rather than iso-8859-1
  t4205 (log-pretty-formats): revert back single quotes
  t4205, t6006, t7102: make functions better readable
  t6006 (rev-list-format): add tests for "%b" and "%s" for the case
i18n.commitEncoding is not set
  t4205 (log-pretty-formats): avoid using `sed`

 t/t4041-diff-submodule-option.sh |   4 +-
 t/t4205-log-pretty-formats.sh| 143 +++
 t/t6006-rev-list-format.sh   |  83 ---
 t/t7102-reset.sh |  20 +++---
 4 files changed, 125 insertions(+), 125 deletions(-)

-- 
1.8.3.2.16.gb1f0d63

--
To unsubscribe from this list: send the line "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 v10 1/5] t4041, t4205, t6006, t7102: use iso8859-1 rather than iso-8859-1

[Alexey Shumkin]

Both "iso8859-1" and "iso-8859-1" are understood as latin-1
by modern platforms, but the latter is not understood by
older platforms; update tests to use the former.

This is in line with 3994e8a9 (t4201: use ISO8859-1 rather
than ISO-8859-1, 2009-12-03), which did the same.

Signed-off-by: Alexey Shumkin 
Suggested-by: Johannes Sixt 
---
 t/t4041-diff-submodule-option.sh |  4 ++--
 t/t4205-log-pretty-formats.sh|  8 
 t/t6006-rev-list-format.sh   | 14 +++---
 t/t7102-reset.sh | 10 +-
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 0a4f496..1751c83 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -23,8 +23,8 @@ add_file () {
echo "$name" >"$name" &&
git add "$name" &&
test_tick &&
-   msg_added_iso88591=$(echo "Add $name ($added $name)" | 
iconv -f utf-8 -t iso-8859-1) &&
-   git -c 'i18n.commitEncoding=iso-8859-1' commit -m 
"$msg_added_iso88591"
+   msg_added_iso88591=$(echo "Add $name ($added $name)" | 
iconv -f utf-8 -t iso8859-1) &&
+   git -c 'i18n.commitEncoding=iso8859-1' commit -m 
"$msg_added_iso88591"
done >/dev/null &&
git rev-parse --short --verify HEAD
)
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 3cfb744..c283842 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -32,8 +32,8 @@ test_expect_success 'set up basic repos' '
>bar &&
git add foo &&
test_tick &&
-   git config i18n.commitEncoding iso-8859-1 &&
-   git commit -m "$(commit_msg iso-8859-1)" &&
+   git config i18n.commitEncoding iso8859-1 &&
+   git commit -m "$(commit_msg iso8859-1)" &&
git add bar &&
test_tick &&
git commit -m "add bar" &&
@@ -61,8 +61,8 @@ test_expect_success 'alias user-defined format' '
test_cmp expected actual
 '
 
-test_expect_success 'alias user-defined tformat with %s (iso-8859-1 encoding)' 
'
-   git config i18n.logOutputEncoding iso-8859-1 &&
+test_expect_success 'alias user-defined tformat with %s (iso8859-1 encoding)' '
+   git config i18n.logOutputEncoding iso8859-1 &&
git log --oneline >expected-s &&
git log --pretty="tformat:%h %s" >actual-s &&
git config --unset i18n.logOutputEncoding &&
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 380c85b..4751d22 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -12,15 +12,15 @@ test_tick
 # String "added" in German (translated with Google Translate), encoded in 
UTF-8,
 # used as a commit log message below.
 added=$(printf "added (hinzugef\303\274gt) foo")
-added_iso88591=$(echo "$added" | iconv -f utf-8 -t iso-8859-1)
+added_iso88591=$(echo "$added" | iconv -f utf-8 -t iso8859-1)
 # same but "changed"
 changed=$(printf "changed (ge\303\244ndert) foo")
-changed_iso88591=$(echo "$changed" | iconv -f utf-8 -t iso-8859-1)
+changed_iso88591=$(echo "$changed" | iconv -f utf-8 -t iso8859-1)
 
 test_expect_success 'setup' '
: >foo &&
git add foo &&
-   git config i18n.commitEncoding iso-8859-1 &&
+   git config i18n.commitEncoding iso8859-1 &&
git commit -m "$added_iso88591" &&
head1=$(git rev-parse --verify HEAD) &&
head1_short=$(git rev-parse --verify --short $head1) &&
@@ -136,9 +136,9 @@ EOF
 
 test_format encoding %e >secondfile &&
-   git -c "i18n.commitEncoding=iso-8859-1" commit -a -m "$(commit_msg 
iso-8859-1)" &&
+   git -c "i18n.commitEncoding=iso8859-1" commit -a -m "$(commit_msg 
iso8859-1)" &&
head5=$(git rev-parse --verify HEAD)
 '
 # git log --pretty=oneline # to see those SHA1 involved
@@ -62,10 +62,10 @@ test_expect_success 'reset --hard message' '
test_cmp .expected .actual
 '
 
-test_expect_success 'reset --hard message (iso-8859-1 logoutencoding)' '
+test_expect_success 'reset --hard message (iso8859-1 logoutputencoding)' '
hex=$(git log -1 --format="%h") &&
-   git -c "i18n.logOutputEncoding=iso-8859-1" reset --hard > .actual &&
-   echo HEAD is now at $hex $(commit_msg iso-8859-1) > .expected &&
+   git -c "i18n.logOutputEncoding=iso8859-1" reset --hard > .actual &&
+   echo HEAD is now at $hex $(commit_msg iso8859-1) > .expected &&
test_cmp .expected .actual
 '
 
@@ -329,7 +329,7 @@ test_expect_success 'redoing the last two commits should 
succeed' '
 
echo "1st line 2nd file" >secondfile &&
echo "2nd line 2nd file" >>secondfile &&
-   git -c "i18n.commitEncoding=iso-8859-1" commit -a -m "$(commit_msg 
iso-8859-1)" &&
+   git -c "i18n.commitEncoding=iso8859-1" commit -a -m "$(commit_msg 
i

[PATCH v10 2/5] t4205 (log-pretty-formats): revert back single quotes

[Alexey Shumkin]

In previuos commit de6029a (pretty: Add failing tests: --format output
should honor logOutputEncoding, 2013-06-26) single quotes were replaced
with double quotes to make "$(commit_msg)" expression in heredoc to
work. The same effect can be achieved by using "EOF" as a heredoc
delimiter instead of "\EOF".

Signed-off-by: Alexey Shumkin 
Suggested-by: Johannes Sixt 
---
 t/t4205-log-pretty-formats.sh | 106 +-
 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index c283842..ef9770a 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -145,174 +145,174 @@ test_expect_success 'setup more commits' '
head4=$(git rev-parse --verify --short HEAD~3)
 '
 
-test_expect_success 'left alignment formatting' "
-   git log --pretty='format:%<(40)%s' >actual &&
+test_expect_success 'left alignment formatting' '
+   git log --pretty="format:%<(40)%s" >actual &&
# complete the incomplete line at the end
echo >>actual &&
-   qz_to_tab_space <<\EOF >expected &&
+   qz_to_tab_space actual &&
+test_expect_success 'left alignment formatting at the nth column' '
+   git log --pretty="format:%h %<|(40)%s" >actual &&
# complete the incomplete line at the end
echo >>actual &&
-   qz_to_tab_space <<\EOF >expected &&
+   qz_to_tab_space actual &&
+test_expect_success 'left alignment formatting with no padding' '
+   git log --pretty="format:%<(1)%s" >actual &&
# complete the incomplete line at the end
echo >>actual &&
-   cat <<\EOF >expected &&
+   cat actual &&
+test_expect_success 'left alignment formatting with trunc' '
+   git log --pretty="format:%<(10,trunc)%s" >actual &&
# complete the incomplete line at the end
echo >>actual &&
-   qz_to_tab_space <<\EOF >expected &&
+   qz_to_tab_space actual &&
+test_expect_success 'left alignment formatting with ltrunc' '
+   git log --pretty="format:%<(10,ltrunc)%s" >actual &&
# complete the incomplete line at the end
echo >>actual &&
-   qz_to_tab_space <<\EOF >expected &&
+   qz_to_tab_space actual &&
+test_expect_success 'left alignment formatting with mtrunc' '
+   git log --pretty="format:%<(10,mtrunc)%s" >actual &&
# complete the incomplete line at the end
echo >>actual &&
-   qz_to_tab_space <<\EOF >expected &&
+   qz_to_tab_space actual &&
+test_expect_success 'right alignment formatting' '
+   git log --pretty="format:%>(40)%s" >actual &&
# complete the incomplete line at the end
echo >>actual &&
-   qz_to_tab_space <<\EOF >expected &&
+   qz_to_tab_space actual &&
+test_expect_success 'right alignment formatting at the nth column' '
+   git log --pretty="format:%h %>|(40)%s" >actual &&
# complete the incomplete line at the end
echo >>actual &&
-   qz_to_tab_space <<\EOF >expected &&
+   qz_to_tab_space 

Re: [PATCH] send-email: squelch warning from Net::SMTP::SSL

[Matthieu Moy]

Ramkumar Ramachandra  writes:

> John Keeping wrote:
>> I don't think this is really "fix", it's more plastering over the
>> problem.
>
> It defaulted to SSL_VERIFY_NONE before Net::SMTP::SSL was updated, and
> the behavior hasn't changed now.  The new version simply asks us to be
> explicit about SSL_VERIFY_NONE, so we are aware about it.

"We" as "the Git developers", yes. But your change makes sure users are
_not_ aware about it. There's a long history of software ignoring SSL
certificates by default, I don't think we should cast in stone that we
don't want SSL certificate verification.

-- 
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] send-email: squelch warning from Net::SMTP::SSL

[Ramkumar Ramachandra]

John Keeping wrote:
> I don't think this is really "fix", it's more plastering over the
> problem.

It defaulted to SSL_VERIFY_NONE before Net::SMTP::SSL was updated, and
the behavior hasn't changed now.  The new version simply asks us to be
explicit about SSL_VERIFY_NONE, so we are aware about it.

> I'd rather leave this as it is (complete with the big scary error
> message) and eventually fix it properly by letting the user specify the
> ca_file or ca_path.  Perhaps we can even set a sensible default,
> although I expect this needs to be platform-specific.

Nothing scary about it: it eats up real estate, and that is annoying.
I personally couldn't care less about ca_file, since all my emails are
to public mailing lists anyway.  Work on specifying a proper ca_file
as a follow-up, if you so desire.
--
To unsubscribe from this list: send the line "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: squelch warning from Net::SMTP::SSL

[John Keeping]

On Fri, Jul 05, 2013 at 03:48:31PM +0530, Ramkumar Ramachandra wrote:
> Due to a recent change in the Net::SMTP::SSL module, send-email emits
> the following ugly warning everytime a email is sent via SSL:
> 
> ***
>  Using the default of SSL_verify_mode of SSL_VERIFY_NONE for client
>  is deprecated! Please set SSL_verify_mode to SSL_VERIFY_PEER
>  together with SSL_ca_file|SSL_ca_path for verification.
>  If you really don't want to verify the certificate and keep the
>  connection open to Man-In-The-Middle attacks please set
>  SSL_verify_mode explicitly to SSL_VERIFY_NONE in your application.
> ***
> 
> Fix this by explicitly specifying SSL_verify_mode => SSL_VERIFY_NONE in
> Net::SMTP::SSL->start_SSL().

I don't think this is really "fix", it's more plastering over the
problem.  As the message from OpenSSL says, specifying this means that
we're explicitly saying that we don't want to check the server
certificate which loses half of the security of SSL.

I'd rather leave this as it is (complete with the big scary error
message) and eventually fix it properly by letting the user specify the
ca_file or ca_path.  Perhaps we can even set a sensible default,
although I expect this needs to be platform-specific.
--
To unsubscribe from this list: send the line "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: squelch warning from Net::SMTP::SSL

[Ramkumar Ramachandra]

Due to a recent change in the Net::SMTP::SSL module, send-email emits
the following ugly warning everytime a email is sent via SSL:

***
 Using the default of SSL_verify_mode of SSL_VERIFY_NONE for client
 is deprecated! Please set SSL_verify_mode to SSL_VERIFY_PEER
 together with SSL_ca_file|SSL_ca_path for verification.
 If you really don't want to verify the certificate and keep the
 connection open to Man-In-The-Middle attacks please set
 SSL_verify_mode explicitly to SSL_VERIFY_NONE in your application.
***

Fix this by explicitly specifying SSL_verify_mode => SSL_VERIFY_NONE in
Net::SMTP::SSL->start_SSL().

Helped-by: brian m. carlson 
Signed-off-by: Ramkumar Ramachandra 
---
 git-send-email.perl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index ecbf56f..758100d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1193,10 +1193,12 @@ X-Mailer: git-send-email $gitversion
 Debug => $debug_net_smtp);
if ($smtp_encryption eq 'tls' && $smtp) {
require Net::SMTP::SSL;
+   use IO::Socket::SSL qw(SSL_VERIFY_NONE);
$smtp->command('STARTTLS');
$smtp->response();
if ($smtp->code == 220) {
-   $smtp = Net::SMTP::SSL->start_SSL($smtp)
+   $smtp = Net::SMTP::SSL->start_SSL($smtp,
+ 
SSL_verify_mode => SSL_VERIFY_NONE)
or die "STARTTLS failed! 
".$smtp->message;
$smtp_encryption = '';
# Send EHLO again to receive fresh
-- 
1.8.3.2.722.g3244e19.dirty

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

Re: [PATCH] diff-options: document default similarity index

[Fraser Tweedale]

On Fri, Jul 05, 2013 at 02:00:40AM -0700, Junio C Hamano wrote:
> Fraser Tweedale  writes:
> 
> > The default similarity index of 50% is documented in gitdiffcore(7)
> > but it is worth also mentioning it in the description of the
> > -M/--find-renames option.
> 
> Is it?  I am not sure if it is worth it.

The -M description goes on at some length to explain how to specify
a particular value.  Surely a brief mention of the default value is
also warranted, especially since the pointer to gitdiffcore(7) for a
more detailed explanation of the options appears quite a way below.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

What's cooking in git.git (Jul 2013, #02; Fri, 5)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

We are in the middle of 5th week now in the 11-week releace cycle
for 1.8.4 (http://tinyurl.com/gitCal), and quite a few topics have
graduated to 'master'.  I'd expect the rest of the week to be slow.

Please help ensure the quality of the upcoming release by testing
the tip of 'master' (and if you are so inclined, 'next') early.

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

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

--
[Graduated to "master"]

* tr/test-v-and-v-subtest-only (2013-06-29) 9 commits
  (merged to 'next' on 2013-06-30 at 1c5fac1)
 + perf-lib: fix start/stop of perf tests
  (merged to 'next' on 2013-06-26 at 8ff4d84)
 + test-lib: support running tests under valgrind in parallel
 + test-lib: allow prefixing a custom string before "ok N" etc.
 + test-lib: valgrind for only tests matching a pattern
 + test-lib: verbose mode for only tests matching a pattern
 + test-lib: self-test that --verbose works
 + test-lib: rearrange start/end of test_expect_* and test_skip
 + test-lib: refactor $GIT_SKIP_TESTS matching
 + test-lib: enable MALLOC_* for the actual tests

 Allows N instances of tests run in parallel, each running 1/N parts
 of the test suite under Valgrind, to speed things up.

--
[New Topics]

* jc/t1512-fix (2013-07-01) 2 commits
 - get_short_sha1(): correctly disambiguate type-limited abbreviation
 - t1512: correct leftover constants from earlier edition

 A test that should have failed but didn't revealed a bug that needs
 to be corrected.

 Will merge to 'next'.


* jk/fetch-pack-many-refs (2013-07-02) 3 commits
 - fetch-pack: avoid quadratic behavior in rev_list_push
 - commit.c: make compare_commits_by_commit_date global
 - fetch-pack: avoid quadratic list insertion in mark_complete

 Fetching between repositories with many refs employed O(n^2)
 algorithm to match up the common objects, which has been corrected.

 Will merge to 'next'.


* jk/format-patch-from (2013-07-03) 2 commits
 - teach format-patch to place other authors into in-body "From"
 - pretty.c: drop const-ness from pretty_print_context

 "git format-patch" learned "--from[=whom]" option, which sets the
 "From: " header to the specified person (or the person who runs the
 command, if "=whom" part is missing) and move the original author
 information to an in-body From: header as necessary.

 Will merge to 'next'.


* ms/remote-tracking-branches-in-doc (2013-07-03) 1 commit
 - Change "remote tracking" to "remote-tracking"

 Will merge to 'next'.


* tf/gitweb-extra-breadcrumbs (2013-07-04) 1 commit
 - gitweb: allow extra breadcrumbs to prefix the trail

 An Gitweb installation that is a part of larger site can optionally
 show extra links that point at the levels higher than the Gitweb
 pages itself in the link hierarchy of pages.

 Will merge to 'next'.


* bc/commit-invalid-utf8 (2013-07-04) 2 commits
 - commit: reject overlong UTF-8 sequences
 - commit: reject invalid UTF-8 codepoints

 Will merge to 'next'.


* bc/send-email-use-port-as-separate-param (2013-07-04) 1 commit
 - send-email: provide port separately from hostname

 Will merge to 'next'.


* ml/cygwin-does-not-have-fifo (2013-07-05) 1 commit
 - test-lib.sh - cygwin does not have usable FIFOs

 Will merge to 'next'.

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


* hv/config-from-blob (2013-05-12) 5 commits
 - do not die when error in config parsing of buf occurs
 - teach config --blob option to parse config from database
 - config: make parsing stack struct independent from actual data source
 - config: drop cf validity check in get_next_char()
 - config: factor out config file stack management

 Waiting for a reroll.
 $gmane/223964


* jh/shorten-refname (2013-05-07) 4 commits
 - t1514: refname shortening is done after dereferencing symbolic refs
 - shorten_unambiguous_ref(): Fix shortening refs/remotes/origin/HEAD to origin
 - t1514: Demonstrate failure to correctly shorten "refs/remotes/origin/HEAD"
 - t1514: Add tests of shortening refnames in strict/loose mode

 When remotes/origin/HEAD is not a symbolic ref, "rev-parse
 --abbrev-ref remotes/origin/HEAD" ought to show "origin", not
 "origin/HEAD", which is fixed with this series (if it is a symbolic
 ref that points at remotes/origin/something, then it should show
 "or

Re: [PATCH] diff-options: document default similarity index

[Junio C Hamano]

Fraser Tweedale  writes:

> The default similarity index of 50% is documented in gitdiffcore(7)
> but it is worth also mentioning it in the description of the
> -M/--find-renames option.

Is it?  I am not sure if it is worth 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 v9 3/5] t4205, t6006, t7102: make functions more readable

[Junio C Hamano]

Alexey Shumkin  writes:

>> > Could you point me to the coding style guide, please?
>> 
>> Documentation/CodingGuidelines::
> Oh! :)
> thank you

The most important part of the coding guidelines is to match the
style to existing code when writing a new one:

$ git grep '^[a-z_]* ()' -- *.sh | wc -l
132
$ git grep '^[a-z_]*()' -- *.sh | wc -l
55

We have acquired quite a few violators, but that is no reason to add
more.
--
To unsubscribe from this list: send the line "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 v9 1/5] t4041, t4205, t6006, t7102: use iso8859-1 rather than iso-8859-1

[Junio C Hamano]

Alexey Shumkin  writes:

>> OK, then I'll queue this patch (but not 2-4/5 yet) with log message
>> amended.
> Excuse me, you've said "Ok" for 2/5 message, and then explained (as I
> understood) then "subtle difference" between EOF and \EOF.
> Should I change the message somehow?

I left it up to you.  If I queued without waiting, it wouldn't have
been up-to-you, so...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 3/5] t4205, t6006, t7102: make functions more readable

[Alexey Shumkin]

On Fri, Jul 05, 2013 at 01:44:07AM -0700, Junio C Hamano wrote:
> Alexey Shumkin  writes:
> 
> >> Perhaps like this.
> >> 
> >> Function 'test_format' has become harder to read after its
> >> change in de6029a2 (pretty: Add failing tests: --format output
> >> should honor logOutputEncoding, 2013-06-26).  Simplify it by
> >> moving its "should we expect it to fail?" parameter to the end.
> > I'm not sure whether this "last parameter" is needed in that code as far as 
> > we
> > already removed expected to fail tests
> 
> Whatever.
> 
> The above is an example of justifying a more vague "simple" ("is
> better" is implied) with a concrete point (i.e. By moving that to
> the end, you removed the need to conditionally shift $@ in the
> function to simplify the codepath), based on my _guess_ of what you
> possibly meant to say, from reading your description that did not
> give much clue for me to guess why you thought the result was "more
> elegant".  If my guess missed what your true justification was,
> please replace it with the more correct one ;-)
Ok
> 
> >> I cannot read why you think the updated commit_msg is "more pretty"
> >> in the message or in the patch.
> >> 
> >> > -commit_msg () {
> >> > -# String "initial. initial" partly in German (translated with 
> >> > Google Translate),
> >> > +commit_msg() {
> >> 
> >> Style.  Have SP on both sides of () in a shell function definition.
> > Could you point me to the coding style guide, please?
> 
> Documentation/CodingGuidelines::
Oh! :)
thank you
>  
>  - We prefer a space between the function name and the parentheses. The
>opening "{" should also be on the same line.
>E.g.: my_function () {
Aha

-- 
Alexey Shumkin
--
To unsubscribe from this list: send the line "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 v9 3/5] t4205, t6006, t7102: make functions more readable

[Junio C Hamano]

Alexey Shumkin  writes:

>> Perhaps like this.
>> 
>> Function 'test_format' has become harder to read after its
>> change in de6029a2 (pretty: Add failing tests: --format output
>> should honor logOutputEncoding, 2013-06-26).  Simplify it by
>> moving its "should we expect it to fail?" parameter to the end.
> I'm not sure whether this "last parameter" is needed in that code as far as we
> already removed expected to fail tests

Whatever.

The above is an example of justifying a more vague "simple" ("is
better" is implied) with a concrete point (i.e. By moving that to
the end, you removed the need to conditionally shift $@ in the
function to simplify the codepath), based on my _guess_ of what you
possibly meant to say, from reading your description that did not
give much clue for me to guess why you thought the result was "more
elegant".  If my guess missed what your true justification was,
please replace it with the more correct one ;-)

>> I cannot read why you think the updated commit_msg is "more pretty"
>> in the message or in the patch.
>> 
>> > -commit_msg () {
>> > -  # String "initial. initial" partly in German (translated with Google 
>> > Translate),
>> > +commit_msg() {
>> 
>> Style.  Have SP on both sides of () in a shell function definition.
> Could you point me to the coding style guide, please?

Documentation/CodingGuidelines::

 - We prefer a space between the function name and the parentheses. The
   opening "{" should also be on the same line.
   E.g.: my_function () {
--
To unsubscribe from this list: send the line "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 v9 1/5] t4041, t4205, t6006, t7102: use iso8859-1 rather than iso-8859-1

[Alexey Shumkin]

On Fri, Jul 05, 2013 at 01:11:49AM -0700, Junio C Hamano wrote:
> Alexey Shumkin  writes:
> 
> >>Both "iso8859-1" and "iso-8859-1" are understood as latin-1
> >>by modern platforms, but the latter is not understood by
> >>older platforms;update tests to use the former.
> >> 
> >> This is in line with 3994e8a9 (t4201: use ISO8859-1 rather
> >>than ISO-8859-1, 2009-12-03), which did the same.
> > Yep, it whould be better, I thought to do like this but I didn't )
> >> 
> >> > Signed-off-by: Alexey Shumkin 
> >> > Reviewed-by: Johannes Sixt 
> >> 
> >> I do not recall this exact patch reviewed by J6t, but perhaps I
> >> missed a message on the list?
> > I've reread 'SubmittingPatches' doc, and I can say I used "Reviewed-by"
> > incorrectly. Sorry for this. It must be "Suggested-by" there, I guess.
> 
> OK, then I'll queue this patch (but not 2-4/5 yet) with log message
> amended.
Excuse me, you've said "Ok" for 2/5 message, and then explained (as I
understood) then "subtle difference" between EOF and \EOF.
Should I change the message somehow?
> 
> Thanks.

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

[PATCH] diff-options: document default similarity index

[Fraser Tweedale]

The default similarity index of 50% is documented in gitdiffcore(7)
but it is worth also mentioning it in the description of the
-M/--find-renames option.

Signed-off-by: Fraser Tweedale 
---
 Documentation/diff-options.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index a85288f..d6e4db4 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -333,7 +333,7 @@ endif::git-log[]
a fraction, with a decimal point before it.  I.e., `-M5` becomes
0.5, and is thus the same as `-M50%`.  Similarly, `-M05` is
the same as `-M5%`.  To limit detection to exact renames, use
-   `-M100%`.
+   `-M100%`.  The default similarity index is 50%.
 
 -C[]::
 --find-copies[=]::
-- 
1.8.3.1

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

[PATCH] documentation: add git transport security notice

[Fraser Tweedale]

The fact that the git transport does not do any authentication is
easily overlooked.  For example, DNS poisoning may result in
fetching from somewhere that was not intended.

Add a brief security notice to the "GIT URLS" section of the
documentation stating that the git transport should be used with
caution on unsecured networks.

Signed-off-by: Fraser Tweedale 
---
 Documentation/urls.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index 60ba300..9c8f524 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -11,6 +11,9 @@ and ftps can be used for fetching and rsync can be used for 
fetching
 and pushing, but these are inefficient and deprecated; do not use
 them).
 
+The git transport does not do any authentication and should be used
+with caution on unsecured networks.
+
 The following syntaxes may be used with them:
 
 - ssh://{startsb}user@{endsb}host.xz{startsb}:port{endsb}/path/to/repo.git/
-- 
1.8.3.1

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

Re: [PATCH v9 3/5] t4205, t6006, t7102: make functions more readable

[Alexey Shumkin]

On Thu, Jul 04, 2013 at 11:45:57PM -0700, Junio C Hamano wrote:
> Alexey Shumkin  writes:
> 
> > Function 'test_format' is become hard to read after its change in
> > de6029a2d7734a93a9e27b9c4471862a47dd8123. So, make it more elegant.
> > Also, change 'commit_msg' function to make it more pretty.
> 
> I do not know where you pick up these "more elegant" and "more
> pretty" from, but please refrain from using _only_ such vague and
> subjective phrases to describe the change in the log message.
> Saying "make it <> by doing X" (with various subjective
> adjectives to say "better") is fine, but make sure you have "doing
> X" part in the explanation.
> 
> Perhaps like this.
> 
> Function 'test_format' has become harder to read after its
> change in de6029a2 (pretty: Add failing tests: --format output
> should honor logOutputEncoding, 2013-06-26).  Simplify it by
> moving its "should we expect it to fail?" parameter to the end.
I'm not sure whether this "last parameter" is needed in that code as far as we
already removed expected to fail tests
> 
> I cannot read why you think the updated commit_msg is "more pretty"
> in the message or in the patch.
> 
> > -commit_msg () {
> > -   # String "initial. initial" partly in German (translated with Google 
> > Translate),
> > +commit_msg() {
> 
> Style.  Have SP on both sides of () in a shell function definition.
Could you point me to the coding style guide, please?
> 
> > +   # String "initial. initial" partly in German
> > +   # (translated with Google Translate),
> > # encoded in UTF-8, used as a commit log message below.
> > msg=$(printf "initial. anf\303\244nglich")
> > if test -n "$1"
> 
> This is not "more pretty" but "better commented".
Well, this is "better formatted comment", I guess :)
> 
> > diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
> > index 2ef96e9..73a1bdb 100755
> > --- a/t/t7102-reset.sh
> > +++ b/t/t7102-reset.sh
> > @@ -9,15 +9,17 @@ Documented tests for git reset'
> >  
> >  . ./test-lib.sh
> >  
> > -commit_msg () {
> > -   # String "modify 2nd file (changed)" partly in German(translated with 
> > Google Translate),
> > +commit_msg() {
> > +   # String "modify 2nd file (changed)" partly in German
> > +   # (translated with Google Translate),
> > # encoded in UTF-8, used as a commit log message below.
> > -   msg=$(printf "modify 2nd file (ge\303\244ndert)")
> > +   printf "modify 2nd file (ge\303\244ndert)" |
> > if test -n "$1"
> > then
> > -   msg=$(echo $msg | iconv -f utf-8 -t $1)
> > +   iconv -f utf-8 -t $1
> > +   else
> > +   cat
> > fi
> > -   echo $msg
> 
> Is it "more pretty"?  The "we have to have cat only because we want
> to pipe into a conditional" look somewhat ugly.
That was a proposition of J6t :-D
(see http://article.gmane.org/gmane.comp.version-control.git/229291):
>If you wanted to, you could write this as
>
>commit_msg () {
># String "modify 2nd file (changed)" partly in German
>#(translated with Google Translate),
># encoded in UTF-8, used as a commit log message below.
>printf "modify 2nd file (ge\303\244ndert)" |
>if test -n "$1"
>then
>iconv -f utf-8 -t $1
>else
>cat
>fi
>}
>
>but I'm not sure whether it's a lot better.

Last sentence has apperared to be a key

> 
>   msg="modify 2nd file (ge\303\244ndert)"
> if test -n "$1"
>   then
>   printf "$msg" | iconv -f utf-8 -t "$1"
>   else
>   printf "$msg"
>   fi
> 
> >  }
> >  
> >  test_expect_success 'creating initial files and commits' '

-- 
Alexey Shumkin
--
To unsubscribe from this list: send the line "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 v9 1/5] t4041, t4205, t6006, t7102: use iso8859-1 rather than iso-8859-1

[Junio C Hamano]

Alexey Shumkin  writes:

>>  Both "iso8859-1" and "iso-8859-1" are understood as latin-1
>>  by modern platforms, but the latter is not understood by
>>  older platforms;update tests to use the former.
>> 
>> This is in line with 3994e8a9 (t4201: use ISO8859-1 rather
>>  than ISO-8859-1, 2009-12-03), which did the same.
> Yep, it whould be better, I thought to do like this but I didn't )
>> 
>> > Signed-off-by: Alexey Shumkin 
>> > Reviewed-by: Johannes Sixt 
>> 
>> I do not recall this exact patch reviewed by J6t, but perhaps I
>> missed a message on the list?
> I've reread 'SubmittingPatches' doc, and I can say I used "Reviewed-by"
> incorrectly. Sorry for this. It must be "Suggested-by" there, I guess.

OK, then I'll queue this patch (but not 2-4/5 yet) with log message
amended.

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 v9 4/5] t6006: add two more tests for the case i18n.commitEncoding is not set

[Junio C Hamano]

Alexey Shumkin  writes:

>> Perhaps we should update test_format so that we can feed a quoted
>> input, e.g.
>> 
>> +include an iso8859-1 character: bueno!
>> 
>> or something?
> We could use this file whole in UTF-8 but just make a conversion of
> expected output as it's done a few lines above with a commit message
> (stored to a file 'commit-msg' before the test 'setup complex body').
> +iconv -f utf-8 -t iso8859-1 > commit-msg < +Test printing of complex bodies

That is way better than my bueno!.

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 v9 1/5] t4041, t4205, t6006, t7102: use iso8859-1 rather than iso-8859-1

[Alexey Shumkin]

On Thu, Jul 04, 2013 at 11:47:04PM -0700, Junio C Hamano wrote:
> Alexey Shumkin  writes:
> 
> > This is actually a fixup of de6029a2d7734a93a9e27b9c4471862a47dd8123,
> > which was applied before final patch series was sent.
> >
> > Also, see 3994e8a98dc7bbf67e61d23c8125f44383499a1f for the explanation
> > of such a replacement.
> 
> These are not very useful in a log message.  People who read the
> history 6 months down the road would want to see why we want to use
> iso8859-1 not iso-8859-1 explained.
> 
>   Both "iso8859-1" and "iso-8859-1" are understood as latin-1
>   by modern platforms, but the latter is not understood by
>   older platforms;update tests to use the former.
> 
> This is in line with 3994e8a9 (t4201: use ISO8859-1 rather
>   than ISO-8859-1, 2009-12-03), which did the same.
Yep, it whould be better, I thought to do like this but I didn't )
> 
> > Signed-off-by: Alexey Shumkin 
> > Reviewed-by: Johannes Sixt 
> 
> I do not recall this exact patch reviewed by J6t, but perhaps I
> missed a message on the list?
I've reread 'SubmittingPatches' doc, and I can say I used "Reviewed-by"
incorrectly. Sorry for this. It must be "Suggested-by" there, I guess.


-- 
Alexey Shumkin
--
To unsubscribe from this list: send the line "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 v9 4/5] t6006: add two more tests for the case i18n.commitEncoding is not set

[Alexey Shumkin]

On Fri, Jul 05, 2013 at 12:04:34AM -0700, Junio C Hamano wrote:
> Alexey Shumkin  writes:
> 
> > +test_format complex-body %b < > +commit $head3
> > +This commit message is much longer than the others,
> > +and it will be encoded in iso8859-1. We should therefore
> > +include an iso8859 character: ¡bueno!
> 
> This is not such a good idea, as the resulting file will be in mixed
> encoding (it already has a line with non-ascii that is in UTF-8),
> and many editors would not like such a file.
I agree, there was issues when I edited that file
> 
> Perhaps we should update test_format so that we can feed a quoted
> input, e.g.
> 
> +include an iso8859-1 character: bueno!
> 
> or something?
We could use this file whole in UTF-8 but just make a conversion of
expected output as it's done a few lines above with a commit message
(stored to a file 'commit-msg' before the test 'setup complex body').
+iconv -f utf-8 -t iso8859-1 > commit-msg < 
> > +commit $head2
> > +commit $head1
> > +EOF
> > +
> > +# Git uses i18n.commitEncoding if no i18n.logOutputEncoding set
> > +# so unset i18n.commitEncoding to test encoding conversion
> > +git config --unset i18n.commitEncoding
> > +
> > +test_format complex-subject-commitencoding-unset %s < > +commit $head3
> > +Test printing of complex bodies
> > +commit $head2
> >  $changed
> >  commit $head1
> >  $added
> >  EOF
> >  
> > -test_format complex-body %b < > +test_format complex-body-commitencoding-unset %b < >  commit $head3
> >  This commit message is much longer than the others,
> >  and it will be encoded in iso8859-1. We should therefore

-- 
Alexey Shumkin
--
To unsubscribe from this list: send the line "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: unexpected file deletion after using git rebase --abort

[Junio C Hamano]

"Paul A. Kennedy"  writes:

>> "rebase --abort" is typically used to get rid of conflicted mess the
>> user does not want to resolve right now, and "stash" would not be a
>> sensible thing to use in such a situation, I think.  Doesn't it even
>> refuse to work if there is a conflicted entry in the index?
>
> Thanks for thinking about this with me.  
>
> After contemplating your messages, I think that it's unreasonable to
> expect git rebase --abort to be able to properly handle content from
> completely outside the repo and only placed in the index.

The essense of "--abort" is to bring your index and the working tree
state back to the state you had before you got into a conflicted
mess.

For paths that still have higher/conflicted stages in the index, it
is easy to decide what should happen.  Because we do not even allow
rebase to start with an index with conflicted paths, they must have
come from the rebase operation itself, so matching them to what is
in the HEAD (and removing such conflicted paths that are not in the
HEAD) is always the right thing.

But for paths in the index that are already resolved at stage #0,
it is not simple.  They can come from

 (1) a clean automatic resolution (including the case where the
 rebase did not even touch the path).

 (2) a conflict that was resolved manually and then "git add"ed.

 (3) manual "git add" of a path that a mechanical part of the rebase
 operation did not touch at all.

For (1) and (2), it is the right thing to match with HEAD and
to remove the path if it is not in HEAD (e.g. the path may have
appeared by attempting to replay a change to add it).

For (3), a change may update existing files in HEAD or it may add a
new path not in HEAD.  Ideally, the former should be reverted to
HEAD, and the latter should be only reverted in the index but leave
the file in the working tree untracked.

But there is no good way to tell these three classes apart,
unfortunately.  With recent Git, I think you can tell paths in
category (2) by noticing that they have "REUC" entries for them in
the index extension section.

The only case we may want to behave differently is "git add" that
adds a path that does not exist (at any stage) to the index, so
theoretically, we could teach the end-user facing "git add" to leave
a new extension entry to the index, teach "git reset --no-so-hard"
to notice the index extension to leave the path in the working tree
when reverting to a HEAD that does not have such a path and use it
in "rebase --abort" codepath.  Which would also allow us to be less
aggressive in a case like this:

... have many untracked and unignored paths
$ git add .
... oops, I did not mean it
$ git reset --hard
... double-oops, these untracked paths are all gone

But that is "theoretical" primarily because it is unclear what the
exhaustive set of situations where we must clear such a "this is
newly added" mark to avoid false positives.  After "git commit" that
records such a path in a commit is obviously one of them, but that
is not the only one, and every place we forget to clear the mark
will be a cause of confusion.

Alternatively, perhaps we could create the "REUC" extension for
category (1) paths as well.  Then "git reset --not-so-hard $commit"
could be changed to behave the same way as it does today, except
that it would not remove paths without "REUC" extension entry from
the working tree.

> + Untracked files added to the index will not be unstaged, and
> + therefore, not present in the working directory upon abort.
> + Unstage files before the abort, or stash untracked content before
> + starting the rebase (see linkgit:git-stash[1]).

"Unstage files" is a less than ideal suggestion.

$ git rebase
... oops conflicted
... ouch, this is too much for me to resolve
$ git reset
$ git rebase --abort

would leave a path that was introduced by the change being replayed
in the working tree, which may later interfere when you try to
checkout a branch that does have that path.  At least it has to be
"Unstage such files that were untracked and added by you" to be
helpful, I think.

--
To unsubscribe from this list: send the line "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 v9 2/5] t4205: revert back single quotes

[Junio C Hamano]

Alexey Shumkin  writes:

> In previuos commit de6029a2d7734a93a9e27b9c4471862a47dd8123 single
> quotes were replaced with double quotes to make "$(commit_msg)"
> expression in heredoc to work. The same effect can be achieved by using
> "EOF" as a heredoc delimiter instead of "\EOF".

OK.

> -test_expect_success 'left alignment formatting' "
> - git log --pretty='format:%<(40)%s' >actual &&
> +test_expect_success 'left alignment formatting' '
> + git log --pretty="format:%<(40)%s" >actual &&
>   # complete the incomplete line at the end
>   echo >>actual &&
> - qz_to_tab_space <<\EOF >expected &&
> + qz_to_tab_space   message twoZ
>  message oneZ
>  add barZ
>  $(commit_msg)Z
>  EOF
>   test_cmp expected actual
> -"
> +'

A subtle difference is that a call to commit_msg is made when the
test is actually run, not when the test script is prepared to be
passed (as a parameter) to test_expect_success helper.  I think the
result of applying this patch, i.e. running $(commit_msg) inside the
test, is easier to read and understand.
--
To unsubscribe from this list: send the line "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 v9 4/5] t6006: add two more tests for the case i18n.commitEncoding is not set

[Junio C Hamano]

Alexey Shumkin  writes:

> +test_format complex-body %b < +commit $head3
> +This commit message is much longer than the others,
> +and it will be encoded in iso8859-1. We should therefore
> +include an iso8859 character: ¡bueno!

This is not such a good idea, as the resulting file will be in mixed
encoding (it already has a line with non-ascii that is in UTF-8),
and many editors would not like such a file.

Perhaps we should update test_format so that we can feed a quoted
input, e.g.

+include an iso8859-1 character: bueno!

or something?

> +commit $head2
> +commit $head1
> +EOF
> +
> +# Git uses i18n.commitEncoding if no i18n.logOutputEncoding set
> +# so unset i18n.commitEncoding to test encoding conversion
> +git config --unset i18n.commitEncoding
> +
> +test_format complex-subject-commitencoding-unset %s < +commit $head3
> +Test printing of complex bodies
> +commit $head2
>  $changed
>  commit $head1
>  $added
>  EOF
>  
> -test_format complex-body %b < +test_format complex-body-commitencoding-unset %b <  commit $head3
>  This commit message is much longer than the others,
>  and it will be encoded in iso8859-1. We should therefore
--
To unsubscribe from this list: send the line "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 v8 3/7] git-remote-mediawiki: New git bin-wrapper for developement

[Matthieu Moy]

benoit.per...@ensimag.fr writes:

> --- a/contrib/mw-to-git/Makefile
> +++ b/contrib/mw-to-git/Makefile
> @@ -2,6 +2,12 @@
>  # Copyright (C) 2013
>  # Matthieu Moy 
>  #
> +# To build and test:
> +#
> +#   make:
> +# bin-wrapper/git mw preview Some_page.mw
> +# bin-wrapper/git clone mediawiki::http://example.com/wiki/
> +#

The colon after "make" and the indentation look weird. Shouldn't this be

+# To build and test:
+#
+#   make
+#   bin-wrapper/git mw preview Some_page.mw
+#   bin-wrapper/git clone mediawiki::http://example.com/wiki/
+#

?

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