Re: bin/53919: please revert changes that make /bin/sh evaluate $ENV non-interactively

[Robert Elz <kre%munnari.OZ.AU@localhost>]

The following reply was made to PR bin/53919; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: bin/53919: please revert changes that make /bin/sh evaluate $ENV non-interactively
Date: Tue, 29 Jan 2019 13:44:27 +0700

     Date:        Mon, 28 Jan 2019 20:45:00 +0000 (UTC)
     From:        "Greg A. Woods" <woods%planix.ca@localhost>
     Message-ID:  <20190128204500.DD68C7A1FE%mollari.NetBSD.org@localhost>
 
   | 	Please revert the changes to /bin/sh which cause it to evaluate
   | 	$ENV when not running in "interactive" mode.
 
 I can interpret this in several ways.   If you are requesting that
 we re-open PR 42828, then sorry, no.
 
 If ...
   | 	I.e. revert, or disable, the changes made for PR 42829
 
 That (alone) I am certainly willing to condsider, that one was only
 processed in the name of being more POSIX compatible, and with
 the assumption that it would do no harm.   I personally see no
 rational benefit in having $ENV parameter expanded, it ought simply
 be set to a file name (with any expansions needed being done
 when it is set) - though from your usage I doubt that you agree.
 
 I do not think it would be rational or sane to make the decision on
 whether or not to parameter expand $ENV be done based upon
 whether the shell is interactive or not - whatever happens, it should
 be the same both ways (again, no plans to avoid using $ENV in
 non-ineractive shells) - not that I believe that you are suggesting
 that change.   Here I am just mentioning all the possibilities.
 
   | 	The expansion of $ENV by non-interactive /bin/sh process makes
   | 	the system effectively and entirely unusable by some of us users
   | 	of /bin/ksh who have been happily using a certain feature of
   | 	/bin/ksh and similar shells for over 30 years now.
 
 I don't really belive this is a rational thing to do, or to expect to work.
 While it may be been functional 30 years ago, when the sh choices
 available were limited, these days there are lots more shells around,
 and when they extend POSIX, which almost all do, one way or another,
 they all tend to be different, though there are some common extensions.
 
 ENV is a POSIX specified variable, and parameter expanding it, is
 a POSIX mandated activity (whether we decide to continue that
 practice in /bin/sh or not), so any setting of $ENV ought to be limited
 to use only POSIX defnied mechanisms if you have any real hope
 that it is going to work.
 
 Just doing something as simple as "I see a new shell appeared in pkgsrc,
 let's see if it is any good" ... so you install it using whatever method you
 use for installing packages, and then try it out
 	newshell
 to see what happens - only to find that being a POSIX compatible (or
 mostly) shell, and interactive, it parameter expands $ENV and because
 your version of that uses non-POSIX mechanisms, it fails.
 
 What you're effectively doing in this PR is requesting that all shells
 which do not happen to have the extensions that you want to use to
 ignore the POSIX spec and not parameter expand $ENV - only shells
 that happen to implement the extensions that you want to use are to
 be permitted to parameter expand $ENV.
 
 [Aside: for casual readers, note it is $ENV that this relates to, the
 name ENV has always been expanded, what is of concern here is
 whether the string produced from generating ${ENV} is expanded
 again before being used.]
 
 Your usage has always been broken, it just has not affected you
 until now.
 
 You should also be aware that your particular usage not only
 requires that arrays be implemented in the shell in question (something
 that will never happen in /bin/sh while I am maintaing it, they are
 entirely unnecessary, and cause all kinds of bizarre parsing issues
 as commonly defined) - but you're also expecting that they be
 implemented and accessed the way that ksh first implemened them,
 and that the array indexes be interpreted and expanded using the
 parsing rules that ksh defined for them, neither of which is guaranteed
 in anything except ksh.   That is, you should ensure that your magic
 expression is only ever expanded by a ksh type shell, and is never
 seen by anything else.   Including it in $ENV 30 years ago only ever
 worked because only ksh (and bash, which copied ksh for this) actually
 did parameter expansion -- and I note you are, or were in 2010 when
 you commented on PR 42828, also expecting tilde expansion of $ENV,
 which POSIX does not specify (and we do not implement) - of course,
 that would easily be accommodated, and perhaps has been since,
 by just using '${HOME}' instead of '~'.   (You were (maybe still are)
 expecting ~ expansion to happen (for this anyway) after parameter
 expansion, which never happens anywhere in a POSIX shell.)
 
 A better solution for your problem, which should be much more
 portable, is simply to make $ENV be a very short file, that determines
 whether it should be used at all (if you don't want it in non-interactive
 shells, it should simply return) and then determines which shell is
 being run (without using unsafe tricks like uses of $RANDOM unless
 there is absolutely no other way) and then sources a shell specific
 setup script, or a generic one (which uses only standard POSIX) if
 it cannot determine with some degree of certainty, which shell is
 currently being used.   This has to be done inside the file set as
 $ENV, and not as part of login time init, as the shell expanding
 $ENV and then reading and processing the result might not even
 exist at the time you logged in.
 
   | >Fix:
   |
   | 	I don't know what the best fix is.  As I said in PR 42828 I
   | 	think the best solution would be _full_ compatability with the
   | 	original source of this feature, namely the original Korn Shell.
   | 	However as I discussed this would require implementation of
   | 	arrays for /bin/sh, and that's probably not a short term fix.
 
 Not short term, not long term, not any term as far as I am concerned.
 If you want ksh to be /bin/sh you know how to make that happen, I
 presume...    If you use NetBSD's (ancient pdksh version in /bin/ksh)
 for that purpose, don't expect the rest f the system to work properly.
 
   | 	My sort-term hack, just to make my system usable again, is to
   | 	change the name of the variable to SHENV.  However I doubt I
   | 	will ever set $SHENV for my own use, because:
 
 That would be to acceed to PR 42828, which we have already decided
 we are not going to do (more than what was done years ago.)
 
   | 	The reason the current implementation is unacceptable is because
   | 	it forces the shell to open the file referenced by $ENV and to
   | 	read the entire file's contents
 
 That depends upon the shell.   /bin/sh reads only as much as it needs
 to before the script in there is done.   But it should be a very small file,
 if it exceeds one read buffer worth, then it is probably doing more than
 it should.   Some other shells wll read the entire thing (though those also
 tend to be ones that only process ENV in interactive shells.)
 
   | 	recommended hack for interactive-only use) and, if I'm not
   | 	mistaken, parse all of what it reads as well.
 
 You're mistaken for /bin/sh - it reads and parses a command at a
 time, then executes it.  If that results in a "return" we're done, and
 it neither reads (though more was probably already read into the
 file buffer), nor parses, any more.  Other shells (perhaps ksh - or
 some variants - and maybe bosh) parse the whole thing before
 executing any of it (they are more likely to do that for files read
 using '.'  but sometimes the same mechanism is used to process
 $ENV).
 
   | 	However I still can't think of a better fix than to implement
   | 	arrays in /bin/sh.
 
 You could just not sent ENV, and instead of modifying sh to use
 a different variable name, instead modify ksh to use KSH_ENV or
 something, in which you could put as much KSH specific syntax
 as you like.   ksh could continue processing ENV as well, for
 posix conformance, but you wouldn't be using that.
 
 That would be a much more portable solution.   I believe bash
 has that functionality - it has its own startup script var name
 which it will read.
 
 That is something I could do for sh as well, if people would like
 that capability, a NetBSD sh specific var which names a startup
 file only processed by the NetBSD sh (or two, one for interactive
 shells, and one for others) if people would like that (if so, please
 file a "change-request" PR to suggest it.)   That would not help for
 this issue however, as $ENV is still going to be there, and processed
 the same way (whichever way that ends up being - the -8 way, or
 the HEAD way.)
 
   | 	I also add a small error message to try to help explain the
   | 	mystery error message.
 
 Since there are many ways that expanding $ENV can lead to problems
 if it is not set correctly, and not all of them are related to expansion
 errors, I am not sure that would help a lot, though it might be useful
 in some cases.
 
 Your implementation won't work in general (it works for you because
 you're expecting the expanded $ENV to be different to the unexpanded
 one, but that is not the usual case) but that would be easily fixed if we
 decide getting (more) errors from faulty settings of $ENV is a good idea.
 
 Now I believe we need input from others, before any changes are made,
 but pending that, my position would be that reverting the PR42829 change
 is not out of the question, but that (aside from more possible error messages,
 or perhaps just enhancing what is there now to be more specific as to
 context, if we don't revert 42829 changes) that is the only change that is
 likely to result from this PR.
 
 kre