Patchwork [RFC,10/10] send-pull-request: verify git sendemail config

login
register
mail settings
Submitter Darren Hart
Date May 13, 2011, 11:38 p.m.
Message ID <33b417d1457fc40c41c4957d62e6758ec1cfcc42.1305329659.git.dvhart@linux.intel.com>
Download mbox | patch
Permalink /patch/4017/
State New, archived
Headers show

Comments

Darren Hart - May 13, 2011, 11:38 p.m.
Perform a quick sanity check to be able to direct users to configure
git.sendemail if they haven't yet.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
---
 scripts/send-pull-request |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)
Bruce Ashfield - May 14, 2011, 5:43 a.m.
On Fri, May 13, 2011 at 7:38 PM, Darren Hart <dvhart@linux.intel.com> wrote:
> Perform a quick sanity check to be able to direct users to configure
> git.sendemail if they haven't yet.
>
> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
> ---
>  scripts/send-pull-request |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/send-pull-request b/scripts/send-pull-request
> index f94596f..2ccb8e8 100755
> --- a/scripts/send-pull-request
> +++ b/scripts/send-pull-request
> @@ -38,6 +38,18 @@ harvest_recipients()
>        unset IFS
>  }
>
> +check_git_sendemail_config()
> +{
> +       GIT_SMTP=$(git config sendemail.smtpserver)
> +       GIT_FROM=$(git config sendemail.from)
> +       if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then
> +               echo "ERROR: git sendemail is not configured."
> +               echo "Please read GIT-SEND-EMAIL(1) and configure:"
> +               echo "  sendemail.smtpserver"
> +               echo "  sendemail.from"

I was going to ask for this to be optional, since I always pass the smtp
settings on the command line .. but then I realized that I invoke git send-email
directly anyway, so I don't need it to be optional. :)

Regardless, reading this from git config is good idea, so the above is
only a comment.

Bruce

> +               exit 1
> +       fi
> +}
>
>  # Parse and verify arguments
>  while getopts "ahp:t:" OPT; do
> @@ -67,6 +79,9 @@ while getopts "ahp:t:" OPT; do
>        esac
>  done
>
> +# Abort early if git-send-email is not properly configured
> +check_git_sendemail_config
> +
>  if [ -z "$PDIR" ]; then
>        echo "ERROR: you must specify a pull-dir."
>        usage
> --
> 1.7.1
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
Darren Hart - May 14, 2011, 6:45 p.m.
On 05/13/2011 10:43 PM, Bruce Ashfield wrote:
> On Fri, May 13, 2011 at 7:38 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>> Perform a quick sanity check to be able to direct users to configure
>> git.sendemail if they haven't yet.
>>
>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>> ---
>>  scripts/send-pull-request |   15 +++++++++++++++
>>  1 files changed, 15 insertions(+), 0 deletions(-)
>>
>> diff --git a/scripts/send-pull-request b/scripts/send-pull-request
>> index f94596f..2ccb8e8 100755
>> --- a/scripts/send-pull-request
>> +++ b/scripts/send-pull-request
>> @@ -38,6 +38,18 @@ harvest_recipients()
>>        unset IFS
>>  }
>>
>> +check_git_sendemail_config()
>> +{
>> +       GIT_SMTP=$(git config sendemail.smtpserver)
>> +       GIT_FROM=$(git config sendemail.from)
>> +       if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then
>> +               echo "ERROR: git sendemail is not configured."
>> +               echo "Please read GIT-SEND-EMAIL(1) and configure:"
>> +               echo "  sendemail.smtpserver"
>> +               echo "  sendemail.from"
> 
> I was going to ask for this to be optional, since I always pass the smtp
> settings on the command line .. but then I realized that I invoke git send-email
> directly anyway, so I don't need it to be optional. :)
> 
> Regardless, reading this from git config is good idea, so the above is
> only a comment.

This only impacts the send-pull-request script which requires
git-send-email to be configured in order to work, so it really can't be
an optional thing.

--
Darren

> 
> Bruce
> 
>> +               exit 1
>> +       fi
>> +}
>>
>>  # Parse and verify arguments
>>  while getopts "ahp:t:" OPT; do
>> @@ -67,6 +79,9 @@ while getopts "ahp:t:" OPT; do
>>        esac
>>  done
>>
>> +# Abort early if git-send-email is not properly configured
>> +check_git_sendemail_config
>> +
>>  if [ -z "$PDIR" ]; then
>>        echo "ERROR: you must specify a pull-dir."
>>        usage
>> --
>> 1.7.1
>>
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>>
> 
> 
>
Bruce Ashfield - May 15, 2011, 2:48 a.m.
On Sat, May 14, 2011 at 2:45 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>
>
> On 05/13/2011 10:43 PM, Bruce Ashfield wrote:
>> On Fri, May 13, 2011 at 7:38 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>> Perform a quick sanity check to be able to direct users to configure
>>> git.sendemail if they haven't yet.
>>>
>>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>>> ---
>>>  scripts/send-pull-request |   15 +++++++++++++++
>>>  1 files changed, 15 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/scripts/send-pull-request b/scripts/send-pull-request
>>> index f94596f..2ccb8e8 100755
>>> --- a/scripts/send-pull-request
>>> +++ b/scripts/send-pull-request
>>> @@ -38,6 +38,18 @@ harvest_recipients()
>>>        unset IFS
>>>  }
>>>
>>> +check_git_sendemail_config()
>>> +{
>>> +       GIT_SMTP=$(git config sendemail.smtpserver)
>>> +       GIT_FROM=$(git config sendemail.from)
>>> +       if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then
>>> +               echo "ERROR: git sendemail is not configured."
>>> +               echo "Please read GIT-SEND-EMAIL(1) and configure:"
>>> +               echo "  sendemail.smtpserver"
>>> +               echo "  sendemail.from"
>>
>> I was going to ask for this to be optional, since I always pass the smtp
>> settings on the command line .. but then I realized that I invoke git send-email
>> directly anyway, so I don't need it to be optional. :)
>>
>> Regardless, reading this from git config is good idea, so the above is
>> only a comment.
>
> This only impacts the send-pull-request script which requires
> git-send-email to be configured in order to work, so it really can't be
> an optional thing.

I meant making it optional from being in your .gitconfig. I use
git send-email with a smtp server almost every day .. and it's
not in my .gitconfig :)

