| Message ID | 20180507071134.5213-1-riku.voipio@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Riku, 2018-05-07 16:11 GMT+09:00 <riku.voipio@linaro.org>: > From: Riku Voipio <riku.voipio@linaro.org> > > There is multiple issues with the genaration of maintainer string > > It uses DEBEMAIL and EMAIL enviroment variables, which may contain angle brackets, > creating invalid maintainer strings. The documented KBUILD_BUILD_USER and > KBUILD_BUILD_HOST variables are not used. Undocumented and uncommon NAME > variable is used. Sorry, I missed to ask you about 'NAME' variable. I checked the Debian Administrator's Handbook. I see the following description TIP Maintainer’s name and email address Most of the programs involved in package maintenance will look for your name and email address in the DEBFULLNAME and DEBEMAIL or EMAIL environment variables. Defining them once and for all will avoid you having to type them multiple times. If your usual shell is bash , it is a simple matter of adding the following two lines in your ~/.bashrc file (you will obviously replace the values with more relevant ones!): export EMAIL=”hertzog@debian.org” export DEBFULLNAME=”Raphael Hertzog” Indeed, 'NAME' is not mentioned at all here. On the other hand, I also checked the following link referred by Mathieu: https://manpages.debian.org/unstable/devscripts/dch.1.en.html If the environment variable DEBFULLNAME is set, this will be used for the maintainer full name; if not, then NAME will be checked. If the environment variable DEBEMAIL is set, this will be used for the email address. If this variable has the form "name <email>", then the maintainer name will also be taken from here if neither DEBFULLNAME nor NAME is set. Hmm, debchange checks 'NAME' too. I think you are more familiar with Debian. I just took a pause to ask which to follow. > Refactor the Maintainer string to: > > - use EMAIL or DEBEMAIL directly if they are in form "name <user@host>" > - use KBUILD_BUILD_USER and KBUILD_BUILD_HOST if set before falling > back to autodetection > - no longer use NAME variable or the useless Anonymous string > > The logic is switched from multiline if/then/fi statements to compact > shell variable substition commands. > > Reported-by: Mathieu Malaterre <malat@debian.org> > Signed-off-by: Riku Voipio <riku.voipio@linaro.org> > --- > v2: include improvements suggested by Masahiro-san > > scripts/package/mkdebian | 27 +++++++++++++-------------- > 1 file changed, 13 insertions(+), 14 deletions(-) > > diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian > index 6adb3a16ba3b..985d72d1ab34 100755 > --- a/scripts/package/mkdebian > +++ b/scripts/package/mkdebian > @@ -71,22 +71,21 @@ if [ "$ARCH" = "um" ] ; then > packagename=user-mode-linux-$version > fi > > -# Try to determine maintainer and email values > -if [ -n "$DEBEMAIL" ]; then > - email=$DEBEMAIL > -elif [ -n "$EMAIL" ]; then > - email=$EMAIL > -else > - email=$(id -nu)@$(hostname -f 2>/dev/null || hostname) > -fi > -if [ -n "$DEBFULLNAME" ]; then > - name=$DEBFULLNAME > -elif [ -n "$NAME" ]; then > - name=$NAME > +email=${DEBEMAIL-$EMAIL} > + > +# use email string directly if it contains <email> > +if echo $email | grep -q '<.*>'; then > + maintainer=$email > else > - name="Anonymous" > + # or construct the maintainer string > + user=${KBUILD_BUILD_USER-$(id -nu)} > + name=${DEBFULLNAME-$user} > + if [ -z "$email" ]; then > + buildhost=${KBUILD_BUILD_HOST-$(hostname -f 2>/dev/null || hostname)} > + email="$user@$buildhost" > + fi > + maintainer="$name <$email>" > fi > -maintainer="$name <$email>" > > # Try to determine distribution > if [ -n "$KDEB_CHANGELOG_DIST" ]; then > -- > 2.14.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7 May 2018 at 16:35, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > Hi Riku, > > 2018-05-07 16:11 GMT+09:00 <riku.voipio@linaro.org>: >> From: Riku Voipio <riku.voipio@linaro.org> >> >> There is multiple issues with the genaration of maintainer string >> >> It uses DEBEMAIL and EMAIL enviroment variables, which may contain angle brackets, >> creating invalid maintainer strings. The documented KBUILD_BUILD_USER and >> KBUILD_BUILD_HOST variables are not used. Undocumented and uncommon NAME >> variable is used. > > Sorry, I missed to ask you about 'NAME' variable. > > > I checked the Debian Administrator's Handbook. > > I see the following description > > > TIP > Maintainer’s name and email address > > Most of the programs involved in package maintenance will look for > your name and > email address in the DEBFULLNAME and DEBEMAIL or EMAIL environment variables. > Defining them once and for all will avoid you having to type them > multiple times. > If your usual shell is bash , it is a simple matter of adding the > following two lines > in your ~/.bashrc file (you will obviously replace the values with > more relevant > ones!): > > export EMAIL=”hertzog@debian.org” > export DEBFULLNAME=”Raphael Hertzog” > > > Indeed, 'NAME' is not mentioned at all here. > > > On the other hand, I also checked the following link > referred by Mathieu: > https://manpages.debian.org/unstable/devscripts/dch.1.en.html > > If the environment variable DEBFULLNAME is set, this will be used for the > maintainer full name; if not, then NAME will be checked. If the environment > variable DEBEMAIL is set, this will be used for the email address. If this > variable has the form "name <email>", then the maintainer name will also be > taken from here if neither DEBFULLNAME nor NAME is set. > > > Hmm, debchange checks 'NAME' too. dch is symlink to debchange. I found one common tool that falls back from DEBFULLNAME to NAME, reportbug. But almost all other users of DEBFULLNAME don't: https://codesearch.debian.net/search?q=DEBFULLNAME&perpkg=1 Supporting DEBFULLNAME and DEBEMAIL makes sense, since they are explicitly documented Debian variables. EMAIL is commonly used elsewhere (such as with git). NAME otoh is is not used outside Debian. And you will have a poor experience in Debian if you have only NAME set - reportbug will use it, but bts wont. The main reason to keep NAME, would be historic reasons ("has been supported in deb-pkg before"). Given that setting a) either DEBFULLNAME or KBUILD_BUILD_USER is trivial, and b) it's a cosmetic issue to begin with, I'm not sure it's worth it. Riku -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2018-05-08 20:56 GMT+09:00 Riku Voipio <riku.voipio@linaro.org>: > On 7 May 2018 at 16:35, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: >> Hi Riku, >> >> 2018-05-07 16:11 GMT+09:00 <riku.voipio@linaro.org>: >>> From: Riku Voipio <riku.voipio@linaro.org> >>> >>> There is multiple issues with the genaration of maintainer string >>> >>> It uses DEBEMAIL and EMAIL enviroment variables, which may contain angle brackets, >>> creating invalid maintainer strings. The documented KBUILD_BUILD_USER and >>> KBUILD_BUILD_HOST variables are not used. Undocumented and uncommon NAME >>> variable is used. >> >> Sorry, I missed to ask you about 'NAME' variable. >> >> >> I checked the Debian Administrator's Handbook. >> >> I see the following description >> >> >> TIP >> Maintainer’s name and email address >> >> Most of the programs involved in package maintenance will look for >> your name and >> email address in the DEBFULLNAME and DEBEMAIL or EMAIL environment variables. >> Defining them once and for all will avoid you having to type them >> multiple times. >> If your usual shell is bash , it is a simple matter of adding the >> following two lines >> in your ~/.bashrc file (you will obviously replace the values with >> more relevant >> ones!): >> >> export EMAIL=”hertzog@debian.org” >> export DEBFULLNAME=”Raphael Hertzog” >> >> >> Indeed, 'NAME' is not mentioned at all here. >> >> >> On the other hand, I also checked the following link >> referred by Mathieu: >> https://manpages.debian.org/unstable/devscripts/dch.1.en.html >> >> If the environment variable DEBFULLNAME is set, this will be used for the >> maintainer full name; if not, then NAME will be checked. If the environment >> variable DEBEMAIL is set, this will be used for the email address. If this >> variable has the form "name <email>", then the maintainer name will also be >> taken from here if neither DEBFULLNAME nor NAME is set. >> >> >> Hmm, debchange checks 'NAME' too. > > dch is symlink to debchange. I found one common tool that falls back > from DEBFULLNAME to NAME, reportbug. But almost all other users of > DEBFULLNAME don't: > > https://codesearch.debian.net/search?q=DEBFULLNAME&perpkg=1 > > Supporting DEBFULLNAME and DEBEMAIL makes sense, since they are > explicitly documented Debian variables. EMAIL is commonly used > elsewhere (such as with git). NAME otoh is is not used outside Debian. > And you will have a poor experience in Debian if you have only NAME > set - reportbug will use it, but bts wont. > > The main reason to keep NAME, would be historic reasons ("has been > supported in deb-pkg before"). Given that setting a) either > DEBFULLNAME or KBUILD_BUILD_USER is trivial, and b) it's a cosmetic > issue to begin with, I'm not sure it's worth it. > Okay, applied now. Thanks for detailed explanation!
Hello, On Mon, May 07, 2018 at 10:11:34AM +0300, riku.voipio@linaro.org wrote: > From: Riku Voipio <riku.voipio@linaro.org> > > There is multiple issues with the genaration of maintainer string > > It uses DEBEMAIL and EMAIL enviroment variables, which may contain angle brackets, > creating invalid maintainer strings. The documented KBUILD_BUILD_USER and > KBUILD_BUILD_HOST variables are not used. Undocumented and uncommon NAME > variable is used. Refactor the Maintainer string to: > > - use EMAIL or DEBEMAIL directly if they are in form "name <user@host>" > - use KBUILD_BUILD_USER and KBUILD_BUILD_HOST if set before falling > back to autodetection > - no longer use NAME variable or the useless Anonymous string > > The logic is switched from multiline if/then/fi statements to compact > shell variable substition commands. > > Reported-by: Mathieu Malaterre <malat@debian.org> > Signed-off-by: Riku Voipio <riku.voipio@linaro.org> > --- > v2: include improvements suggested by Masahiro-san > > scripts/package/mkdebian | 27 +++++++++++++-------------- > 1 file changed, 13 insertions(+), 14 deletions(-) > > diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian > index 6adb3a16ba3b..985d72d1ab34 100755 > --- a/scripts/package/mkdebian > +++ b/scripts/package/mkdebian > @@ -71,22 +71,21 @@ if [ "$ARCH" = "um" ] ; then > packagename=user-mode-linux-$version > fi > > -# Try to determine maintainer and email values > -if [ -n "$DEBEMAIL" ]; then > - email=$DEBEMAIL > -elif [ -n "$EMAIL" ]; then > - email=$EMAIL > -else > - email=$(id -nu)@$(hostname -f 2>/dev/null || hostname) > -fi > -if [ -n "$DEBFULLNAME" ]; then > - name=$DEBFULLNAME > -elif [ -n "$NAME" ]; then > - name=$NAME > +email=${DEBEMAIL-$EMAIL} > + > +# use email string directly if it contains <email> > +if echo $email | grep -q '<.*>'; then > + maintainer=$email Alternatively you can do: case "$email" in *\<*\>*) maintainer="$email" ;; *) ... esac which might be a tad quicker because case is shell-builtin. > else > - name="Anonymous" > + # or construct the maintainer string > + user=${KBUILD_BUILD_USER-$(id -nu)} > + name=${DEBFULLNAME-$user} > + if [ -z "$email" ]; then > + buildhost=${KBUILD_BUILD_HOST-$(hostname -f 2>/dev/null || hostname)} On Debian machines there is also /etc/mailname which should be preferred over hostname -f. Best regards Uwe
diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian index 6adb3a16ba3b..985d72d1ab34 100755 --- a/scripts/package/mkdebian +++ b/scripts/package/mkdebian @@ -71,22 +71,21 @@ if [ "$ARCH" = "um" ] ; then packagename=user-mode-linux-$version fi -# Try to determine maintainer and email values -if [ -n "$DEBEMAIL" ]; then - email=$DEBEMAIL -elif [ -n "$EMAIL" ]; then - email=$EMAIL -else - email=$(id -nu)@$(hostname -f 2>/dev/null || hostname) -fi -if [ -n "$DEBFULLNAME" ]; then - name=$DEBFULLNAME -elif [ -n "$NAME" ]; then - name=$NAME +email=${DEBEMAIL-$EMAIL} + +# use email string directly if it contains <email> +if echo $email | grep -q '<.*>'; then + maintainer=$email else - name="Anonymous" + # or construct the maintainer string + user=${KBUILD_BUILD_USER-$(id -nu)} + name=${DEBFULLNAME-$user} + if [ -z "$email" ]; then + buildhost=${KBUILD_BUILD_HOST-$(hostname -f 2>/dev/null || hostname)} + email="$user@$buildhost" + fi + maintainer="$name <$email>" fi -maintainer="$name <$email>" # Try to determine distribution if [ -n "$KDEB_CHANGELOG_DIST" ]; then