-
Notifications
You must be signed in to change notification settings - Fork 338
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
Rewrite all bash-specific code to be POSIX shell compatible #77
Comments
On 11/07/2017 04:34 AM, Twisted Pair in my Hair wrote:
Most systems have bash installed, so that is not an issue for them, but
some lack it. I suggest changing default interpreter of |scripts/*.sh|
and |tests/*.test| from |/bin/bash| to |/bin/sh| and checking that they
run fine with |dash| which is the closest to full POSIX shell
implementation. I'm not proposing any changes yet, I'm just starting a
discussion here (or maybe it should be reposted to the mailing list?)
The toybox shell is intended to be a reasonable bash replacement, so
rather than remove bashisms from our shell code the plan is to implement
support in toysh. That hasn't been done yet, but modifying toybox's shell
code to run in something other than toysh seems counterproductive.
Rob
|
In my opinion both toysh and the scripts should be improved. And both should be made POSIX compatible. Implementing support for bashisms in toysh may be a very time consuming task that will result in bloating. I think it is reasonable to only implement POSIX standard in toysh and check that all the scripts comply with this standard. I know that there are some bashisms used in those scripts, but I doubt that it would be easier to support them in toysh than rewrite to something simpler to implement. Doesn't seem counterproductive at all to me. |
I'd like to voice my strong support for build/test scripts that require only a POSIX compliant shell. That toysh aspires to become bash-like isn't my personal preference, but this aspiration seems mostly divorced from what happens in the scripts that build and test this project. |
If you actually look at the code, 99% is already sh compliant. [ "${i:0:1}" == "/" ] || i=$(which $i)` and it could be replaced by case "$i" in
/*);;
*) i=$(which $i);;
esac I personally don't care that toybox requires a GPL licensed package to build, nor that bash is slower and slower with each version, nor that the only standard applicable to bash is Posix sh - with a bunch of unspecified addons that vary between versions, yet has no versioning standard for the shebang (Hell even perl and php do better). ...but even autotools can make their scripts Posix compliant. |
On 11/22/2017 12:07 PM, technosaurus wrote:
If you actually look at the code, 99% is already sh compliant.
So you want me to add enough bashisms until removing them is _not_
trivial, in order to justify what I already told you I'm going to do anyway?
Seems like a lot of effort, but if you really think it's necessary...
Rob
|
I'm having trouble understanding the situation: Are those scripts required in order to build toybox, or at least toybox' shell implementation? If so, there is a hen and egg problem that could only be solved by using bash, if the bashism are not removed. |
On 11/23/2017 08:30 AM, Max Bruckner wrote:
I'm having trouble understanding the situation: Are those scripts
required in order to build toybox, or at least toybox' shell
implementation? If so, there is a hen and egg problem that could only be
solved by using bash, if the bashism are not removed.
Toybox should build under itself (which is regression tested using the
"make install_airlock" target), so it eventually builds under android.
In theory generated/build.sh is supposed to contain a build script for
building it (without make) against a snapshot "generated" directory.
The main "toybox is ready for its 1.0 release" goal is for toybox to
provide all the tools that aren't set in "TOOLCHAIN=" in
scripts/install.sh which are needed to build
https://github.com/landley/mkroot and then build Linux From Scratch
under that.
I have a todo item to provide a directory under scripts/ with a snapshot
of generated files and build.sh for a minimal build of just the host
tools needed to build toybox, so it can build its own tools and then
build a full version with those tools on systems like macosx where the
sed doesn't work. (In the meantime, it uses gsed if it's installed. I
can add toysh to this, although the build would then need to "./sh
script.sh" to bypass the #!/bin/bash at the top.)
But that's pretty far down the todo list, and ensuring that the
generated files are sufficiently portable (the .config options for host
library features, the -lutil and similar link options) without
build-time probing of the environment is nontrivial.
In the meantime, no. I do not intend to remove the bashisms from the
build. I may add more if sufficiently poked about it. Bash was not only
the standard shell of Linux for 15 years, it was the first program Linux
ever _ran_. I wrote about that at some length in
http://lists.landley.net/pipermail/toybox-landley.net/2015-January/003831.html
I also note that github is the wrong place to raise this sort of issue:
the project's mailing list is where design decisions and discussions
occur. I only use github to publish the git repository, the project has
a mailing list for development discussion. The argument put forward for
"not bash" has been ideological purity not "I couldn't get it to work".
Toybox has never used posix as more than a frame of reference: they
standardized pax instead of tar, removed cpio after susv2 despite
initramfs requiring it, don't have a "mount" command... See the
roadmap's posix section for more.
Also, this month I'm in tokyo for work and kind of swamped, so probably
wouldn't do anything about it before flying back to the states on the
5th anyway.
Rob
|
Most systems have bash installed, so that is not an issue for them, but some lack it. I suggest changing default interpreter of
scripts/*.sh
andtests/*.test
from/bin/bash
to/bin/sh
and checking that they run fine withdash
which is the closest to full POSIX shell implementation. I'm not proposing any changes yet, I'm just starting a discussion here (or maybe it should be reposted to the mailing list?)The text was updated successfully, but these errors were encountered: