Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should SHOPT_SYSRC and the code predicated on it be removed? #330

Closed
krader1961 opened this issue Jan 5, 2018 · 5 comments
Closed

Should SHOPT_SYSRC and the code predicated on it be removed? #330

krader1961 opened this issue Jan 5, 2018 · 5 comments

Comments

@krader1961
Copy link
Contributor

The SHOPT_SYSRC symbol is another build time option I stumbled across. It is not enabled by default in the 2016-01-10-beta branch. It is not enabled in the current master branch. From lib/package/ast-open.README:

05-03-25 +Building ksh with SHOPT_SYSRC=1 causes interactive ksh to source
          /etc/ksh.kshrc (if it exists) before sourcing the $ENV file.

This feature should either be enabled unconditionally or removed. We should not be enabling people to build ksh with features enabled or disabled based solely on their whims without regard to whether the platform requires a specific choice (in which case we should auto-detect it at build time).

@dannyweldon
Copy link

I just checked the system ksh on my ubuntu 16.04 machine:

$ strings /usr/bin/ksh | grep kshrc
%s/.kshrc
/etc/ksh.kshrc
/etc/ksh.kshrc
${ENV-$HOME/.kshrc}
.kshrc

Then I checked my very out-of-date rhel7 vm and it does not have the /etc/ksh.kshrc references.

@catull
Copy link
Contributor

catull commented Jan 5, 2018

Searching for strings in the binary I downloaded years ago for macOS 10.6(?) reveals ...

strings ksh.2012-08-06.darwin.i386-64 | fgrep kshrc
%s/.kshrc
.kshrc
${ENV-$HOME/.kshrc}

Even if this shows a different configuration, I favour the unconditional inclusion of the code.
If the file /etc/ksh.kshrc` does exist, it should be sourced.

macOS does not come with /etc/ksh.kshrc.
Thus including the code has no impact on this platform.

+1 on unconditional inclusion of code, i.e. removal of #if SHOPT_SYSRC/#endif only!

Having said that ....

strings ksh.2012-08-06.darwin.i386-64| fgrep /etc

shows

/etc/ksh_audit
/etc/profile
/etc/suid_profile
/etc/suid_exec
/etc/profile

@dannyweldon
Can you confirm these, too ?

@krader1961
Copy link
Contributor Author

The /etc/ksh_audit file is issue #240. The other paths appear to be enabled unconditionally in the current code base.

@dannyweldon
Copy link

From ubuntu 16.04:

$ strings /usr/bin/ksh | grep /etc
/etc/ksh_audit
/etc/suid_exec
/etc/ksh.kshrc
/etc/suid_profile
/etc/profile
/etc/profile
/etc/ksh.kshrc
/etc/mtab

From my rhel7 vm:

$ strings /bin/ksh | grep /etc
/etc/ksh_audit
/etc/suid_exec
/etc/suid_profile
/etc/profile
/etc/profile
/etc/mtab

@krader1961
Copy link
Contributor Author

I am not convinced the SHOPT_SYSRC feature is worthwhile but since it is enabled in some distros (e.g., Ubuntu) we should enable it unconditionally. So I'm going to remove that symbol and unconditionally compile the code predicated on it. This type of feature should not be at the discretion of a particular OS distro. OS distros that don't want to support it can simply omit providing a /etc/ksh.kshrc file.

We should not be making it trivial for people to build ksh variants that behave in fundamentally different ways unrelated to the quirks of a specific architecture.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants