D6789: check-code: allow command substitution with $(command)
Augie Fackler
raf at durin42.com
Mon Sep 9 10:43:46 EDT 2019
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.
> On Sep 7, 2019, at 02:32, martinvonz (Martin von Zweigbergk) <phabricator at mercurial-scm.org> wrote:
>
> martinvonz created this revision.
> Herald added a subscriber: mercurial-devel.
> Herald added a reviewer: hg-reviewers.
>
> REVISION SUMMARY
> 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).
>
> REPOSITORY
> rHG Mercurial
>
> REVISION DETAIL
> https://phab.mercurial-scm.org/D6789
>
> AFFECTED FILES
> contrib/check-code.py
>
> CHANGE DETAILS
>
> diff --git a/contrib/check-code.py b/contrib/check-code.py
> --- a/contrib/check-code.py
> +++ b/contrib/check-code.py
> @@ -116,7 +116,6 @@
> (r'\bls\b.*-\w*R', "don't use 'ls -R', use 'find'"),
> (r'printf.*[^\\]\\([1-9]|0\d)', r"don't use 'printf \NNN', use Python"),
> (r'printf.*[^\\]\\x', "don't use printf \\x, use Python"),
> - (r'\$\(.*\)', "don't use $(expr), use `expr`"),
> (r'rm -rf \*', "don't use naked rm -rf, target a directory"),
> (r'\[[^\]]+==', '[ foo == bar ] is a bashism, use [ foo = bar ] instead'),
> (r'(^|\|\s*)grep (-\w\s+)*[^|]*[(|]\w',
>
>
>
> To: martinvonz, #hg-reviewers
> Cc: mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
More information about the Mercurial-devel
mailing list