[Pam-developers] Re: Patches from ticket #46

[Dmitry V. Levin]

On Fri, Apr 01, 2016 at 03:42:11PM +0200, Tomas Mraz wrote:
>  On Pá, 2016-04-01 at 14:39 +0200, Thorsten Kukuk wrote:
>  > Hi,
>  > 
>  > There is the ticket #46:
>  > https://fedorahosted.org/linux-pam/ticket/46
>  > Fix build with musl libc
>  > 
>  > The documentation to the patches are pretty bad :(, but
>  > there is some usefull/correct stuff, that's why I want to
>  > discuss them here:
>  > 
>  > 0001-build-use-host_cpu-for-lib64-directory-handling.patch
>  > 
>  > I think the patch is correct, we should not use uname but the
>  > autoconf $host_cpu variable. Else the options the user specifies
>  > are ignored.
>  > 
>  > 
>  > 0002-build-ignore-pam_rhosts-if-neither-ruserok-nor-ruser.patch
>  > 
>  > While the idea behind the patch is Ok, the patch itself is crap.
>  > I wouldn't add that patch as long as nobody rewrites it to do it
>  > the same way as we do for the other modules.
>  > 
>  > 
>  > 0003-build-ignore-pam_lastlog-when-logwtmp-is-not-availab.patch
>  > 
>  > Same here.
>  > 
>  > 
>  > 0004-build-fix-build-when-crypt-is-not-part-of-crypt_libs.patch
>  > 
>  > The patch is correct, fixes a real bug and should be commited.
>  > 
>  > 
>  > 0005-build-fix-doc-build.patch
>  > 
>  > I'm strongly against that patch, it can break everything. If I want
>  > to compile a git checkout and not from the official sources, I should
>  > have the minimal required tools for this.
>  > 
>  > 
>  > 0006-build-build-xxx_MANS-only-if-ENABLE_REGENERATE_MAN.patch
>  > 
>  > I'm unsure about this patch. Looks correct, but I'm not sure
>  > about side effects. 
It looks OK but doesn't apply and has to be regenerated anyway.

>  > 0007-modules-check-if-innetgr-is-available-at-compile-tim.patch
>  > 
>  > That patch is fine for me.
>  > 
>  > 
>  > 0008-pam_unix-fix-compilation-in-case-rpc-rpc.h-is-missin.patch
>  > 
>  > This patch doesn't make any sense to me. If HAVE_NIS is defined,
>  > but rpc/rpc.h does not exist, we will still include rpc/rpc.h. So
>  > this patch cannot fix anything. I would reject that.
>  > 
>  > 0009-pam_exec-fix-build-when-strndupa-is-not-available.patch
>  > 
>  > I don't like that patch at all.
>  > 
>  > Your comments? 
>  > I would like to apply Patch 1, 4 and 7 and close that report.
>  
>  I agree with you and I am OK with committing patches 1, 4 and 7. Also
>  please mention to the reporter when closing the ticket that for patches
>  2 and 3 they need to be reworked similarly to how pam_selinux is
>  conditionally compiled when SELinux support is not available. 
I also agree with your review.


-- 
ldv