-
Notifications
You must be signed in to change notification settings - Fork 13
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
Pleas use POSIX shell script instead of dash #42
Comments
Is a dash dependency that much of a problem? |
Is a dash dependency that much of a problem?
Of course it is! Only Debian derivative distro have `/bin/sh` symlinked
to `/bin/dash`. Other distros uses other shells... who know whatever
might be used for that. This is where using a standard is usefull when
targetting numerous distros out there whithout needing a specific shell
when a default shell is already available. Hence using POSIX shell
scripts. And this how it's done in numerous projects by default.
|
Huh? This has nothing to do with the #!/bin/sh link. vdev has dash in the shebangs. There is no risk of using the wrong shell here. P.S. I'm not saying I'm against using POSIX shell in a piece of software that's meant to be distro-agnostic, but #!/bin/sh not being dash in a non-problem. |
Well, scripts in general and especially in cases like this really should be POSIX compliant, if possible. Assumption that everyone would be willing to "install all the shells!" on their systems is also a little bit strange in my opinion. |
Especially to execute a couple of commands... there is no excuse to require dash for this; and then, requiring dash at all in this case is practically (dare I say unforgivable?) mistake. I did took a look on various scripts... really nothing stand out but a couple of easy things are done each time. So, using |
@fbt: Ignone the symlink part if you like or if you think it's out side the scope. -- That's only a side track of the main issue. |
To the best of my knowledge, the only non-POSIX feature I make use of in the helper scripts is the My reason for using dash is not because I need dash-specific features, but instead because I need to test with a minimal shell that is readily available and as POSIX-compliant as possible, modulo the availability of the @tokiclover The only script that does not end in .sh is the daemonlet script (vdevd/helpers/LINUX/daemonlet). This is because the daemonlet program is considered to be part of vdevd, not a helper script (it's meant to supervise and communicate with a long-running subprocess on vdevd's behalf, such as a stateful helper script, or a helper script that will frequently receive many device requests). The fact that it is implemented as a shell script today is an implementation detail that is likely to change in the future. It is unfortunate that it is located in the helpers directory, since placing it there seems to have caused confusion. I will move it. |
On the topic of local: I've just tested in in some of the more conservative shells, and it seems to be implemented everywhere. Even posh supports it. |
There should not be an issue with the It's just that the script should be modified to use plain Please fix this ASAP to permit wide testing for no Debian derivative distros. You cannot expect every one to have many shells for no reasons at all. More complicated projects like autotools et al use plain POSIX shell conformance. You cannot avoid this issue at all. And it's best to fix it sooner rather than later. Thanks. |
As a packager, I don't really get the ASAP nature of the problem. You can just depend your package on dash till vdev is no longer in heavy testing and doesn't need this, no? I do. |
@tokiclover You can also symlink EDIT: Symlink your favorite shell to /bin/dash, provided that you mention as much in any subsequent bug reports (thanks @fbt!). |
That might not be a good idea. To elaborate: bash, as an example, has an sh mode that is enabled if it's called as sh. Not as dash. So there might be inconsistencies that would produce unwanted behaviour in vdev's handlers. |
@fbt Thanks for the listing! Point 31 is particularly concerning--if I'm reading it right, it basically means that shell functions won't be able to set globally-scoped variables. That would severely restrict the programming model I'm aiming to provide with the I mis-typed earlier (fixed above)--I meant to suggest to @tokiclover that one alternative to replacing /bin/dash in each helper would be to choose a preferred POSIX-y shell (e.g. posh, pdksh) and symlink that to /bin/dash, so the scripts would not need to be modified. |
Come on @jcnelson... I am not talking about only my own preferences nor my own conveniences; I am just pointing the way it's done in really ubiquitous projects out there to facilitate testing and integration without requiring absolutely useless dependencies. -- There is absolutely no reasons to require dash at all. So remove it because this is way it's done in this kind of project aimed to wide usage. Really, no need to turn around looking for unnecessary reasons. |
@tokiclover As I have explained, there is no dependency on dash because the scripts do not rely on any dash-specific features. Any dash-isms in the code are considered bugs. As you yourself have pointed out, you may assume that it will be safe for downstream packagers to replace all instances of However, my tree will continue to use You are of course free to make whatever downstream changes you deem necessary. Again, any dash-isms you encounter are considered bugs, so feel free to report them here and I will remove them. Again, you should have no problem picking an alternative shell that meets the above two requirements (I have successfully used vdev with bash in non-POSIX mode, for example). Once we have a stable release, we can revisit the issue of supporting multiple shells via the traditional Now, is there anything else I can help you with? EDIT: (*) With pdksh in particular, it will be necessary to alias |
Note: ZSh is a very portable and awesome shell. |
Please consider removing
dash
hardcoding usage for plain POSIX shell script instead. See 1 reference for a wealth of POSIX sh usage. Normally replacingdash
withsh
should work out the box if not using dash-ism which I did for sys-fs/vdev Gentoo (derivative) distro package.This issue is really a serious one and should get a bit of attention... because you cannot expect other distros to be using
dash
as Debian derivative ones do.Thanks.
The text was updated successfully, but these errors were encountered: