Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.kernel > #60898 > unrolled thread

[PATCH v2] kbuild: deb-pkg improve maintainer address generation

Back to article view | Back to linux.debian.kernel

Contents

  [PATCH v2] kbuild: deb-pkg improve maintainer address generation riku.voipio@linaro.org - 2018-05-07 09:30 +0200
    Re: [PATCH v2] kbuild: deb-pkg improve maintainer address generation Masahiro Yamada <yamada.masahiro@socionext.com> - 2018-05-07 16:00 +0200
      Re: [PATCH v2] kbuild: deb-pkg improve maintainer address generation Riku Voipio <riku.voipio@linaro.org> - 2018-05-08 14:20 +0200
        Re: [PATCH v2] kbuild: deb-pkg improve maintainer address generation Masahiro Yamada <yamada.masahiro@socionext.com> - 2018-05-09 02:30 +0200
    Re: [PATCH v2] kbuild: deb-pkg improve maintainer address generation Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2018-05-09 09:40 +0200

#60898 — [PATCH v2] kbuild: deb-pkg improve maintainer address generation

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
 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

[toc] | [next] | [standalone]

#60905

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



-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#60931

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

[toc] | [prev] | [next] | [standalone]

#60934

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!


-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

#60936

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.kernel

csiph-web