{"thread":{"id":"35730","subject":"[PATCH] Added get sendmail from .mailrc","startedAt":"2014-01-25T09:46:50Z","lastAt":"2014-01-28T06:19:22Z","messageCount":8,"participants":["Brilliantov Kirill Vladimirovich","Eric Wong","Jeff King","Kyle J. McKay"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"233747","messageId":"1390643210-5748-1-git-send-email-brilliantov@inbox.ru","threadId":"35730","inReplyTo":null,"subject":"[PATCH] Added get sendmail from .mailrc","fromName":"Brilliantov Kirill Vladimirovich","fromEmail":"brilliantov@inbox.ru","sentAt":"2014-01-25T09:46:50Z","receivedAt":"2014-01-25T09:46:50Z","isPatch":true,"sender":{"key":"brilliantov@inbox.ru","avatar":null},"body":"Signed-off-by: Brilliantov Kirill Vladimirovich \n---\n git-send-email.perl | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 2016d9c..5345fdb 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -28,6 +28,7 @@ use File::Temp qw/ tempdir tempfile /;\n use File::Spec::Functions qw(catfile);\n use Error qw(:try);\n use Git;\n+use File::HomeDir;\n \n Getopt::Long::Configure qw/ pass_through /;\n \n@@ -804,6 +805,23 @@ if (!defined $smtp_server) {\n \t\t\tlast;\n \t\t}\n \t}\n+\n+\tif (!defined $smtp_server) {\n+\t\tmy $mailrc = File::HomeDir->my_home . \"/.mailrc\";\n+\t\tif (-e $mailrc) {\n+\t\t\topen FILE, $mailrc or die \"Failed open $mailrc: $!\";\n+\t\t\twhile () {\n+\t\t\t chomp;\n+\t\t\t if (/set sendmail=.*/) {\n+\t\t\t\tmy @data = split '\"';\n+\t\t\t\t$smtp_server = $data[1];\n+\t\t\t\tlast;\n+\t\t\t }\n+\t\t\t}\n+\t\t\tclose FILE;\n+\t\t}\n+\t}\n+\n \t$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*\n }\n \n-- \n1.8.5.2\n"},{"id":"233759","messageId":"20140125223721.GA2684@dcvr.yhbt.net","threadId":"35730","inReplyTo":"1390643210-5748-1-git-send-email-brilliantov@inbox.ru","subject":"Re: [PATCH] Added get sendmail from .mailrc","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2014-01-25T22:37:21Z","receivedAt":"2014-01-25T22:37:21Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Brilliantov Kirill Vladimirovich wrote:\n> Signed-off-by: Brilliantov Kirill Vladimirovich \n> ---\n> git-send-email.perl | 18 ++++++++++++++++++\n> 1 file changed, 18 insertions(+)\n\nSome documentation references to .mailrc and its format would be nice.\n\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -28,6 +28,7 @@ use File::Temp qw/ tempdir tempfile /;\n> use File::Spec::Functions qw(catfile);\n> use Error qw(:try);\n> use Git;\n> +use File::HomeDir;\n\nWe should probably avoid a new dependency and also remain consistent\nwith the rest of git handles home directories.\n\nUnfortunately, expand_user_path()/git_config_pathname() isn't currently\nexposed to scripters right now...\n"},{"id":"233761","messageId":"20140126073438.GA3858@kirill.netbynet.ru","threadId":"35730","inReplyTo":"20140125223721.GA2684@dcvr.yhbt.net","subject":"Re: [PATCH] Added get sendmail from .mailrc","fromName":"Brilliantov Kirill Vladimirovich","fromEmail":"brilliantov@inbox.ru","sentAt":"2014-01-26T07:34:38Z","receivedAt":"2014-01-26T07:34:38Z","isPatch":true,"sender":{"key":"brilliantov@inbox.ru","avatar":null},"body":"On 2014-01-25 22:37:21, Eric Wong wrote:\n> Brilliantov Kirill Vladimirovich wrote:\n> > Signed-off-by: Brilliantov Kirill Vladimirovich \n> > ---\n> > git-send-email.perl | 18 ++++++++++++++++++\n> > 1 file changed, 18 insertions(+)\n> \n> Some documentation references to .mailrc and its format would be nice.\n> \n\nUnfortunally I can't found official documentation on this option:\nhttp://linux.die.net/man/1/mailx\nhttp://publib.boulder.ibm.com/infocenter/aix/v6r1/topic/com.ibm.aix.files/doc/aixfiles/mailrc.htm\n\nOn my system (Debian GNU/Linux 7.3) documentation on mailx not conteins\ndescription senmail options.\n\n> > --- a/git-send-email.perl\n> > +++ b/git-send-email.perl\n> > @@ -28,6 +28,7 @@ use File::Temp qw/ tempdir tempfile /;\n> > use File::Spec::Functions qw(catfile);\n> > use Error qw(:try);\n> > use Git;\n> > +use File::HomeDir;\n> \n> We should probably avoid a new dependency and also remain consistent\n> with the rest of git handles home directories.\n> \n> Unfortunately, expand_user_path()/git_config_pathname() isn't currently\n> exposed to scripters right now...\n> \n\nOk, if new dependency is not allowed I see next ways:\n- add new argument\n- add new configuration parameters\n"},{"id":"233762","messageId":"20140126081147.GA5048@kirill.netbynet.ru","threadId":"35730","inReplyTo":"20140126073438.GA3858@kirill.netbynet.ru","subject":"Re: [PATCH] Added get sendmail from .mailrc","fromName":"Brilliantov Kirill Vladimirovich","fromEmail":"brilliantov@inbox.ru","sentAt":"2014-01-26T08:11:47Z","receivedAt":"2014-01-26T08:11:47Z","isPatch":true,"sender":{"key":"brilliantov@inbox.ru","avatar":null},"body":"On 2014-01-26 11:34:38, Brilliantov Kirill Vladimirovich wrote:\n> On 2014-01-25 22:37:21, Eric Wong wrote:\n> > \n> > We should probably avoid a new dependency and also remain consistent\n> > with the rest of git handles home directories.\n> > \n> > Unfortunately, expand_user_path()/git_config_pathname() isn't currently\n> > exposed to scripters right now...\n> > \n> \n> Ok, if new dependency is not allowed I see next ways:\n> - add new argument\n> - add new configuration parameters\n\nOk, git support setting path to sendmail-like program via sendemail.smtpserver\nconfiguration option.\nIt is not very convenient because I need have separated configuration for mail\nand git.\n"},{"id":"233763","messageId":"20140126091709.GA31073@dcvr.yhbt.net","threadId":"35730","inReplyTo":"20140126073438.GA3858@kirill.netbynet.ru","subject":"Re: [PATCH] Added get sendmail from .mailrc","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2014-01-26T09:17:09Z","receivedAt":"2014-01-26T09:17:09Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Brilliantov Kirill Vladimirovich wrote:\n> On 2014-01-25 22:37:21, Eric Wong wrote:\n> > Brilliantov Kirill Vladimirovich wrote:\n> > > --- a/git-send-email.perl\n> > > +++ b/git-send-email.perl\n> > > @@ -28,6 +28,7 @@ use File::Temp qw/ tempdir tempfile /;\n> > > use File::Spec::Functions qw(catfile);\n> > > use Error qw(:try);\n> > > use Git;\n> > > +use File::HomeDir;\n> > \n> > We should probably avoid a new dependency and also remain consistent\n> > with the rest of git handles home directories.\n> > \n> > Unfortunately, expand_user_path()/git_config_pathname() isn't currently\n> > exposed to scripters right now...\n> \n> Ok, if new dependency is not allowed I see next ways:\n\nNot saying it's not allowed. I meant we should probably expose\nexpand_user_path()/git_config_pathname() C functions to script helpers\n(so git-config or git-rev-parse can provide them to sh or perl scripts).\n"},{"id":"233856","messageId":"20140128010459.GA17945@sigill.intra.peff.net","threadId":"35730","inReplyTo":"20140126091709.GA31073@dcvr.yhbt.net","subject":"Re: [PATCH] Added get sendmail from .mailrc","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-01-28T01:04:59Z","receivedAt":"2014-01-28T01:04:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jan 26, 2014 at 09:17:09AM +0000, Eric Wong wrote:\n\n> > > > +use File::HomeDir;\n> > > \n> > > We should probably avoid a new dependency and also remain consistent\n> > > with the rest of git handles home directories.\n> > > \n> > > Unfortunately, expand_user_path()/git_config_pathname() isn't currently\n> > > exposed to scripters right now...\n> > \n> > Ok, if new dependency is not allowed I see next ways:\n> \n> Not saying it's not allowed. I meant we should probably expose\n> expand_user_path()/git_config_pathname() C functions to script helpers\n> (so git-config or git-rev-parse can provide them to sh or perl scripts).\n\nI do not think we need anything so complex. Most of the logic in\nexpand_user_path is about handling \"~\" and \"~user\". But here we _just_\nwant to know the current user's home directory, and for that\nexpand_user_path always just looks in $HOME.\n\nSo I think $ENV{HOME} would be fine to match what git does. My\nunderstanding is that File::HomeDir does some magic that may work better\non non-Unix platforms. I do not know if we even care for this feature,\nsince .mailrc is presumably a Unix thing. But if we do, I think our\nusual strategy with such things is to optionally use the dependency if\navailable, and fall back to something sane. Like:\n\n sub homedir {\n if (eval { require File::HomeDir; 1 }) {\n return File::HomeDir->my_home;\n }\n return $ENV{HOME};\n }\n\nWhichever code path is followed, you should probably also check the\nresult for \"undef\", which the original patch did not do.\n\n-Peff\n"},{"id":"233857","messageId":"20140128011512.GB17945@sigill.intra.peff.net","threadId":"35730","inReplyTo":"1390643210-5748-1-git-send-email-brilliantov@inbox.ru","subject":"Re: [PATCH] Added get sendmail from .mailrc","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-01-28T01:15:12Z","receivedAt":"2014-01-28T01:15:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 25, 2014 at 01:46:50PM +0400, Brilliantov Kirill Vladimirovich wrote:\n\n> +\tif (!defined $smtp_server) {\n> +\t\tmy $mailrc = File::HomeDir->my_home . \"/.mailrc\";\n\nThe new module dependency has been discussed elsewhere in the thread.\n\n> +\t\tif (-e $mailrc) {\n> +\t\t\topen FILE, $mailrc or die \"Failed open $mailrc: $!\";\n\nPlease use the three-argument of 'open', and use a regular scalar\ninstead of a glob. Both are safer, and we assume a modern enough perl to\nsupport both. I.e.:\n\n open(my $file, '<', $mailrc)\n or die \"failed to open $mailrc: $!\";\n\n> +\t\t\twhile () {\n> +\t\t\t chomp;\n> +\t\t\t if (/set sendmail=.*/) {\n> +\t\t\t\tmy @data = split '\"';\n> +\t\t\t\t$smtp_server = $data[1];\n> +\t\t\t\tlast;\n> +\t\t\t }\n\nYour split is a rather unusual way to do the parsing, and it took me a\nminute to figure it out. It might be more obvious as:\n\n if (/set sendmail=\"(.*)\"/) {\n $smtp_server = $1;\n last;\n }\n\nI do not know anything about the mailrc format, nor does it seem to be\nwell documented. Are the double-quotes required? If not, then the above\nregex can easily make them optional. I also wonder if any whitespace is\nallowed. E.g., this might be more forgiving:\n\n /set sendmail\\s*=\\s*\"?(.*?)\"?/\n\nbut I am just guessing at what the format allows.\n\n-Peff\n"},{"id":"233861","messageId":"1B2852B7-5CDB-4C8E-AF0D-5375DB95A4AB@gmail.com","threadId":"35730","inReplyTo":"20140128011512.GB17945@sigill.intra.peff.net","subject":"Re: [PATCH] Added get sendmail from .mailrc","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2014-01-28T06:19:22Z","receivedAt":"2014-01-28T06:19:22Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Jan 27, 2014, at 17:15, Jeff King wrote:\n> On Sat, Jan 25, 2014 at 01:46:50PM +0400, Brilliantov Kirill \n> Vladimirovich wrote:\n>\n>> +\tif (!defined $smtp_server) {\n>> +\t\tmy $mailrc = File::HomeDir->my_home . \"/.mailrc\";\n\nActually, based on the output of \"man mail\", this should probably be \nsomething more like\n\n my $mailrc = $ENV{'MAILRC'} || \"$ENV{'HOME'}/.mailrc\";\n\nwhich takes into account any MAILRC setting and also avoids the use of \nFile::HomeDir.\n\n>> +\t\t\twhile () {\n>> +\t\t\t chomp;\n>> +\t\t\t if (/set sendmail=.*/) {\n>> +\t\t\t\tmy @data = split '\"';\n>> +\t\t\t\t$smtp_server = $data[1];\n>> +\t\t\t\tlast;\n>> +\t\t\t }\n>\n> Your split is a rather unusual way to do the parsing, and it took me a\n> minute to figure it out. It might be more obvious as:\n>\n> if (/set sendmail=\"(.*)\"/) {\n> $smtp_server = $1;\n> last;\n> }\n>\n> I do not know anything about the mailrc format, nor does it seem to be\n> well documented. Are the double-quotes required? If not, then the \n> above\n> regex can easily make them optional. I also wonder if any whitespace \n> is\n> allowed.\n\n From \"man mail\":\n\n set (se) With no arguments, prints all variable values. \nOtherwise,\n sets option. Arguments are of the form option=value (no space\n before or after `=') or option. Quotation marks may be placed\n around any part of the assignment statement to quote blanks or\n tabs, i.e. ``set indentprefix=\"->\"''\n\nMy version of \"man mail\" does not list all the variables that can be \nset but it refers to \"The Mail Reference Manual\" document which \npresumably does. I did find this [1] that documents many of the \navailable variables including the sendmail one. I then tried this:\n\ncat < /tmp/shim\n#!/bin/sh\nexec cat\nEOF\nchmod a+x /tmp/shim\ncat < /tmp/testrc\n se send\"mail\"=/tm\"p/\"shim\nEOF\necho 'testing' | MAILRC=/tmp/testrc mail -s test nobody\n\nAnd to my surprise the contents of the new message were cat'd out to \nthe terminal rather than being sent. So clearly there's some room for \nimprovement with the \"set\", white space and quote checking.\n\n[1] http://www.cs.fsu.edu/sysinfo/mail/mailrc.html\n\n--Kyle\n"}]}