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