bug#10317: PING - bug#10317: patch to su: -l and -p should not be used together

[Jim Meyering]

Rocky Bernstein wrote:
> On Fri, May 4, 2012 at 4:49 AM, Jim Meyering <address@hidden> wrote:
>
>     Rocky Bernstein wrote:
>     > Any word on this?
>
>     Hi Rocky,
>
>     Sorry about the extended delay.
>     Changing su.c like this has really low priority.
>
>     I've looked at your patch and see that it changes the semantics
>     of su for those who use -l with (-m or -p).
>
>     Before your patch, -l would lead to simulate_login being set even
>     when -l was specified after an -m-or-p option.
>     Now, using e.g., "-l -m" evokes a warning but the -l is otherwise ignored.
>
>     Did you intend that?
>
> I never had a strong feeling on it to the extent that ambiguous behavior 
> should
> be preserved. However much more important is issuing a warning says what's 
> done
> whatever it is and currently, that is not done.  I am sensing (but am not
> completely sure that)  the preferred behavior is to keep compatible behavior
> whatever the current behavior is?

That is what I expected, and would certainly require less justification.
If you have a good case for making the semantic change, I'm open to
that, too.

> May as well get this cleared up before another
> 1/3 of a year goes by.
>
>     Normally such a change would be mentioned in a ChangeLog entry or commit
>     log.
>
>     I tried to use the su from coreutils (with or without your patch)
>     and found that it does not work when it attempts to authenticate.
>     E.g., it cannot su to any user on this Fedora 17 system.  If su
>     remains so broken that it does not work out-of-the-box on F17,
>     then it's not worth patching.  Remember that I wanted to remove su
>     from coreutils altogether, and only reluctantly agreed to consider
>     this change.  If someone who cares about su remaining in coreutils
>     wants to take responsibility for making it usable, that'd be great.
>     It may be as simple as importing a patch or two from Fedora or Suse.
>
>     If you'd like to pursue your change once su is restored to working
>     order, please justify or revert the semantic change in your patch,
>
> Actually, at this point I am loosing interest. You have created a somewhat
> unfriendly environment here to work in.

Please don't interpret my review delays or my misreading your
patch as anything deliberately unfriendly.  I've been trying to keep
this list as open/accommodating as possible for nearly two decades.
I agree that the long delay is off-putting and apologized for that, twice.

> I suggest, but leave up to you, whether
> to just document the behavior as it is leaving the code exactly as it is.
>
>     starting from the change-set below, which includes the following changes:
>
>      - remove some space-before-TAB in tests/Makefile.am
>      - remove space-before-semicolon in su.c
>      - split two longer-than-80-col lines
>      - remove both \n and trailing "." from two new diagnostics
>      - adjust NEWS
>
> Again, if you or anyone reading this can't be bothered to do this, I will also
> put this on my  "low priority" list. You have in mind what you want to see and
> I've been getting this information in drips and drabs which is annoying.

If you had read the contribution guidelines in HACKING, you would have
known to run "make syntax-check", which would highlighted three of the
five issues listed above.

> It's now about 5 months since I sent this and only now have you even attempted
> to try the patch. (The last correspondence you incorrectly made a denigrating
> assumption which just added gratuitous delay.)  Most of the stylistic things
> mentioned here could have been mentioned a couple of months ago.