All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ted Zlatanov <tzz@lifelogs.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCHv6] Add contrib/credentials/netrc with GPG support
Date: Mon, 25 Feb 2013 11:24:02 -0500	[thread overview]
Message-ID: <87621guztp.fsf@lifelogs.com> (raw)
In-Reply-To: <20130208061855.GA11892@sigill.intra.peff.net> (Jeff King's message of "Fri, 8 Feb 2013 01:18:55 -0500")

On Fri, 8 Feb 2013 01:18:55 -0500 Jeff King <peff@peff.net> wrote: 

>> +	# the following check is copied from Net::Netrc, for non-GPG files
>> +	# OS/2 and Win32 do not handle stat in a way compatable with this check :-(

JK> s/compatable/compatible/

This is from the Net::Netrc module.  Fixed in my commit but eh...

JK> You mention os/2 and Win32 here, but the check has more:

>> +	unless ($gpgmode || $options{insecure} ||
>> +		$^O eq 'os2'
>> +		|| $^O eq 'MSWin32'
>> +		|| $^O eq 'MacOS'
>> +		|| $^O =~ /^cygwin/) {

JK> Does MacOS really not handle stat? Or is this old MacOS, not OS X?

This is all out of Net::Netrc, and yes, it's pre-Mac OS X.  I think it's
safe to leave as is, but I can remove OS/2 and MacOS if you prefer.

JK> So this will convert:
JK>   machine foo port smtp
JK> in the netrc into (protocol => "smtp", host => "foo"), but:
JK>   machine foo port 25
JK> into (protocol => undef, host => "foo:25"), right? That makes sense to
JK> me.

Yes.  test.pl checks that host=foo doesn't find the above, as well.

JK> I don't know if you want to take the hit of relying on Git.pm (it is
JK> nice for the helper to be totally standalone and copy-able), but one
JK> obvious possible refactor would be to use the credential read/write
JK> functions recently added there. I'm OK with not doing that, though.

JK> It may also be worth building on top of the regular git test harness.
JK> It's more work, but the resulting code (and the output) will be much
JK> more readable.

At least for now let's leave it standalone.  When and if it moves into
the core, we can change it to use the core's Git.pm and test suite.  The
code and the tests are small enough that I think using Perl's Test
module makes the most sense right now.

JK> Printf? Bleh, isn't this supposed to be perl? :P

What?  Was I supposed to use formats?!?!

JK> You are depending on whatever the user has in their ~/.netrc, no?
JK> Wouldn't it make more sense to ship a sample netrc and run all of the
JK> tests with "-f netrc.example"?

Yes.  See test.netrc.

Thanks
Ted

  reply	other threads:[~2013-02-25 16:24 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-04 19:54 [PATCH] Add contrib/credentials/netrc with GPG support Ted Zlatanov
2013-02-04 21:17 ` Jeff King
2013-02-04 21:32   ` Ted Zlatanov
2013-02-04 21:44   ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Ted Zlatanov
2013-02-04 22:56     ` Junio C Hamano
2013-02-04 23:23       ` Jeff King
2013-02-04 23:36         ` Junio C Hamano
2013-02-04 23:42         ` Ted Zlatanov
2013-02-04 23:28       ` [PATCHv3] Add contrib/credentials/netrc with GPG support Ted Zlatanov
2013-02-04 23:31       ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Ted Zlatanov
2013-02-04 23:40         ` Junio C Hamano
2013-02-04 23:54           ` Ted Zlatanov
2013-02-05  0:15             ` Junio C Hamano
2013-02-05 13:39               ` Ted Zlatanov
2013-02-05 16:07                 ` Junio C Hamano
2013-02-05 16:18                   ` Junio C Hamano
2013-02-05 16:15     ` Junio C Hamano
2013-02-05 17:01       ` Ted Zlatanov
2013-02-05 18:55         ` [PATCHv4] Add contrib/credentials/netrc with GPG support Ted Zlatanov
2013-02-05 19:53           ` Junio C Hamano
2013-02-05 20:47             ` Ted Zlatanov
2013-02-05 22:09               ` Junio C Hamano
2013-02-05 22:30                 ` Ted Zlatanov
2013-02-05 20:55             ` [PATCHv5] " Ted Zlatanov
2013-02-05 22:24               ` Junio C Hamano
2013-02-05 23:58                 ` Junio C Hamano
2013-02-06  0:38                   ` [PATCHv6] " Ted Zlatanov
2013-02-07 23:52                     ` Junio C Hamano
2013-02-08  1:53                       ` Ted Zlatanov
2013-02-08  6:15                         ` Junio C Hamano
2013-02-08  6:18                     ` Jeff King
2013-02-25 16:24                       ` Ted Zlatanov [this message]
2013-02-25 15:49                     ` [PATCH v7] " Ted Zlatanov
2013-02-06  0:34                 ` [PATCHv5] " Ted Zlatanov
2013-02-05 19:47         ` [PATCH] Add contrib/credentials/netrc with GPG support, try #2 Junio C Hamano
2013-02-05 20:03           ` Ted Zlatanov
2013-02-05 20:23             ` Junio C Hamano
2013-02-05 21:00               ` Ted Zlatanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87621guztp.fsf@lifelogs.com \
    --to=tzz@lifelogs.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.