[pacman-dev] [PATCH] Provide a better guess about who the packager is.
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. Use these instead of defaulting to "Unknown Packager" to minimize configuration necessary by the user. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git). Signed-off-by: Kieran Colford <kieran@kcolford.com> --- scripts/makepkg.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 29408929..65114862 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -618,7 +618,7 @@ write_pkginfo() { if [[ -n $PACKAGER ]]; then local packager="$PACKAGER" else - local packager="Unknown Packager" + local packager="${NAME:-$(getent passwd ${USER:-$(whoami)} | cut -d : -f 5 | cut -d , -f 1)} <${EMAIL:-${USER:-$(whoami)}@$(hostname --fqdn)}>" fi local size="$(@DUPATH@ @DUFLAGS@)" -- 2.11.0
On 01/31/2017 05:10 PM, Kieran Colford wrote:
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. Use these instead of defaulting to "Unknown Packager" to minimize configuration necessary by the user. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git).
Signed-off-by: Kieran Colford <kieran@kcolford.com> --- scripts/makepkg.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 29408929..65114862 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -618,7 +618,7 @@ write_pkginfo() { if [[ -n $PACKAGER ]]; then local packager="$PACKAGER" else - local packager="Unknown Packager" + local packager="${NAME:-$(getent passwd ${USER:-$(whoami)} | cut -d : -f 5 | cut -d , -f 1)} <${EMAIL:-${USER:-$(whoami)}@$(hostname --fqdn)}>"
What happens when someone initially created their user without making use of the "useradd --comment" option? I know I didn't bother... Do you have a fallback for when that is empty? More: what about building in a clean chroot? This will invariably produce nothing but "bash: hostname: command not found" on stderr. (But the "$NAME" would evaluate to "builduser" anyway. I am not entirely sure why that field is even filled out with that much in the first place, but that is another matter entirely...) That would be because "hostname" is provided by inetutils, which is not installed as part of base-devel. Warning: Personal opinion follows. I personally dislike its useless output of "localhost.localdomain" too, even when git uses that. (I would much prefer using the local machine name at least, available through e.g. uname -n which is at least guaranteed available as part of coreutils, besides being 100% more distinctive for the average home system.)
fi
local size="$(@DUPATH@ @DUFLAGS@)"
Trying to scrape information from the OS in order to get better defaults is not a terrible idea, but getting rid of any fallback whatsoever in the process doesn't strike me as useful. I also think everyone, without exception, should be configuring their makepkg.conf to provide an *actual* email, rather than falling back to the machine name. For the same reason that git users should do so (and git redeems itself by verbosely warning you every time you rely on that autodetected value). Unless of course they simply don't care about that field, in which case we can probably use "flbkoaasdfklpopefevq" instead of "Unknown Packager", and they'd never notice the difference. Though as I have no actual power here, I probably cannot get *that* enforced. :( -- Eli Schwartz
I would have added better defaults for when getent returns nothing for us to use, but my bash skills aren't up to par for that. I would love to get a better fallback for that. Currently it will just give a blank if you didn't set your full name in /etc/passwd I also didn't realize the hostname command wasn't included in base-devel. It didn't even occur to me to not have it. I can further fallback on uname -n like you suggested or even echo localhost A system configured according to the wiki will give even more distinctive and useful information with hostname --fqdn than it will with just uname -n, so I would keep it as the default and fallback to uname if it doesn't work. The On Tue, Jan 31, 2017, 7:47 PM Eli Schwartz, <eschwartz93@gmail.com> wrote:
On 01/31/2017 05:10 PM, Kieran Colford wrote:
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. Use these instead of defaulting to "Unknown Packager" to minimize configuration necessary by the user. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git).
Signed-off-by: Kieran Colford <kieran@kcolford.com> --- scripts/makepkg.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 29408929..65114862 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -618,7 +618,7 @@ write_pkginfo() { if [[ -n $PACKAGER ]]; then local packager="$PACKAGER" else - local packager="Unknown Packager" + local packager="${NAME:-$(getent passwd ${USER:-$(whoami)} | cut -d : -f 5 | cut -d , -f 1)} <${EMAIL:-${USER:-$(whoami)}@$(hostname --fqdn)}>"
What happens when someone initially created their user without making use of the "useradd --comment" option? I know I didn't bother... Do you have a fallback for when that is empty?
More: what about building in a clean chroot? This will invariably produce nothing but "bash: hostname: command not found" on stderr. (But the "$NAME" would evaluate to "builduser" anyway. I am not entirely sure why that field is even filled out with that much in the first place, but that is another matter entirely...)
That would be because "hostname" is provided by inetutils, which is not installed as part of base-devel.
Warning: Personal opinion follows. I personally dislike its useless output of "localhost.localdomain" too, even when git uses that. (I would much prefer using the local machine name at least, available through e.g. uname -n which is at least guaranteed available as part of coreutils, besides being 100% more distinctive for the average home system.)
fi
local size="$(@DUPATH@ @DUFLAGS@)"
Trying to scrape information from the OS in order to get better defaults is not a terrible idea, but getting rid of any fallback whatsoever in the process doesn't strike me as useful.
I also think everyone, without exception, should be configuring their makepkg.conf to provide an *actual* email, rather than falling back to the machine name. For the same reason that git users should do so (and git redeems itself by verbosely warning you every time you rely on that autodetected value). Unless of course they simply don't care about that field, in which case we can probably use "flbkoaasdfklpopefevq" instead of "Unknown Packager", and they'd never notice the difference. Though as I have no actual power here, I probably cannot get *that* enforced. :(
-- Eli Schwartz
--
Signed, Kieran Colford
On 01/02/17 08:10, Kieran Colford wrote:
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. Use these instead of defaulting to "Unknown Packager" to minimize configuration necessary by the user. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git).
Signed-off-by: Kieran Colford <kieran@kcolford.com>
This returns a pile of shit on my system.... We are not going to work around laziness. A
Would you care to share with us what pile of shit it returns? I can't fix something when I don't know what's wrong. I never suggested that we work around laziness, people should absolutely configure their system properly (I personally think violators should be punished with regular kernel panics, but I don't think Linus will accept my patch). I'm just trying to give default values that conform to what a user expects to see: if they added a real name on account creation then it should show up wherever a real name is needed, if they set the EMAIL environment variable then software should assume that's the user's email and use it. Those are both recommended by the wiki too. On Wed, Feb 1, 2017, 12:47 AM Allan McRae, <allan@archlinux.org> wrote:
On 01/02/17 08:10, Kieran Colford wrote:
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. Use these instead of defaulting to "Unknown Packager" to minimize configuration necessary by the user. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git).
Signed-off-by: Kieran Colford <kieran@kcolford.com>
This returns a pile of shit on my system.... We are not going to work around laziness.
A
-- Signed, Kieran Colford
Anybody could impersonate anybody else as long as a package isn't even signed. I don't get what we are trying to accomplish here. cheers! mar77i
On Feb 1, 2017 08:33, "Kieran Colford" <kieran@kcolford.com> wrote: Would you care to share with us what pile of shit it returns? I can't fix something when I don't know what's wrong. I never suggested that we work around laziness, people should absolutely configure their system properly (I personally think violators should be punished with regular kernel panics, but I don't think Linus will accept my patch). I'm just trying to give default values that conform to what a user expects to see: if they added a real name on account creation then it should show up wherever a real name is needed, if they set the EMAIL environment variable then software should assume that's the user's email and use it. Those are both recommended by the wiki too. And if they set PACKAGER it leaks into makepkg just the same, but better? On Wed, Feb 1, 2017, 12:47 AM Allan McRae, <allan@archlinux.org> wrote:
On 01/02/17 08:10, Kieran Colford wrote:
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. Use these instead of defaulting to "Unknown Packager" to minimize configuration necessary by the user. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git).
Signed-off-by: Kieran Colford <kieran@kcolford.com>
This returns a pile of shit on my system.... We are not going to work around laziness.
A
-- Signed, Kieran Colford
Exactly, if they have PACKAGER set then we don't make any guesses at all, they've already told us exactly what they want. On Wed, Feb 1, 2017, 8:43 AM Dave Reisner, <d@falconindy.com> wrote:
On Feb 1, 2017 08:33, "Kieran Colford" <kieran@kcolford.com> wrote:
Would you care to share with us what pile of shit it returns? I can't fix something when I don't know what's wrong.
I never suggested that we work around laziness, people should absolutely configure their system properly (I personally think violators should be punished with regular kernel panics, but I don't think Linus will accept my patch).
I'm just trying to give default values that conform to what a user expects to see: if they added a real name on account creation then it should show up wherever a real name is needed, if they set the EMAIL environment variable then software should assume that's the user's email and use it. Those are both recommended by the wiki too.
And if they set PACKAGER it leaks into makepkg just the same, but better?
On Wed, Feb 1, 2017, 12:47 AM Allan McRae, <allan@archlinux.org> wrote:
On 01/02/17 08:10, Kieran Colford wrote:
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. Use these instead of defaulting to "Unknown Packager" to minimize configuration necessary by the user. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git).
Signed-off-by: Kieran Colford <kieran@kcolford.com>
This returns a pile of shit on my system.... We are not going to work around laziness.
A
--
Signed, Kieran Colford
-- Signed, Kieran Colford
On Wed, Feb 01, 2017 at 01:48:09PM +0000, Kieran Colford wrote:
Exactly, if they have PACKAGER set then we don't make any guesses at all, they've already told us exactly what they want.
But this patch gets rid of a lovely feature in arch's db-scripts which causes an unset PACKAGER variable (resulting in "Unknown Packager") to reject packages from being pushed into the repos. Your patch breaks that, and we end up having to hunt down whomever "noclaf@rampage" is to make them fix their ~/.makepkg.conf.
On Wed, Feb 1, 2017, 8:43 AM Dave Reisner, <d@falconindy.com> wrote:
On Feb 1, 2017 08:33, "Kieran Colford" <kieran@kcolford.com> wrote:
Would you care to share with us what pile of shit it returns? I can't fix something when I don't know what's wrong.
I never suggested that we work around laziness, people should absolutely configure their system properly (I personally think violators should be punished with regular kernel panics, but I don't think Linus will accept my patch).
I'm just trying to give default values that conform to what a user expects to see: if they added a real name on account creation then it should show up wherever a real name is needed, if they set the EMAIL environment variable then software should assume that's the user's email and use it. Those are both recommended by the wiki too.
And if they set PACKAGER it leaks into makepkg just the same, but better?
On Wed, Feb 1, 2017, 12:47 AM Allan McRae, <allan@archlinux.org> wrote:
On 01/02/17 08:10, Kieran Colford wrote:
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. Use these instead of defaulting to "Unknown Packager" to minimize configuration necessary by the user. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git).
Signed-off-by: Kieran Colford <kieran@kcolford.com>
This returns a pile of shit on my system.... We are not going to work around laziness.
A
--
Signed, Kieran Colford
--
Signed, Kieran Colford
Really????? That is a problem then. Could we improve that feature to check that the PACKAGER variable matches a userid of the key that signs said package? That would not only be a stronger requirement (forcing a correct makepkg.conf, rather than one that's just set so you're running around trying to find "hfkskxfjks"), but it would also remove the reliance on makepkg's implementation defined behavior. I could try submitting a patch to the relevant mailing list of you think it's worthwhile. On Wed, Feb 1, 2017, 9:19 AM Dave Reisner, <d@falconindy.com> wrote: On Wed, Feb 01, 2017 at 01:48:09PM +0000, Kieran Colford wrote:
Exactly, if they have PACKAGER set then we don't make any guesses at all, they've already told us exactly what they want.
On Wed, Feb 1, 2017, 8:43 AM Dave Reisner, <d@falconindy.com> wrote:
On Feb 1, 2017 08:33, "Kieran Colford" <kieran@kcolford.com> wrote:
Would you care to share with us what pile of shit it returns? I can't fix something when I don't know what's wrong.
I never suggested that we work around laziness, people should absolutely configure their system properly (I personally think violators should be punished with regular kernel panics, but I don't think Linus will accept my patch).
I'm just trying to give default values that conform to what a user expects to see: if they added a real name on account creation then it should show up wherever a real name is needed, if they set the EMAIL environment variable then software should assume that's the user's email and use it. Those are both recommended by the wiki too.
And if they set PACKAGER it leaks into makepkg just the same, but better?
On Wed, Feb 1, 2017, 12:47 AM Allan McRae, <allan@archlinux.org> wrote:
On 01/02/17 08:10, Kieran Colford wrote:
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. Use
But this patch gets rid of a lovely feature in arch's db-scripts which causes an unset PACKAGER variable (resulting in "Unknown Packager") to reject packages from being pushed into the repos. Your patch breaks that, and we end up having to hunt down whomever "noclaf@rampage" is to make them fix their ~/.makepkg.conf. these
instead of defaulting to "Unknown Packager" to minimize configuration necessary by the user. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git).
Signed-off-by: Kieran Colford <kieran@kcolford.com>
This returns a pile of shit on my system.... We are not going to work around laziness.
A
--
Signed, Kieran Colford
--
Signed, Kieran Colford
-- Signed, Kieran Colford
After taking in your input, I'd like you to consider this revised patch. I've cleaned up the scraping and set it up to fallback to old behaviour if it can't find an intelligent and resonable default.
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. We can use this to make the defaults more intelligent. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git). Signed-off-by: Kieran Colford <kieran@kcolford.com> --- scripts/makepkg.sh.in | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 29408929..b2a0b89a 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -613,12 +613,22 @@ write_kv_pair() { done } +default_packager() { + local user="${USER:-$(whoami)}" + # Try to find the user's real name is in the environment or + # the passwd databse. + local name="${NAME:-$(getent passwd "${user}" | cut -d : -f 5 | cut -d , -f 1)}" + # Try to find the user's email in the environment only. + local email="${EMAIL}" + echo "${name:-Unknown Packager}${email:+ <${email}>}" +} + write_pkginfo() { local builddate=$(date -u "+%s") if [[ -n $PACKAGER ]]; then local packager="$PACKAGER" else - local packager="Unknown Packager" + local packager="$(default_packager)" fi local size="$(@DUPATH@ @DUFLAGS@)" -- 2.11.0
On 02/02/17 01:59, Kieran Colford wrote:
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. We can use this to make the defaults more intelligent. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git).
Signed-off-by: Kieran Colford <kieran@kcolford.com>
This will not be included in makepkg. Any "guessing" will inevitably be wrong at some stage and we will have more and more fixes requested on top of this. We have a single place to configure this information. That is enough. Allan
On Thu, Feb 02, 2017 at 02:39:21AM +1000, Allan McRae wrote:
On 02/02/17 01:59, Kieran Colford wrote:
The system usually has enough information in various places to guess the name and email of the person running the makepkg script. We can use this to make the defaults more intelligent. This particular implemenation should provide relatively good guess that's compatible with other programs (i.e. git).
Signed-off-by: Kieran Colford <kieran@kcolford.com>
This will not be included in makepkg. Any "guessing" will inevitably be wrong at some stage and we will have more and more fixes requested on top of this.
Perhaps we should bail if PACKAGER isn't set, then.
We have a single place to configure this information. That is enough.
Allan
On 02/01/2017 08:33 AM, Kieran Colford wrote:
Would you care to share with us what pile of shit it returns? I can't fix something when I don't know what's wrong.
I already told you a few things... maybe Allan didn't want to repeat what has already been said.
I never suggested that we work around laziness, people should absolutely configure their system properly (I personally think violators should be punished with regular kernel panics, but I don't think Linus will accept my patch).
I don't understand how you can say this...
I'm just trying to give default values
... and then this. Which is "to minimize configuration necessary by the user" according to your original patch message.
that conform to what a user expects to see: if they added a real name on account creation then it should show up wherever a real name is needed, if they set the EMAIL environment variable then software should assume that's the user's email and use it. Those are both recommended by the wiki too.
Honestly, trying to extract this information from sources that most people don't bother to set up properly, and which I certainly never saw recommended, seems kind of silly. The fact that proper laziness (by looking for as many farfetched sources of substandard information as possible) requires ugly and hackish code, should be a turnoff too; you are trying to do too much. If anything, maybe makepkg.conf should be responsible for properly setting PACKAGER by default, from "$NAME <${EMAIL}>". If they haven't set that data somewhere, I do not see how you can possibly justify the unambiguous laziness involved in using *untrue* values that are *guessed* from hostname analysis, and will *not* be what the user actually wants in any situation! The proper solution is to... use makepkg.conf as it is intended to be used!* It seems to me that you have the (partially) right solution to the wrong problem. And you might be looking in the wrong place to fix it, too. This would be better suited to the tooling used to build package repositories, especially since that will catch the info outside of build chroots and pass it into the chroot. -- Eli Schwartz * I have to wonder, which people aren't setting their makepkg.conf who actually look at the Packager information. The former will be the less savvy AUR users, and the latter will be people looking at packages someone else built. I don't know of anyone publishing pacman repos who don't set their makepkg.conf...
participants (5)
-
Allan McRae
-
Dave Reisner
-
Eli Schwartz
-
Kieran Colford
-
Martin Kühne