https://blogs.gentoo.org/ago/2020/07/04/gentoo-tinderbox/ Issue: net-dns/bind-9.16.29 installs shell script that uses non-POSIX features. Discovered on: amd64 (internal ref: ci)
Created attachment 781553 [details] build.log.xz build log and emerge --info (compressed because it exceeds attachment limit, use 'xzless' to read it)
ci has reproduced this issue with version 9.16.29-r1 - Updating summary.
ci has reproduced this issue with version 9.16.30 - Updating summary.
This is INVALID. Though the script could use a clean-up, the two QA notices in the build log are false positives. The only POSIX violation is the ever-pervasive use of local.
I don't feel like re-writing the whole thing right now but I'll attach a patch shortly that improves those two tests and should keep checkbashisms quiet.
Created attachment 789110 [details, diff] gentoo-bug-848708.patch This is a direct patch against named.init-r14 so as to make the changes easier to review. My proposed commit message is as follows ... Rename _mount to _bindmount and remove the needless option support. The only options ever being passed prior were -o bind. Remove the useless $# check; there are always a sufficient number of arguments. Remove the useless intermediate variable that was holding the value of $?; the eend function already propagates the status code that is given, so one may simply let it fall through to an implicit return. Remove the useless return 0; if already produces a status of 0 in the case that the given command(s) fail. Use mountpoint(1) from util-linux to determine whether the mountpoint exists. Doing so inhibits a spurious warning raised by checkbashisms into the bargain. Resolve a dangling eend by calling ebegin instead of einfo. Apply the aforementioned changes to _umount, where applicable. Do not use the -a operator in tests, as its use is discouraged. Do not initialise the 'reported' variable at the same that it is localised. The effect of using local is still undefined by POSIX, so that's asking for trouble. Fix almost all cases in which expansions are erroneously unquoted (SC2086). Closes: https://bugs.gentoo.org/848708 Signed-off-by: Kerin Millar <kfm@plushkava.net>
Created attachment 789149 [details, diff] gentoo-bug-848708-r1.patch Upon self-review of my patch, I realised that I had accidentally double-quoted the expansion of CHROOT in the invocation of start-stop-daemon. Unfortunately, the present approach to argument composition requires that it be unquoted. For now, make it so. No other changes.
Created attachment 789152 [details, diff] gentoo-bug-848708-r2.patch Take into account three more cases where CHROOT must specifically remain unquoted, this time involving invocations of named-checkconf. Third time should be the charm.
(In reply to Kerin Millar from comment #4) > This is INVALID. If you think this is invalid, the best approach is send improvements to checkbashism or filter the invalid notice in portage.
(In reply to Agostino Sarubbo from comment #9) > (In reply to Kerin Millar from comment #4) > > This is INVALID. > > If you think this is invalid, the best approach is send improvements to > checkbashism or filter the invalid notice in portage. checkbashisms isn't especially adept in its capacity as a shell code parser and is essentially treating the == operator here as if it is shellcode. [ -n "$(awk 'this == that')" ] That is not to say that checkbashisms isn't useful, of course. It's high time that the issue of pervasive POSIX violations was attended to and your initiative is welcome. I do not have any personal interest in attempting to improve chekbashisms itself, because shellcheck is already a much better parser and is already capable of extensively flagging POSIX violations, though there is presently no simply option to make it exclusively report violations in that category. The word "best" can mean different things to different people and it is my view that efforts are better directed towards the shellcheck project. Besides, even the very best linting tools can - and will - produce false positives in some context. I do, however, have an interest in hacking on shell code that could stand to be improved and have made prior contributions in that vein. The _mount and _unmount routines are convoluted and easy to improve. Even if mountpoint(1) did not exist as a utility, there's no need to use a command substitution, nor awk itself. For example, this routine would be more correct than the existing code (it decodes the second field correctly) and would not offend checkbashisms. while read -r _ target _; do target=$(printf %b "$target") if [ "$to" = "$target" ]; then return fi done < /proc/self/mounts # mount stuff here But mountpoint(1) does exist so why not use it? Not all effort is fungible. While I understand your point, a lack of interest in working on checkbashisms should not preclude me from making what I believe to be useful suggestions that can result in bugs being closed, including this one.
ci has reproduced this issue with version 9.16.31 - Updating summary.
ci has reproduced this issue with version 9.16.32 - Updating summary.
ci has reproduced this issue with version 9.16.33 - Updating summary.
ci has reproduced this issue with version 9.16.36 - Updating summary.
ci has reproduced this issue with version 9.16.37 - Updating summary.
ci has reproduced this issue with version 9.16.39 - Updating summary.
ci has reproduced this issue with version 9.16.41 - Updating summary.
ci has reproduced this issue with version 9.16.42 - Updating summary.
ci has reproduced this issue with version 9.16.48 - Updating summary.
