{"thread":{"id":"32770","subject":"[PATCH] git-send-email: add ~/.authinfo parsing","startedAt":"2013-01-29T19:13:40Z","lastAt":"2013-02-08T18:15:56Z","messageCount":78,"participants":["Michal Nazarewicz","Junio C Hamano","Jeff King","Ted Zlatanov","Matthieu Moy","demerphq"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"208207","messageId":"2f93ce7b6b5d3f6c6d1b99958330601a5560d4ba.1359486391.git.mina86@mina86.com","threadId":"32770","inReplyTo":null,"subject":"[PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Michal Nazarewicz","fromEmail":"mpn@google.com","sentAt":"2013-01-29T19:13:40Z","receivedAt":"2013-01-29T19:13:40Z","isPatch":true,"sender":{"key":"mpn@google.com","avatar":null},"body":"From: Michal Nazarewicz <mina86@mina86.com>\n\nMake git-send-email read password from a ~/.authinfo file instead of\nrequiring it to be stored in git configuration, passed as command line\nargument or typed in.\n\nThere are various other applications that use this file for\nauthentication information so letting users use it for git-send-email\nis convinient.  Furthermore, some users store their ~/.gitconfig file\nin a public repository and having to store password there makes it\neasy to publish the password.\n\nNot to introduce any new dependencies, ~/.authinfo file is parsed only\nif Text::CSV Perl module is installed.  If it's not, a notification is\nprinted and the file is ignored.\n\nSigned-off-by: Michal Nazarewicz <mina86@mina86.com>\n---\n Documentation/git-send-email.txt | 15 +++++++--\n git-send-email.perl              | 69 +++++++++++++++++++++++++++++++++-------\n 2 files changed, 70 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex eeb561c..b83576e 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -158,14 +158,25 @@ Sending\n --smtp-pass[=<password>]::\n \tPassword for SMTP-AUTH. The argument is optional: If no\n \targument is specified, then the empty string is used as\n-\tthe password. Default is the value of 'sendemail.smtppass',\n-\thowever '--smtp-pass' always overrides this value.\n+\tthe password. Default is the value of 'sendemail.smtppass'\n+\tor value read from '~/.authinfo' file, however '--smtp-pass'\n+\talways overrides this value.\n +\n Furthermore, passwords need not be specified in configuration files\n or on the command line. If a username has been specified (with\n '--smtp-user' or a 'sendemail.smtpuser'), but no password has been\n specified (with '--smtp-pass' or 'sendemail.smtppass'), then the\n user is prompted for a password while the input is masked for privacy.\n++\n+The '~/.authinfo' file is read if Text::CSV Perl module is installed\n+on the system; if it's missing, a notification message will be printed\n+and the file ignored altogether.  The file should contain a line with\n+the following format:\n++\n+  machine <domain> port <port> login <user> password <pass>\n++\n+Contrary to other tools, 'git-send-email' does not support symbolic\n+port names like 'imap' thus `<port>` must be a number.\n \n --smtp-server=<host>::\n \tIf set, specifies the outgoing SMTP server to use (e.g.\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex be809e5..d824098 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1045,6 +1045,62 @@ sub maildomain {\n \treturn maildomain_net() || maildomain_mta() || 'localhost.localdomain';\n }\n \n+\n+sub read_password_from_stdin {\n+\tmy $line;\n+\n+\tsystem \"stty -echo\";\n+\n+\tdo {\n+\t\tprint \"Password: \";\n+\t\t$line = <STDIN>;\n+\t\tprint \"\\n\";\n+\t} while (!defined $line);\n+\n+\tsystem \"stty echo\";\n+\n+\tchomp $line;\n+\treturn $line;\n+}\n+\n+sub read_password_from_authinfo {\n+\tmy $fd;\n+\tif (!open $fd, '<', $ENV{'HOME'} . '/.authinfo') {\n+\t\treturn;\n+\t}\n+\n+\tif (!eval { require Text::CSV; 1 }) {\n+\t\tprint STDERR \"Text::CSV missing, won't read ~/.authinfo\\n\";\n+\t\tclose $fd;\n+\t\treturn;\n+\t}\n+\n+\tmy $csv = Text::CSV->new( { sep_char => ' ' } );\n+\tmy $password;\n+\twhile (my $line = <$fd>) {\n+\t\tchomp $line;\n+\t\t$csv->parse($line);\n+\t\tmy %row = $csv->fields();\n+\t\tif (defined $row{'machine'} &&\n+\t\t    defined $row{'login'} &&\n+\t\t    defined $row{'port'} &&\n+\t\t    defined $row{'password'} &&\n+\t\t    $row{'machine'} eq $smtp_server &&\n+\t\t    $row{'login'} eq $smtp_authuser &&\n+\t\t    $row{'port'} eq $smtp_server_port) {\n+\t\t\t$password = $row{'password'};\n+\t\t\tlast;\n+\t\t}\n+\t}\n+\n+\tclose $fd;\n+\treturn $password;\n+}\n+\n+sub read_password {\n+\treturn read_password_from_authinfo || read_password_from_stdin;\n+}\n+\n # Returns 1 if the message was sent, and 0 otherwise.\n # In actuality, the whole program dies when there\n # is an error sending a message.\n@@ -1194,18 +1250,7 @@ X-Mailer: git-send-email $gitversion\n \t\t\t};\n \n \t\t\tif (!defined $smtp_authpass) {\n-\n-\t\t\t\tsystem \"stty -echo\";\n-\n-\t\t\t\tdo {\n-\t\t\t\t\tprint \"Password: \";\n-\t\t\t\t\t$_ = <STDIN>;\n-\t\t\t\t\tprint \"\\n\";\n-\t\t\t\t} while (!defined $_);\n-\n-\t\t\t\tchomp($smtp_authpass = $_);\n-\n-\t\t\t\tsystem \"stty echo\";\n+\t\t\t\t$smtp_authpass = read_password\n \t\t\t}\n \n \t\t\t$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;\n-- \n1.8.1\n"},{"id":"208214","messageId":"7vvcafojf4.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"2f93ce7b6b5d3f6c6d1b99958330601a5560d4ba.1359486391.git.mina86@mina86.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-29T19:53:19Z","receivedAt":"2013-01-29T19:53:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Nazarewicz <mpn@google.com> writes:\n\n> From: Michal Nazarewicz <mina86@mina86.com>\n>\n> Make git-send-email read password from a ~/.authinfo file instead of\n> requiring it to be stored in git configuration, passed as command line\n> argument or typed in.\n\nMakes one wonder why .authinfo and not .netrc; \n\nhttp://www.gnu.org/software/emacs/manual/html_node/auth/Help-for-users.html\n\nphrases it amusingly:\n\n        “Netrc” files are usually called .authinfo or .netr\n        nowadays .authinfo seems to be more popular and the\n        auth-source library encourages this confusion by accepting\n        both\n\nEither way it still encourages a plaintext password to be on disk,\nwhich may not be what we want, even though it may be slight if not\nreally much of an improvement.  Again the Help-for-users has this\namusing bit:\n\n\tYou could just say (but we don't recommend it, we're just\n\tshowing that it's possible)\n\n\t     password mypassword\n\n\tto use the same password everywhere. Again, DO NOT DO THIS\n\tor you will be pwned as the kids say.\n\n> +The '~/.authinfo' file is read if Text::CSV Perl module is installed\n> +on the system; if it's missing, a notification message will be printed\n> +and the file ignored altogether.  The file should contain a line with\n> +the following format:\n> ++\n> +  machine <domain> port <port> login <user> password <pass>\n\nIt is rather strange to require a comma-separated-values parser to\nread a file format this simple, isn't it?\n\n> ++\n> +Contrary to other tools, 'git-send-email' does not support symbolic\n> +port names like 'imap' thus `<port>` must be a number.\n\nPerhaps you can convert at least some popular ones yourself?  After\nall, the user may be using an _existing_ .authinfo/.netrc that she\nhas been using with other programs that do understand symbolic port\nnames.  Rather than forcing all such users to update their files,\nthe patch can work a bit harder for them and the world will be a\nbetter place, no?\n"},{"id":"208221","messageId":"d58b0709cd86e0d336902b52d72e06dd9b52d70d.1359493459.git.mina86@mina86.com","threadId":"32770","inReplyTo":"7vvcafojf4.fsf@alter.siamese.dyndns.org","subject":"[PATCHv2] git-send-email: add ~/.authinfo parsing","fromName":"Michal Nazarewicz","fromEmail":"mpn@google.com","sentAt":"2013-01-29T21:08:49Z","receivedAt":"2013-01-29T21:08:49Z","isPatch":false,"sender":{"key":"mpn@google.com","avatar":null},"body":"From: Michal Nazarewicz <mina86@mina86.com>\n\nMake git-send-email read password from a ~/.authinfo or a ~/.netrc\nfile instead of requiring it to be stored in git configuration, passed\nas command line argument or typed in.\n\nThere are various other applications that use this file for\nauthentication information so letting users use it for git-send-email\nis convinient.  Furthermore, some users store their ~/.gitconfig file\nin a public repository and having to store password there makes it\neasy to publish the password.\n\nSigned-off-by: Michal Nazarewicz <mina86@mina86.com>\n---\n Documentation/git-send-email.txt |  34 +++++++++--\n git-send-email.perl              | 124 +++++++++++++++++++++++++++++++++++----\n 2 files changed, 140 insertions(+), 18 deletions(-)\n\nOn Tue, Jan 29 2013, Junio C Hamano wrote:\n> Makes one wonder why .authinfo and not .netrc; \n\nFine… Let's parse both. ;)\n\n> Either way it still encourages a plaintext password to be on disk,\n> which may not be what we want, even though it may be slight if not\n> really much of an improvement.\n\nWell… Users store passwords on disks in a lot of places.  I wager that\nmost have mail clients configured not to ask for password but instead\nstore it on hard drive.  I don't see that changing any time soon, so\nat least we can try and minimise number of places where a password is\nstored.\n\n> It is rather strange to require a comma-separated-values parser to\n> read a file format this simple, isn't it?\n\nI was worried about spaces in password.  CVS should handle such case\nnicely, whereas simple split won't.  Nonetheless, I guess that in the\nend this is not likely enough to add the dependency.\n\n> Perhaps you can convert at least some popular ones yourself?  After\n> all, the user may be using an _existing_ .authinfo/.netrc that she\n> has been using with other programs that do understand symbolic port\n> names.  Rather than forcing all such users to update their files,\n> the patch can work a bit harder for them and the world will be a\n> better place, no?\n\nParsing /etc/services added.\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex eeb561c..ee20714 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -158,14 +158,36 @@ Sending\n --smtp-pass[=<password>]::\n \tPassword for SMTP-AUTH. The argument is optional: If no\n \targument is specified, then the empty string is used as\n-\tthe password. Default is the value of 'sendemail.smtppass',\n-\thowever '--smtp-pass' always overrides this value.\n+\tthe password. Default is the value of 'sendemail.smtppass'\n+\tor value read from ~/.authinfo file, however '--smtp-pass'\n+\talways overrides this value.\n +\n-Furthermore, passwords need not be specified in configuration files\n-or on the command line. If a username has been specified (with\n+Furthermore, passwords need not be specified in configuration files or\n+on the command line. If a username has been specified (with\n '--smtp-user' or a 'sendemail.smtpuser'), but no password has been\n-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the\n-user is prompted for a password while the input is masked for privacy.\n+specified (with '--smtp-pass', 'sendemail.smtppass' or via\n+~/.authinfo file), then the user is prompted for a password while\n+the input is masked for privacy.\n++\n+The ~/.authinfo file should contain a line with the following\n+format:\n++\n+  machine <domain> port <port> login <user> password <pass>\n++\n+Each pair (expect for `password <pass>`) can be omitted which will\n+skip matching of the given value.  Lines are interpreted in order and\n+password from the first line that matches will be used.  `<port>` can\n+be either an integer or a symbolic name.  In the latter case, it is\n+looked up in `/etc/services` file (if it exists).  For instance, you\n+can put\n++\n+  machine example.com login testuser port ssmtp password smtppassword\n+  machine example.com login testuser            password testpassword\n++\n+if you want to use `smtppassword` for authenticating to a service at\n+port 465 (SSMTP) and `testpassword` for all other services.  As shown\n+in the example, `<port>` can use   If ~/.authinfo file is\n+missing, 'git-send-email' will also try ~/.netrc file.\n \n --smtp-server=<host>::\n \tIf set, specifies the outgoing SMTP server to use (e.g.\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex be809e5..2d8fd1b 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1045,6 +1045,117 @@ sub maildomain {\n \treturn maildomain_net() || maildomain_mta() || 'localhost.localdomain';\n }\n \n+\n+sub read_password_from_stdin {\n+\tmy $line;\n+\n+\tsystem \"stty -echo\";\n+\n+\tdo {\n+\t\tprint \"Password: \";\n+\t\t$line = <STDIN>;\n+\t\tprint \"\\n\";\n+\t} while (!defined $line);\n+\n+\tsystem \"stty echo\";\n+\n+\tchomp $line;\n+\treturn $line;\n+}\n+\n+sub read_etc_services {\n+\tmy $fd;\n+\tif (!open $fd, '<', '/etc/services') {\n+\t\treturn {};\n+\t}\n+\n+\tmy $ret = {};\n+\twhile (my $line = <$fd>) {\n+\t\t$line =~ s/^\\s+|\\s*(?:#.*)?$//g;\n+\t\tmy @line = split /\\s+/, $line;\n+\t\tif (@line < 2 || $line[1] !~ m~^(\\d+)/tcp$~) {\n+\t\t\tnext;\n+\t\t}\n+\n+\t\tmy $num = int $1;\n+\t\tundef $line[1];\n+\t\tfor my $service (@line) {\n+\t\t\tif (defined $service && !defined $ret->{$service}) {\n+\t\t\t\t$ret->{$service} = $num;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tclose $fd;\n+\treturn $ret;\n+}\n+\n+my $authinfo_parse_port;\n+\n+sub authinfo_is_eq_port {\n+\tmy ($from_file, $value, $filename) = @_;\n+\n+\tif (!defined $from_file) {\n+\t\treturn 1;\n+\t} elsif ($from_file =~ /^\\d+$/) {\n+\t\treturn $from_file == $value;\n+\t}\n+\n+\tif (!defined $authinfo_parse_port) {\n+\t\t$authinfo_parse_port = read_etc_services;\n+\t}\n+\n+\tmy $port = $authinfo_parse_port->{$from_file};\n+\tif (!defined $port) {\n+\t\tprint STDERR \"$filename: invalid port name: $from_file\\n\";\n+\t\treturn;\n+\t}\n+\n+\treturn $port == $value;\n+}\n+\n+sub authinfo_is_eq {\n+\tmy ($from_file, $value) = @_;\n+\treturn defined $from_file || $from_file eq $value;\n+}\n+\n+sub read_password_from_authinfo {\n+\tmy $filename = join '/', $ENV{'HOME'}, $_[0] // '.authinfo';\n+\tmy $fd;\n+\tif (!open $fd, '<', $filename) {\n+\t\treturn;\n+\t}\n+\n+\tmy $password;\n+\twhile (my $line = <$fd>) {\n+\t\t$line =~ s/^\\s+|\\s+$//g;\n+\t\tmy @line = split /\\s+/, $line;\n+\t\tif (@line % 2) {\n+\t\t\tnext;\n+\t\t}\n+\n+\t\tmy %line = @line;\n+\t\tif (defined $line{'password'} &&\n+\t\t    authinfo_is_eq $line{'machine'}, $smtp_server &&\n+\t\t    authinfo_is_eq $line{'login'}, $smtp_authuser &&\n+\t\t    authinfo_is_eq_port $line{'port'}, $smtp_server_port, $filename) {\n+\t\t\t$password = $line{'password'};\n+\t\t\tlast;\n+\t\t}\n+\t}\n+\n+\tclose $fd;\n+\treturn $password;\n+}\n+\n+sub read_password {\n+\treturn\n+\t  read_password_from_authinfo '.authinfo' ||\n+\t  read_password_from_authinfo '.netrc' ||\n+\t  read_password_from_stdin;\n+}\n+\n+\n # Returns 1 if the message was sent, and 0 otherwise.\n # In actuality, the whole program dies when there\n # is an error sending a message.\n@@ -1194,18 +1305,7 @@ X-Mailer: git-send-email $gitversion\n \t\t\t};\n \n \t\t\tif (!defined $smtp_authpass) {\n-\n-\t\t\t\tsystem \"stty -echo\";\n-\n-\t\t\t\tdo {\n-\t\t\t\t\tprint \"Password: \";\n-\t\t\t\t\t$_ = <STDIN>;\n-\t\t\t\t\tprint \"\\n\";\n-\t\t\t\t} while (!defined $_);\n-\n-\t\t\t\tchomp($smtp_authpass = $_);\n-\n-\t\t\t\tsystem \"stty echo\";\n+\t\t\t\t$smtp_authpass = read_password\n \t\t\t}\n \n \t\t\t$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;\n-- \n1.8.1\n"},{"id":"208227","messageId":"7vehh3obs0.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"d58b0709cd86e0d336902b52d72e06dd9b52d70d.1359493459.git.mina86@mina86.com","subject":"Re: [PATCHv2] git-send-email: add ~/.authinfo parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-29T22:38:23Z","receivedAt":"2013-01-29T22:38:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Nazarewicz <mpn@google.com> writes:\n\n>> It is rather strange to require a comma-separated-values parser to\n>> read a file format this simple, isn't it?\n>\n> I was worried about spaces in password.  CVS should handle such case\n> nicely, whereas simple split won't.  Nonetheless, I guess that in the\n> end this is not likely enough to add the dependency.\n\nBut .netrc/.authinfo format separates its entries with SP, HT, or\nLF.  An entry begins with \"machine <remote-hostname>\" token pair.\n\nsplit(/\\s+/) will not work for an entry that span multiple lines but\nCSV will not help, either.\n\nIs it bad to use Net::Netrc instead?  This looks like exactly the\nuse case that module was written for, no?\n\n>> Perhaps you can convert at least some popular ones yourself?  After\n>> all, the user may be using an _existing_ .authinfo/.netrc that she\n>> has been using with other programs that do understand symbolic port\n>> names.  Rather than forcing all such users to update their files,\n>> the patch can work a bit harder for them and the world will be a\n>> better place, no?\n>\n> Parsing /etc/services added.\n\nHmph.  I would have expected to see getservbyname.\n"},{"id":"208231","messageId":"5d18d777d6ddf6f01bbf460f37af637d3dc28ed5.1359503987.git.mina86@mina86.com","threadId":"32770","inReplyTo":"7vehh3obs0.fsf@alter.siamese.dyndns.org","subject":"[PATCHv3] git-send-email: add ~/.authinfo parsing","fromName":"Michal Nazarewicz","fromEmail":"mpn@google.com","sentAt":"2013-01-30T00:03:51Z","receivedAt":"2013-01-30T00:03:51Z","isPatch":false,"sender":{"key":"mpn@google.com","avatar":null},"body":"From: Michal Nazarewicz <mina86@mina86.com>\n\nMake git-send-email read password from a ~/.authinfo or ~/.netrc file\ninstead of requiring it to be stored in git configuration, passed as\ncommand line argument or typed in.\n\nThere are various other applications that use this file for\nauthentication information so letting users use it for git-send-email\nis convinient.  Furthermore, some users store their ~/.gitconfig file\nin a public repository and having to store password there makes it\neasy to publish the password.\n\nSigned-off-by: Michal Nazarewicz <mina86@mina86.com>\n---\n Documentation/git-send-email.txt | 47 +++++++++++++++++---\n git-send-email.perl              | 93 ++++++++++++++++++++++++++++++++++------\n 2 files changed, 122 insertions(+), 18 deletions(-)\n\nOn Tue, Jan 29 2013, Junio C Hamano wrote:\n> But .netrc/.authinfo format separates its entries with SP, HT, or\n> LF.  An entry begins with \"machine <remote-hostname>\" token pair.\n>\n> split(/\\s+/) will not work for an entry that span multiple lines but\n> CSV will not help, either.\n>\n> Is it bad to use Net::Netrc instead?  This looks like exactly the\n> use case that module was written for, no?\n\nI don't think that's the case.  For one, Net::Netrc does not seem to\nprocess port number.\n\nThere is a Text::Authinfo module but it just uses Text::CSV.\n\nI can change the code to use Net::Netrc, but I dunno if that's really\nthe best option, since I feel people would expect parsing to be\nsomehow compatible with\n<http://www.gnu.org/software/emacs/manual/html_node/gnus/NNTP.html>\nrather than the original .netrc file format.\n\n> Hmph.  I would have expected to see getservbyname.\n\nHa!  Even better. :]\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex eeb561c..ac020d1 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -158,14 +158,49 @@ Sending\n --smtp-pass[=<password>]::\n \tPassword for SMTP-AUTH. The argument is optional: If no\n \targument is specified, then the empty string is used as\n-\tthe password. Default is the value of 'sendemail.smtppass',\n-\thowever '--smtp-pass' always overrides this value.\n+\tthe password. Default is the value of 'sendemail.smtppass'\n+\tor value read from ~/.authinfo file, however '--smtp-pass'\n+\talways overrides this value.\n +\n-Furthermore, passwords need not be specified in configuration files\n-or on the command line. If a username has been specified (with\n+Furthermore, passwords need not be specified in configuration files or\n+on the command line. If a username has been specified (with\n '--smtp-user' or a 'sendemail.smtpuser'), but no password has been\n-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the\n-user is prompted for a password while the input is masked for privacy.\n+specified (with '--smtp-pass', 'sendemail.smtppass' or via\n+~/.authinfo file), then the user is prompted for a password while\n+the input is masked for privacy.\n++\n+The ~/.authinfo file should contain a line with the following\n+format:\n++\n+  machine <domain> port <port> login <user> password <pass>\n++\n+Instead of `machine <domain>` pair a `default` token can be used\n+instead in which case all domains will match.  Similarly, `port\n+<port>` and `login <user>` pairs can be omitted in which case matching\n+of the given value will be skipped.  `<port>` can be either an integer\n+or a symbolic name.  Lines are interpreted in order and password from\n+the first line that matches will be used.  For instance, one may end\n+up with:\n++\n+  machine example.com login jane port ssmtp password smtppassword\n+  machine example.com login jane            password janepassword\n+  default             login janedoe         password doepassword\n++\n+if she wants to use `smtppassword` for authenticating as `jane` to\n+a service at example.com:465 (SSMTP), `janepassword` for all other\n+services at example.com; and `doepassword` when authonticating as\n+`janedoe` to any service.  If ~/.authinfo file is missing,\n+'git-send-email' will also try ~/.netrc file (even though parsing is\n+not fully compatible with ftp's .netrc file format).\n++\n+Note that you should never make ~/.authinfo file world-readable.  To\n+help guarantee that, you might want to create the file with the\n+following command:\n++\n+  ( umask 077; cat >~/.authinfo <<EOF\n+  ... file contents ...\n+  EOF\n+  )\n \n --smtp-server=<host>::\n \tIf set, specifies the outgoing SMTP server to use (e.g.\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex be809e5..a62dfa4 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1045,6 +1045,86 @@ sub maildomain {\n \treturn maildomain_net() || maildomain_mta() || 'localhost.localdomain';\n }\n \n+\n+sub read_password_from_stdin {\n+\tmy $line;\n+\n+\tsystem \"stty -echo\";\n+\n+\tdo {\n+\t\tprint \"Password: \";\n+\t\t$line = <STDIN>;\n+\t\tprint \"\\n\";\n+\t} while (!defined $line);\n+\n+\tsystem \"stty echo\";\n+\n+\tchomp $line;\n+\treturn $line;\n+}\n+\n+sub authinfo_is_port_eq {\n+\tmy ($from_file, $value, $filename) = @_;\n+\n+\tif (!defined $from_file) {\n+\t\treturn 1;\n+\t} elsif ($from_file =~ /^\\d+$/) {\n+\t\treturn $from_file == $value;\n+\t}\n+\n+\tmy $port = getservbyname $from_file, 'tcp';\n+\tif (!defined $port) {\n+\t\tprint STDERR \"$filename: invalid port name: $from_file\\n\";\n+\t\treturn;\n+\t}\n+\n+\treturn $port == $value;\n+}\n+\n+sub read_password_from_authinfo {\n+\tmy $filename = join '/', $ENV{'HOME'}, $_[0] // '.authinfo';\n+\tmy $fd;\n+\tif (!open $fd, '<', $filename) {\n+\t\treturn;\n+\t}\n+\n+\tmy $password;\n+\twhile (my $line = <$fd>) {\n+\t\t$line =~ s/^\\s+|\\s+$//g;\n+\t\tmy @line = split /\\s+/, $line;\n+\t\tmy %line;\n+\t\twhile (@line) {\n+\t\t\tmy $token = shift @line;\n+\t\t\tif ($token eq 'default') {\n+\t\t\t\t$line{'machine'} = $smtp_server;\n+\t\t\t} elsif (@line) {\n+\t\t\t\t$line{$token} = shift @line;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (defined $line{'password'} &&\n+\t\t    defined $line{'machine'} &&\n+\t\t    $line{'machine'} eq $smtp_server &&\n+\t\t    (!defined $line{'login'} ||\n+\t\t     $line{'login'} eq $smtp_authuser) &&\n+\t\t    authinfo_is_port_eq($line{'port'}, $smtp_server_port, $filename)) {\n+\t\t\t$password = $line{'password'};\n+\t\t\tlast;\n+\t\t}\n+\t}\n+\n+\tclose $fd;\n+\treturn $password;\n+}\n+\n+sub read_password {\n+\treturn\n+\t  read_password_from_authinfo '.authinfo' ||\n+\t  read_password_from_authinfo '.netrc' ||\n+\t  read_password_from_stdin;\n+}\n+\n+\n # Returns 1 if the message was sent, and 0 otherwise.\n # In actuality, the whole program dies when there\n # is an error sending a message.\n@@ -1194,18 +1274,7 @@ X-Mailer: git-send-email $gitversion\n \t\t\t};\n \n \t\t\tif (!defined $smtp_authpass) {\n-\n-\t\t\t\tsystem \"stty -echo\";\n-\n-\t\t\t\tdo {\n-\t\t\t\t\tprint \"Password: \";\n-\t\t\t\t\t$_ = <STDIN>;\n-\t\t\t\t\tprint \"\\n\";\n-\t\t\t\t} while (!defined $_);\n-\n-\t\t\t\tchomp($smtp_authpass = $_);\n-\n-\t\t\t\tsystem \"stty echo\";\n+\t\t\t\t$smtp_authpass = read_password\n \t\t\t}\n \n \t\t\t$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;\n-- \n1.8.1\n"},{"id":"208233","messageId":"7v1ud3o6ep.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"5d18d777d6ddf6f01bbf460f37af637d3dc28ed5.1359503987.git.mina86@mina86.com","subject":"Re: [PATCHv3] git-send-email: add ~/.authinfo parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T00:34:22Z","receivedAt":"2013-01-30T00:34:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Nazarewicz <mpn@google.com> writes:\n\n>> Is it bad to use Net::Netrc instead?  This looks like exactly the\n>> use case that module was written for, no?\n>\n> I don't think that's the case.  For one, Net::Netrc does not seem to\n> process port number.\n>\n> There is a Text::Authinfo module but it just uses Text::CSV.\n>\n> I can change the code to use Net::Netrc, but I dunno if that's really\n> the best option, since I feel people would expect parsing to be\n> somehow compatible with\n> <http://www.gnu.org/software/emacs/manual/html_node/gnus/NNTP.html>\n> rather than the original .netrc file format.\n\nThanks for pushing back (I wish more contributors did so when I\nsuggest nonsense ;-)); you are right that both canned modules are\nlacking.\n\nWill queue.  Thanks.\n"},{"id":"208251","messageId":"20130130074306.GA17868@sigill.intra.peff.net","threadId":"32770","inReplyTo":"7vvcafojf4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-30T07:43:06Z","receivedAt":"2013-01-30T07:43:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 29, 2013 at 11:53:19AM -0800, Junio C Hamano wrote:\n\n> Either way it still encourages a plaintext password to be on disk,\n> which may not be what we want, even though it may be slight if not\n> really much of an improvement.  Again the Help-for-users has this\n> amusing bit:\n\nI do not mind a .netrc or .authinfo parser, because while those formats\ndo have security problems, they are standard files that may already be\nin use. So as long as we are not encouraging their use, I do not see a\nproblem in supporting them (and we already do the same with curl's netrc\nsupport).\n\nBut it would probably make sense for send-email to support the existing\ngit-credential subsystem, so that it can take advantage of secure\nsystem-specific storage. And that is where we should be pointing new\nusers. I think contrib/mw-to-git even has credential support written in\nperl, so it would just need to be factored out to Git.pm.\n\n-Peff\n"},{"id":"208268","messageId":"87vcae90hr.fsf@lifelogs.com","threadId":"32770","inReplyTo":"7vvcafojf4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-01-30T15:03:28Z","receivedAt":"2013-01-30T15:03:28Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Tue, 29 Jan 2013 11:53:19 -0800 Junio C Hamano <gitster@pobox.com> wrote: \n\nJCH> Makes one wonder why .authinfo and not .netrc; \n\nJCH> http://www.gnu.org/software/emacs/manual/html_node/auth/Help-for-users.html\n\nJCH> phrases it amusingly:\n\nJCH>         “Netrc” files are usually called .authinfo or .netr\nJCH>         nowadays .authinfo seems to be more popular and the\nJCH>         auth-source library encourages this confusion by accepting\nJCH>         both\n\nI wrote this and the auth-source.el library in Emacs (I'm glad it was\namusing :).  The confusion is further perpetuated by our (in Emacs)\nencouragement to use a .authinfo.gpg file, which is then decrypted on\nthe fly by Emacs through GPG.  The format is the same; by the time\nauth-source.el sees the contents, they are plain text since the decoding\nhappens at the file handler level.\n\nI think it makes sense to write the code to support both\n`git-send-email' and credentials.  I have had it in my TODO list for\nalmost 2 years now to work on credential support, and to support the\n~/.authinfo.gpg decoding specifically.  Ideally this would also support\nthe other formats... Michal, would you be interested in that feature?  I\npromise to get off my rear and help out.\n\n>> +The '~/.authinfo' file is read if Text::CSV Perl module is installed\n>> +on the system; if it's missing, a notification message will be printed\n>> +and the file ignored altogether.  The file should contain a line with\n>> +the following format:\n>> ++\n>> +  machine <domain> port <port> login <user> password <pass>\n\nJCH> It is rather strange to require a comma-separated-values parser to\nJCH> read a file format this simple, isn't it?\n\nI'd recommend a hand-crafted parser.  Among other things, you should\naccept both \"strings\" and 'strings' if possible (I've seen both formats\nin the wild), and the format is simple enough to avoid the module\ndependency.\n\n>> ++\n>> +Contrary to other tools, 'git-send-email' does not support symbolic\n>> +port names like 'imap' thus `<port>` must be a number.\n\nJCH> Perhaps you can convert at least some popular ones yourself?  After\nJCH> all, the user may be using an _existing_ .authinfo/.netrc that she\nJCH> has been using with other programs that do understand symbolic port\nJCH> names.  Rather than forcing all such users to update their files,\nJCH> the patch can work a bit harder for them and the world will be a\nJCH> better place, no?\n\nI agree, \"port imap\" is a nice self-documenting token.  Maybe it can be\ninterpreted by the program that requests the token with a services\nlookup, where supported.\n\nTed\n"},{"id":"208271","messageId":"7v7gmumzo6.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"20130130074306.GA17868@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-30T15:57:29Z","receivedAt":"2013-01-30T15:57:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> But it would probably make sense for send-email to support the existing\n> git-credential subsystem, so that it can take advantage of secure\n> system-specific storage. And that is where we should be pointing new\n> users. I think contrib/mw-to-git even has credential support written in\n> perl, so it would just need to be factored out to Git.pm.\n\nYeah, that sounds like a neat idea.\n"},{"id":"208356","messageId":"87pq0l5qbc.fsf@lifelogs.com","threadId":"32770","inReplyTo":"7v7gmumzo6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-01-31T15:23:51Z","receivedAt":"2013-01-31T15:23:51Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Wed, 30 Jan 2013 07:57:29 -0800 Junio C Hamano <gitster@pobox.com> wrote: \n\nJCH> Jeff King <peff@peff.net> writes:\n>> But it would probably make sense for send-email to support the existing\n>> git-credential subsystem, so that it can take advantage of secure\n>> system-specific storage. And that is where we should be pointing new\n>> users. I think contrib/mw-to-git even has credential support written in\n>> perl, so it would just need to be factored out to Git.pm.\n\nJCH> Yeah, that sounds like a neat idea.\n\nJeff, is there a way for git-credential to currently support\nauthinfo/netrc parsing?  I assume that's the right way, instead of using\nMichal's proposal to parse internally?\n\nI'd like to add that, plus support for the 'string' and \"string\"\nformats, and authinfo.gpg decoding through GPG.  I'd write it in Perl,\nif there's a choice.\n\nTed\n"},{"id":"208394","messageId":"20130131193844.GA14460@sigill.intra.peff.net","threadId":"32770","inReplyTo":"87pq0l5qbc.fsf@lifelogs.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-31T19:38:45Z","receivedAt":"2013-01-31T19:38:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 31, 2013 at 10:23:51AM -0500, Ted Zlatanov wrote:\n\n> Jeff, is there a way for git-credential to currently support\n> authinfo/netrc parsing?  I assume that's the right way, instead of using\n> Michal's proposal to parse internally?\n> \n> I'd like to add that, plus support for the 'string' and \"string\"\n> formats, and authinfo.gpg decoding through GPG.  I'd write it in Perl,\n> if there's a choice.\n\nYes, you could write a credential helper that understands netrc and\nfriends; git talks to the helpers over a socket, so there is no problem\nwith writing it in Perl. See Documentation/technical/api-credentials.txt\nfor an overview, or the sample implementation in credential-store.c for a\nsimple example.\n\n-Peff\n"},{"id":"208523","messageId":"87k3qrx712.fsf@lifelogs.com","threadId":"32770","inReplyTo":"20130131193844.GA14460@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-02T11:57:29Z","receivedAt":"2013-02-02T11:57:29Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Thu, 31 Jan 2013 14:38:45 -0500 Jeff King <peff@peff.net> wrote: \n\nJK> On Thu, Jan 31, 2013 at 10:23:51AM -0500, Ted Zlatanov wrote:\n>> Jeff, is there a way for git-credential to currently support\n>> authinfo/netrc parsing?  I assume that's the right way, instead of using\n>> Michal's proposal to parse internally?\n>> \n>> I'd like to add that, plus support for the 'string' and \"string\"\n>> formats, and authinfo.gpg decoding through GPG.  I'd write it in Perl,\n>> if there's a choice.\n\nJK> Yes, you could write a credential helper that understands netrc and\nJK> friends; git talks to the helpers over a socket, so there is no problem\nJK> with writing it in Perl. See Documentation/technical/api-credentials.txt\nJK> for an overview, or the sample implementation in credential-store.c for a\nJK> simple example.\n\nI wrote a Perl credential helper for netrc parsing which is pretty\nrobust, has built-in docs with -h, and doesn't depend on external\nmodules.  The netrc parser regex was stolen from Net::Netrc.\n\nIt will by default use ~/.authinfo.gpg, ~/.netrc.gpg, ~/.authinfo, and\n~/.netrc (whichever is found first) and this can be overridden with -f.\n\nIf the file name ends with \".gpg\", it will run \"gpg --decrypt FILE\" and\nuse the output.  So non-interactively, that could hang if GPG was\nwaiting for input.  Does Git handle that, or should I check for a TTY?\n\nTake a look at the proposed patch and let me know if it's usable, if you\nneed a formal copyright assignment, etc.\n\nThanks\nTed\n\n\n\ncommit 3d28bc2a610ebcc988eba5443d82d0ded92c24bc\nAuthor: Ted Zlatanov <tzz@lifelogs.com>\nDate:   Sat Feb 2 06:42:13 2013 -0500\n\n    Add contrib/credentials/netrc with GPG support\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc\nnew file mode 100755\nindex 0000000..92fc306\n--- /dev/null\n+++ b/contrib/credential/netrc/git-credential-netrc\n@@ -0,0 +1,242 @@\n+#!/usr/bin/perl\n+\n+use strict;\n+use warnings;\n+\n+use Data::Dumper;\n+\n+use Getopt::Long;\n+use File::Basename;\n+\n+my $VERSION = \"0.1\";\n+\n+my %options = (\n+               help => 0,\n+               debug => 0,\n+\n+               # identical token maps, e.g. host -> host, will be inserted later\n+               tmap => {\n+                        port => 'protocol',\n+                        machine => 'host',\n+                        path => 'path',\n+                        login => 'username',\n+                        user => 'username',\n+                        password => 'password',\n+                       }\n+              );\n+\n+foreach my $v (values %{$options{tmap}})\n+{\n+ $options{tmap}->{$v} = $v;\n+}\n+\n+foreach my $suffix ('.gpg', '')\n+{\n+ foreach my $base (qw/authinfo netrc/)\n+ {\n+  my $file = glob(\"~/.$base$suffix\");\n+  next unless (defined $file && -f $file);\n+  $options{file} = $file ;\n+ }\n+}\n+\n+Getopt::Long::Configure(\"bundling\");\n+\n+# TODO: maybe allow the token map $options{tmap} to be configurable.\n+GetOptions(\\%options,\n+           \"help|h\",\n+           \"debug|d\",\n+           \"file|f=s\",\n+          );\n+\n+if ($options{help})\n+{\n+ my $shortname = basename($0);\n+ $shortname =~ s/git-credential-//;\n+\n+ print <<EOHIPPUS;\n+\n+$0 [-f AUTHFILE] [-d] get\n+\n+Version $VERSION by tzz\\@lifelogs.com.  License: any use is OK.\n+\n+Options:\n+  -f AUTHFILE: specify a netrc-style file\n+  -d: turn on debugging\n+\n+To enable (note that Git will prepend \"git-credential-\" to the helper\n+name and look for it in the path):\n+\n+  git config credential.helper '$shortname -f AUTHFILE'\n+\n+And if you want lots of debugging info:\n+\n+  git config credential.helper '$shortname -f AUTHFILE -d'\n+\n+Only \"get\" mode is supported by this credential helper.  It opens\n+AUTHFILE and looks for entries that match the requested search\n+criteria:\n+\n+ 'port|protocol':\n+   The protocol that will be used (e.g., https). (protocol=X)\n+\n+ 'machine|host':\n+   The remote hostname for a network credential. (host=X)\n+\n+ 'path':\n+   The path with which the credential will be used. (path=X)\n+\n+ 'login|user|username':\n+   The credential’s username, if we already have one. (username=X)\n+\n+Thus, when we get \"protocol=https\\nusername=tzz\", this credential\n+helper will look for lines in AUTHFILE that match\n+\n+port https login tzz\n+\n+OR\n+\n+protocol https login tzz\n+\n+OR... etc. acceptable tokens as listed above.  Any unknown tokens are\n+simply ignored.\n+\n+Then, the helper will print out whatever tokens it got from the line,\n+including \"password\" tokens, mapping e.g. \"port\" back to \"protocol\".\n+\n+The first matching line is used.  Tokens can be quoted as 'STRING' or\n+\"STRING\".\n+\n+No caching is performed by this credential helper.\n+\n+EOHIPPUS\n+\n+ exit;\n+}\n+\n+my $mode = shift @ARGV;\n+\n+# credentials may get 'get', 'store', or 'erase' as parameters but\n+# only acknowledge 'get'\n+die \"Syntax: $0 [-f AUTHFILE] [-d] get\" unless defined $mode;\n+\n+# only support 'get' mode\n+exit unless $mode eq 'get';\n+\n+my $debug = $options{debug};\n+my $file = $options{file};\n+\n+die \"Sorry, you need to specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE\"\n+ unless defined $file;\n+\n+die \"Sorry, the specified netrc $file is not accessible\"\n+ unless -f $file;\n+\n+if ($file =~ m/\\.gpg$/)\n+{\n+ $file = \"gpg --decrypt $file|\";\n+}\n+\n+my @data = load($file);\n+chomp @data;\n+\n+die \"Sorry, we could not load data from [$file]\"\n+ unless (scalar @data);\n+\n+# the query\n+my %q;\n+\n+foreach my $v (values %{$options{tmap}})\n+{\n+ undef $q{$v};\n+}\n+\n+while (<STDIN>)\n+{\n+ next unless m/([a-z]+)=(.+)/;\n+\n+ my ($token, $value) = ($1, $2);\n+ die \"Unknown search token $1\" unless exists $q{$token};\n+ $q{$token} = $value;\n+}\n+\n+# build reverse token map\n+my %rmap;\n+foreach my $k (keys %{$options{tmap}})\n+{\n+ push @{$rmap{$options{tmap}->{$k}}}, $k;\n+}\n+\n+# there are CPAN modules to do this better, but we want to avoid\n+# dependencies and generally, complex netrc-style files are rare\n+\n+if ($debug)\n+{\n+ foreach (sort keys %q)\n+ {\n+  printf STDERR \"searching for %s = %s\\n\",\n+   $_, $q{$_} || '(any value)';\n+ }\n+}\n+\n+LINE: foreach my $line (@data)\n+{\n+\n+ print STDERR \"line [$line]\\n\" if $debug;\n+ my @tok;\n+ # gratefully stolen from Net::Netrc\n+ while (length $line &&\n+        $line =~ s/^(\"((?:[^\"]+|\\\\.)*)\"|((?:[^\\\\\\s]+|\\\\.)*))\\s*//)\n+ {\n+  (my $tok = $+) =~ s/\\\\(.)/$1/g;\n+  push(@tok, $tok);\n+ }\n+\n+ my %tokens;\n+ while (@tok)\n+ {\n+  my ($k, $v) = (shift @tok, shift @tok);\n+  next unless defined $v;\n+  next unless exists $options{tmap}->{$k};\n+  $tokens{$options{tmap}->{$k}} = $v;\n+ }\n+\n+ foreach my $check (sort keys %q)\n+ {\n+  if (exists $tokens{$check} && defined $q{$check})\n+  {\n+   print STDERR \"comparing [$tokens{$check}] to [$q{$check}] in line [$line]\\n\" if $debug;\n+   next LINE unless $tokens{$check} eq $q{$check};\n+  }\n+  else\n+  {\n+   print STDERR \"we could not find [$check] but it's OK\\n\" if $debug;\n+  }\n+ }\n+\n+ print STDERR \"line has passed all the search checks\\n\" if $debug;\n+ foreach my $token (sort keys %rmap)\n+ {\n+  print STDERR \"looking for useful token $token\\n\" if $debug;\n+  next unless exists $tokens{$token}; # did we match?\n+\n+  foreach my $rctoken (@{$rmap{$token}})\n+  {\n+   next if defined $q{$rctoken};           # don't re-print given tokens\n+  }\n+\n+  print STDERR \"FOUND: $token=$tokens{$token}\\n\" if $debug;\n+  printf \"%s=%s\\n\", $token, $tokens{$token};\n+ }\n+\n+ last;\n+}\n+\n+sub load\n+{\n+ my $file = shift;\n+ # this supports pipes too\n+ my $io = new IO::File($file) or die \"Could not open $file: $!\\n\";\n+\n+ return <$io>;                          # whole file\n+}\n"},{"id":"208557","messageId":"20130203194148.GA26318@sigill.intra.peff.net","threadId":"32770","inReplyTo":"87k3qrx712.fsf@lifelogs.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-03T19:41:49Z","receivedAt":"2013-02-03T19:41:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 02, 2013 at 06:57:29AM -0500, Ted Zlatanov wrote:\n\n> I wrote a Perl credential helper for netrc parsing which is pretty\n> robust, has built-in docs with -h, and doesn't depend on external\n> modules.  The netrc parser regex was stolen from Net::Netrc.\n>\n> It will by default use ~/.authinfo.gpg, ~/.netrc.gpg, ~/.authinfo, and\n> ~/.netrc (whichever is found first) and this can be overridden with -f.\n\nCool, thanks for working on this.\n\n> If the file name ends with \".gpg\", it will run \"gpg --decrypt FILE\" and\n> use the output.  So non-interactively, that could hang if GPG was\n> waiting for input.  Does Git handle that, or should I check for a TTY?\n\nNo, git does not do anything special with respect to credential helpers\nand ttys (nor should it, since one use of helpers is to get credentials\nwhen there is no tty). I think it is GPG's problem to deal with, though.\nWe will invoke it, and it is up to it to decide whether it can acquire\nthe passphrase or not (either through the tty, or possibly from\ngpg-agent). So it would be wrong to do the tty check yourself.\n\nI haven't tested GPG, but I assume it properly tries to read from\n/dev/tty and not stdin. Your helper's stdio is connected to git and\nspeaking the credential-helper protocol, so GPG reading from stdin would\neither steal your input (if run before you read it), or just get EOF (if\nyou have read all of the pipe content already). If GPG isn't well\nbehaved, it may be worth redirecting its stdin from /dev/null as a\nsafety measure.\n\n> Take a look at the proposed patch and let me know if it's usable, if you\n> need a formal copyright assignment, etc.\n\nOverall looks sane to me, though my knowledge of .netrc is not\nespecially good. Usually we try to send patches inline in the email\n(i.e., as generated by git-format-patch), and include a \"Signed-off-by\"\nline indicating that content is released to the project; see\nDocumentation/SubmittingPatches.\n\n> +use Data::Dumper;\n\nI don't see it used here. Leftover from debugging?\n\n> + print <<EOHIPPUS;\n\nCute, I haven't seen that one before.\n\n> +$0 [-f AUTHFILE] [-d] get\n> +\n> +Version $VERSION by tzz\\@lifelogs.com.  License: any use is OK.\n\nI don't know if we have a particular policy for items in contrib/, but\nthis license may be too vague. In particular, it does not explicitly\nallow redistribution, which would make Junio shipping a release with it\na copyright violation.\n\nAny objection to just putting it under some well-known simple license\n(GPL, BSD, or whatever)?\n\n> +if ($file =~ m/\\.gpg$/)\n> +{\n> + $file = \"gpg --decrypt $file|\";\n> +}\n\nDoes this need to quote $file, since the result will get passed to the\nshell? It might be easier to just use the list form of open(), like:\n\n  my @data = $file =~ /\\.gpg$/ ?\n             load('-|', qw(gpg --decrypt), $file) :\n             load('<', $file);\n\n(and then obviously update load to just dump all of @_ to open()).\n\n> +die \"Sorry, we could not load data from [$file]\"\n> + unless (scalar @data);\n\nProbably not that interesting a corner case, but this means we die on an\nempty .netrc, whereas it might be more sensible for it to behave as \"no\nmatch\".\n\nFor the same reason, it might be worth silently exiting when we don't\nfind a .netrc (or any of its variants). That lets people who share their\ndot-files across machines configure git globally, even if they don't\nnecessarily have a netrc on every machine.\n\n> +# the query\n> +my %q;\n> +\n> +foreach my $v (values %{$options{tmap}})\n> +{\n> + undef $q{$v};\n> +}\n\nJust my personal style, but I find the intent more obvious with \"map\" (I\nknow some people find it unreadable, though):\n\n  my %q = map { $_ => undef } values(%{$options{tmap}});\n\n> +while (<STDIN>)\n> +{\n> + next unless m/([a-z]+)=(.+)/;\n\nWe don't currently have any exotic tokens that this would not match, nor\ndo I plan to add them, but the credential documentation defines a valid\nline as /^([^=]+)=(.+)/.\n\nIt's also possible for the value to be empty, but I do not think\noff-hand that current git will ever send such an empty value.\n\n> [...]\n\nThe rest of it looks fine to me. I don't think any of my comments are\nshow-stoppers. Tests would be nice, but integrating contrib/ stuff with\nthe test harness is kind of a pain.\n\n-Peff\n"},{"id":"208614","messageId":"xa1tmwvk9gy1.fsf@mina86.com","threadId":"32770","inReplyTo":"20130130074306.GA17868@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Michal Nazarewicz","fromEmail":"mina86@mina86.com","sentAt":"2013-02-04T16:33:58Z","receivedAt":"2013-02-04T16:33:58Z","isPatch":true,"sender":{"key":"mina86@mina86.com","avatar":"https://avatars.githubusercontent.com/u/32383?v=4"},"body":"On Wed, Jan 30 2013, Jeff King wrote:\n> I do not mind a .netrc or .authinfo parser, because while those formats\n> do have security problems, they are standard files that may already be\n> in use. So as long as we are not encouraging their use, I do not see a\n> problem in supporting them (and we already do the same with curl's netrc\n> support).\n>\n> But it would probably make sense for send-email to support the existing\n> git-credential subsystem, so that it can take advantage of secure\n> system-specific storage. And that is where we should be pointing new\n> users. I think contrib/mw-to-git even has credential support written in\n> perl, so it would just need to be factored out to Git.pm.\n\nAs far as I understand, there could be a git-credential helper that\nreads ~/.authinfo and than git-send-email would just call “git\ncredential fill”, right?\n\nI've noticed though, that git-credential does not support port argument,\nwhich makes it slightly incompatible with ~/.authinfo.\n\n-- \nBest regards,                                         _     _\n.o. | Liege of Serenely Enlightened Majesty of      o' \\,=./ `o\n..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)\nooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--\n\n"},{"id":"208616","messageId":"87wquovxpl.fsf@lifelogs.com","threadId":"32770","inReplyTo":"20130203194148.GA26318@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T16:40:54Z","receivedAt":"2013-02-04T16:40:54Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Sun, 3 Feb 2013 14:41:49 -0500 Jeff King <peff@peff.net> wrote: \n\nJK> On Sat, Feb 02, 2013 at 06:57:29AM -0500, Ted Zlatanov wrote:\n>> If the file name ends with \".gpg\", it will run \"gpg --decrypt FILE\" and\n>> use the output.  So non-interactively, that could hang if GPG was\n>> waiting for input.  Does Git handle that, or should I check for a TTY?\n\nJK> No, git does not do anything special with respect to credential helpers\nJK> and ttys (nor should it, since one use of helpers is to get credentials\nJK> when there is no tty). I think it is GPG's problem to deal with, though.\nJK> We will invoke it, and it is up to it to decide whether it can acquire\nJK> the passphrase or not (either through the tty, or possibly from\nJK> gpg-agent). So it would be wrong to do the tty check yourself.\n\nJK> I haven't tested GPG, but I assume it properly tries to read from\nJK> /dev/tty and not stdin. Your helper's stdio is connected to git and\nJK> speaking the credential-helper protocol, so GPG reading from stdin would\nJK> either steal your input (if run before you read it), or just get EOF (if\nJK> you have read all of the pipe content already). If GPG isn't well\nJK> behaved, it may be worth redirecting its stdin from /dev/null as a\nJK> safety measure.\n\nIn my testing GPG did the right thing, so I think this is OK.\n\n>> Take a look at the proposed patch and let me know if it's usable, if you\n>> need a formal copyright assignment, etc.\n\nJK> Overall looks sane to me, though my knowledge of .netrc is not\nJK> especially good. Usually we try to send patches inline in the email\nJK> (i.e., as generated by git-format-patch), and include a \"Signed-off-by\"\nJK> line indicating that content is released to the project; see\nJK> Documentation/SubmittingPatches.\n\nOK, thanks.  I will fire that off.\n\n>> +use Data::Dumper;\n\nJK> I don't see it used here. Leftover from debugging?\n\nIt's part of my Perl new script skeleton, sorry.\n\n>> + print <<EOHIPPUS;\n\nJK> Cute, I haven't seen that one before.\n\nHeh heh.  I've had to explain that one in code review many times.  \"See,\nit's the precursor to the modern horse...\"\n\n>> +$0 [-f AUTHFILE] [-d] get\n>> +\n>> +Version $VERSION by tzz\\@lifelogs.com.  License: any use is OK.\n\nJK> I don't know if we have a particular policy for items in contrib/, but\nJK> this license may be too vague. In particular, it does not explicitly\nJK> allow redistribution, which would make Junio shipping a release with it\nJK> a copyright violation.\n\nJK> Any objection to just putting it under some well-known simple license\nJK> (GPL, BSD, or whatever)?\n\nNo, I didn't know what Git requires, and I'd like it to be the least\nrestrictive, so BSD is OK.  Stated in -h now.\n\n>> +if ($file =~ m/\\.gpg$/)\n>> +{\n>> + $file = \"gpg --decrypt $file|\";\n>> +}\n\nJK> Does this need to quote $file, since the result will get passed to the\nJK> shell? It might be easier to just use the list form of open(), like:\n\nJK>   my @data = $file =~ /\\.gpg$/ ?\nJK>              load('-|', qw(gpg --decrypt), $file) :\nJK>              load('<', $file);\n\nJK> (and then obviously update load to just dump all of @_ to open()).\n\nYes, thanks.  Done.\n\n>> +die \"Sorry, we could not load data from [$file]\"\n>> + unless (scalar @data);\n\nJK> Probably not that interesting a corner case, but this means we die on an\nJK> empty .netrc, whereas it might be more sensible for it to behave as \"no\nJK> match\".\n\nJK> For the same reason, it might be worth silently exiting when we don't\nJK> find a .netrc (or any of its variants). That lets people who share their\nJK> dot-files across machines configure git globally, even if they don't\nJK> necessarily have a netrc on every machine.\n\nOK; done.\n\n>> +# the query\n>> +my %q;\n>> +\n>> +foreach my $v (values %{$options{tmap}})\n>> +{\n>> + undef $q{$v};\n>> +}\n\nJK> Just my personal style, but I find the intent more obvious with \"map\" (I\nJK> know some people find it unreadable, though):\n\nJK>   my %q = map { $_ => undef } values(%{$options{tmap}});\n\nYes, changed.\n\n>> +while (<STDIN>)\n>> +{\n>> + next unless m/([a-z]+)=(.+)/;\n\nJK> We don't currently have any exotic tokens that this would not match, nor\nJK> do I plan to add them, but the credential documentation defines a valid\nJK> line as /^([^=]+)=(.+)/.\n\nJK> It's also possible for the value to be empty, but I do not think\nJK> off-hand that current git will ever send such an empty value.\n\nYes, changed.\n\nJK> The rest of it looks fine to me. I don't think any of my comments are\nJK> show-stoppers. Tests would be nice, but integrating contrib/ stuff with\nJK> the test harness is kind of a pain.\n\n\"I tested it on AIX, it works great!\" :)\n\nIt's pretty easy to write a local Makefile with a test target, if you\nthink it worthwhile.\n\nTed\n"},{"id":"208617","messageId":"87sj5cvxnf.fsf_-_@lifelogs.com","threadId":"32770","inReplyTo":"20130203194148.GA26318@sigill.intra.peff.net","subject":"[PATCH 1/3] Add contrib/credentials/netrc with GPG support","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T16:42:12Z","receivedAt":"2013-02-04T16:42:12Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"\nSigned-off-by: Ted Zlatanov <tzz@lifelogs.com>\n---\n contrib/credential/netrc/git-credential-netrc |  242 +++++++++++++++++++++++++\n 1 files changed, 242 insertions(+), 0 deletions(-)\n create mode 100755 contrib/credential/netrc/git-credential-netrc\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc\nnew file mode 100755\nindex 0000000..92fc306\n--- /dev/null\n+++ b/contrib/credential/netrc/git-credential-netrc\n@@ -0,0 +1,242 @@\n+#!/usr/bin/perl\n+\n+use strict;\n+use warnings;\n+\n+use Data::Dumper;\n+\n+use Getopt::Long;\n+use File::Basename;\n+\n+my $VERSION = \"0.1\";\n+\n+my %options = (\n+               help => 0,\n+               debug => 0,\n+\n+               # identical token maps, e.g. host -> host, will be inserted later\n+               tmap => {\n+                        port => 'protocol',\n+                        machine => 'host',\n+                        path => 'path',\n+                        login => 'username',\n+                        user => 'username',\n+                        password => 'password',\n+                       }\n+              );\n+\n+foreach my $v (values %{$options{tmap}})\n+{\n+ $options{tmap}->{$v} = $v;\n+}\n+\n+foreach my $suffix ('.gpg', '')\n+{\n+ foreach my $base (qw/authinfo netrc/)\n+ {\n+  my $file = glob(\"~/.$base$suffix\");\n+  next unless (defined $file && -f $file);\n+  $options{file} = $file ;\n+ }\n+}\n+\n+Getopt::Long::Configure(\"bundling\");\n+\n+# TODO: maybe allow the token map $options{tmap} to be configurable.\n+GetOptions(\\%options,\n+           \"help|h\",\n+           \"debug|d\",\n+           \"file|f=s\",\n+          );\n+\n+if ($options{help})\n+{\n+ my $shortname = basename($0);\n+ $shortname =~ s/git-credential-//;\n+\n+ print <<EOHIPPUS;\n+\n+$0 [-f AUTHFILE] [-d] get\n+\n+Version $VERSION by tzz\\@lifelogs.com.  License: any use is OK.\n+\n+Options:\n+  -f AUTHFILE: specify a netrc-style file\n+  -d: turn on debugging\n+\n+To enable (note that Git will prepend \"git-credential-\" to the helper\n+name and look for it in the path):\n+\n+  git config credential.helper '$shortname -f AUTHFILE'\n+\n+And if you want lots of debugging info:\n+\n+  git config credential.helper '$shortname -f AUTHFILE -d'\n+\n+Only \"get\" mode is supported by this credential helper.  It opens\n+AUTHFILE and looks for entries that match the requested search\n+criteria:\n+\n+ 'port|protocol':\n+   The protocol that will be used (e.g., https). (protocol=X)\n+\n+ 'machine|host':\n+   The remote hostname for a network credential. (host=X)\n+\n+ 'path':\n+   The path with which the credential will be used. (path=X)\n+\n+ 'login|user|username':\n+   The credential’s username, if we already have one. (username=X)\n+\n+Thus, when we get \"protocol=https\\nusername=tzz\", this credential\n+helper will look for lines in AUTHFILE that match\n+\n+port https login tzz\n+\n+OR\n+\n+protocol https login tzz\n+\n+OR... etc. acceptable tokens as listed above.  Any unknown tokens are\n+simply ignored.\n+\n+Then, the helper will print out whatever tokens it got from the line,\n+including \"password\" tokens, mapping e.g. \"port\" back to \"protocol\".\n+\n+The first matching line is used.  Tokens can be quoted as 'STRING' or\n+\"STRING\".\n+\n+No caching is performed by this credential helper.\n+\n+EOHIPPUS\n+\n+ exit;\n+}\n+\n+my $mode = shift @ARGV;\n+\n+# credentials may get 'get', 'store', or 'erase' as parameters but\n+# only acknowledge 'get'\n+die \"Syntax: $0 [-f AUTHFILE] [-d] get\" unless defined $mode;\n+\n+# only support 'get' mode\n+exit unless $mode eq 'get';\n+\n+my $debug = $options{debug};\n+my $file = $options{file};\n+\n+die \"Sorry, you need to specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE\"\n+ unless defined $file;\n+\n+die \"Sorry, the specified netrc $file is not accessible\"\n+ unless -f $file;\n+\n+if ($file =~ m/\\.gpg$/)\n+{\n+ $file = \"gpg --decrypt $file|\";\n+}\n+\n+my @data = load($file);\n+chomp @data;\n+\n+die \"Sorry, we could not load data from [$file]\"\n+ unless (scalar @data);\n+\n+# the query\n+my %q;\n+\n+foreach my $v (values %{$options{tmap}})\n+{\n+ undef $q{$v};\n+}\n+\n+while (<STDIN>)\n+{\n+ next unless m/([a-z]+)=(.+)/;\n+\n+ my ($token, $value) = ($1, $2);\n+ die \"Unknown search token $1\" unless exists $q{$token};\n+ $q{$token} = $value;\n+}\n+\n+# build reverse token map\n+my %rmap;\n+foreach my $k (keys %{$options{tmap}})\n+{\n+ push @{$rmap{$options{tmap}->{$k}}}, $k;\n+}\n+\n+# there are CPAN modules to do this better, but we want to avoid\n+# dependencies and generally, complex netrc-style files are rare\n+\n+if ($debug)\n+{\n+ foreach (sort keys %q)\n+ {\n+  printf STDERR \"searching for %s = %s\\n\",\n+   $_, $q{$_} || '(any value)';\n+ }\n+}\n+\n+LINE: foreach my $line (@data)\n+{\n+\n+ print STDERR \"line [$line]\\n\" if $debug;\n+ my @tok;\n+ # gratefully stolen from Net::Netrc\n+ while (length $line &&\n+        $line =~ s/^(\"((?:[^\"]+|\\\\.)*)\"|((?:[^\\\\\\s]+|\\\\.)*))\\s*//)\n+ {\n+  (my $tok = $+) =~ s/\\\\(.)/$1/g;\n+  push(@tok, $tok);\n+ }\n+\n+ my %tokens;\n+ while (@tok)\n+ {\n+  my ($k, $v) = (shift @tok, shift @tok);\n+  next unless defined $v;\n+  next unless exists $options{tmap}->{$k};\n+  $tokens{$options{tmap}->{$k}} = $v;\n+ }\n+\n+ foreach my $check (sort keys %q)\n+ {\n+  if (exists $tokens{$check} && defined $q{$check})\n+  {\n+   print STDERR \"comparing [$tokens{$check}] to [$q{$check}] in line [$line]\\n\" if $debug;\n+   next LINE unless $tokens{$check} eq $q{$check};\n+  }\n+  else\n+  {\n+   print STDERR \"we could not find [$check] but it's OK\\n\" if $debug;\n+  }\n+ }\n+\n+ print STDERR \"line has passed all the search checks\\n\" if $debug;\n+ foreach my $token (sort keys %rmap)\n+ {\n+  print STDERR \"looking for useful token $token\\n\" if $debug;\n+  next unless exists $tokens{$token}; # did we match?\n+\n+  foreach my $rctoken (@{$rmap{$token}})\n+  {\n+   next if defined $q{$rctoken};           # don't re-print given tokens\n+  }\n+\n+  print STDERR \"FOUND: $token=$tokens{$token}\\n\" if $debug;\n+  printf \"%s=%s\\n\", $token, $tokens{$token};\n+ }\n+\n+ last;\n+}\n+\n+sub load\n+{\n+ my $file = shift;\n+ # this supports pipes too\n+ my $io = new IO::File($file) or die \"Could not open $file: $!\\n\";\n+\n+ return <$io>;                          # whole file\n+}\n-- \n1.7.9.rc2\n"},{"id":"208618","messageId":"87obg0vxmr.fsf_-_@lifelogs.com","threadId":"32770","inReplyTo":"20130203194148.GA26318@sigill.intra.peff.net","subject":"[PATCH 2/3] Skip blank and commented lines in contrib/credentials/netrc","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T16:42:36Z","receivedAt":"2013-02-04T16:42:36Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"\nSigned-off-by: Ted Zlatanov <tzz@lifelogs.com>\n---\n contrib/credential/netrc/git-credential-netrc |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc\nindex 92fc306..a47a223 100755\n--- a/contrib/credential/netrc/git-credential-netrc\n+++ b/contrib/credential/netrc/git-credential-netrc\n@@ -192,6 +192,9 @@ LINE: foreach my $line (@data)\n   push(@tok, $tok);\n  }\n \n+ # skip blank lines, comments, etc.\n+ next LINE unless scalar @tok;\n+\n  my %tokens;\n  while (@tok)\n  {\n-- \n1.7.9.rc2\n"},{"id":"208619","messageId":"87k3qovxlp.fsf_-_@lifelogs.com","threadId":"32770","inReplyTo":"20130203194148.GA26318@sigill.intra.peff.net","subject":"[PATCH 3/3] Fix contrib/credentials/netrc minor issues: exit quietly; use 3-parameter open; etc.","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T16:43:14Z","receivedAt":"2013-02-04T16:43:14Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"\nSigned-off-by: Ted Zlatanov <tzz@lifelogs.com>\n---\n contrib/credential/netrc/git-credential-netrc |   38 +++++++++++++------------\n 1 files changed, 20 insertions(+), 18 deletions(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc\nindex a47a223..0e35918 100755\n--- a/contrib/credential/netrc/git-credential-netrc\n+++ b/contrib/credential/netrc/git-credential-netrc\n@@ -3,8 +3,6 @@\n use strict;\n use warnings;\n \n-use Data::Dumper;\n-\n use Getopt::Long;\n use File::Basename;\n \n@@ -58,7 +56,7 @@ if ($options{help})\n \n $0 [-f AUTHFILE] [-d] get\n \n-Version $VERSION by tzz\\@lifelogs.com.  License: any use is OK.\n+Version $VERSION by tzz\\@lifelogs.com.  License: BSD.\n \n Options:\n   -f AUTHFILE: specify a netrc-style file\n@@ -129,31 +127,36 @@ my $file = $options{file};\n die \"Sorry, you need to specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE\"\n  unless defined $file;\n \n-die \"Sorry, the specified netrc $file is not accessible\"\n- unless -f $file;\n+unless (-f $file)\n+{\n+ print STDERR \"Sorry, the specified netrc $file is not accessible\\n\" if $debug;\n+ exit 0;\n+}\n \n+my @data;\n if ($file =~ m/\\.gpg$/)\n {\n- $file = \"gpg --decrypt $file|\";\n+ @data = load('-|', qw(gpg --decrypt), $file)\n+}\n+else\n+{\n+ @data = load('<', $file);\n }\n \n-my @data = load($file);\n chomp @data;\n \n-die \"Sorry, we could not load data from [$file]\"\n- unless (scalar @data);\n-\n-# the query\n-my %q;\n-\n-foreach my $v (values %{$options{tmap}})\n+unless (scalar @data)\n {\n- undef $q{$v};\n+ print STDERR \"Sorry, we could not load data from [$file]\\n\" if $debug;\n+ exit;\n }\n \n+# the query: start with every token with no value\n+my %q = map { $_ => undef } values(%{$options{tmap}});\n+\n while (<STDIN>)\n {\n- next unless m/([a-z]+)=(.+)/;\n+ next unless m/([^=]+)=(.+)/;\n \n  my ($token, $value) = ($1, $2);\n  die \"Unknown search token $1\" unless exists $q{$token};\n@@ -237,9 +240,8 @@ LINE: foreach my $line (@data)\n \n sub load\n {\n- my $file = shift;\n  # this supports pipes too\n- my $io = new IO::File($file) or die \"Could not open $file: $!\\n\";\n+ my $io = new IO::File(@_) or die \"Could not open [@_]: $!\\n\";\n \n  return <$io>;                          # whole file\n }\n-- \n1.7.9.rc2\n"},{"id":"208623","messageId":"87fw1cvwxy.fsf@lifelogs.com","threadId":"32770","inReplyTo":"87sj5cvxnf.fsf_-_@lifelogs.com","subject":"Re: [PATCH 1/3] Add contrib/credentials/netrc with GPG support","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T16:57:29Z","receivedAt":"2013-02-04T16:57:29Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"Grr, sorry for the bad formatting.  First time doing format-patch.\n\nTed\n"},{"id":"208624","messageId":"878v74vwst.fsf@lifelogs.com","threadId":"32770","inReplyTo":"xa1tmwvk9gy1.fsf@mina86.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T17:00:34Z","receivedAt":"2013-02-04T17:00:34Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Mon, 04 Feb 2013 17:33:58 +0100 Michal Nazarewicz <mina86@mina86.com> wrote: \n\nMN> As far as I understand, there could be a git-credential helper that\nMN> reads ~/.authinfo and than git-send-email would just call “git\nMN> credential fill”, right?\n\nMN> I've noticed though, that git-credential does not support port argument,\nMN> which makes it slightly incompatible with ~/.authinfo.\n\nMy proposed netrc credential helper does this :)\n\nThe token mapping I use:\n\nport, protocol        => protocol\nmachine, host         => host\npath                  => path\nlogin, username, user => username\npassword              => password\n\nI think that's sensible.\n\nTed\n"},{"id":"208627","messageId":"7vk3qo2dsc.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"87sj5cvxnf.fsf_-_@lifelogs.com","subject":"Re: [PATCH 1/3] Add contrib/credentials/netrc with GPG support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-04T17:24:03Z","receivedAt":"2013-02-04T17:24:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"[administrivia: I would really wish you didn't put \"Mail-copies-to:\nnever\" above].\n\nTed Zlatanov <tzz@lifelogs.com> writes:\n\n> +foreach my $v (values %{$options{tmap}})\n> +{\n> + $options{tmap}->{$v} = $v;\n> +}\n\nPlease follow the styles of existing Perl scripts, e.g. indent with\ntab, etc.  Style requests are not optional; it is a prerequisite to\nmake the patch readable and reviewable.\n\n> + print <<EOHIPPUS;\n> + ...\n> +EOHIPPUS\n\nDo we really need to refer readers to Wikipedia or something to\nlearn about extinct equid ungulates ;-)?\n"},{"id":"208628","messageId":"7vfw1c2dm5.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"87k3qovxlp.fsf_-_@lifelogs.com","subject":"Re: [PATCH 3/3] Fix contrib/credentials/netrc minor issues: exit quietly; use 3-parameter open; etc.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-04T17:27:46Z","receivedAt":"2013-02-04T17:27:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ted Zlatanov <tzz@lifelogs.com> writes:\n\n> Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>\n> ---\n>  contrib/credential/netrc/git-credential-netrc |   38 +++++++++++++------------\n>  1 files changed, 20 insertions(+), 18 deletions(-)\n\nEspecially because this is an initial submission, please equash\nthree patches into one, instead of sending three \"here is my first\nattempt with many problems I know I do not want to be there\", \"one\nsmall improvement\", \"another one to fix remaining issues\".\n\nOtherwise you will waste reviewers' time, getting distracted by\nundesirable details they find in an earlier patch while reviewing,\nwithout realizing that some of them are fixed in a later one.\n\nThanks.\n"},{"id":"208633","messageId":"87k3qoudxp.fsf@lifelogs.com","threadId":"32770","inReplyTo":"7vk3qo2dsc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] Add contrib/credentials/netrc with GPG support","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T18:33:22Z","receivedAt":"2013-02-04T18:33:22Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Mon, 04 Feb 2013 09:24:03 -0800 Junio C Hamano <gitster@pobox.com> wrote: \n\nJCH> [administrivia: I would really wish you didn't put \"Mail-copies-to:\nJCH> never\" above].\n\nI normally post through GMane and don't need the extra CC on any list I\nread.  I'll make an effort to remove that header here, and apologize for\nthe inconvenience.\n\nJCH> Ted Zlatanov <tzz@lifelogs.com> writes:\n\n>> +foreach my $v (values %{$options{tmap}})\n>> +{\n>> + $options{tmap}->{$v} = $v;\n>> +}\n\nJCH> Please follow the styles of existing Perl scripts, e.g. indent with\nJCH> tab, etc.  Style requests are not optional; it is a prerequisite to\nJCH> make the patch readable and reviewable.\n\nSorry, I didn't realize contrib/ stuff was under the same rules.  I will\nattempt to make my contributions fit the project's requirements.\n\nIt would help if the requirements were codified as the fairly standard\nEmacs file-local variables, so I can just put them in the Perl code or\nin .dir-locals.el in the source tree.  At least for Perl I'd like that,\nand it could be nice for the Emacs users who write C too.\n\nWould you like me to propose that as a patch?\n\nEither way, I guessed that these settings are what you want as far as\ntabs and indentation (I use cperl-mode but perl-mode is the same):\n\n# -*- mode: cperl; tab-width: 8; cperl-indent-level: 4; indent-tabs-mode: t; -*-\n\n...plus hanging braces and avoiding one-line blocks.  I hope that's right.\n\n>> + print <<EOHIPPUS;\n>> + ...\n>> +EOHIPPUS\n\nJCH> Do we really need to refer readers to Wikipedia or something to\nJCH> learn about extinct equid ungulates ;-)?\n\nI think the marker's name is irrelevant, and hope you are OK with\nleaving it.\n\nSince the change is a pretty big reformatting, should I squash my 3\ncommits plus the reformatting commit into one patch, or keep them as a\nseries?\n\nI am appending the script in its current form so you can review it and\ntell me if there's anything else I should add or change in the\nformatting.\n\nThanks\nTed\n\n\n\n#!/usr/bin/perl\n# -*- mode: cperl; tab-width: 8; cperl-indent-level: 4; indent-tabs-mode: t; -*-\n\nuse strict;\nuse warnings;\n\nuse Getopt::Long;\nuse File::Basename;\n\nmy $VERSION = \"0.1\";\n\nmy %options = (\n               help => 0,\n               debug => 0,\n\n               # identical token maps, e.g. host -> host, will be inserted later\n               tmap => {\n                        port => 'protocol',\n                        machine => 'host',\n                        path => 'path',\n                        login => 'username',\n                        user => 'username',\n                        password => 'password',\n                       }\n              );\n\n# map each credential protocol token to itself on the netrc side\n$options{tmap}->{$_} = $_ foreach my $v (values %{$options{tmap}});\n\nforeach my $suffix ('.gpg', '') {\n    foreach my $base (qw/authinfo netrc/) {\n\tmy $file = glob(\"~/.$base$suffix\");\n\tnext unless (defined $file && -f $file);\n\t$options{file} = $file ;\n    }\n}\n\nGetopt::Long::Configure(\"bundling\");\n\n# TODO: maybe allow the token map $options{tmap} to be configurable.\nGetOptions(\\%options,\n           \"help|h\",\n           \"debug|d\",\n           \"file|f=s\",\n          );\n\nif ($options{help}) {\n    my $shortname = basename($0);\n    $shortname =~ s/git-credential-//;\n\n    print <<EOHIPPUS;\n\n$0 [-f AUTHFILE] [-d] get\n\nVersion $VERSION by tzz\\@lifelogs.com.  License: BSD.\n\nOptions:\n  -f AUTHFILE: specify a netrc-style file\n  -d: turn on debugging\n\nTo enable (note that Git will prepend \"git-credential-\" to the helper\nname and look for it in the path):\n\n  git config credential.helper '$shortname -f AUTHFILE'\n\nAnd if you want lots of debugging info:\n\n  git config credential.helper '$shortname -f AUTHFILE -d'\n\nOnly \"get\" mode is supported by this credential helper.  It opens\nAUTHFILE and looks for entries that match the requested search\ncriteria:\n\n 'port|protocol':\n   The protocol that will be used (e.g., https). (protocol=X)\n\n 'machine|host':\n   The remote hostname for a network credential. (host=X)\n\n 'path':\n   The path with which the credential will be used. (path=X)\n\n 'login|user|username':\n   The credential’s username, if we already have one. (username=X)\n\nThus, when we get \"protocol=https\\nusername=tzz\", this credential\nhelper will look for lines in AUTHFILE that match\n\nport https login tzz\n\nOR\n\nprotocol https login tzz\n\nOR... etc. acceptable tokens as listed above.  Any unknown tokens are\nsimply ignored.\n\nThen, the helper will print out whatever tokens it got from the line,\nincluding \"password\" tokens, mapping e.g. \"port\" back to \"protocol\".\n\nThe first matching line is used.  Tokens can be quoted as 'STRING' or\n\"STRING\".\n\nNo caching is performed by this credential helper.\n\nEOHIPPUS\n\n    exit;\n}\n\nmy $mode = shift @ARGV;\n\n# credentials may get 'get', 'store', or 'erase' as parameters but\n# only acknowledge 'get'\ndie \"Syntax: $0 [-f AUTHFILE] [-d] get\" unless defined $mode;\n\n# only support 'get' mode\nexit unless $mode eq 'get';\n\nmy $debug = $options{debug};\nmy $file = $options{file};\n\ndie \"Sorry, you need to specify an existing netrc file (with or without a .gpg extension) with -f AUTHFILE\"\n unless defined $file;\n\nunless (-f $file) {\n    print STDERR \"Sorry, the specified netrc $file is not accessible\\n\" if $debug;\n    exit 0;\n}\n\nmy @data;\nif ($file =~ m/\\.gpg$/) {\n    @data = load('-|', qw(gpg --decrypt), $file)\n}\nelse {\n    @data = load('<', $file);\n}\n\nchomp @data;\n\nunless (scalar @data) {\n    print STDERR \"Sorry, we could not load data from [$file]\\n\" if $debug;\n    exit;\n}\n\n# the query: start with every token with no value\nmy %q = map { $_ => undef } values(%{$options{tmap}});\n\nwhile (<STDIN>) {\n    next unless m/([^=]+)=(.+)/;\n\n    my ($token, $value) = ($1, $2);\n    die \"Unknown search token $1\" unless exists $q{$token};\n    $q{$token} = $value;\n}\n\n# build reverse token map\nmy %rmap;\nforeach my $k (keys %{$options{tmap}}) {\n    push @{$rmap{$options{tmap}->{$k}}}, $k;\n}\n\n# there are CPAN modules to do this better, but we want to avoid\n# dependencies and generally, complex netrc-style files are rare\n\nif ($debug) {\n    printf STDERR \"searching for %s = %s\\n\", $_, $q{$_} || '(any value)'\n     foreach sort keys %q;\n}\n\nLINE: foreach my $line (@data) {\n\n    print STDERR \"line [$line]\\n\" if $debug;\n    my @tok;\n    # gratefully stolen from Net::Netrc\n    while (length $line &&\n\t   $line =~ s/^(\"((?:[^\"]+|\\\\.)*)\"|((?:[^\\\\\\s]+|\\\\.)*))\\s*//) {\n\t(my $tok = $+) =~ s/\\\\(.)/$1/g;\n\tpush(@tok, $tok);\n    }\n\n    # skip blank lines, comments, etc.\n    next LINE unless scalar @tok;\n\n    my %tokens;\n    while (@tok) {\n\tmy ($k, $v) = (shift @tok, shift @tok);\n\tnext unless defined $v;\n\tnext unless exists $options{tmap}->{$k};\n\t$tokens{$options{tmap}->{$k}} = $v;\n    }\n\n    foreach my $check (sort keys %q) {\n\tif (exists $tokens{$check} && defined $q{$check}) {\n\t    print STDERR \"comparing [$tokens{$check}] to [$q{$check}] in line [$line]\\n\" if $debug;\n\t    next LINE unless $tokens{$check} eq $q{$check};\n\t}\n\telse {\n\t    print STDERR \"we could not find [$check] but it's OK\\n\" if $debug;\n\t}\n    }\n\n    print STDERR \"line has passed all the search checks\\n\" if $debug;\n TOKEN:\n    foreach my $token (sort keys %rmap) {\n\tprint STDERR \"looking for useful token $token\\n\" if $debug;\n\tnext unless exists $tokens{$token}; # did we match?\n\n\tforeach my $rctoken (@{$rmap{$token}}) {\n\t    next TOKEN if defined $q{$rctoken};           # don't re-print given tokens\n\t}\n\n\tprint STDERR \"FOUND: $token=$tokens{$token}\\n\" if $debug;\n\tprintf \"%s=%s\\n\", $token, $tokens{$token};\n    }\n\n    last;\n}\n\nsub load {\n    # this supports pipes too\n    my $io = new IO::File(@_) or die \"Could not open [@_]: $!\\n\";\n    return <$io>;                          # whole file\n}\n"},{"id":"208635","messageId":"874nhrvs4s.fsf@lifelogs.com","threadId":"32770","inReplyTo":"7vfw1c2dm5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] Fix contrib/credentials/netrc minor issues: exit quietly; use 3-parameter open; etc.","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T18:41:23Z","receivedAt":"2013-02-04T18:41:23Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Mon, 04 Feb 2013 09:27:46 -0800 Junio C Hamano <gitster@pobox.com> wrote: \n\nJCH> Ted Zlatanov <tzz@lifelogs.com> writes:\n>> Signed-off-by: Ted Zlatanov <tzz@lifelogs.com>\n>> ---\n>> contrib/credential/netrc/git-credential-netrc |   38 +++++++++++++------------\n>> 1 files changed, 20 insertions(+), 18 deletions(-)\n\nJCH> Especially because this is an initial submission, please equash\nJCH> three patches into one, instead of sending three \"here is my first\nJCH> attempt with many problems I know I do not want to be there\", \"one\nJCH> small improvement\", \"another one to fix remaining issues\".\n\nJCH> Otherwise you will waste reviewers' time, getting distracted by\nJCH> undesirable details they find in an earlier patch while reviewing,\nJCH> without realizing that some of them are fixed in a later one.\n\nOK, thanks.  I wasn't sure, since Jeff already reviewed it, if it was\nbetter to squash or not.  Ignore this same question in my other reply to\nyou, and thanks for your patience.\n\nTed\n"},{"id":"208636","messageId":"7vvca7291z.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"87k3qoudxp.fsf@lifelogs.com","subject":"Re: [PATCH 1/3] Add contrib/credentials/netrc with GPG support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-04T19:06:16Z","receivedAt":"2013-02-04T19:06:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ted Zlatanov <tzz@lifelogs.com> writes:\n\n> Sorry, I didn't realize contrib/ stuff was under the same rules.\n\nI had a feeling that this may start out from contrib/ but will soon\nprove to be fairly important to be part of the Git proper.\n\n> It would help if the requirements were codified as the fairly standard\n> Emacs file-local variables, so I can just put them in the Perl code or\n> in .dir-locals.el in the source tree.  At least for Perl I'd like that,\n> and it could be nice for the Emacs users who write C too.\n>\n> Would you like me to propose that as a patch?\n\nI thought that we tend to avoid Emacs/Vim formatting cruft left in\nthe file.  Do we have any in existing file outside contrib/?\n\n> Either way, I guessed that these settings are what you want as far as\n> tabs and indentation (I use cperl-mode but perl-mode is the same):\n>\n> # -*- mode: cperl; tab-width: 8; cperl-indent-level: 4; indent-tabs-mode: t; -*-\n\nIndent is done with a Tab and indent level is 8 places (check add--interactive.perl\nand imitate it, perhaps?).\n\nThanks.\n"},{"id":"208638","messageId":"87lib3uats.fsf@lifelogs.com","threadId":"32770","inReplyTo":"7vvca7291z.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] Add contrib/credentials/netrc with GPG support","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T19:40:31Z","receivedAt":"2013-02-04T19:40:31Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Mon, 04 Feb 2013 11:06:16 -0800 Junio C Hamano <gitster@pobox.com> wrote: \n\nJCH> Ted Zlatanov <tzz@lifelogs.com> writes:\n>> Sorry, I didn't realize contrib/ stuff was under the same rules.\n\nJCH> I had a feeling that this may start out from contrib/ but will soon\nJCH> prove to be fairly important to be part of the Git proper.\n\nCool!\n\n>> It would help if the requirements were codified as the fairly standard\n>> Emacs file-local variables, so I can just put them in the Perl code or\n>> in .dir-locals.el in the source tree.  At least for Perl I'd like that,\n>> and it could be nice for the Emacs users who write C too.\n>> \n>> Would you like me to propose that as a patch?\n\nJCH> I thought that we tend to avoid Emacs/Vim formatting cruft left in\nJCH> the file.  Do we have any in existing file outside contrib/?\n\nNo, but it's a nice way to express the settings so no one is guessing\nwhat the project prefers.  At least for me it's not an issue anymore,\nsince I understand your criteria better now, so let me know if you want\nme to express it in the CodingGuidelines, in a dir-locals.el file, or\nsomewhere else.\n\n>> Either way, I guessed that these settings are what you want as far as\n>> tabs and indentation (I use cperl-mode but perl-mode is the same):\n>> \n>> # -*- mode: cperl; tab-width: 8; cperl-indent-level: 4; indent-tabs-mode: t; -*-\n\nJCH> Indent is done with a Tab and indent level is 8 places (check add--interactive.perl\nJCH> and imitate it, perhaps?).\n\nYup, got it.  My mistake on the size-4 indents.\n\nI found this helpful, at least while I was indenting, for anyone else\nwho might want to indent Perl appropriately to imitate existing Perl code:\n\n# -*- mode: cperl; tab-width: 8; cperl-indent-level: 8; indent-tabs-mode: t; -*-\n\nI'll resubmit now.\n\nThanks\nTed\n"},{"id":"208642","messageId":"20130204201040.GA13272@sigill.intra.peff.net","threadId":"32770","inReplyTo":"878v74vwst.fsf@lifelogs.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-04T20:10:40Z","receivedAt":"2013-02-04T20:10:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 04, 2013 at 12:00:34PM -0500, Ted Zlatanov wrote:\n\n> On Mon, 04 Feb 2013 17:33:58 +0100 Michal Nazarewicz <mina86@mina86.com> wrote: \n> \n> MN> As far as I understand, there could be a git-credential helper that\n> MN> reads ~/.authinfo and than git-send-email would just call “git\n> MN> credential fill”, right?\n> \n> MN> I've noticed though, that git-credential does not support port argument,\n> MN> which makes it slightly incompatible with ~/.authinfo.\n> \n> My proposed netrc credential helper does this :)\n> \n> The token mapping I use:\n> \n> port, protocol        => protocol\n> machine, host         => host\n> path                  => path\n> login, username, user => username\n> password              => password\n> \n> I think that's sensible.\n\nTechnically you can speak a particular protocol on an alternate port:\n\n  https://example.com:31337/repo.git\n\nIn this case, git will send you the host as:\n\n  example.com:31337\n\nYou might want to map this to \"port\" in .autoinfo separately if it's\navailable.\n\n-Peff\n"},{"id":"208644","messageId":"87a9rju8l7.fsf@lifelogs.com","threadId":"32770","inReplyTo":"20130204201040.GA13272@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T20:28:52Z","receivedAt":"2013-02-04T20:28:52Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Mon, 4 Feb 2013 15:10:40 -0500 Jeff King <peff@peff.net> wrote: \n\nJK> Technically you can speak a particular protocol on an alternate port:\n\nJK>   https://example.com:31337/repo.git\n\nJK> In this case, git will send you the host as:\n\nJK>   example.com:31337\n\nJK> You might want to map this to \"port\" in .autoinfo separately if it's\nJK> available.\n\nThat would create the following possibilities:\n\n* host example.com:31337, protocol https\n* host example.com:31337, protocol unspecified\n* host example.com, protocol https\n* host example.com, protocol unspecified\n\nHow would you like each one to be handled?  My preference would be to\nmake the user say \"host example.com:31337\" in the netrc file (the\ncurrent situation); that's what we do in Emacs and it lets applications\nrequest credentials for a logical service no matter what the port is.\n\nIt means that example.com credentials won't be used for\nexample.com:31337.  In practice, that has not been a problem for us.\n\nTed\n"},{"id":"208646","messageId":"20130204205911.GA13186@sigill.intra.peff.net","threadId":"32770","inReplyTo":"87a9rju8l7.fsf@lifelogs.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-04T20:59:11Z","receivedAt":"2013-02-04T20:59:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 04, 2013 at 03:28:52PM -0500, Ted Zlatanov wrote:\n\n> JK> You might want to map this to \"port\" in .autoinfo separately if it's\n> JK> available.\n> \n> That would create the following possibilities:\n> \n> * host example.com:31337, protocol https\n> * host example.com:31337, protocol unspecified\n> * host example.com, protocol https\n> * host example.com, protocol unspecified\n\nPossibilities for .netrc, or for git? Git will always specify the\nprotocol.\n\n> How would you like each one to be handled?  My preference would be to\n> make the user say \"host example.com:31337\" in the netrc file (the\n> current situation); that's what we do in Emacs and it lets applications\n> request credentials for a logical service no matter what the port is.\n> \n> It means that example.com credentials won't be used for\n> example.com:31337.  In practice, that has not been a problem for us.\n\nYeah, I think that is a good thing. The credentials used for\nexample.com:31337 are not necessarily the same as for the main site.\nIt's less convenient, but a more secure default.\n\nWhat I was more wondering (and I know very little about .netrc, so this\nmight not be a possibility at all) is a line like:\n\n  host example.com port 5001 protocol https username foo password bar\n\nTo match git's representation on a token-by-token basis, you would have\nto either split out git's \"host:port\" pair, or combine the .netrc's\nrepresentation to \"example.com:5001\".\n\n-Peff\n"},{"id":"208647","messageId":"8738xbu6qj.fsf@lifelogs.com","threadId":"32770","inReplyTo":"20130204205911.GA13186@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T21:08:52Z","receivedAt":"2013-02-04T21:08:52Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Mon, 4 Feb 2013 15:59:11 -0500 Jeff King <peff@peff.net> wrote: \n\nJK> On Mon, Feb 04, 2013 at 03:28:52PM -0500, Ted Zlatanov wrote:\nJK> You might want to map this to \"port\" in .autoinfo separately if it's\nJK> available.\n>> \n>> That would create the following possibilities:\n>> \n>> * host example.com:31337, protocol https\n>> * host example.com:31337, protocol unspecified\n>> * host example.com, protocol https\n>> * host example.com, protocol unspecified\n\nJK> Possibilities for .netrc, or for git? Git will always specify the\nJK> protocol.\n\nPossibilities for the netrc data.  How clever do we want to be with\ntaking 31337 and mapping it to the \"protocol\"?  My preference is to be\nvery simple here.\n\nJK> What I was more wondering (and I know very little about .netrc, so this\nJK> might not be a possibility at all) is a line like:\n\nJK>   host example.com port 5001 protocol https username foo password bar\n\nJK> To match git's representation on a token-by-token basis, you would have\nJK> to either split out git's \"host:port\" pair, or combine the .netrc's\nJK> representation to \"example.com:5001\".\n\nCurrently, we map both the \"port\" and \"protocol\" netrc tokens to the\ncredential helper protocol's \"protocol\".  So this will have undefined\nresults.  To do what you specify could be pretty simple: we could do a\npreliminary scan of the tokens, looking for \"host X port Y\" where Y is\nan integer, and rewriting the host to be \"X:Y\".  That would be clean and\nsimple, unless the user breaks it with \"host x:23 port 22\".  Let me know\nif you agree and I'll do.\n\nTed\n"},{"id":"208649","messageId":"20130204212203.GC13186@sigill.intra.peff.net","threadId":"32770","inReplyTo":"8738xbu6qj.fsf@lifelogs.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-04T21:22:03Z","receivedAt":"2013-02-04T21:22:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 04, 2013 at 04:08:52PM -0500, Ted Zlatanov wrote:\n\n> >> That would create the following possibilities:\n> >> \n> >> * host example.com:31337, protocol https\n> >> * host example.com:31337, protocol unspecified\n> >> * host example.com, protocol https\n> >> * host example.com, protocol unspecified\n> \n> JK> Possibilities for .netrc, or for git? Git will always specify the\n> JK> protocol.\n> \n> Possibilities for the netrc data.  How clever do we want to be with\n> taking 31337 and mapping it to the \"protocol\"?  My preference is to be\n> very simple here.\n\nI think simple is OK, as we can iterate on it as specific use-cases come\nup. The important thing is to make sure we err on the side of \"does not\nmatch\" and not \"oops, we accidentally sent your plaintext credentials to\nthe wrong server\".\n\n> Currently, we map both the \"port\" and \"protocol\" netrc tokens to the\n> credential helper protocol's \"protocol\".  So this will have undefined\n> results.  To do what you specify could be pretty simple: we could do a\n> preliminary scan of the tokens, looking for \"host X port Y\" where Y is\n> an integer, and rewriting the host to be \"X:Y\".  That would be clean and\n> simple, unless the user breaks it with \"host x:23 port 22\".  Let me know\n> if you agree and I'll do.\n\nYeah, I think that is simple and obvious. If the user is saying \"host\nx:23 port 22\", that is nonsensical.\n\n-Peff\n"},{"id":"208651","messageId":"87r4kvsqnc.fsf@lifelogs.com","threadId":"32770","inReplyTo":"20130204212203.GC13186@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-04T21:41:43Z","receivedAt":"2013-02-04T21:41:43Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Mon, 4 Feb 2013 16:22:03 -0500 Jeff King <peff@peff.net> wrote: \n\n>> Currently, we map both the \"port\" and \"protocol\" netrc tokens to the\n>> credential helper protocol's \"protocol\".  So this will have undefined\n>> results.  To do what you specify could be pretty simple: we could do a\n>> preliminary scan of the tokens, looking for \"host X port Y\" where Y is\n>> an integer, and rewriting the host to be \"X:Y\".  That would be clean and\n>> simple, unless the user breaks it with \"host x:23 port 22\".  Let me know\n>> if you agree and I'll do.\n\nJK> Yeah, I think that is simple and obvious. If the user is saying \"host\nJK> x:23 port 22\", that is nonsensical.\n\nOK; added.\n\nTed\n"},{"id":"208654","messageId":"7v7gmn1xqi.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"87lib3uats.fsf@lifelogs.com","subject":"Re: [PATCH 1/3] Add contrib/credentials/netrc with GPG support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-04T23:10:45Z","receivedAt":"2013-02-04T23:10:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ted Zlatanov <tzz@lifelogs.com> writes:\n\n> JCH> I thought that we tend to avoid Emacs/Vim formatting cruft left in\n> JCH> the file.  Do we have any in existing file outside contrib/?\n>\n> No, but it's a nice way to express the settings so no one is guessing\n> what the project prefers.  At least for me it's not an issue anymore,\n> since I understand your criteria better now, so let me know if you want\n> me to express it in the CodingGuidelines, in a dir-locals.el file, or\n> somewhere else.\n\nHistorically we treated this from CodingGuidelines a sufficient\nclue:\n\n    As for more concrete guidelines, just imitate the existing code\n    (this is a good guideline, no matter which project you are\n    contributing to). It is always preferable to match the _local_\n    convention. New code added to git suite is expected to match\n    the overall style of existing code. Modifications to existing\n    code is expected to match the style the surrounding code already\n    uses (even if it doesn't match the overall style of existing code).\n\nbut over time people wanted more specific guidelines and added\nlanguage specific style guides there.  We have sections that cover\nC, shell and Python, and I do not think adding Perl would not hurt.\n"},{"id":"208775","messageId":"7v6226pdb7.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"20130130074306.GA17868@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-05T23:10:20Z","receivedAt":"2013-02-05T23:10:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Jan 29, 2013 at 11:53:19AM -0800, Junio C Hamano wrote:\n>\n>> Either way it still encourages a plaintext password to be on disk,\n>> which may not be what we want, even though it may be slight if not\n>> really much of an improvement.  Again the Help-for-users has this\n>> amusing bit:\n>\n> I do not mind a .netrc or .authinfo parser, because while those formats\n> do have security problems, they are standard files that may already be\n> in use. So as long as we are not encouraging their use, I do not see a\n> problem in supporting them (and we already do the same with curl's netrc\n> support).\n>\n> But it would probably make sense for send-email to support the existing\n> git-credential subsystem, so that it can take advantage of secure\n> system-specific storage. And that is where we should be pointing new\n> users. I think contrib/mw-to-git even has credential support written in\n> perl, so it would just need to be factored out to Git.pm.\n\nI see a lot of rerolls on the credential helper front, but is there\nanybody working on hooking send-email to the credential framework?\n"},{"id":"208787","messageId":"vpqa9rhaml6.fsf@grenoble-inp.fr","threadId":"32770","inReplyTo":"7v6226pdb7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-02-06T08:11:17Z","receivedAt":"2013-02-06T08:11:17Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I see a lot of rerolls on the credential helper front, but is there\n> anybody working on hooking send-email to the credential framework?\n\nNot answering the question, but git-remote-mediawiki supports the\ncredential framework. It is written in perl, and the credential support\nis rather cleanly written and doesn't have dependencies on the wiki\npart, so the way to go for send-email is probably to libify the\ncredential support in git-remote-mediawiki, and to use it in send-email.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"208799","messageId":"xa1tpq0dpo89.fsf@mina86.com","threadId":"32770","inReplyTo":"7v6226pdb7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Michal Nazarewicz","fromEmail":"mina86@mina86.com","sentAt":"2013-02-06T13:26:46Z","receivedAt":"2013-02-06T13:26:46Z","isPatch":true,"sender":{"key":"mina86@mina86.com","avatar":"https://avatars.githubusercontent.com/u/32383?v=4"},"body":"On Wed, Feb 06 2013, Junio C Hamano <gitster@pobox.com> wrote:\n> I see a lot of rerolls on the credential helper front, but is there\n> anybody working on hooking send-email to the credential framework?\n\nI assumed someone had, but if not I can take a stab at it.  I'm not sure\nhowever how should I map server, server-port, and user to credential\nkey-value pairs.  I'm leaning towards\n\n\tprotocol=smtp\n\thost=<smtp-server>:<smtp-port>\n\tuser=<user>\n\nand than netrc/authinfo helper splitting host to host name and port\nnumber, unless port is not in host in which case protocol is assumed as\nport.\n\n-- \nBest regards,                                         _     _\n.o. | Liege of Serenely Enlightened Majesty of      o' \\,=./ `o\n..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)\nooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--\n\n"},{"id":"208800","messageId":"87a9rho5xv.fsf@lifelogs.com","threadId":"32770","inReplyTo":"xa1tpq0dpo89.fsf@mina86.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T14:47:08Z","receivedAt":"2013-02-06T14:47:08Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Wed, 06 Feb 2013 14:26:46 +0100 Michal Nazarewicz <mina86@mina86.com> wrote: \n\nMN> On Wed, Feb 06 2013, Junio C Hamano <gitster@pobox.com> wrote:\n>> I see a lot of rerolls on the credential helper front, but is there\n>> anybody working on hooking send-email to the credential framework?\n\nMN> I assumed someone had, but if not I can take a stab at it.  I'm not sure\nMN> however how should I map server, server-port, and user to credential\nMN> key-value pairs.  I'm leaning towards\n\nMN> \tprotocol=smtp\nMN> \thost=<smtp-server>:<smtp-port>\nMN> \tuser=<user>\n\nMN> and than netrc/authinfo helper splitting host to host name and port\nMN> number, unless port is not in host in which case protocol is assumed as\nMN> port.\n\nThat would work (with my PATCHv6 of the netrc credential helper) as\nfollows:\n\n1) just host\n\nhost=H\n\nmaps to\n\nmachine H login Y password Z\n\n2) host + protocol smtp\n\nhost=H\nprotocol=smtp\n\nmaps to any of:\n\nmachine H port smtp login Y password Z\nmachine H protocol smtp login Y password Z\n\n3) host:port + protocol smtp\n\nhost=H:25\nprotocol=smtp\n\nmaps to any of:\n\nmachine H port 25 protocol smtp login Y password Z\nmachine H:25 port smtp login Y password Z\nmachine H:25 protocol smtp login Y password Z\n\nThat's my understanding of what we discussed with Peff and Junio about\ntoken mapping.  Note we don't split the input host, but instead say \"if\ntoken 'port' is numeric, append it to the host token\" on the netrc side.\n\nDoes that sound reasonable?  If yes, I can add it to the testing\nMakefile for the netrc credential helper, to make sure it's clearly\nstated and tested.\n\nTed\n"},{"id":"208801","messageId":"876225o5mj.fsf@lifelogs.com","threadId":"32770","inReplyTo":"vpqa9rhaml6.fsf@grenoble-inp.fr","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T14:53:56Z","receivedAt":"2013-02-06T14:53:56Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Wed, 06 Feb 2013 09:11:17 +0100 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote: \n\nMM> Junio C Hamano <gitster@pobox.com> writes:\n>> I see a lot of rerolls on the credential helper front, but is there\n>> anybody working on hooking send-email to the credential framework?\n\nMM> Not answering the question, but git-remote-mediawiki supports the\nMM> credential framework. It is written in perl, and the credential support\nMM> is rather cleanly written and doesn't have dependencies on the wiki\nMM> part, so the way to go for send-email is probably to libify the\nMM> credential support in git-remote-mediawiki, and to use it in send-email.\n\nI looked and that's indeed very useful.  If it's put in a library, I'd\nuse credential_read() and credential_write() in my netrc credential\nhelper.  But I would formalize it a little more about the token names\nand output, and I wouldn't necessarily die() on error.  Maybe this can\nbe merged with the netrc credential helper's\nread_credential_data_from_stdin() and print_credential_data()?\n\nLet me know if you'd like me to libify this...  I'm happy to leave it to\nMatthieu or Michal, or anyone else interested.\n\nTed\n"},{"id":"208808","messageId":"871ucto4vj.fsf_-_@lifelogs.com","threadId":"32770","inReplyTo":"7v7gmn1xqi.fsf@alter.siamese.dyndns.org","subject":"CodingGuidelines Perl amendment (was: [PATCH 1/3] Add contrib/credentials/netrc with GPG support)","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T15:10:08Z","receivedAt":"2013-02-06T15:10:08Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Mon, 04 Feb 2013 15:10:45 -0800 Junio C Hamano <gitster@pobox.com> wrote: \n\nJCH> Ted Zlatanov <tzz@lifelogs.com> writes:\nJCH> I thought that we tend to avoid Emacs/Vim formatting cruft left in\nJCH> the file.  Do we have any in existing file outside contrib/?\n>> \n>> No, but it's a nice way to express the settings so no one is guessing\n>> what the project prefers.  At least for me it's not an issue anymore,\n>> since I understand your criteria better now, so let me know if you want\n>> me to express it in the CodingGuidelines, in a dir-locals.el file, or\n>> somewhere else.\n\nJCH> Historically we treated this from CodingGuidelines a sufficient\nJCH> clue:\n\nJCH>     As for more concrete guidelines, just imitate the existing code\nJCH>     (this is a good guideline, no matter which project you are\nJCH>     contributing to). It is always preferable to match the _local_\nJCH>     convention. New code added to git suite is expected to match\nJCH>     the overall style of existing code. Modifications to existing\nJCH>     code is expected to match the style the surrounding code already\nJCH>     uses (even if it doesn't match the overall style of existing code).\n\nJCH> but over time people wanted more specific guidelines and added\nJCH> language specific style guides there.  We have sections that cover\nJCH> C, shell and Python, and I do not think adding Perl would not hurt.\n\nThe following is how I have interpreted the Perl guidelines.  I hope\nit's OK to include Emacs-specific settings; they make it much easier to\nreindent code to be acceptable.\n\nI will submit as a patch if you think this is reasonable at all.\n\nThe org-mode markers around the code are just a suggestion.\n\nFor Perl 5 programs:\n\n - Most of the C guidelines above apply.\n\n - We try to support Perl 5.8 and later (\"use Perl 5.008\").\n\n - use strict and use warnings are strongly preferred.\n\n - As in C (see above), we avoid using braces unnecessarily (but Perl\n   forces braces around if/unless/else/foreach blocks, so this is not\n   always possible).\n\n - Don't abuse statement modifiers (unless $youmust).\n\n - We try to avoid assignments inside if().\n\n - Learn and use Git.pm if you need that functionality.\n\n - For Emacs, it's useful to put the following in\n   GIT_CHECKOUT/.dir-locals.el, assuming you use cperl-mode:\n\n#+begin_src lisp\n((nil . ((indent-tabs-mode . t)\n              (tab-width . 8)\n              (fill-column . 80)))\n (cperl-mode . ((cperl-indent-level . 8)\n                (cperl-extra-newline-before-brace . nil)\n                (cperl-merge-trailing-else . t))))\n#+end_src\n"},{"id":"208809","messageId":"vpqmwvhxyuj.fsf@grenoble-inp.fr","threadId":"32770","inReplyTo":"876225o5mj.fsf@lifelogs.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-02-06T15:10:12Z","receivedAt":"2013-02-06T15:10:12Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ted Zlatanov <tzz@lifelogs.com> writes:\n\n> MM> [...] so the way to go for send-email is probably to libify the\n> MM> credential support in git-remote-mediawiki, and to use it in send-email.\n>\n> I looked and that's indeed very useful.  If it's put in a library, I'd\n> use credential_read() and credential_write() in my netrc credential\n> helper.  But I would formalize it a little more about the token names\n> and output,\n\nCan you elaborate on this? The idea of the Perl code was to mimick a\ncall to the C API, keeping essentially the same names.\n\n> and I wouldn't necessarily die() on error. \n\nSure, die()ing in a library is bad.\n\n> Maybe this can be merged with the netrc credential helper's\n> read_credential_data_from_stdin() and print_credential_data()?\n\nI don't know about the netrc credential helper, but I guess that's\nanother layer. The git-remote-mediawiki code is the code to call the\ncredential C API, that in turn may (or may not) call a credential\nhelper.\n\n> Let me know if you'd like me to libify this...  I'm happy to leave it to\n> Matthieu or Michal, or anyone else interested.\n\nI'd happily let you do the job, but I can help if needed. One thing to\nbe careful about: git-remote-mediawiki is currently a standalone script,\nso it can be installed with a plain \"cp git-remote-mediawiki $somewhere/\".\nOne consequence of libification is that it adds a dependency on the\nlibrary (e.g. Git.pm). We should be carefull to keep it easy for the\nuser to install it (e.g. some kind of \"make install\", or update the doc).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"208816","messageId":"87sj59mo2y.fsf@lifelogs.com","threadId":"32770","inReplyTo":"vpqmwvhxyuj.fsf@grenoble-inp.fr","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T15:58:13Z","receivedAt":"2013-02-06T15:58:13Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Wed, 06 Feb 2013 16:10:12 +0100 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote: \n\nMM> Ted Zlatanov <tzz@lifelogs.com> writes:\nMM> [...] so the way to go for send-email is probably to libify the\nMM> credential support in git-remote-mediawiki, and to use it in send-email.\n>> \n>> I looked and that's indeed very useful.  If it's put in a library, I'd\n>> use credential_read() and credential_write() in my netrc credential\n>> helper.  But I would formalize it a little more about the token names\n>> and output,\n\nMM> Can you elaborate on this? The idea of the Perl code was to mimick a\nMM> call to the C API, keeping essentially the same names.\n\nNone of these are a big deal, and Michal said he's working on libifying\nthis anyhow:\n\n- making 'fill' a special operation is weird\n- anchor the key regex to beginning of line (not strictly necessary)\n- sort the output tokens (after 'url' is extracted) so the output is consistent and testable\n\n>> and I wouldn't necessarily die() on error. \n\nMM> Sure, die()ing in a library is bad.\n\n>> Maybe this can be merged with the netrc credential helper's\n>> read_credential_data_from_stdin() and print_credential_data()?\n\nMM> I don't know about the netrc credential helper, but I guess that's\nMM> another layer. The git-remote-mediawiki code is the code to call the\nMM> credential C API, that in turn may (or may not) call a credential\nMM> helper.\n\nYup.  But what you call \"read\" and \"write\" are, to the credential\nhelper, \"write\" and \"read\" but it's the same protocol :)  So maybe the\nnames should be changed to reflect that, e.g. \"query\" and \"response.\"\n\nMM> One thing to be careful about: git-remote-mediawiki is currently a\nMM> standalone script, so it can be installed with a plain \"cp\nMM> git-remote-mediawiki $somewhere/\".  One consequence of libification\nMM> is that it adds a dependency on the library (e.g. Git.pm). We should\nMM> be carefull to keep it easy for the user to install it (e.g. some\nMM> kind of \"make install\", or update the doc).\n\nI don't know--it's up to the `git-remote-mediawiki' maintainers...  But\nI think anywhere you have Git, you also have Git.pm, right?  Maybe?  But\nthen you also have to look at whether Git.pm has the functionality you\nneed... so I better go quiet :)\n\nTed\n"},{"id":"208818","messageId":"7vvca5mmmt.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"871ucto4vj.fsf_-_@lifelogs.com","subject":"Re: CodingGuidelines Perl amendment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T16:29:30Z","receivedAt":"2013-02-06T16:29:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ted Zlatanov <tzz@lifelogs.com> writes:\n\n>  - As in C (see above), we avoid using braces unnecessarily (but Perl\n>    forces braces around if/unless/else/foreach blocks, so this is not\n>    always possible).\n\nIs it ever (as opposed to \"not always\") possible to omit braces?\n\nIt sounds as if we encourage the use of statement modifiers, which\ncertainly is not what I want to see.\n\nYou probably would want to mention that opening braces for\n\"if/else/elsif\" do not sit on their own line, and closing braces for\nthem will be followed the next \"else/elseif\" on the same line\ninstead, but that is part of \"most of the C guidelines above apply\"\nso it may be redundant.\n\n>  - Don't abuse statement modifiers (unless $youmust).\n\nIt does not make a useful guidance to leave $youmust part\nunspecified.\n\nIncidentally, your sentence is a good example of where use of\nstatement modifiers is appropriate: $youmust is rarely true.\n\nIn general:\n\n\t... do something ...\n\tdo_this() unless (condition);\n        ... do something else ...\n\nis easier to follow the flow of the logic than\n\n\t... do something ...\n\tunless (condition) {\n\t\tdo_this();\n\t}\n        ... do something else ...\n\n*only* when condition is extremely rare, iow, when do_this() is\nexpected to be almost always called.\n"},{"id":"208819","messageId":"vpqobfxwg2q.fsf@grenoble-inp.fr","threadId":"32770","inReplyTo":"87sj59mo2y.fsf@lifelogs.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-02-06T16:41:01Z","receivedAt":"2013-02-06T16:41:01Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ted Zlatanov <tzz@lifelogs.com> writes:\n\n> None of these are a big deal, and Michal said he's working on libifying\n> this anyhow:\n>\n> - making 'fill' a special operation is weird\n\nWell, 'fill' is the only operation that mutates the credential structure\n(i.e. the only one for which \"git credential\" emits an output to be\nparsed), so you don't have much choice.\n\n> - anchor the key regex to beginning of line (not strictly necessary)\n\nRight. The greedyness of * ensures correction, but I like explicit\nanchors ^...$ too.\n\n> - sort the output tokens (after 'url' is extracted) so the output is consistent and testable\n\nWhy not, if you want to use the output of credential_write in tests. But\ncredential_write is essentially used to talk to \"git credential\", so the\nimportant information is the content of the hash before credential_write\nand after credential_read. They are unordered, but consistent and\ntestable.\n\n>>> Maybe this can be merged with the netrc credential helper's\n>>> read_credential_data_from_stdin() and print_credential_data()?\n>\n> MM> I don't know about the netrc credential helper, but I guess that's\n> MM> another layer. The git-remote-mediawiki code is the code to call the\n> MM> credential C API, that in turn may (or may not) call a credential\n> MM> helper.\n>\n> Yup.  But what you call \"read\" and \"write\" are, to the credential\n> helper, \"write\" and \"read\" but it's the same protocol :)  So maybe the\n> names should be changed to reflect that, e.g. \"query\" and \"response.\"\n\nI don't think that would be a better naming. Maybe \"serialize\" and\n\"parse\" would be better, but \"query\" would sound like it establishes the\nconnection and possibly reads the response to me.\n\n> MM> One thing to be careful about: git-remote-mediawiki is currently a\n> MM> standalone script, so it can be installed with a plain \"cp\n> MM> git-remote-mediawiki $somewhere/\".  One consequence of libification\n> MM> is that it adds a dependency on the library (e.g. Git.pm). We should\n> MM> be carefull to keep it easy for the user to install it (e.g. some\n> MM> kind of \"make install\", or update the doc).\n>\n> I don't know--it's up to the `git-remote-mediawiki' maintainers...\n\nThat is, me ;-).\n\n> But I think anywhere you have Git, you also have Git.pm, right?\n\nYes, but you have to find out where it is installed. Git's Makefile\nhardcodes the path to Git.pm at build time, inserting one line in the\nperl script:\n\nuse lib (split(/:/, $ENV{GITPERLLIB} || \"$INSTLIBDIR\"));\n\nThe same needs to be done for git-remote-mediawiki. As much as possible,\nI'd rather avoid copy-pasting from Git's Makefile, so this means\nextracting the perl part of Git's Makefile and make it available in\ncontrib/.\n\nI'll try a patch in this direction.\n\n> Maybe? But then you also have to look at whether Git.pm has the\n> functionality you need...\n\nIf git-remote-mediawiki is installed from Git's source, I think it's OK\nto assume that Git.pm will be up to date, but that would be even better\nif we can issue a clean error message when the functions to be called do\nnot exist.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"208823","messageId":"874nhpibn0.fsf@lifelogs.com","threadId":"32770","inReplyTo":"vpqobfxwg2q.fsf@grenoble-inp.fr","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T17:40:35Z","receivedAt":"2013-02-06T17:40:35Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Wed, 06 Feb 2013 17:41:01 +0100 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote: \n\nMM> Ted Zlatanov <tzz@lifelogs.com> writes:\n>> - sort the output tokens (after 'url' is extracted) so the output is consistent and testable\n\nMM> Why not, if you want to use the output of credential_write in tests. But\nMM> credential_write is essentially used to talk to \"git credential\", so the\nMM> important information is the content of the hash before credential_write\nMM> and after credential_read. They are unordered, but consistent and\nMM> testable.\n\nI like testing output (especially when it's part of an API), so we\nshould make the externally observable output consistent and testable.\n\nThe change is tiny, just sort the keys instead of calling each(), so I\nhope it makes it in the final version.\n\n>> Yup.  But what you call \"read\" and \"write\" are, to the credential\n>> helper, \"write\" and \"read\" but it's the same protocol :)  So maybe the\n>> names should be changed to reflect that, e.g. \"query\" and \"response.\"\n\nMM> I don't think that would be a better naming. Maybe \"serialize\" and\nMM> \"parse\" would be better, but \"query\" would sound like it establishes the\nMM> connection and possibly reads the response to me.\n\nI'm OK with anything unambiguous.\n\nThanks!\nTed\n"},{"id":"208824","messageId":"CANgJU+V5bhdpN_kWxQPEJgx24LXLtQJWRbnHwkSgm9zFwzm+fA@mail.gmail.com","threadId":"32770","inReplyTo":"7vvca5mmmt.fsf@alter.siamese.dyndns.org","subject":"Re: CodingGuidelines Perl amendment","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2013-02-06T17:45:56Z","receivedAt":"2013-02-06T17:45:56Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 6 February 2013 17:29, Junio C Hamano <gitster@pobox.com> wrote:\n> Ted Zlatanov <tzz@lifelogs.com> writes:\n>\n>>  - As in C (see above), we avoid using braces unnecessarily (but Perl\n>>    forces braces around if/unless/else/foreach blocks, so this is not\n>>    always possible).\n>\n> Is it ever (as opposed to \"not always\") possible to omit braces?\n\nOnly in a statement modifier.\n\n> It sounds as if we encourage the use of statement modifiers, which\n> certainly is not what I want to see.\n\nAs you mention below statement modifiers have their place. For instance\n\n  next if $whatever;\n\nIs considered preferable to\n\nif ($whatever) {\n  next;\n}\n\nSimilarly\n\nopen my $fh, \">\", $filename\n   or die \"Failed to open '$filename': $!\";\n\nIs considered preferable by most Perl programmers to:\n\nmy $fh;\nif ( not open $fh, \">\", $filename ) {\n  die \"Failed to open '$filename': $!\";\n}\n\n> You probably would want to mention that opening braces for\n> \"if/else/elsif\" do not sit on their own line,\n> and closing braces for\n> them will be followed the next \"else/elseif\" on the same line\n> instead, but that is part of \"most of the C guidelines above apply\"\n> so it may be redundant.\n>\n>>  - Don't abuse statement modifiers (unless $youmust).\n>\n> It does not make a useful guidance to leave $youmust part\n> unspecified.\n>\n> Incidentally, your sentence is a good example of where use of\n> statement modifiers is appropriate: $youmust is rarely true.\n\n\"unless\" often leads to maintenance errors as the expression gets more\ncomplicated over time, more branches need to be added to the\nstatement, etc. Basically people are bad at doing De Morgans law in\ntheir head.\n\n> In general:\n>\n>         ... do something ...\n>         do_this() unless (condition);\n>         ... do something else ...\n>\n> is easier to follow the flow of the logic than\n>\n>         ... do something ...\n>         unless (condition) {\n>                 do_this();\n>         }\n>         ... do something else ...\n>\n> *only* when condition is extremely rare, iow, when do_this() is\n> expected to be almost always called.\n\nif (not $condition) {\n  do_this();\n}\n\nIs much less error prone in terms of maintenance than\n\nunless ($condition) {\n  do_this();\n}\n\nSimilarly\n\ndo_this() if not $condition;\n\nleads to less maintenance errors than\n\ndo_this() unless $condition;\n\nSo if you objective is maintainability I would just ban \"unless\" outright.\n\nCheers,\nYves\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"208825","messageId":"87zjzhgwef.fsf_-_@lifelogs.com","threadId":"32770","inReplyTo":"7vvca5mmmt.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Update CodingGuidelines for Perl 5","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T17:55:04Z","receivedAt":"2013-02-06T17:55:04Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"Update the coding guidelines for Perl 5.\n\nSigned-off-by: Ted Zlatanov <tzz@lifelogs.com>\n---\n Documentation/CodingGuidelines |   44 ++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 44 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 1d7de5f..951d74c 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -179,6 +179,50 @@ For C programs:\n  - Use Git's gettext wrappers to make the user interface\n    translatable. See \"Marking strings for translation\" in po/README.\n \n+For Perl 5 programs:\n+\n+ - Most of the C guidelines above apply.\n+\n+ - We try to support Perl 5.8 and later (\"use Perl 5.008\").\n+\n+ - use strict and use warnings are strongly preferred.\n+\n+ - As in C (see above), we avoid using braces unnecessarily (but Perl forces\n+   braces around if/unless/else/foreach blocks, so this is not always possible).\n+   At least make sure braces do not sit on their own line, like with C.\n+\n+ - Don't abuse statement modifiers--they are discouraged.  But in general:\n+\n+\t... do something ...\n+\tdo_this() unless (condition);\n+        ... do something else ...\n+\n+   should be used instead of\n+\n+\t... do something ...\n+\tunless (condition) {\n+\t\tdo_this();\n+\t}\n+        ... do something else ...\n+\n+   *only* when when the condition is so rare that do_this() will be called\n+   almost always.\n+\n+ - We try to avoid assignments inside if().\n+\n+ - Learn and use Git.pm if you need that functionality.\n+\n+ - For Emacs, it's useful to put the following in\n+   GIT_CHECKOUT/.dir-locals.el, assuming you use cperl-mode:\n+\n+    ;; note the first part is useful for C editing, too\n+    ((nil . ((indent-tabs-mode . t)\n+                  (tab-width . 8)\n+                  (fill-column . 80)))\n+     (cperl-mode . ((cperl-indent-level . 8)\n+                    (cperl-extra-newline-before-brace . nil)\n+                    (cperl-merge-trailing-else . t))))\n+\n Writing Documentation:\n \n  Every user-visible change should be reflected in the documentation.\n-- \n1.7.9.rc2\n"},{"id":"208826","messageId":"87vca5gvx6.fsf@lifelogs.com","threadId":"32770","inReplyTo":"7vvca5mmmt.fsf@alter.siamese.dyndns.org","subject":"Re: CodingGuidelines Perl amendment","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T18:05:25Z","receivedAt":"2013-02-06T18:05:25Z","isPatch":false,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Wed, 06 Feb 2013 08:29:30 -0800 Junio C Hamano <gitster@pobox.com> wrote: \n\nJCH> Is it ever (as opposed to \"not always\") possible to omit braces?\n\nOh yes!  Not that I recommend it, and I'm not even going to touch on\nPerl Golf :)\n\nJCH> It sounds as if we encourage the use of statement modifiers, which\nJCH> certainly is not what I want to see.\n\nYup.  I think I captured that in the patch, but please feel free to\nrevise it after applying or throw it back to me.\n\nJCH> You probably would want to mention that opening braces for\nJCH> \"if/else/elsif\" do not sit on their own line, and closing braces for\nJCH> them will be followed the next \"else/elseif\" on the same line\nJCH> instead, but that is part of \"most of the C guidelines above apply\"\nJCH> so it may be redundant.\n\nOK; done.\n\n>> - Don't abuse statement modifiers (unless $youmust).\n\nJCH> It does not make a useful guidance to leave $youmust part\nJCH> unspecified.\n\nJCH> Incidentally, your sentence is a good example of where use of\nJCH> statement modifiers is appropriate: $youmust is rarely true.\n\nI was trying to be funny, honestly.  But OK; reworded.\n\nTed\n"},{"id":"208827","messageId":"87r4ktgvsj.fsf@lifelogs.com","threadId":"32770","inReplyTo":"CANgJU+V5bhdpN_kWxQPEJgx24LXLtQJWRbnHwkSgm9zFwzm+fA@mail.gmail.com","subject":"Re: CodingGuidelines Perl amendment","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T18:08:12Z","receivedAt":"2013-02-06T18:08:12Z","isPatch":false,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Wed, 6 Feb 2013 18:45:56 +0100 demerphq <demerphq@gmail.com> wrote: \n\nd> So if you objective is maintainability I would just ban \"unless\" outright.\n\nPlease consider me opposed to such a ban.\n\nTed\n"},{"id":"208832","messageId":"1360174292-14793-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"32770","inReplyTo":"vpqobfxwg2q.fsf@grenoble-inp.fr","subject":"[PATCH 0/4] Allow contrib/ to use Git's Makefile for perl code","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-02-06T18:11:28Z","receivedAt":"2013-02-06T18:11:28Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The very final goal is to be able to move painlessly (credential) code\nfrom git-remote-mediawiki to Git.pm, but then it's nice for the user\nto be able to say just \"cd contrib/mw-to-git && make install\" and let\nthe Makefile set perl's library path just like other Git commands\nwritten in perl.\n\nThis series does this while trying to minimize code duplication, and\nto make it easy for future other tools in contrib to do the same.\n\nMatthieu Moy (4):\n  Makefile: extract perl-related rules to make them available from other\n    dirs\n  perl.mak: introduce $(GIT_ROOT_DIR) to allow inclusion from other\n    directories\n  Makefile: factor common configuration in git-default-config.mak\n  git-remote-mediawiki: use Git's Makefile to build the script\n\n Makefile                                           | 108 +--------------------\n contrib/mw-to-git/.gitignore                       |   1 +\n contrib/mw-to-git/Makefile                         |  45 ++++++---\n ...-remote-mediawiki => git-remote-mediawiki.perl} |   0\n default-config.mak                                 |  61 ++++++++++++\n perl.mak                                           |  52 ++++++++++\n 6 files changed, 145 insertions(+), 122 deletions(-)\n create mode 100644 contrib/mw-to-git/.gitignore\n rename contrib/mw-to-git/{git-remote-mediawiki => git-remote-mediawiki.perl} (100%)\n create mode 100644 default-config.mak\n create mode 100644 perl.mak\n\n-- \n1.8.1.2.526.gf51a757\n"},{"id":"208831","messageId":"1360174292-14793-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"32770","inReplyTo":"1360174292-14793-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 1/4] Makefile: extract perl-related rules to make them available from other dirs","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-02-06T18:11:29Z","receivedAt":"2013-02-06T18:11:29Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The final goal is to make it easy to write Git commands in perl in the\ncontrib/ directory. It is currently possible to do so, but without the\nbenefits of Git's Makefile: adapt first line with $(PERL_PATH),\nhardcode the path to Git.pm, ...\n\nWe make the perl-related part of the Makefile available from directories\nother than the toplevel so that:\n\n* Developers can include it, to avoid code duplication\n\n* Users can get a consistent behavior of \"make install\"\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n Makefile | 46 +---------------------------------------------\n perl.mak | 49 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 50 insertions(+), 45 deletions(-)\n create mode 100644 perl.mak\n\ndiff --git a/Makefile b/Makefile\nindex 731b6a8..f39d4a9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -573,14 +573,10 @@ BINDIR_PROGRAMS_NO_X += git-cvsserver\n ifndef SHELL_PATH\n \tSHELL_PATH = /bin/sh\n endif\n-ifndef PERL_PATH\n-\tPERL_PATH = /usr/bin/perl\n-endif\n ifndef PYTHON_PATH\n \tPYTHON_PATH = /usr/bin/python\n endif\n \n-export PERL_PATH\n export PYTHON_PATH\n \n LIB_FILE = libgit.a\n@@ -1441,10 +1437,6 @@ ifeq ($(TCLTK_PATH),)\n NO_TCLTK = NoThanks\n endif\n \n-ifeq ($(PERL_PATH),)\n-NO_PERL = NoThanks\n-endif\n-\n ifeq ($(PYTHON_PATH),)\n NO_PYTHON = NoThanks\n endif\n@@ -1522,7 +1514,6 @@ prefix_SQ = $(subst ','\\'',$(prefix))\n gitwebdir_SQ = $(subst ','\\'',$(gitwebdir))\n \n SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n-PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))\n PYTHON_PATH_SQ = $(subst ','\\'',$(PYTHON_PATH))\n TCLTK_PATH_SQ = $(subst ','\\'',$(TCLTK_PATH))\n DIFF_SQ = $(subst ','\\'',$(DIFF))\n@@ -1715,9 +1706,6 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tmv $@+ $@\n \n-ifndef NO_PERL\n-$(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n-\n perl/perl.mak: perl/PM.stamp\n \n perl/PM.stamp: FORCE\n@@ -1728,39 +1716,7 @@ perl/PM.stamp: FORCE\n perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL\n \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n \n-$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE\n-\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \\\n-\tsed -e '1{' \\\n-\t    -e '\ts|#!.*perl|#!$(PERL_PATH_SQ)|' \\\n-\t    -e '\th' \\\n-\t    -e '\ts=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || \"'\"$$INSTLIBDIR\"'\"));=' \\\n-\t    -e '\tH' \\\n-\t    -e '\tx' \\\n-\t    -e '}' \\\n-\t    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n-\t    $@.perl >$@+ && \\\n-\tchmod +x $@+ && \\\n-\tmv $@+ $@\n-\n-\n-.PHONY: gitweb\n-gitweb:\n-\t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all\n-\n-git-instaweb: git-instaweb.sh gitweb GIT-SCRIPT-DEFINES\n-\t$(QUIET_GEN)$(cmd_munge_script) && \\\n-\tchmod +x $@+ && \\\n-\tmv $@+ $@\n-else # NO_PERL\n-$(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh\n-\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n-\t    -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \\\n-\t    unimplemented.sh >$@+ && \\\n-\tchmod +x $@+ && \\\n-\tmv $@+ $@\n-endif # NO_PERL\n+include perl.mak\n \n ifndef NO_PYTHON\n $(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS\ndiff --git a/perl.mak b/perl.mak\nnew file mode 100644\nindex 0000000..8bbeef3\n--- /dev/null\n+++ b/perl.mak\n@@ -0,0 +1,49 @@\n+# Rules to build Git commands written in perl\n+\n+ifndef PERL_PATH\n+\tPERL_PATH = /usr/bin/perl\n+endif\n+export PERL_PATH\n+PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))\n+\n+ifeq ($(PERL_PATH),)\n+NO_PERL = NoThanks\n+endif\n+\n+ifndef NO_PERL\n+$(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n+\n+\n+$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE\n+\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n+\tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \\\n+\tsed -e '1{' \\\n+\t    -e '\ts|#!.*perl|#!$(PERL_PATH_SQ)|' \\\n+\t    -e '\th' \\\n+\t    -e '\ts=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || \"'\"$$INSTLIBDIR\"'\"));=' \\\n+\t    -e '\tH' \\\n+\t    -e '\tx' \\\n+\t    -e '}' \\\n+\t    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n+\t    $@.perl >$@+ && \\\n+\tchmod +x $@+ && \\\n+\tmv $@+ $@\n+\n+\n+.PHONY: gitweb\n+gitweb:\n+\t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all\n+\n+git-instaweb: git-instaweb.sh gitweb GIT-SCRIPT-DEFINES\n+\t$(QUIET_GEN)$(cmd_munge_script) && \\\n+\tchmod +x $@+ && \\\n+\tmv $@+ $@\n+else # NO_PERL\n+$(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh\n+\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n+\tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n+\t    -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \\\n+\t    unimplemented.sh >$@+ && \\\n+\tchmod +x $@+ && \\\n+\tmv $@+ $@\n+endif # NO_PERL\n-- \n1.8.1.2.526.gf51a757\n"},{"id":"208829","messageId":"1360174292-14793-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"32770","inReplyTo":"1360174292-14793-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/4] perl.mak: introduce $(GIT_ROOT_DIR) to allow inclusion from other directories","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-02-06T18:11:30Z","receivedAt":"2013-02-06T18:11:30Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"perl.mak uses relative path, which is OK when called from the toplevel,\nbut won't be anymore if one includes it from elsewhere. It is now\npossible to include the file using:\n\nGIT_ROOT_DIR=<whatever>\ninclude $(GIT_ROOT_DIR)/perl.mak\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n perl.mak | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/perl.mak b/perl.mak\nindex 8bbeef3..a2b8717 100644\n--- a/perl.mak\n+++ b/perl.mak\n@@ -1,5 +1,9 @@\n # Rules to build Git commands written in perl\n \n+ifndef GIT_ROOT_DIR\n+\tGIT_ROOT_DIR = .\n+endif\n+\n ifndef PERL_PATH\n \tPERL_PATH = /usr/bin/perl\n endif\n@@ -11,12 +15,11 @@ NO_PERL = NoThanks\n endif\n \n ifndef NO_PERL\n-$(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n-\n+$(patsubst %.perl,%,$(SCRIPT_PERL)): $(GIT_ROOT_DIR)/perl/perl.mak\n \n-$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE\n+$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl $(GIT_ROOT_DIR)/GIT-VERSION-FILE\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \\\n+\tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C $(GIT_ROOT_DIR)/perl -s --no-print-directory instlibdir` && \\\n \tsed -e '1{' \\\n \t    -e '\ts|#!.*perl|#!$(PERL_PATH_SQ)|' \\\n \t    -e '\th' \\\n-- \n1.8.1.2.526.gf51a757\n"},{"id":"208830","messageId":"1360174292-14793-4-git-send-email-Matthieu.Moy@imag.fr","threadId":"32770","inReplyTo":"1360174292-14793-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 3/4] Makefile: factor common configuration in git-default-config.mak","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-02-06T18:11:31Z","receivedAt":"2013-02-06T18:11:31Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Similarly to the extraction of perl-related code in perl.mak, we extract\ngeneral default configuration from the Makefile to make it available from\ndirectories other than the toplevel.\n\nThis is required to make perl.mak usable because it requires $(pathsep)\nto be set.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n Makefile           | 62 +-----------------------------------------------------\n default-config.mak | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 62 insertions(+), 61 deletions(-)\n create mode 100644 default-config.mak\n\ndiff --git a/Makefile b/Makefile\nindex f39d4a9..9649a41 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -346,67 +346,7 @@ GIT-VERSION-FILE: FORCE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n -include GIT-VERSION-FILE\n \n-# CFLAGS and LDFLAGS are for the users to override from the command line.\n-\n-CFLAGS = -g -O2 -Wall\n-LDFLAGS =\n-ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n-ALL_LDFLAGS = $(LDFLAGS)\n-STRIP ?= strip\n-\n-# Among the variables below, these:\n-#   gitexecdir\n-#   template_dir\n-#   mandir\n-#   infodir\n-#   htmldir\n-#   sysconfdir\n-# can be specified as a relative path some/where/else;\n-# this is interpreted as relative to $(prefix) and \"git\" at\n-# runtime figures out where they are based on the path to the executable.\n-# This can help installing the suite in a relocatable way.\n-\n-prefix = $(HOME)\n-bindir_relative = bin\n-bindir = $(prefix)/$(bindir_relative)\n-mandir = share/man\n-infodir = share/info\n-gitexecdir = libexec/git-core\n-mergetoolsdir = $(gitexecdir)/mergetools\n-sharedir = $(prefix)/share\n-gitwebdir = $(sharedir)/gitweb\n-localedir = $(sharedir)/locale\n-template_dir = share/git-core/templates\n-htmldir = share/doc/git-doc\n-ETC_GITCONFIG = $(sysconfdir)/gitconfig\n-ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes\n-lib = lib\n-# DESTDIR =\n-pathsep = :\n-\n-export prefix bindir sharedir sysconfdir gitwebdir localedir\n-\n-CC = cc\n-AR = ar\n-RM = rm -f\n-DIFF = diff\n-TAR = tar\n-FIND = find\n-INSTALL = install\n-RPMBUILD = rpmbuild\n-TCL_PATH = tclsh\n-TCLTK_PATH = wish\n-XGETTEXT = xgettext\n-MSGFMT = msgfmt\n-PTHREAD_LIBS = -lpthread\n-PTHREAD_CFLAGS =\n-GCOV = gcov\n-\n-export TCL_PATH TCLTK_PATH\n-\n-SPARSE_FLAGS =\n-\n-\n+include default-config.mak\n \n ### --- END CONFIGURATION SECTION ---\n \ndiff --git a/default-config.mak b/default-config.mak\nnew file mode 100644\nindex 0000000..b2aab3d\n--- /dev/null\n+++ b/default-config.mak\n@@ -0,0 +1,61 @@\n+# CFLAGS and LDFLAGS are for the users to override from the command line.\n+\n+CFLAGS = -g -O2 -Wall\n+LDFLAGS =\n+ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n+ALL_LDFLAGS = $(LDFLAGS)\n+STRIP ?= strip\n+\n+# Among the variables below, these:\n+#   gitexecdir\n+#   template_dir\n+#   mandir\n+#   infodir\n+#   htmldir\n+#   sysconfdir\n+# can be specified as a relative path some/where/else;\n+# this is interpreted as relative to $(prefix) and \"git\" at\n+# runtime figures out where they are based on the path to the executable.\n+# This can help installing the suite in a relocatable way.\n+\n+prefix = $(HOME)\n+bindir_relative = bin\n+bindir = $(prefix)/$(bindir_relative)\n+mandir = share/man\n+infodir = share/info\n+gitexecdir = libexec/git-core\n+mergetoolsdir = $(gitexecdir)/mergetools\n+sharedir = $(prefix)/share\n+gitwebdir = $(sharedir)/gitweb\n+localedir = $(sharedir)/locale\n+template_dir = share/git-core/templates\n+htmldir = share/doc/git-doc\n+ETC_GITCONFIG = $(sysconfdir)/gitconfig\n+ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes\n+lib = lib\n+# DESTDIR =\n+pathsep = :\n+\n+export prefix bindir sharedir sysconfdir gitwebdir localedir\n+\n+CC = cc\n+AR = ar\n+RM = rm -f\n+DIFF = diff\n+TAR = tar\n+FIND = find\n+INSTALL = install\n+RPMBUILD = rpmbuild\n+TCL_PATH = tclsh\n+TCLTK_PATH = wish\n+XGETTEXT = xgettext\n+MSGFMT = msgfmt\n+PTHREAD_LIBS = -lpthread\n+PTHREAD_CFLAGS =\n+GCOV = gcov\n+\n+export TCL_PATH TCLTK_PATH\n+\n+SPARSE_FLAGS =\n+\n+\n-- \n1.8.1.2.526.gf51a757\n"},{"id":"208833","messageId":"1360174292-14793-5-git-send-email-Matthieu.Moy@imag.fr","threadId":"32770","inReplyTo":"1360174292-14793-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-02-06T18:11:32Z","receivedAt":"2013-02-06T18:11:32Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The configuration of the install directory is not reused from the\ntoplevel Makefile: we assume Git is already built, hence just call\n\"git --exec-path\". This avoids too much surgery in the toplevel Makefile.\n\ngit-remote-mediawiki.perl can now \"use Git;\".\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n contrib/mw-to-git/.gitignore                       |  1 +\n contrib/mw-to-git/Makefile                         | 45 ++++++++++++++--------\n ...-remote-mediawiki => git-remote-mediawiki.perl} |  0\n 3 files changed, 30 insertions(+), 16 deletions(-)\n create mode 100644 contrib/mw-to-git/.gitignore\n rename contrib/mw-to-git/{git-remote-mediawiki => git-remote-mediawiki.perl} (100%)\n\ndiff --git a/contrib/mw-to-git/.gitignore b/contrib/mw-to-git/.gitignore\nnew file mode 100644\nindex 0000000..b919655\n--- /dev/null\n+++ b/contrib/mw-to-git/.gitignore\n@@ -0,0 +1 @@\n+git-remote-mediawiki\ndiff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile\nindex 3ed728b..ed8073b 100644\n--- a/contrib/mw-to-git/Makefile\n+++ b/contrib/mw-to-git/Makefile\n@@ -8,40 +8,53 @@\n #\n ## Build git-remote-mediawiki\n \n--include ../../config.mak.autogen\n--include ../../config.mak\n+all:\n+\n+GIT_ROOT_DIR=../../\n+include $(GIT_ROOT_DIR)/default-config.mak\n+-include $(GIT_ROOT_DIR)/config.mak.autogen\n+-include $(GIT_ROOT_DIR)/config.mak\n+-include $(GIT_ROOT_DIR)/GIT-VERSION-FILE\n+\n+\n+SCRIPT_PERL = git-remote-mediawiki.perl\n+ALL_PROGRAMS = $(patsubst %.perl,%,$(SCRIPT_PERL))\n+\n+include $(GIT_ROOT_DIR)/perl.mak\n \n-ifndef PERL_PATH\n-\tPERL_PATH = /usr/bin/perl\n-endif\n ifndef gitexecdir\n \tgitexecdir = $(shell git --exec-path)\n endif\n \n-PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))\n-gitexecdir_SQ = $(subst ','\\'',$(gitexecdir))\n-SCRIPT = git-remote-mediawiki\n+ifneq ($(filter /%,$(firstword $(gitexecdir))),)\n+gitexec_instdir = $(gitexecdir)\n+else\n+gitexec_instdir = $(prefix)/$(gitexecdir)\n+endif\n+gitexec_instdir_SQ = $(subst ','\\'',$(gitexec_instdir))\n \n .PHONY: install help doc test clean\n \n help:\n \t@echo 'This is the help target of the Makefile. Current configuration:'\n-\t@echo '  gitexecdir = $(gitexecdir_SQ)'\n+\t@echo '  gitexec_instdir = $(gitexec_instdir_SQ)'\n \t@echo '  PERL_PATH = $(PERL_PATH_SQ)'\n-\t@echo 'Run \"$(MAKE) install\" to install $(SCRIPT) in gitexecdir'\n+\t@echo 'Run \"$(MAKE) all\" to build the script'\n+\t@echo 'Run \"$(MAKE) install\" to install $(ALL_PROGRAMS) in gitexec_instdir'\n \t@echo 'Run \"$(MAKE) test\" to run the testsuite'\n \n-install:\n-\tsed -e '1s|#!.*/perl|#!$(PERL_PATH_SQ)|' $(SCRIPT) \\\n-\t\t> '$(gitexecdir_SQ)/$(SCRIPT)'\n-\tchmod +x '$(gitexecdir)/$(SCRIPT)'\n+all: $(ALL_PROGRAMS)\n+\n+install: $(ALL_PROGRAMS)\n+\t$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \n doc:\n-\t@echo 'Sorry, \"make doc\" is not implemented yet for $(SCRIPT)'\n+\t@echo 'Sorry, \"make doc\" is not implemented yet for $(ALL_PROGRAMS)'\n \n test:\n \t$(MAKE) -C t/ test\n \n clean:\n-\t$(RM) '$(gitexecdir)/$(SCRIPT)'\n+\t$(RM) $(ALL_PROGRAMS)\n+\t$(RM) $(patsubst %,$(gitexec_instdir)/%,/$(ALL_PROGRAMS))\n \t$(MAKE) -C t/ clean\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki.perl\nsimilarity index 100%\nrename from contrib/mw-to-git/git-remote-mediawiki\nrename to contrib/mw-to-git/git-remote-mediawiki.perl\n-- \n1.8.1.2.526.gf51a757\n"},{"id":"208834","messageId":"7vip65cnt3.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"CANgJU+V5bhdpN_kWxQPEJgx24LXLtQJWRbnHwkSgm9zFwzm+fA@mail.gmail.com","subject":"Re: CodingGuidelines Perl amendment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T18:14:16Z","receivedAt":"2013-02-06T18:14:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"demerphq <demerphq@gmail.com> writes:\n\n> As you mention below statement modifiers have their place. For instance\n>\n>   next if $whatever;\n>\n> Is considered preferable to\n>\n> if ($whatever) {\n>   next;\n> }\n>\n> Similarly\n>\n> open my $fh, \">\", $filename\n>    or die \"Failed to open '$filename': $!\";\n>\n> Is considered preferable by most Perl programmers to:\n>\n> my $fh;\n> if ( not open $fh, \">\", $filename ) {\n>   die \"Failed to open '$filename': $!\";\n> }\n\nYeah, and that is for the same reason.  When you are trying to get a\nbirds-eye view of the codeflow, the former makes it clear that \"we\ndo something, and then we open, and then we ...\", without letting\nthe error handling (which also is rare case) distract us.\n\n> \"unless\" often leads to maintenance errors as the expression gets more\n> complicated over time,...\n\nThat might also be true, but my comment was not an endorsement for\n(or suggestion against) use of unless.  I was commenting on\nstatement modifiers, which some people tend to overuse (or abuse)\nand make the resulting code harder to follow.\n"},{"id":"208835","messageId":"7vehgtcnpm.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"87vca5gvx6.fsf@lifelogs.com","subject":"Re: CodingGuidelines Perl amendment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T18:16:21Z","receivedAt":"2013-02-06T18:16:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ted Zlatanov <tzz@lifelogs.com> writes:\n\n> On Wed, 06 Feb 2013 08:29:30 -0800 Junio C Hamano <gitster@pobox.com> wrote: \n>\n> JCH> Is it ever (as opposed to \"not always\") possible to omit braces?\n>\n> Oh yes!  Not that I recommend it, and I'm not even going to touch on\n> Perl Golf :)\n>\n> JCH> It sounds as if we encourage the use of statement modifiers, which\n> JCH> certainly is not what I want to see.\n>\n> Yup.  I think I captured that in the patch, but please feel free to\n> revise it after applying or throw it back to me.\n\nI'd suggest to just drop that \"try to write without braces\" entirely.\n\n> JCH> Incidentally, your sentence is a good example of where use of\n> JCH> statement modifiers is appropriate: $youmust is rarely true.\n>\n> I was trying to be funny, honestly.  But OK; reworded.\n\nIt wasn't a useful guidance, but it _was_ funny.  \n"},{"id":"208837","messageId":"CANgJU+Waa2WFGEQz=UmQkS+CRjq94CTeQtobaY=EMiveC_sMww@mail.gmail.com","threadId":"32770","inReplyTo":"7vip65cnt3.fsf@alter.siamese.dyndns.org","subject":"Re: CodingGuidelines Perl amendment","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2013-02-06T18:18:41Z","receivedAt":"2013-02-06T18:18:41Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 6 February 2013 19:14, Junio C Hamano <gitster@pobox.com> wrote:\n> demerphq <demerphq@gmail.com> writes:\n>\n>> As you mention below statement modifiers have their place. For instance\n>>\n>>   next if $whatever;\n>>\n>> Is considered preferable to\n>>\n>> if ($whatever) {\n>>   next;\n>> }\n>>\n>> Similarly\n>>\n>> open my $fh, \">\", $filename\n>>    or die \"Failed to open '$filename': $!\";\n>>\n>> Is considered preferable by most Perl programmers to:\n>>\n>> my $fh;\n>> if ( not open $fh, \">\", $filename ) {\n>>   die \"Failed to open '$filename': $!\";\n>> }\n>\n> Yeah, and that is for the same reason.  When you are trying to get a\n> birds-eye view of the codeflow, the former makes it clear that \"we\n> do something, and then we open, and then we ...\", without letting\n> the error handling (which also is rare case) distract us.\n\nperldoc perlstyle has language which explains this well if you want to\ncrib a description from somewhere.\n\n>> \"unless\" often leads to maintenance errors as the expression gets more\n>> complicated over time,...\n>\n> That might also be true, but my comment was not an endorsement for\n> (or suggestion against) use of unless.  I was commenting on\n> statement modifiers, which some people tend to overuse (or abuse)\n> and make the resulting code harder to follow.\n\nThat's also my point about unless. They tend to get abused and then\nlead to maint devs making errors, and people misunderstanding the\ncode. The only time that unless IMO is \"ok\" (ish) is when it really is\na very simple statement. As soon as it mentions more than one var it\nshould be converted to an if. This applies even more so to the\nmodifier form.\n\nYves\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"208838","messageId":"CANgJU+VbkQ+xa+_sSAu-3pMe+6gycHi9J4VR18M5YJt=pa9QUw@mail.gmail.com","threadId":"32770","inReplyTo":"87vca5gvx6.fsf@lifelogs.com","subject":"Re: CodingGuidelines Perl amendment","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2013-02-06T18:25:43Z","receivedAt":"2013-02-06T18:25:43Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 6 February 2013 19:05, Ted Zlatanov <tzz@lifelogs.com> wrote:\n> On Wed, 06 Feb 2013 08:29:30 -0800 Junio C Hamano <gitster@pobox.com> wrote:\n>\n> JCH> Is it ever (as opposed to \"not always\") possible to omit braces?\n>\n> Oh yes!  Not that I recommend it, and I'm not even going to touch on\n> Perl Golf :)\n\nI think you are wrong. Can you provide an example?\n\nLarry specifically wanted to avoid the \"dangling else\" problem that C\nsuffers from, and made it so that blocks are mandatory. The only\nexception is statement modifiers, which are not only allowed to omit\nthe braces but also the parens on the condition.\n\nYves\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"208839","messageId":"87mwvhguwa.fsf@lifelogs.com","threadId":"32770","inReplyTo":"7vehgtcnpm.fsf@alter.siamese.dyndns.org","subject":"Re: CodingGuidelines Perl amendment","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T18:27:33Z","receivedAt":"2013-02-06T18:27:33Z","isPatch":false,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Wed, 06 Feb 2013 10:16:21 -0800 Junio C Hamano <gitster@pobox.com> wrote: \n\nJCH> I'd suggest to just drop that \"try to write without braces\" entirely.\n\nOK, I'll do it on the reroll, or you can just make the change directly.\n\nI agree it was not going anywhere :)\n\nTed\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 951d74c..857f4e2 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -187,10 +187,6 @@ For Perl 5 programs:\n \n  - use strict and use warnings are strongly preferred.\n \n- - As in C (see above), we avoid using braces unnecessarily (but Perl forces\n-   braces around if/unless/else/foreach blocks, so this is not always possible).\n-   At least make sure braces do not sit on their own line, like with C.\n-\n  - Don't abuse statement modifiers--they are discouraged.  But in general:\n \n        ... do something ...\n"},{"id":"208842","messageId":"87ip65guj8.fsf@lifelogs.com","threadId":"32770","inReplyTo":"CANgJU+VbkQ+xa+_sSAu-3pMe+6gycHi9J4VR18M5YJt=pa9QUw@mail.gmail.com","subject":"Re: CodingGuidelines Perl amendment","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T18:35:23Z","receivedAt":"2013-02-06T18:35:23Z","isPatch":false,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Wed, 6 Feb 2013 19:25:43 +0100 demerphq <demerphq@gmail.com> wrote: \n\nd> On 6 February 2013 19:05, Ted Zlatanov <tzz@lifelogs.com> wrote:\n>> On Wed, 06 Feb 2013 08:29:30 -0800 Junio C Hamano <gitster@pobox.com> wrote:\n>> \nJCH> Is it ever (as opposed to \"not always\") possible to omit braces?\n>> \n>> Oh yes!  Not that I recommend it, and I'm not even going to touch on\n>> Perl Golf :)\n\nd> I think you are wrong. Can you provide an example?\n\nd> Larry specifically wanted to avoid the \"dangling else\" problem that C\nd> suffers from, and made it so that blocks are mandatory. The only\nd> exception is statement modifiers, which are not only allowed to omit\nd> the braces but also the parens on the condition.\n\nOh, perhaps I didn't state it correctly.  You can avoid braces, but not\nif you want to use if/elsif/else/unless/etc. which require them:\n\ncondition && do_this();\ncondition || do_this();\ncondition ? do_this() : do_that();\n\n(and others I can't recall right now)\n\nBut my point was only that it's always possible to get around these\nartificial restrictions; it's more important to ask for legible sensible\ncode.  Sorry if that was unclear!\n\nTed\n"},{"id":"208843","messageId":"CANgJU+X=Bb=ncqOxsd1hZDWsnFkt-bJw=Zbtuz8_KC0gO-dLaQ@mail.gmail.com","threadId":"32770","inReplyTo":"87ip65guj8.fsf@lifelogs.com","subject":"Re: CodingGuidelines Perl amendment","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2013-02-06T18:44:16Z","receivedAt":"2013-02-06T18:44:16Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On 6 February 2013 19:35, Ted Zlatanov <tzz@lifelogs.com> wrote:\n> On Wed, 6 Feb 2013 19:25:43 +0100 demerphq <demerphq@gmail.com> wrote:\n>\n> d> On 6 February 2013 19:05, Ted Zlatanov <tzz@lifelogs.com> wrote:\n>>> On Wed, 06 Feb 2013 08:29:30 -0800 Junio C Hamano <gitster@pobox.com> wrote:\n>>>\n> JCH> Is it ever (as opposed to \"not always\") possible to omit braces?\n>>>\n>>> Oh yes!  Not that I recommend it, and I'm not even going to touch on\n>>> Perl Golf :)\n>\n> d> I think you are wrong. Can you provide an example?\n>\n> d> Larry specifically wanted to avoid the \"dangling else\" problem that C\n> d> suffers from, and made it so that blocks are mandatory. The only\n> d> exception is statement modifiers, which are not only allowed to omit\n> d> the braces but also the parens on the condition.\n>\n> Oh, perhaps I didn't state it correctly.  You can avoid braces, but not\n> if you want to use if/elsif/else/unless/etc. which require them:\n>\n> condition && do_this();\n> condition || do_this();\n> condition ? do_this() : do_that();\n>\n> (and others I can't recall right now)\n>\n> But my point was only that it's always possible to get around these\n> artificial restrictions; it's more important to ask for legible sensible\n> code.  Sorry if that was unclear!\n\nAh ok. Right, at a low level:\n\nif (condition) { do_this() }\n\nis identical to\n\ncondition && do_this();\n\nIOW, Perl allows logical operators to act as control flow statements.\n\nI hope your document include something that says that using logical\noperators as control flow statements should be used sparingly, and\ngenerally should be restricted to low precedence operators and should\nnever involve more than one operator.\n\nYves\n\n\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"208844","messageId":"87bobxgtmw.fsf@lifelogs.com","threadId":"32770","inReplyTo":"CANgJU+X=Bb=ncqOxsd1hZDWsnFkt-bJw=Zbtuz8_KC0gO-dLaQ@mail.gmail.com","subject":"Re: CodingGuidelines Perl amendment","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T18:54:47Z","receivedAt":"2013-02-06T18:54:47Z","isPatch":false,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Wed, 6 Feb 2013 19:44:16 +0100 demerphq <demerphq@gmail.com> wrote: \n\nd> Ah ok. Right, at a low level:\n\nd> if (condition) { do_this() }\n\nd> is identical to\n\nd> condition && do_this();\n\nd> IOW, Perl allows logical operators to act as control flow statements.\n\nd> I hope your document include something that says that using logical\nd> operators as control flow statements should be used sparingly, and\nd> generally should be restricted to low precedence operators and should\nd> never involve more than one operator.\n\nI'd stay away from wording it so tightly, but instead just say\n\n\"Make your code readable and sensible, and don't try to be clever.\"\n\nBut this is good C and shell advice too, so I'd put it under \"General\nGuidelines\" and leave it for Junio to decide if it's appropriate.\n\nTed\n"},{"id":"208847","messageId":"7vwqulb5el.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"87bobxgtmw.fsf@lifelogs.com","subject":"Re: CodingGuidelines Perl amendment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-06T19:37:06Z","receivedAt":"2013-02-06T19:37:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ted Zlatanov <tzz@lifelogs.com> writes:\n\n> \"Make your code readable and sensible, and don't try to be clever.\"\n>\n> But this is good C and shell advice too,...\n\nSounds sensible.\n"},{"id":"208850","messageId":"877gmlgr4i.fsf_-_@lifelogs.com","threadId":"32770","inReplyTo":"7vwqulb5el.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] Update CodingGuidelines for Perl 5","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T19:49:01Z","receivedAt":"2013-02-06T19:49:01Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"Update the coding guidelines for Perl 5.\n\nSigned-off-by: Ted Zlatanov <tzz@lifelogs.com>\n---\nChanges since PATCHv1:\n- removed brace guidelines\n- add \"don't try to be clever\" at beginning\n\n Documentation/CodingGuidelines |   42 ++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 42 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 1d7de5f..166c141 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -18,6 +18,8 @@ code.  For Git in general, three rough rules are:\n    judgement call, the decision based more on real world\n    constraints people face than what the paper standard says.\n \n+For any programming language below, make your code readable and sensible, and\n+don't try to be clever.\n \n As for more concrete guidelines, just imitate the existing code\n (this is a good guideline, no matter which project you are\n@@ -179,6 +181,46 @@ For C programs:\n  - Use Git's gettext wrappers to make the user interface\n    translatable. See \"Marking strings for translation\" in po/README.\n \n+For Perl 5 programs:\n+\n+ - Most of the C guidelines above apply.\n+\n+ - We try to support Perl 5.8 and later (\"use Perl 5.008\").\n+\n+ - use strict and use warnings are strongly preferred.\n+\n+ - Don't abuse statement modifiers--they are discouraged.  But in general:\n+\n+\t... do something ...\n+\tdo_this() unless (condition);\n+        ... do something else ...\n+\n+   should be used instead of\n+\n+\t... do something ...\n+\tunless (condition) {\n+\t\tdo_this();\n+\t}\n+        ... do something else ...\n+\n+   *only* when when the condition is so rare that do_this() will be called\n+   almost always.\n+\n+ - We try to avoid assignments inside if().\n+\n+ - Learn and use Git.pm if you need that functionality.\n+\n+ - For Emacs, it's useful to put the following in\n+   GIT_CHECKOUT/.dir-locals.el, assuming you use cperl-mode:\n+\n+    ;; note the first part is useful for C editing, too\n+    ((nil . ((indent-tabs-mode . t)\n+                  (tab-width . 8)\n+                  (fill-column . 80)))\n+     (cperl-mode . ((cperl-indent-level . 8)\n+                    (cperl-extra-newline-before-brace . nil)\n+                    (cperl-merge-trailing-else . t))))\n+\n Writing Documentation:\n \n  Every user-visible change should be reflected in the documentation.\n-- \n1.7.9.rc2\n"},{"id":"208868","messageId":"20130206215724.GA27507@sigill.intra.peff.net","threadId":"32770","inReplyTo":"87sj59mo2y.fsf@lifelogs.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-06T21:57:24Z","receivedAt":"2013-02-06T21:57:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 06, 2013 at 10:58:13AM -0500, Ted Zlatanov wrote:\n\n> MM> I don't know about the netrc credential helper, but I guess that's\n> MM> another layer. The git-remote-mediawiki code is the code to call the\n> MM> credential C API, that in turn may (or may not) call a credential\n> MM> helper.\n> \n> Yup.  But what you call \"read\" and \"write\" are, to the credential\n> helper, \"write\" and \"read\" but it's the same protocol :)  So maybe the\n> names should be changed to reflect that, e.g. \"query\" and \"response.\"\n\nIs that true? As a user of the credential system, git-remote-mediawiki\nwould want to \"write\" to git-credential, then \"read\" the response. As a\nhelper, git-credential-netrc would want to \"read\" the query then\n\"write\" the response. The order is different, but the operations should\nbe the same in both cases.\n\nThe big difference is that mediawiki would want an additional function\nto open a pipe to \"git credential\" and operate on that, whereas the\nhelper will be reading/writing stdio.\n\n-Peff\n"},{"id":"208883","messageId":"8738x9ghpu.fsf@lifelogs.com","threadId":"32770","inReplyTo":"20130206215724.GA27507@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-06T23:12:13Z","receivedAt":"2013-02-06T23:12:13Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Wed, 6 Feb 2013 16:57:24 -0500 Jeff King <peff@peff.net> wrote: \n\nJK> On Wed, Feb 06, 2013 at 10:58:13AM -0500, Ted Zlatanov wrote:\nMM> I don't know about the netrc credential helper, but I guess that's\nMM> another layer. The git-remote-mediawiki code is the code to call the\nMM> credential C API, that in turn may (or may not) call a credential\nMM> helper.\n>> \n>> Yup.  But what you call \"read\" and \"write\" are, to the credential\n>> helper, \"write\" and \"read\" but it's the same protocol :)  So maybe the\n>> names should be changed to reflect that, e.g. \"query\" and \"response.\"\n\nJK> Is that true? As a user of the credential system, git-remote-mediawiki\nJK> would want to \"write\" to git-credential, then \"read\" the response. As a\nJK> helper, git-credential-netrc would want to \"read\" the query then\nJK> \"write\" the response. The order is different, but the operations should\nJK> be the same in both cases.\n\nLogically they are different steps (query and response), even though the\ndata protocol is the same.  But it's really not a big deal, I know what\nit means either way.\n\nJK> The big difference is that mediawiki would want an additional function\nJK> to open a pipe to \"git credential\" and operate on that, whereas the\nJK> helper will be reading/writing stdio.\n\nYup.\n\nTed\n"},{"id":"208896","messageId":"vpq4nhowqgk.fsf@grenoble-inp.fr","threadId":"32770","inReplyTo":"8738x9ghpu.fsf@lifelogs.com","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-02-07T07:08:59Z","receivedAt":"2013-02-07T07:08:59Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ted Zlatanov <tzz@lifelogs.com> writes:\n\n> Logically they are different steps (query and response), even though the\n> data protocol is the same.  But it's really not a big deal, I know what\n> it means either way.\n\nYes, but if you rename write() to query(), then on the helper side,\nyou'll have to call query() to send the response, and response() to read\nthe query. Much worse than keeping read/write.\n\nPlus, read/write has already been used for a while in the C API, so I'd\nrather keep the same names for the Perl equivalent.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"208919","messageId":"87wqukci2v.fsf@lifelogs.com","threadId":"32770","inReplyTo":"vpq4nhowqgk.fsf@grenoble-inp.fr","subject":"Re: [PATCH] git-send-email: add ~/.authinfo parsing","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-02-07T14:30:16Z","receivedAt":"2013-02-07T14:30:16Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Thu, 07 Feb 2013 08:08:59 +0100 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote: \n\nMM> Plus, read/write has already been used for a while in the C API, so I'd\nMM> rather keep the same names for the Perl equivalent.\n\nThat makes perfect sense.\n\nTed\n"},{"id":"208934","messageId":"7v62247x5b.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"1360174292-14793-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 1/4] Makefile: extract perl-related rules to make them available from other dirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-07T19:16:00Z","receivedAt":"2013-02-07T19:16:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> The final goal is to make it easy to write Git commands in perl in the\n> contrib/ directory. It is currently possible to do so, but without the\n> benefits of Git's Makefile: adapt first line with $(PERL_PATH),\n> hardcode the path to Git.pm, ...\n>\n> We make the perl-related part of the Makefile available from directories\n> other than the toplevel so that:\n>\n> * Developers can include it, to avoid code duplication\n>\n> * Users can get a consistent behavior of \"make install\"\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n\nThe goal may be worthy, but the split does not look quite right.\n\nWhat business do contrib/ scripts have knowing how gitweb and\ngit-instaweb are built and what they depend on, for example?\n"},{"id":"208935","messageId":"7vobfv7wkl.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"1360174292-14793-4-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 3/4] Makefile: factor common configuration in git-default-config.mak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-07T19:28:26Z","receivedAt":"2013-02-07T19:28:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> Similarly to the extraction of perl-related code in perl.mak, we extract\n> general default configuration from the Makefile to make it available from\n> directories other than the toplevel.\n>\n> This is required to make perl.mak usable because it requires $(pathsep)\n> to be set.\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n\nI really think this is going in a wrong direction.  Whatever you\nhappen to have chosen in this patch will be available to others,\nwhile whatever are left out will not be.  When adding new things,\npeople need to ask if it needs to be sharable or not, and the right\nanswer to that question will even change over time.\n"},{"id":"208936","messageId":"7vhaln7wkg.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"1360174292-14793-5-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-07T19:28:31Z","receivedAt":"2013-02-07T19:28:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> The configuration of the install directory is not reused from the\n> toplevel Makefile: we assume Git is already built, hence just call\n> \"git --exec-path\". This avoids too much surgery in the toplevel Makefile.\n>\n> git-remote-mediawiki.perl can now \"use Git;\".\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n\nContinuing to the comment on 3/4, I wonder if it would be a lot\nsimpler and more maintainable if you replaced 1/4 to 3/4 with a\nsmaller patch to the top-level Makefile to teach it to munge\narbitrary path/to/foo.perl to path/to/foo the same way as we do to\nother path/tool.perl that are known to the top-level Makefile\n(similarly, another target to install the resulting path/to/foo at\nan arbitrary place).  Then do something like\n\n\tall::\n\t\t$(MAKE) -C ../.. \\\n\t\t\tPERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \\\n                        build-perl-script\n\tinstall::\n\t\t$(MAKE) -C ../.. \\\n\t\t\tPERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \\\n                        install-perl-script\n\nin this step.\n"},{"id":"208970","messageId":"20130208042800.GB4157@sigill.intra.peff.net","threadId":"32770","inReplyTo":"7vhaln7wkg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-08T04:28:00Z","receivedAt":"2013-02-08T04:28:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 07, 2013 at 11:28:31AM -0800, Junio C Hamano wrote:\n\n> Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n> \n> > The configuration of the install directory is not reused from the\n> > toplevel Makefile: we assume Git is already built, hence just call\n> > \"git --exec-path\". This avoids too much surgery in the toplevel Makefile.\n> >\n> > git-remote-mediawiki.perl can now \"use Git;\".\n> >\n> > Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> > ---\n> \n> Continuing to the comment on 3/4, I wonder if it would be a lot\n> simpler and more maintainable if you replaced 1/4 to 3/4 with a\n> smaller patch to the top-level Makefile to teach it to munge\n> arbitrary path/to/foo.perl to path/to/foo the same way as we do to\n> other path/tool.perl that are known to the top-level Makefile\n> (similarly, another target to install the resulting path/to/foo at\n> an arbitrary place).  Then do something like\n> \n> \tall::\n> \t\t$(MAKE) -C ../.. \\\n> \t\t\tPERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \\\n>                         build-perl-script\n> \tinstall::\n> \t\t$(MAKE) -C ../.. \\\n> \t\t\tPERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \\\n>                         install-perl-script\n> \n> in this step.\n\nThat seems much cleaner to me. If done right, it could also let people\nput:\n\n  CONTRIB_PERL += contrib/mw-to-git/git-remote-mediawiki\n\nor similar into their config.mak, and just get specific contrib bits\nbuilt and installed along with the rest of git.\n\n-Peff\n"},{"id":"209001","messageId":"vpq4nhmbusp.fsf@grenoble-inp.fr","threadId":"32770","inReplyTo":"7vobfv7wkl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/4] Makefile: factor common configuration in git-default-config.mak","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-02-08T17:05:26Z","receivedAt":"2013-02-08T17:05:26Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I really think this is going in a wrong direction.  Whatever you\n> happen to have chosen in this patch will be available to others,\n> while whatever are left out will not be.  When adding new things,\n> people need to ask if it needs to be sharable or not, and the right\n> answer to that question will even change over time.\n\nMy feeling is that Git's toplevel Makefile has become too large to\nremain completely monolithic, and splitting is good to organize it (and\nyes, splitting code into several files imply that future added code will\nhave to be added in the right file, but that's not very different from\nsplitting C code into several .c files to me). But that is another\nmatter, and ...\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Then do something like\n>\n> \tall::\n> \t\t$(MAKE) -C ../.. \\\n> \t\t\tPERL_SCRIPT=contrib/mw-to-git/git-remote-mediawiki.perl \\\n>                         build-perl-script\n\nThis ended up being very simple to implement (essentially, the Makefile\nalready knows how to do this, so this just means adding convenience\nbuild-perl-script target to the toplevel), so 2 new patches follow doing\nthis, with a ridiculously small new version of mw-to-git/Makefile.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"209005","messageId":"1360344677-5962-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"32770","inReplyTo":"vpq4nhmbusp.fsf@grenoble-inp.fr","subject":"[PATCH 1/2] Makefile: make script-related rules usable from subdirectories","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-02-08T17:31:16Z","receivedAt":"2013-02-08T17:31:16Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Git's Makefile provides a few nice features for script build and\ninstallation (substitute the first line with the right path, hardcode the\npath to Git library, ...).\n\nThe Makefile already knows how to process files outside the toplevel\ndirectory with e.g.\n\n  make SCRIPT_PERL=path/to/file.perl path/to/file\n\nbut we can make it simpler for callers by exposing build, install and\nclean rules as .PHONY targets.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nThe goal of this series is to use perl, but it is as easy to do it\nwith sh and python too, so I did it for them too. I tested a manual\n\"make -C ../../\" in contrib/subtree and contrib/hg-to-git/ to check that\nit actually works.\n\n Makefile | 35 ++++++++++++++++++++++++++++++++---\n 1 file changed, 32 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 5a2e02d..b4af30d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -480,9 +480,38 @@ SCRIPT_PERL += git-svn.perl\n SCRIPT_PYTHON += git-remote-testpy.py\n SCRIPT_PYTHON += git-p4.py\n \n-SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \\\n-\t  $(patsubst %.perl,%,$(SCRIPT_PERL)) \\\n-\t  $(patsubst %.py,%,$(SCRIPT_PYTHON)) \\\n+# Generated files for scripts\n+SCRIPT_SH_GEN = $(patsubst %.sh,%,$(SCRIPT_SH))\n+SCRIPT_PERL_GEN = $(patsubst %.perl,%,$(SCRIPT_PERL))\n+SCRIPT_PYTHON_GEN = $(patsubst %.py,%,$(SCRIPT_PYTHON))\n+\n+# Individual rules to allow e.g.\n+# \"make -C ../.. SCRIPT_PERL=contrib/foo/bar.perl build-perl-script\"\n+# from subdirectories like contrib/*/\n+.PHONY: build-perl-script build-sh-script build-python-script\n+build-perl-script: $(SCRIPT_PERL_GEN)\n+build-sh-script: $(SCRIPT_SH_GEN)\n+build-python-script: $(SCRIPT_PYTHON_GEN)\n+\n+.PHONY: install-perl-script install-sh-script install-python-script\n+install-sh-script: $(SCRIPT_SH_GEN)\n+\t$(INSTALL) $(SCRIPT_SH_GEN) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+install-perl-script: $(SCRIPT_PERL_GEN)\n+\t$(INSTALL) $(SCRIPT_PERL_GEN) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+install-python-script: $(SCRIPT_PYTHON_GEN)\n+\t$(INSTALL) $(SCRIPT_PYTHON_GEN) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\n+.PHONY: clean-perl-script clean-sh-script clean-python-script\n+clean-sh-script:\n+\t$(RM) $(SCRIPT_SH_GEN)\n+clean-perl-script:\n+\t$(RM) $(SCRIPT_PERL_GEN)\n+clean-python-script:\n+\t$(RM) $(SCRIPT_PYTHON_GEN)\n+\n+SCRIPTS = $(SCRIPT_SH_GEN) \\\n+\t  $(SCRIPT_PERL_GEN) \\\n+\t  $(SCRIPT_PYTHON_GEN) \\\n \t  git-instaweb\n \n ETAGS_TARGET = TAGS\n-- \n1.8.1.2.530.g3cc16e4.dirty\n"},{"id":"209007","messageId":"1360344677-5962-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"32770","inReplyTo":"1360344677-5962-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/2] git-remote-mediawiki: use toplevel's Makefile","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2013-02-08T17:31:17Z","receivedAt":"2013-02-08T17:31:17Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"This makes the Makefile simpler, while providing more features, and more\nconsistency (the exact same rules with the exact same configuration as\nGit official commands are applied with the new version).\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n contrib/mw-to-git/.gitignore                       |  1 +\n contrib/mw-to-git/Makefile                         | 64 ++++++----------------\n ...-remote-mediawiki => git-remote-mediawiki.perl} |  0\n 3 files changed, 18 insertions(+), 47 deletions(-)\n create mode 100644 contrib/mw-to-git/.gitignore\n rewrite contrib/mw-to-git/Makefile (96%)\n rename contrib/mw-to-git/{git-remote-mediawiki => git-remote-mediawiki.perl} (100%)\n\ndiff --git a/contrib/mw-to-git/.gitignore b/contrib/mw-to-git/.gitignore\nnew file mode 100644\nindex 0000000..b919655\n--- /dev/null\n+++ b/contrib/mw-to-git/.gitignore\n@@ -0,0 +1 @@\n+git-remote-mediawiki\ndiff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile\ndissimilarity index 96%\nindex 3ed728b..f149719 100644\n--- a/contrib/mw-to-git/Makefile\n+++ b/contrib/mw-to-git/Makefile\n@@ -1,47 +1,17 @@\n-#\n-# Copyright (C) 2012\n-#     Charles Roussel <charles.roussel@ensimag.imag.fr>\n-#     Simon Cathebras <simon.cathebras@ensimag.imag.fr>\n-#     Julien Khayat <julien.khayat@ensimag.imag.fr>\n-#     Guillaume Sasdy <guillaume.sasdy@ensimag.imag.fr>\n-#     Simon Perrat <simon.perrat@ensimag.imag.fr>\n-#\n-## Build git-remote-mediawiki\n-\n--include ../../config.mak.autogen\n--include ../../config.mak\n-\n-ifndef PERL_PATH\n-\tPERL_PATH = /usr/bin/perl\n-endif\n-ifndef gitexecdir\n-\tgitexecdir = $(shell git --exec-path)\n-endif\n-\n-PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))\n-gitexecdir_SQ = $(subst ','\\'',$(gitexecdir))\n-SCRIPT = git-remote-mediawiki\n-\n-.PHONY: install help doc test clean\n-\n-help:\n-\t@echo 'This is the help target of the Makefile. Current configuration:'\n-\t@echo '  gitexecdir = $(gitexecdir_SQ)'\n-\t@echo '  PERL_PATH = $(PERL_PATH_SQ)'\n-\t@echo 'Run \"$(MAKE) install\" to install $(SCRIPT) in gitexecdir'\n-\t@echo 'Run \"$(MAKE) test\" to run the testsuite'\n-\n-install:\n-\tsed -e '1s|#!.*/perl|#!$(PERL_PATH_SQ)|' $(SCRIPT) \\\n-\t\t> '$(gitexecdir_SQ)/$(SCRIPT)'\n-\tchmod +x '$(gitexecdir)/$(SCRIPT)'\n-\n-doc:\n-\t@echo 'Sorry, \"make doc\" is not implemented yet for $(SCRIPT)'\n-\n-test:\n-\t$(MAKE) -C t/ test\n-\n-clean:\n-\t$(RM) '$(gitexecdir)/$(SCRIPT)'\n-\t$(MAKE) -C t/ clean\n+#\n+# Copyright (C) 2013\n+#     Matthieu Moy <Matthieu.Moy@imag.fr>\n+#\n+## Build git-remote-mediawiki\n+\n+SCRIPT_PERL=git-remote-mediawiki.perl\n+GIT_ROOT_DIR=../..\n+HERE=contrib/mw-to-git/\n+\n+SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))\n+\n+all: build\n+\n+build install clean:\n+\t$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \\\n+                $@-perl-script\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki.perl\nsimilarity index 100%\nrename from contrib/mw-to-git/git-remote-mediawiki\nrename to contrib/mw-to-git/git-remote-mediawiki.perl\n-- \n1.8.1.2.530.g3cc16e4.dirty\n"},{"id":"209008","messageId":"vpqzjzeaevm.fsf@grenoble-inp.fr","threadId":"32770","inReplyTo":"20130208042800.GB4157@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-02-08T17:34:37Z","receivedAt":"2013-02-08T17:34:37Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King <peff@peff.net> writes:\n\n> That seems much cleaner to me. If done right, it could also let people\n> put:\n>\n>   CONTRIB_PERL += contrib/mw-to-git/git-remote-mediawiki\n\nActually, you can already do this:\n\n  SCRIPT_PERL += contrib/mw-to-git/git-remote-mediawiki.perl\n\nprobably not by design, but it works!\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"209010","messageId":"20130208174350.GA28266@sigill.intra.peff.net","threadId":"32770","inReplyTo":"vpqzjzeaevm.fsf@grenoble-inp.fr","subject":"Re: [PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-08T17:43:50Z","receivedAt":"2013-02-08T17:43:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 08, 2013 at 06:34:37PM +0100, Matthieu Moy wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > That seems much cleaner to me. If done right, it could also let people\n> > put:\n> >\n> >   CONTRIB_PERL += contrib/mw-to-git/git-remote-mediawiki\n> \n> Actually, you can already do this:\n> \n>   SCRIPT_PERL += contrib/mw-to-git/git-remote-mediawiki.perl\n> \n> probably not by design, but it works!\n\nSo putting:\n\n  ROOT=contrib/mw-to-git\n  git-remote-mediawiki: FORCE\n          @make -C ../.. SCRIPT_PERL=$(ROOT)/$@.perl $(ROOT)/$@\n\nin contrib/mw-to-git/Makefile would already work? Neat.\n\n-Peff\n"},{"id":"209014","messageId":"7v62223c8s.fsf@alter.siamese.dyndns.org","threadId":"32770","inReplyTo":"20130208174350.GA28266@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-08T18:13:23Z","receivedAt":"2013-02-08T18:13:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Feb 08, 2013 at 06:34:37PM +0100, Matthieu Moy wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > That seems much cleaner to me. If done right, it could also let people\n>> > put:\n>> >\n>> >   CONTRIB_PERL += contrib/mw-to-git/git-remote-mediawiki\n>> \n>> Actually, you can already do this:\n>> \n>>   SCRIPT_PERL += contrib/mw-to-git/git-remote-mediawiki.perl\n>> \n>> probably not by design, but it works!\n>\n> So putting:\n>\n>   ROOT=contrib/mw-to-git\n>   git-remote-mediawiki: FORCE\n>           @make -C ../.. SCRIPT_PERL=$(ROOT)/$@.perl $(ROOT)/$@\n>\n> in contrib/mw-to-git/Makefile would already work? Neat.\n\nThat essentially is what [v2 2/2] does, no?\n"},{"id":"209015","messageId":"20130208181556.GA387@sigill.intra.peff.net","threadId":"32770","inReplyTo":"7v62223c8s.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-08T18:15:56Z","receivedAt":"2013-02-08T18:15:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 08, 2013 at 10:13:23AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > On Fri, Feb 08, 2013 at 06:34:37PM +0100, Matthieu Moy wrote:\n> >\n> >> Jeff King <peff@peff.net> writes:\n> >> \n> >> > That seems much cleaner to me. If done right, it could also let people\n> >> > put:\n> >> >\n> >> >   CONTRIB_PERL += contrib/mw-to-git/git-remote-mediawiki\n> >> \n> >> Actually, you can already do this:\n> >> \n> >>   SCRIPT_PERL += contrib/mw-to-git/git-remote-mediawiki.perl\n> >> \n> >> probably not by design, but it works!\n> >\n> > So putting:\n> >\n> >   ROOT=contrib/mw-to-git\n> >   git-remote-mediawiki: FORCE\n> >           @make -C ../.. SCRIPT_PERL=$(ROOT)/$@.perl $(ROOT)/$@\n> >\n> > in contrib/mw-to-git/Makefile would already work? Neat.\n> \n> That essentially is what [v2 2/2] does, no?\n\nYes (this one was cc'd to me, but the others were not, so I read it in\nisolation). I think Matthieu's series is nicer than just that, though,\nbecause it handles the single-file case installation, too, which\nrequires more support from the parent Makefile.\n\n-Peff\n"}]}