Bruce

>
> --
> Darren
>
>>
>> Bruce
>>
>>> +               exit 1
>>> +       fi
>>> +}
>>>
>>>  # Parse and verify arguments
>>>  while getopts "ahp:t:" OPT; do
>>> @@ -67,6 +79,9 @@ while getopts "ahp:t:" OPT; do
>>>        esac
>>>  done
>>>
>>> +# Abort early if git-send-email is not properly configured
>>> +check_git_sendemail_config
>>> +
>>>  if [ -z "$PDIR" ]; then
>>>        echo "ERROR: you must specify a pull-dir."
>>>        usage
>>> --
>>> 1.7.1
>>>
>>>
>>> _______________________________________________
>>> Openembedded-core mailing list
>>> Openembedded-core@lists.openembedded.org
>>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>>>
>>
>>
>>
>
> --
> Darren Hart
> Intel Open Source Technology Center
> Yocto Project - Linux Kernel
>
Darren Hart - May 16, 2011, 3:13 p.m.
On 05/14/2011 07:48 PM, Bruce Ashfield wrote:
> On Sat, May 14, 2011 at 2:45 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>
>>
>> On 05/13/2011 10:43 PM, Bruce Ashfield wrote:
>>> On Fri, May 13, 2011 at 7:38 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>>> Perform a quick sanity check to be able to direct users to configure
>>>> git.sendemail if they haven't yet.
>>>>
>>>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>>>> ---
>>>>  scripts/send-pull-request |   15 +++++++++++++++
>>>>  1 files changed, 15 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/scripts/send-pull-request b/scripts/send-pull-request
>>>> index f94596f..2ccb8e8 100755
>>>> --- a/scripts/send-pull-request
>>>> +++ b/scripts/send-pull-request
>>>> @@ -38,6 +38,18 @@ harvest_recipients()
>>>>        unset IFS
>>>>  }
>>>>
>>>> +check_git_sendemail_config()
>>>> +{
>>>> +       GIT_SMTP=$(git config sendemail.smtpserver)
>>>> +       GIT_FROM=$(git config sendemail.from)
>>>> +       if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then
>>>> +               echo "ERROR: git sendemail is not configured."
>>>> +               echo "Please read GIT-SEND-EMAIL(1) and configure:"
>>>> +               echo "  sendemail.smtpserver"
>>>> +               echo "  sendemail.from"
>>>
>>> I was going to ask for this to be optional, since I always pass the smtp
>>> settings on the command line .. but then I realized that I invoke git send-email
>>> directly anyway, so I don't need it to be optional. :)
>>>
>>> Regardless, reading this from git config is good idea, so the above is
>>> only a comment.
>>
>> This only impacts the send-pull-request script which requires
>> git-send-email to be configured in order to work, so it really can't be
>> an optional thing.
> 
> I meant making it optional from being in your .gitconfig. I use
> git send-email with a smtp server almost every day .. and it's
> not in my .gitconfig :)

