[PATCH 0/1] V2 Fix libpam's chmod

[PATCH 0/1] V2 Fix libpam's chmod

[Robert Yang]

Hi Saul,

This patch is based on oe-core, but I don't have permission to put the pull
request on git://git.openembedded.org/openembedded-core-contrib, so I still
put the pull request on poky-contrib.

Changes of V2:

* Update the PR bump as Koen pointed out.

//Robert


The following changes since commit 6b6aa170d5c618cfcd016dd1de926db647f7f932:

  busybox: Restore 'date' compatability (2012-03-01 23:16:43 +0000)

are available in the git repository at:
  git://git.pokylinux.org/poky-contrib robert/libpam
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=robert/libpam

Robert Yang (1):
  Fix libpam's chmod

 meta/recipes-extended/pam/libpam_1.1.5.bb |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

-- 
1.7.4.1

[PATCH 1/1] V2 Fix libpam's chmod

[Robert Yang]

The libpam's has an error when generating the rootfs:

chmod: cannot access `/usr/sbin/unix_chkpwd': No such file or directory

This is because the following code in libpam_1.1.5.bb:

pkg_postinst_pam-plugin-unix () {
    # below is necessary to allow unix_chkpwd get user info from shadow file
    # on lsb images
    chmod 4755 ${sbindir}/unix_chkpwd
}

This is to set the setuid permission for unix_chkpwd (the lsb test
requires this), but it lacks a "${D}", and we can do this in the install
stage.

[YOCTO #2049]

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 meta/recipes-extended/pam/libpam_1.1.5.bb |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/meta/recipes-extended/pam/libpam_1.1.5.bb b/meta/recipes-extended/pam/libpam_1.1.5.bb
index 283f477..8dd5ac5 100644
--- a/meta/recipes-extended/pam/libpam_1.1.5.bb
+++ b/meta/recipes-extended/pam/libpam_1.1.5.bb
@@ -9,7 +9,7 @@ SECTION = "base"
 LICENSE = "GPLv2+ | BSD"
 LIC_FILES_CHKSUM = "file://COPYING;md5=ca0395de9a86191a078b8b79302e3083"
 
-PR = "r2"
+PR = "r3"
 
 SRC_URI = "https://fedorahosted.org/releases/l/i/linux-pam/Linux-PAM-${PV}.tar.bz2 \
            file://99_pam \
@@ -85,10 +85,7 @@ do_install() {
 
 	install -d ${D}${sysconfdir}/pam.d/     
 	install -m 0644 ${WORKDIR}/pam.d/* ${D}${sysconfdir}/pam.d/
-}
 
-pkg_postinst_pam-plugin-unix () {
-    # below is necessary to allow unix_chkpwd get user info from shadow file
-    # on lsb images
-    chmod 4755 ${sbindir}/unix_chkpwd
+    # The lsb requires unix_chkpwd has setuid permission
+    chmod 4755 ${D}${sbindir}/unix_chkpwd
 }
-- 
1.7.4.1

Re: [PATCH 1/1] V2 Fix libpam's chmod

[Richard Purdie]

On Fri, 2012-03-02 at 18:34 +0800, Robert Yang wrote:
> The libpam's has an error when generating the rootfs:
> 
> chmod: cannot access `/usr/sbin/unix_chkpwd': No such file or directory
> 
> This is because the following code in libpam_1.1.5.bb:
> 
> pkg_postinst_pam-plugin-unix () {
>     # below is necessary to allow unix_chkpwd get user info from shadow file
>     # on lsb images
>     chmod 4755 ${sbindir}/unix_chkpwd
> }
> 
> This is to set the setuid permission for unix_chkpwd (the lsb test
> requires this), but it lacks a "${D}", and we can do this in the install
> stage.
> 
> [YOCTO #2049]
> 
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
>  meta/recipes-extended/pam/libpam_1.1.5.bb |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)

Merged to master, thanks.

Richard

Re: [PATCH 1/1] V2 Fix libpam's chmod

[Richard Purdie]

