| Summary: | New eselect module for /bin/sh | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Dan Wallis <mrdanwallis> |
| Component: | New packages | Assignee: | Michał Górny <mgorny> |
| Status: | RESOLVED FIXED | ||
| Severity: | enhancement | CC: | admin, alecm_88, eselect, esigra, grobian, nbowler, pesa, slong, spatz |
| Priority: | Lowest | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 154511, 302535 | ||
| Attachments: |
/usr/share/eselect/modules/sh.eselect
/usr/share/eselect/modules/sh.eselect eselect module with fixes …and diff of it Cleanup and improvements, rebranch for Sunrise Implementation of proposed changes |
||
|
Description
Dan Wallis
2008-03-26 02:30:44 UTC
You can find some documentation about writing eselect modules here: http://www.gentoo.org/proj/en/eselect/dev-guide.xml Great idea. BASH run as sh isn't POSIX compliant in its default state, although it is a lot harder to work with ;) As far as I know, busybox is always installed on Gentoo giving ash, although such a virtual could dep on ed and any other missing pieces of the POSIX system. I look forward to seeing what you come up with for review. (If you're worried about bugspam, try #gentoo-dev-help@irc.freenode.org or some of us are in #friendly-coders too.) Created attachment 176757 [details]
/usr/share/eselect/modules/sh.eselect
Created attachment 176759 [details]
/usr/share/eselect/modules/sh.eselect
Eselect module for /bin/sh. I have only added the implementations I have
installed myself since I'm not sure about the binary names of ksh etc.
On line 121 you've got:
elif [[ -L "${ROOT}/usr/bin/vi" ]] ; then
*** This bug has been confirmed by popular vote. *** Created attachment 199627 [details]
eselect module with fixes
* vi references replaced with sh,
* -f checks replaced with -x (no sense in linking non-executable shell),
* create relative symlink,
* removed busybox from the list as it uses its' internal tools (like mount) incompatible with Gentoo ones.
Created attachment 199629 [details, diff]
…and diff of it
Created attachment 203348 [details]
Cleanup and improvements, rebranch for Sunrise
(In reply to comment #9) > Created an attachment (id=203348) [edit] > Cleanup and improvements, rebranch for Sunrise Some comments: - Option-like arguments like "--if-unset" should be avoided below the subaction level, so better use "ifunset". (Yes, I know that it's wrong in the vi module. ;-) Or accept both forms, see ctags.eselect for an example. - "canonicalise" should be replaced by simple "readlink" throughout, since in all cases only the last path component should be expanded, but not any symlinks inside ROOT. - Generally, don't use quotes on LHS in [[ ]]. - This is a bit weird: target=${targets[$(( ${target} - 1 ))]} target=${targets[target-1]} is better readable. Otherwise, the module looks good to me. Created attachment 204658 [details, diff]
Implementation of proposed changes
Ulrich, will that be fine?
Adding two bugs as dependencies: - Direct use of the highlight() function should be avoided in eselect-1.2, see bug 154511 comment #5. - Prefix support (not sure if this is relevant here), see bug 302535. (In reply to comment #12) > - Prefix support (not sure if this is relevant here), see bug 302535. If this going to be default and ending up in base or something, it needs to respect the prefix. There is no need to consider real /bin/sh on most prefix systems, since that one remains /bin/sh, and the prefix bin/sh may be any shell that the user prefers. I would be interested in having this nowadays, since dash appears to be so much faster than bash for configure, libtool and friends. What's blocking this module from hitting the tree? (In reply to comment #14) > I would be interested in having this nowadays, since dash appears to be so much > faster than bash for configure, libtool and friends. What's blocking this > module from hitting the tree? Hmm, that's a good question. I guess I simply forgot about it. Will commit it today. And fixed. Note that (unless this changed recently) OpenRC runs init scripts using /bin/sh, and there are some scripts that use non-POSIX syntax - for instance bug 340175. There are quite a few others. (In reply to comment #12) > Adding two bugs as dependencies: > - Direct use of the highlight() function should be avoided in eselect-1.2, > see bug 154511 comment #5. > - Prefix support (not sure if this is relevant here), see bug 302535. These two issues haven't been addressed. Reopening. (In reply to comment #18) > (In reply to comment #12) > > Adding two bugs as dependencies: > > - Direct use of the highlight() function should be avoided in eselect-1.2, > > see bug 154511 comment #5. > > - Prefix support (not sure if this is relevant here), see bug 302535. > > These two issues haven't been addressed. Reopening. I'm sorry, must have missed that mail. Do the two commits I've made [1] fix them in a satisfying way? Also, should 'show' output be adjusted somehow for that '--brief' option as well? [1]:https://github.com/mgorny/eselect-sh/compare/cd1552fbde...5c8b3299c1 (In reply to comment #19) > I'm sorry, must have missed that mail. Do the two commits I've made [1] fix > them in a satisfying way? I think the surrounding "if" statement in do_list (line 86) should be removed, or the "none found" message will never be displayed. And while you're at it, you could add a bunch of quotes in lines 67 and 88: "$(basename "$(readlink "${EROOT}/bin/sh")")" Otherwise it will fail if EROOT contains whitespace or wildcard characters. > Also, should 'show' output be adjusted somehow for that '--brief' option as > well? This one looks good to me. (In reply to comment #14) > I would be interested in having this nowadays, since dash appears to be so > much faster than bash for configure, libtool and friends. What's blocking > this module from hitting the tree? Unfortunately, this module (by itself) won't actually help for autoconf-generated configure scripts, because (for better or for worse) the first thing they do is try to find bash and use that. In order to avoid this behaviour, one needs to set CONFIG_SHELL in their environment. But if you're going to do that, then you don't need to change the /bin/sh symlink for configure scripts because you can just set CONFIG_SHELL=/bin/dash directly. (In reply to comment #21) > (In reply to comment #14) > > I would be interested in having this nowadays, since dash appears to be so > > much faster than bash for configure, libtool and friends. What's blocking > > this module from hitting the tree? > > Unfortunately, this module (by itself) won't actually help for > autoconf-generated configure scripts, because (for better or for worse) the > first thing they do is try to find bash and use that. In order to avoid this > behaviour, one needs to set CONFIG_SHELL in their environment. But if you're > going to do that, then you don't need to change the /bin/sh symlink for > configure scripts because you can just set CONFIG_SHELL=/bin/dash directly. I think they started to recognize dash as bash lately because of some new features but that might be fixed already. (In reply to comment #20) > (In reply to comment #19) > > I'm sorry, must have missed that mail. Do the two commits I've made [1] fix > > them in a satisfying way? > > I think the surrounding "if" statement in do_list (line 86) should be removed, > or the "none found" message will never be displayed. > > And while you're at it, you could add a bunch of quotes in lines 67 and 88: > "$(basename "$(readlink "${EROOT}/bin/sh")")" > Otherwise it will fail if EROOT contains whitespace or wildcard characters. > > > Also, should 'show' output be adjusted somehow for that '--brief' option as > > well? > > This one looks good to me. Done and done. |
