Both command and $(command) are specified by POSIX. The latter nests
better. I don't see why we shouldn't allow both (or only the latter).
Details
- Reviewers
indygreg - Group Reviewers
hg-reviewers - Commits
- rHG4257c33e24b7: check-code: allow command substitution with $(command)
Diff Detail
- Repository
- rHG Mercurial
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
I went through the history of this line and sadly it came into existence with the initial check-code.py commit. However, I got to use hg grep for the purpose nobody uses (!) and found this:
augie% hg grep -r '0::e7d3b509af8b7cb31ec1b419dd7cc05ea5a6c7c1' '\$\(' tests
tests/test-http-proxy:2337:kill $(cat proxy.pid a/hg.pid)
tests/test-bisect:2924: count=$(( $count + 1 ))
tests/README:2968:- don't use math expressions like let, (( ... )), or $(( ... )); use "expr"
tests/test-diff-change:7628:#rev=$(hg log -r 1 --template '{node|short}')
tests/test-gendoc:9444:RST2HTML=$(which rst2html 2> /dev/null || which rst2html.py)
tests/test-diffstat:9640:i=0; while (( $i < 213 )); do echo a >> a; i=$(($i + 1)); done
tests/test-subrepo-svn:10180:escapedwd=$(pwd | \
tests/test-subrepo-svn:10180:WCROOT="$(pwd)/svn-wc"
looking at the history for test-http-proxy, I found this:
https://www.mercurial-scm.org/repo/hg/rev/a20877c8a3e2:
Use more compatible cmd instead of $(cmd) in test-http-proxy
IOW, my foggy memory that some sh implementations don't understand $() seems justified. That doesn't mean we shouldn't make this change, but we should at a minimum call out in the relnotes that $() is now a required feature of your shell if you want to run the tests.
There's also https://www.mercurial-scm.org/repo/hg/rev/d77022db1bca but I suspect $(()) is a bashism. The only other explicit cleanup of $() I can find is https://www.mercurial-scm.org/repo/hg/rev/7e3a685be2f3, which calls it a bashism but that's seemingly not quite true.
I've left the change not-accepted for now, but I don't object if someone wants to accept it. If you do, please send a follow-up release note for packagers.
The only information about it that I could find is that Bourne Shell doesn't support $() syntax. I'll still send a patch to mention it in the release notes.
Oh, the most likely reason one would be using Bourne Shell is probably if one is running Solaris before version 11 (2011): https://docs.oracle.com/cd/E23824_01/html/E24456/userenv-1.html
FYI, Fish also doesn't support the $(expr) syntax but it's not a POSIX compatible shell
This shouldn't be about the user's chosen shell, though, right? This is about the /bin/sh implementation, which you wouldn't/can't replace with fish, so I think it won't bother users that use fish, just distros with really old implementations of /bin/sh.