On Fri, 2012-03-02 at 18:34 +0800, Robert Yang wrote:
> The libpam's has an error when generating the rootfs:
> 
> chmod: cannot access `/usr/sbin/unix_chkpwd': No such file or directory
> 
> This is because the following code in libpam_1.1.5.bb:
> 
> pkg_postinst_pam-plugin-unix () {
>     # below is necessary to allow unix_chkpwd get user info from shadow file
>     # on lsb images
>     chmod 4755 ${sbindir}/unix_chkpwd
> }
> 
> This is to set the setuid permission for unix_chkpwd (the lsb test
> requires this), but it lacks a "${D}", and we can do this in the install
> stage.
> 
> [YOCTO #2049]
> 
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>

Robert, in future patches could you use a subject line like:

[PATCH 1/1 v2] Fix libpam's chmod

instead of:

[PATCH 1/1] V2 Fix libpam's chmod

so that when the patches get applied, the V2 is stripped off?

The reason is that the value isn't particularly within the repository
once merged and doesn't look good when looking through commit history.

Thanks!

Richard

Re: [PATCH 1/1] V2 Fix libpam's chmod

[Koen Kooi]

Op 2 mrt. 2012, om 14:26 heeft Richard Purdie het volgende geschreven:

> On Fri, 2012-03-02 at 18:34 +0800, Robert Yang wrote:
>> The libpam's has an error when generating the rootfs:
>> 
>> chmod: cannot access `/usr/sbin/unix_chkpwd': No such file or directory
>> 
>> This is because the following code in libpam_1.1.5.bb:
>> 
>> pkg_postinst_pam-plugin-unix () {
>>    # below is necessary to allow unix_chkpwd get user info from shadow file
>>    # on lsb images
>>    chmod 4755 ${sbindir}/unix_chkpwd
>> }
>> 
>> This is to set the setuid permission for unix_chkpwd (the lsb test
>> requires this), but it lacks a "${D}", and we can do this in the install
>> stage.
>> 
>> [YOCTO #2049]
>> 
>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> 
> Robert, in future patches could you use a subject line like:
> 
> [PATCH 1/1 v2] Fix libpam's chmod
> 
> instead of:
> 
> [PATCH 1/1] V2 Fix libpam's chmod
> 
> so that when the patches get applied, the V2 is stripped off?
> 
> The reason is that the value isn't particularly within the repository
> once merged and doesn't look good when looking through commit history.

Actually both versions above are wrong, the right version would be similar to:

libpam 1.1.5: fix chmod in postinst

We can debate the usefulness of the version specifier, but we do need to follow the commit guide for the rest.

regards,

Koen

Re: [PATCH 1/1] V2 Fix libpam's chmod

[Richard Purdie]

On Fri, 2012-03-02 at 14:31 +0100, Koen Kooi wrote:
> Op 2 mrt. 2012, om 14:26 heeft Richard Purdie het volgende geschreven:
> 
> > On Fri, 2012-03-02 at 18:34 +0800, Robert Yang wrote:
> >> The libpam's has an error when generating the rootfs:
> >> 
> >> chmod: cannot access `/usr/sbin/unix_chkpwd': No such file or directory
> >> 
> >> This is because the following code in libpam_1.1.5.bb:
> >> 
> >> pkg_postinst_pam-plugin-unix () {
> >>    # below is necessary to allow unix_chkpwd get user info from shadow file
> >>    # on lsb images
> >>    chmod 4755 ${sbindir}/unix_chkpwd
> >> }
> >> 
> >> This is to set the setuid permission for unix_chkpwd (the lsb test
> >> requires this), but it lacks a "${D}", and we can do this in the install
> >> stage.
> >> 
> >> [YOCTO #2049]
> >> 
> >> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> > 
> > Robert, in future patches could you use a subject line like:
> > 
> > [PATCH 1/1 v2] Fix libpam's chmod
> > 
> > instead of:
> > 
> > [PATCH 1/1] V2 Fix libpam's chmod
> > 
> > so that when the patches get applied, the V2 is stripped off?
> > 
> > The reason is that the value isn't particularly within the repository
> > once merged and doesn't look good when looking through commit history.
> 
> Actually both versions above are wrong, the right version would be similar to:
> 
> libpam 1.1.5: fix chmod in postinst
> 
> We can debate the usefulness of the version specifier, but we do need to follow the commit guide for the rest.

Good point, I was just commenting that the patch version specifier
didn't look good. As you say, the package name should be first too, as
per http://wiki.openembedded.org/wiki/Commit_Patch_Message_Guidelines
under Common Errors in Patch and Commit Messages: '- Short log does not
start with the file or component being modified. Such as "foo: Update to
new upstream version 5.8.9"'.

If I was going to be picky, I'd suggest capitalisation of "fix" in your
version ;-)

Cheers,

Richard

Re: [PATCH 1/1] V2 Fix libpam's chmod

[Koen Kooi]

Op 2 mrt. 2012, om 14:39 heeft Richard Purdie het volgende geschreven:

> On Fri, 2012-03-02 at 14:31 +0100, Koen Kooi wrote:
>> Op 2 mrt. 2012, om 14:26 heeft Richard Purdie het volgende geschreven:
>> 
>>> On Fri, 2012-03-02 at 18:34 +0800, Robert Yang wrote:
>>>> The libpam's has an error when generating the rootfs:
>>>> 
>>>> chmod: cannot access `/usr/sbin/unix_chkpwd': No such file or directory
>>>> 
>>>> This is because the following code in libpam_1.1.5.bb:
>>>> 
>>>> pkg_postinst_pam-plugin-unix () {
>>>>   # below is necessary to allow unix_chkpwd get user info from shadow file
>>>>   # on lsb images
>>>>   chmod 4755 ${sbindir}/unix_chkpwd
>>>> }
>>>> 
>>>> This is to set the setuid permission for unix_chkpwd (the lsb test
>>>> requires this), but it lacks a "${D}", and we can do this in the install
>>>> stage.
>>>> 
>>>> [YOCTO #2049]
>>>> 
>>>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
>>> 
>>> Robert, in future patches could you use a subject line like:
>>> 
>>> [PATCH 1/1 v2] Fix libpam's chmod
>>> 
>>> instead of:
>>> 
>>> [PATCH 1/1] V2 Fix libpam's chmod
>>> 
>>> so that when the patches get applied, the V2 is stripped off?
>>> 
>>> The reason is that the value isn't particularly within the repository
>>> once merged and doesn't look good when looking through commit history.
>> 
>> Actually both versions above are wrong, the right version would be similar to:
>> 
>> libpam 1.1.5: fix chmod in postinst
>> 
>> We can debate the usefulness of the version specifier, but we do need to follow the commit guide for the rest.
> 
> Good point, I was just commenting that the patch version specifier
> didn't look good. As you say, the package name should be first too, as
> per http://wiki.openembedded.org/wiki/Commit_Patch_Message_Guidelines
> under Common Errors in Patch and Commit Messages: '- Short log does not
> start with the file or component being modified. Such as "foo: Update to
> new upstream version 5.8.9"'.
> 
> If I was going to be picky, I'd suggest capitalisation of "fix" in your
> version ;-)

I'm a bit conflicted on that one. If I'm going to treat it as a sentence by using caps, I'd also like to use proper punctuation, which the guidelines don't allow. And that's why I said 'similar' :)

regards,

Koen