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

[Rocky Bernstein]

On Fri, May 4, 2012 at 8:19 AM, Jim Meyering <address@hidden> wrote:

> 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 didn't say *deliberately unfriendly*. I said there was an *unfriendly
environment*. No doubt, it is unintentional. But for me, for this, it is
there. More below.

 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.
>

Understood and accepted. But really there is not much to show that this has
moved forward in the 5 months. Well, okay, for 5 months of time, you have
now at least tried the patch.

So perhaps for the delays, you or someone reading could make the stylistic
changes that would take, what, maybe a few minutes? I promise in the future
with respect to coreutils I will follow coreutils conventions more
carefully. But is this about me getting following coreutils conventions or
is this about getting a "su" problem solved? Right now, if feels like the
former, hence "unfriendly environment".

Also if you had taken the few minutes to make the stylistic changes, you
would have known that things would be properly and it would show that this
you have some *interest* in seeing things improved. Deeds sometimes speak
volumes more than apologies.


>
> > 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.
>

Well, that too could have been mentioned 3 months ago -- the *first* time
you talked about stylistic things. Did I say information in drips and
drabs? And it wouldn't have caught the other two. I guess we can both play
the "you did something wrong, let's delay this" game.


> > It's now about 5 months since I snt 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.
>