bug#7572: [PATCH] PAM support for su

[Jim Meyering]

Ludwig Nussel wrote:
> Jim Meyering wrote:
>> Ludwig Nussel wrote:
>> > Are there any concerns with the patch? It would be really nice to
>> > have this merged upstream to avoid further fragmentation.
>>
>> The main concern is that by default coreutils doesn't even build su anymore.
>
> Does that mean you intend to drop su from coreutils? If so is there

I would have dropped it long ago but for some distributions
for which the switch to e.g., util-linux was not an option.

> any suggested alternative? Should we move su to e.g. util-linux
> instead?

It's worth considering.

>> However, if this makes it easier on Fedora and Suse packagers, then
>> I suppose it's worthwhile.
>>
>> If you'd like to pursue the matter, there are a few missing pieces:
>>
>>  - Ensure that "make syntax-check" still passes with this patch.
>>    I see cpp indentation that may fail the test that runs cppi.
>>    That test is run only when cppi is installed, so you may have
>>    to install it.
>>
>>  - it will need a ChangeLog entry, including attribution if you can
>>    dig that up.
>
> Ok, I'll check both.
>
>>  - I haven't looked carefully, but considering the size, I'd be
>>    surprised if there is no need to document changes -- in
>>    coreutils.texi
>
> Yes. Our package actually has a separate patch that modifies the
> docu. For upstream the pam support is optional though so any
> addition to coreutils.texi would need to be conditional I suppose.
> So we'd need e.g. a coreutils.texi.in that gets rewritten by
> configure.

Simpler is to start a paragraph/section with a few words saying that
some additional functionality is available when PAM support is enabled.

>>  - include a NEWS entry
>
> ok
>
>>  - tests would be most welcome, but I won't insist on those
>
> Hmm, I'm not sure that's feasible. Tests would need to run as
> root and they'd likely have to modify /etc/pam.d.

root-only tests are not a problem.
There are already quite a few.  For examples,
see the scripts under tests/ that use "require_root_".
However, as you imply, if the only way to test is
by changing the likes of /etc/pam.d, then it's easy:
automated tests are not an option ;-)