Right, duh. OK, so it order to make this optional, we would have to add
all the necessary options to pass through from the script to
git-send-email. I'd really rather avoid that.

What is your objection to having a default smtp/from in your git config?
You can always override it on the command line, and with confirm=always
in your git config (you're sane right?) there is little risk of sending
from the wrong account accidentally.

--
Darren

> 
> Bruce
> 
>>
>> --
>> Darren
>>
>>>
>>> Bruce
>>>
>>>> +               exit 1
>>>> +       fi
>>>> +}
>>>>
>>>>  # Parse and verify arguments
>>>>  while getopts "ahp:t:" OPT; do
>>>> @@ -67,6 +79,9 @@ while getopts "ahp:t:" OPT; do
>>>>        esac
>>>>  done
>>>>
>>>> +# Abort early if git-send-email is not properly configured
>>>> +check_git_sendemail_config
>>>> +
>>>>  if [ -z "$PDIR" ]; then
>>>>        echo "ERROR: you must specify a pull-dir."
>>>>        usage
>>>> --
>>>> 1.7.1
>>>>
>>>>
>>>> _______________________________________________
>>>> Openembedded-core mailing list
>>>> Openembedded-core@lists.openembedded.org
>>>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>>>>
>>>
>>>
>>>
>>
>> --
>> Darren Hart
>> Intel Open Source Technology Center
>> Yocto Project - Linux Kernel
>>
> 
> 
>
Bruce Ashfield - May 16, 2011, 3:16 p.m.
On Mon, May 16, 2011 at 11:13 AM, Darren Hart <dvhart@linux.intel.com> wrote:
> On 05/14/2011 07:48 PM, Bruce Ashfield wrote:
>> On Sat, May 14, 2011 at 2:45 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>>
>>>
>>> On 05/13/2011 10:43 PM, Bruce Ashfield wrote:
>>>> On Fri, May 13, 2011 at 7:38 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>>>> Perform a quick sanity check to be able to direct users to configure
>>>>> git.sendemail if they haven't yet.
>>>>>
>>>>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>>>>> ---
>>>>>  scripts/send-pull-request |   15 +++++++++++++++
>>>>>  1 files changed, 15 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/scripts/send-pull-request b/scripts/send-pull-request
>>>>> index f94596f..2ccb8e8 100755
>>>>> --- a/scripts/send-pull-request
>>>>> +++ b/scripts/send-pull-request
>>>>> @@ -38,6 +38,18 @@ harvest_recipients()
>>>>>        unset IFS
>>>>>  }
>>>>>
>>>>> +check_git_sendemail_config()
>>>>> +{
>>>>> +       GIT_SMTP=$(git config sendemail.smtpserver)
>>>>> +       GIT_FROM=$(git config sendemail.from)
>>>>> +       if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then
>>>>> +               echo "ERROR: git sendemail is not configured."
>>>>> +               echo "Please read GIT-SEND-EMAIL(1) and configure:"
>>>>> +               echo "  sendemail.smtpserver"
>>>>> +               echo "  sendemail.from"
>>>>
>>>> I was going to ask for this to be optional, since I always pass the smtp
>>>> settings on the command line .. but then I realized that I invoke git send-email
>>>> directly anyway, so I don't need it to be optional. :)
>>>>
>>>> Regardless, reading this from git config is good idea, so the above is
>>>> only a comment.
>>>
>>> This only impacts the send-pull-request script which requires
>>> git-send-email to be configured in order to work, so it really can't be
>>> an optional thing.
>>
>> I meant making it optional from being in your .gitconfig. I use
>> git send-email with a smtp server almost every day .. and it's
>> not in my .gitconfig :)
>
> Right, duh. OK, so it order to make this optional, we would have to add
> all the necessary options to pass through from the script to
> git-send-email. I'd really rather avoid that.

Exactly. that's why I dropped my idea of asking for it to be optional, you'd
need to chain it down, and that's a bad idea.

>
> What is your objection to having a default smtp/from in your git config?
> You can always override it on the command line, and with confirm=always
> in your git config (you're sane right?) there is little risk of sending
> from the wrong account accidentally.

I just have a zillion boxes, some with different connectivity, so that is
typically left out of my config. The hard error would cause me minimal
pain, but I invoke git send-email by hand, so it's no problem for me in
the long run.

So there's nothing here for you to change, if I needed something, I
could do it locally.

Cheers,

Bruce


>
> --
> Darren
>
>>
>> Bruce
>>
>>>
>>> --
>>> Darren
>>>
>>>>
>>>> Bruce
>>>>
>>>>> +               exit 1
>>>>> +       fi
>>>>> +}
>>>>>
>>>>>  # Parse and verify arguments
>>>>>  while getopts "ahp:t:" OPT; do
>>>>> @@ -67,6 +79,9 @@ while getopts "ahp:t:" OPT; do
>>>>>        esac
>>>>>  done
>>>>>
>>>>> +# Abort early if git-send-email is not properly configured
>>>>> +check_git_sendemail_config
>>>>> +
>>>>>  if [ -z "$PDIR" ]; then
>>>>>        echo "ERROR: you must specify a pull-dir."
>>>>>        usage
>>>>> --
>>>>> 1.7.1
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Openembedded-core mailing list
>>>>> Openembedded-core@lists.openembedded.org
>>>>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>>>>>
>>>>
>>>>
>>>>
>>>
>>> --
>>> Darren Hart
>>> Intel Open Source Technology Center
>>> Yocto Project - Linux Kernel
>>>
>>
>>
>>
>
> --
> Darren Hart
> Intel Open Source Technology Center
> Yocto Project - Linux Kernel
>

Patch

diff --git a/scripts/send-pull-request b/scripts/send-pull-request
index f94596f..2ccb8e8 100755
--- a/scripts/send-pull-request
+++ b/scripts/send-pull-request
@@ -38,6 +38,18 @@  harvest_recipients()
 	unset IFS
 }
 
+check_git_sendemail_config()
+{
+	GIT_SMTP=$(git config sendemail.smtpserver)
+	GIT_FROM=$(git config sendemail.from)
+	if [ -z "$GIT_SMTP" ] || [ -z "$GIT_FROM" ]; then
+		echo "ERROR: git sendemail is not configured."
+		echo "Please read GIT-SEND-EMAIL(1) and configure:"
+		echo "  sendemail.smtpserver"
+		echo "  sendemail.from"
+		exit 1
+	fi
+}
 
 # Parse and verify arguments
 while getopts "ahp:t:" OPT; do
@@ -67,6 +79,9 @@  while getopts "ahp:t:" OPT; do
 	esac
 done
 
+# Abort early if git-send-email is not properly configured
+check_git_sendemail_config
+
 if [ -z "$PDIR" ]; then
 	echo "ERROR: you must specify a pull-dir."
 	usage