From: Eric Sunshine <sunshine@sunshineco.com>
To: git@vger.kernel.org
Cc: Elijah Newren <newren@gmail.com>, Johannes Sixt <j6t@kdbg.org>,
Jonathan Nieder <jrnieder@gmail.com>,
Jonathan Tan <jonathantanmy@google.com>,
Stefan Beller <sbeller@google.com>,
Junio C Hamano <gitster@pobox.com>,
Luke Diamand <luke@diamand.org>, Jeff King <peff@peff.net>,
Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH v2 00/10] detect broken &&-chains in subshells
Date: Wed, 11 Jul 2018 02:46:32 -0400 [thread overview]
Message-ID: <20180711064642.6933-1-sunshine@sunshineco.com> (raw)
In-Reply-To: <20180626073001.6555-1-sunshine@sunshineco.com>
This is a re-roll of [1] which teaches --chain-lint to detect broken
&&-chains in subshells since the existing implementation[2] detects
breakage only at the top-level. It is built atop 'es/test-fixes', which
was split off of [1] and submitted separately[3], and which fixes many
broken &&-chains.
The major difference since v1 is that broken &&-chains in subshells are
now detected by pure textual inspection rather than by merging subshell
commands into the top-level &&-chain. Thus, v2 entirely sidesteps the
primary objection[4] raised during v1 review of possible unintended
side-effects of test code executing outside of the intended directory.
The pure textual detection implemented by v2 can have no such
side-effects.
A second important difference since v1 is that the "linter" itself, a
complex 'sed' script, now has its own tests to ensure correct behavior.
Not only do the tests protect against regressions, but they help to
document (for humans) expected behavior, which is important since 'sed'
scripts can seem rather inscrutable due to looking like line-noise and
due to the 'sed' "language" being stack-oriented (albeit with a very
tiny stack) which, like other stack-oriented languages (Forth,
Postscript, etc.) can be difficult to reason about.
Although the 'sed' script in v1 was already well-commented, the comments
have been improved in v2. More importantly, a high-level overview of the
script's operation has been added at the top of the file to aid
comprehension.
Thanks to Elijah, Hannes, Jonathan Nieder, Jonathan Tan, Junio, Luke,
Peff, and Stefan for comments on v1.
[1]: https://public-inbox.org/git/20180626073001.6555-1-sunshine@sunshineco.com/
[2]: https://public-inbox.org/git/20150320100429.GA17354@peff.net/
[3]: https://public-inbox.org/git/20180702002405.3042-1-sunshine@sunshineco.com/
[4]: https://public-inbox.org/git/xmqqwouljr5e.fsf@gitster-ct.c.googlers.com/
Eric Sunshine (10):
t/test-lib: teach --chain-lint to detect broken &&-chains in subshells
t/Makefile: add machinery to check correctness of chainlint.sed
t/chainlint: add chainlint "basic" test cases
t/chainlint: add chainlint "whitespace" test cases
t/chainlint: add chainlint "one-liner" test cases
t/chainlint: add chainlint "nested subshell" test cases
t/chainlint: add chainlint "loop" and "conditional" test cases
t/chainlint: add chainlint "cuddled" test cases
t/chainlint: add chainlint "complex" test cases
t/chainlint: add chainlint "specialized" test cases
t/.gitignore | 1 +
t/Makefile | 25 +-
t/chainlint.sed | 346 ++++++++++++++++++
t/chainlint/arithmetic-expansion.expect | 9 +
t/chainlint/arithmetic-expansion.test | 11 +
t/chainlint/bash-array.expect | 10 +
t/chainlint/bash-array.test | 12 +
t/chainlint/blank-line.expect | 4 +
t/chainlint/blank-line.test | 10 +
t/chainlint/block.expect | 12 +
t/chainlint/block.test | 15 +
t/chainlint/broken-chain.expect | 6 +
t/chainlint/broken-chain.test | 8 +
t/chainlint/case.expect | 19 +
t/chainlint/case.test | 23 ++
.../close-nested-and-parent-together.expect | 4 +
.../close-nested-and-parent-together.test | 3 +
t/chainlint/close-subshell.expect | 25 ++
t/chainlint/close-subshell.test | 27 ++
t/chainlint/command-substitution.expect | 9 +
t/chainlint/command-substitution.test | 11 +
t/chainlint/comment.expect | 4 +
t/chainlint/comment.test | 11 +
t/chainlint/complex-if-in-cuddled-loop.expect | 10 +
t/chainlint/complex-if-in-cuddled-loop.test | 11 +
t/chainlint/cuddled-if-then-else.expect | 7 +
t/chainlint/cuddled-if-then-else.test | 7 +
t/chainlint/cuddled-loop.expect | 5 +
t/chainlint/cuddled-loop.test | 7 +
t/chainlint/cuddled.expect | 21 ++
t/chainlint/cuddled.test | 23 ++
t/chainlint/exit-loop.expect | 24 ++
t/chainlint/exit-loop.test | 27 ++
t/chainlint/exit-subshell.expect | 5 +
t/chainlint/exit-subshell.test | 6 +
t/chainlint/for-loop.expect | 11 +
t/chainlint/for-loop.test | 19 +
t/chainlint/here-doc.expect | 3 +
t/chainlint/here-doc.test | 16 +
t/chainlint/if-in-loop.expect | 12 +
t/chainlint/if-in-loop.test | 15 +
t/chainlint/if-then-else.expect | 19 +
t/chainlint/if-then-else.test | 28 ++
t/chainlint/incomplete-line.expect | 4 +
t/chainlint/incomplete-line.test | 12 +
t/chainlint/inline-comment.expect | 9 +
t/chainlint/inline-comment.test | 12 +
t/chainlint/loop-in-if.expect | 12 +
t/chainlint/loop-in-if.test | 15 +
...ti-line-nested-command-substitution.expect | 9 +
...ulti-line-nested-command-substitution.test | 9 +
t/chainlint/multi-line-string.expect | 9 +
t/chainlint/multi-line-string.test | 15 +
t/chainlint/negated-one-liner.expect | 5 +
t/chainlint/negated-one-liner.test | 7 +
t/chainlint/nested-cuddled-subshell.expect | 19 +
t/chainlint/nested-cuddled-subshell.test | 31 ++
t/chainlint/nested-here-doc.expect | 5 +
t/chainlint/nested-here-doc.test | 23 ++
t/chainlint/nested-subshell-comment.expect | 11 +
t/chainlint/nested-subshell-comment.test | 13 +
t/chainlint/nested-subshell.expect | 12 +
t/chainlint/nested-subshell.test | 14 +
t/chainlint/one-liner.expect | 9 +
t/chainlint/one-liner.test | 12 +
t/chainlint/p4-filespec.expect | 4 +
t/chainlint/p4-filespec.test | 5 +
t/chainlint/pipe.expect | 8 +
t/chainlint/pipe.test | 12 +
t/chainlint/semicolon.expect | 20 +
t/chainlint/semicolon.test | 25 ++
t/chainlint/subshell-here-doc.expect | 5 +
t/chainlint/subshell-here-doc.test | 23 ++
t/chainlint/subshell-one-liner.expect | 14 +
t/chainlint/subshell-one-liner.test | 24 ++
t/chainlint/while-loop.expect | 11 +
t/chainlint/while-loop.test | 19 +
t/test-lib.sh | 3 +-
78 files changed, 1316 insertions(+), 5 deletions(-)
create mode 100644 t/chainlint.sed
create mode 100644 t/chainlint/arithmetic-expansion.expect
create mode 100644 t/chainlint/arithmetic-expansion.test
create mode 100644 t/chainlint/bash-array.expect
create mode 100644 t/chainlint/bash-array.test
create mode 100644 t/chainlint/blank-line.expect
create mode 100644 t/chainlint/blank-line.test
create mode 100644 t/chainlint/block.expect
create mode 100644 t/chainlint/block.test
create mode 100644 t/chainlint/broken-chain.expect
create mode 100644 t/chainlint/broken-chain.test
create mode 100644 t/chainlint/case.expect
create mode 100644 t/chainlint/case.test
create mode 100644 t/chainlint/close-nested-and-parent-together.expect
create mode 100644 t/chainlint/close-nested-and-parent-together.test
create mode 100644 t/chainlint/close-subshell.expect
create mode 100644 t/chainlint/close-subshell.test
create mode 100644 t/chainlint/command-substitution.expect
create mode 100644 t/chainlint/command-substitution.test
create mode 100644 t/chainlint/comment.expect
create mode 100644 t/chainlint/comment.test
create mode 100644 t/chainlint/complex-if-in-cuddled-loop.expect
create mode 100644 t/chainlint/complex-if-in-cuddled-loop.test
create mode 100644 t/chainlint/cuddled-if-then-else.expect
create mode 100644 t/chainlint/cuddled-if-then-else.test
create mode 100644 t/chainlint/cuddled-loop.expect
create mode 100644 t/chainlint/cuddled-loop.test
create mode 100644 t/chainlint/cuddled.expect
create mode 100644 t/chainlint/cuddled.test
create mode 100644 t/chainlint/exit-loop.expect
create mode 100644 t/chainlint/exit-loop.test
create mode 100644 t/chainlint/exit-subshell.expect
create mode 100644 t/chainlint/exit-subshell.test
create mode 100644 t/chainlint/for-loop.expect
create mode 100644 t/chainlint/for-loop.test
create mode 100644 t/chainlint/here-doc.expect
create mode 100644 t/chainlint/here-doc.test
create mode 100644 t/chainlint/if-in-loop.expect
create mode 100644 t/chainlint/if-in-loop.test
create mode 100644 t/chainlint/if-then-else.expect
create mode 100644 t/chainlint/if-then-else.test
create mode 100644 t/chainlint/incomplete-line.expect
create mode 100644 t/chainlint/incomplete-line.test
create mode 100644 t/chainlint/inline-comment.expect
create mode 100644 t/chainlint/inline-comment.test
create mode 100644 t/chainlint/loop-in-if.expect
create mode 100644 t/chainlint/loop-in-if.test
create mode 100644 t/chainlint/multi-line-nested-command-substitution.expect
create mode 100644 t/chainlint/multi-line-nested-command-substitution.test
create mode 100644 t/chainlint/multi-line-string.expect
create mode 100644 t/chainlint/multi-line-string.test
create mode 100644 t/chainlint/negated-one-liner.expect
create mode 100644 t/chainlint/negated-one-liner.test
create mode 100644 t/chainlint/nested-cuddled-subshell.expect
create mode 100644 t/chainlint/nested-cuddled-subshell.test
create mode 100644 t/chainlint/nested-here-doc.expect
create mode 100644 t/chainlint/nested-here-doc.test
create mode 100644 t/chainlint/nested-subshell-comment.expect
create mode 100644 t/chainlint/nested-subshell-comment.test
create mode 100644 t/chainlint/nested-subshell.expect
create mode 100644 t/chainlint/nested-subshell.test
create mode 100644 t/chainlint/one-liner.expect
create mode 100644 t/chainlint/one-liner.test
create mode 100644 t/chainlint/p4-filespec.expect
create mode 100644 t/chainlint/p4-filespec.test
create mode 100644 t/chainlint/pipe.expect
create mode 100644 t/chainlint/pipe.test
create mode 100644 t/chainlint/semicolon.expect
create mode 100644 t/chainlint/semicolon.test
create mode 100644 t/chainlint/subshell-here-doc.expect
create mode 100644 t/chainlint/subshell-here-doc.test
create mode 100644 t/chainlint/subshell-one-liner.expect
create mode 100644 t/chainlint/subshell-one-liner.test
create mode 100644 t/chainlint/while-loop.expect
create mode 100644 t/chainlint/while-loop.test
--
2.18.0.203.gfac676dfb9
next prev parent reply other threads:[~2018-07-11 6:48 UTC|newest]
Thread overview: 123+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-26 7:29 [PATCH 00/29] t: detect and fix broken &&-chains in subshells Eric Sunshine
2018-06-26 7:29 ` [PATCH 01/29] t7508: use test_when_finished() instead of managing exit code manually Eric Sunshine
2018-06-26 7:29 ` [PATCH 02/29] t0001: use "{...}" block around "||" expression rather than subshell Eric Sunshine
2018-06-26 7:29 ` [PATCH 03/29] t1300: use sane_unset() to avoid breaking &&-chain Eric Sunshine
2018-06-26 7:29 ` [PATCH 04/29] t3303: use standard here-doc tag "EOF" to avoid fooling --chain-lint Eric Sunshine
2018-06-26 7:29 ` [PATCH 05/29] t5505: modernize and simplify hard-to-digest test Eric Sunshine
2018-06-26 7:29 ` [PATCH 06/29] t6036: fix broken "merge fails but has appropriate contents" tests Eric Sunshine
2018-06-26 8:44 ` Elijah Newren
2018-06-26 7:29 ` [PATCH 07/29] t7201: drop pointless "exit 0" at end of subshell Eric Sunshine
2018-06-26 7:29 ` [PATCH 08/29] t7400: fix broken "submodule add/reconfigure --force" test Eric Sunshine
2018-06-27 18:04 ` Stefan Beller
2018-06-26 7:29 ` [PATCH 09/29] t7810: use test_expect_code() instead of hand-rolled comparison Eric Sunshine
2018-06-26 7:29 ` [PATCH 10/29] t9001: fix broken "invoke hook" test Eric Sunshine
2018-06-26 17:07 ` Jonathan Tan
2018-06-26 7:29 ` [PATCH 11/29] t9104: use "{...}" block around "||" expression rather than subshell Eric Sunshine
2018-06-26 7:29 ` [PATCH 12/29] t9401: drop unnecessary nested subshell Eric Sunshine
2018-06-26 7:29 ` [PATCH 13/29] t/lib-submodule-update: fix broken "replace submodule must-fail" test Eric Sunshine
2018-06-27 18:30 ` [PATCH] t/lib-submodule-update: fix absorbing test Stefan Beller
2018-06-27 18:38 ` Eric Sunshine
2018-06-26 7:29 ` [PATCH 14/29] t: drop subshell with missing &&-chain in favor of simpler construct Eric Sunshine
2018-06-26 19:31 ` Junio C Hamano
2018-06-26 20:06 ` Eric Sunshine
2018-06-26 7:29 ` [PATCH 15/29] t: drop unnecessary terminating semicolons in subshell Eric Sunshine
2018-06-26 7:29 ` [PATCH 16/29] t: use test_might_fail() instead of manipulating exit code manually Eric Sunshine
2018-06-26 7:29 ` [PATCH 17/29] t: use test_must_fail() instead of checking " Eric Sunshine
2018-06-26 7:59 ` Luke Diamand
2018-06-26 8:58 ` Elijah Newren
2018-06-26 9:21 ` Eric Sunshine
2018-06-26 18:05 ` Johannes Sixt
2018-06-26 18:14 ` Eric Sunshine
2018-06-26 21:00 ` Johannes Sixt
2018-06-26 7:29 ` [PATCH 18/29] t0000-t0999: fix broken &&-chains in subshells Eric Sunshine
2018-06-26 7:29 ` [PATCH 19/29] t1000-t1999: " Eric Sunshine
2018-06-26 7:29 ` [PATCH 20/29] t2000-t2999: " Eric Sunshine
2018-06-26 7:29 ` [PATCH 21/29] t3000-t3999: " Eric Sunshine
2018-06-26 7:29 ` [PATCH 22/29] t3030: " Eric Sunshine
2018-06-26 7:29 ` [PATCH 23/29] t4000-t4999: " Eric Sunshine
2018-06-26 7:29 ` [PATCH 24/29] t5000-t5999: " Eric Sunshine
2018-06-26 7:29 ` [PATCH 25/29] t6000-t6999: " Eric Sunshine
2018-06-26 7:29 ` [PATCH 26/29] t7000-t7999: " Eric Sunshine
2018-06-26 7:29 ` [PATCH 27/29] t9000-t9999: " Eric Sunshine
2018-06-26 7:30 ` [PATCH 28/29] t9119: " Eric Sunshine
2018-06-26 7:30 ` [PATCH 29/29] t/test-lib: teach --chain-lint to detect " Eric Sunshine
2018-06-26 19:15 ` Junio C Hamano
2018-06-26 19:52 ` Eric Sunshine
2018-06-26 20:17 ` Jeff King
2018-06-26 20:22 ` Jeff King
2018-06-26 20:59 ` Eric Sunshine
2018-06-26 21:33 ` Elijah Newren
2018-06-26 21:42 ` Eric Sunshine
2018-06-26 20:46 ` Eric Sunshine
2018-06-26 21:01 ` Jeff King
2018-06-26 21:13 ` Eric Sunshine
2018-06-28 14:35 ` Jeff King
2018-06-27 2:15 ` Elijah Newren
2018-06-27 6:27 ` Johannes Sixt
2018-06-27 6:48 ` Eric Sunshine
2018-06-28 14:37 ` Jeff King
2018-06-26 21:09 ` Junio C Hamano
2018-06-26 9:20 ` [PATCH 00/29] t: detect and fix " Elijah Newren
2018-06-26 9:31 ` Eric Sunshine
2018-06-26 15:34 ` Elijah Newren
2018-06-26 19:38 ` Junio C Hamano
2018-06-26 21:25 ` Eric Sunshine
2018-06-26 22:31 ` Junio C Hamano
2018-06-27 0:22 ` Jonathan Nieder
2018-07-11 6:46 ` Eric Sunshine [this message]
2018-07-11 6:46 ` [PATCH v2 01/10] t/test-lib: teach --chain-lint to detect " Eric Sunshine
2018-07-11 21:37 ` Junio C Hamano
2018-07-12 10:50 ` Eric Sunshine
2018-07-12 16:56 ` Jeff King
2018-07-12 19:32 ` Eric Sunshine
2018-07-12 19:54 ` Junio C Hamano
2018-07-30 18:13 ` Jonathan Nieder
2018-07-30 19:06 ` [PATCH 0/2] subtree: fix &&-chain and simplify tests (Re: [PATCH v2 01/10] t/test-lib: teach --chain-lint to detect broken &&-chains in subshells) Jonathan Nieder
2018-07-30 19:07 ` [PATCH 1/2] subtree test: add missing && to &&-chain Jonathan Nieder
2018-07-30 19:07 ` [PATCH 2/2] subtree test: simplify preparation of expected results Jonathan Nieder
2018-07-30 20:25 ` [PATCH v2 01/10] t/test-lib: teach --chain-lint to detect broken &&-chains in subshells Eric Sunshine
2018-07-30 20:59 ` Jonathan Nieder
2018-07-30 21:38 ` Eric Sunshine
2018-07-31 12:50 ` Jeff King
2018-07-31 18:55 ` Eric Sunshine
2018-07-31 19:08 ` Jeff King
2018-08-23 18:02 ` Ævar Arnfjörð Bjarmason
2018-08-23 18:27 ` Eric Sunshine
2018-07-11 6:46 ` [PATCH v2 02/10] t/Makefile: add machinery to check correctness of chainlint.sed Eric Sunshine
2018-07-11 6:46 ` [PATCH v2 03/10] t/chainlint: add chainlint "basic" test cases Eric Sunshine
2018-07-11 6:46 ` [PATCH v2 04/10] t/chainlint: add chainlint "whitespace" " Eric Sunshine
2018-07-11 6:46 ` [PATCH v2 05/10] t/chainlint: add chainlint "one-liner" " Eric Sunshine
2018-07-11 6:46 ` [PATCH v2 06/10] t/chainlint: add chainlint "nested subshell" " Eric Sunshine
2018-07-11 6:46 ` [PATCH v2 07/10] t/chainlint: add chainlint "loop" and "conditional" " Eric Sunshine
2018-07-11 6:46 ` [PATCH v2 08/10] t/chainlint: add chainlint "cuddled" " Eric Sunshine
2018-07-11 6:46 ` [PATCH v2 09/10] t/chainlint: add chainlint "complex" " Eric Sunshine
2018-07-11 6:46 ` [PATCH v2 10/10] t/chainlint: add chainlint "specialized" " Eric Sunshine
2018-08-07 8:21 ` [PATCH 0/5] chainlint: improve robustness against "unusual" shell coding Eric Sunshine
2018-08-07 8:21 ` [PATCH 1/5] chainlint: match arbitrary here-docs tags rather than hard-coded names Eric Sunshine
2018-08-08 22:50 ` Jeff King
2018-08-09 5:58 ` Eric Sunshine
2018-08-09 14:26 ` Jeff King
2018-08-07 8:21 ` [PATCH 2/5] chainlint: recognize multi-line $(...) when command cuddled with "$(" Eric Sunshine
2018-08-07 8:21 ` [PATCH 3/5] chainlint: let here-doc and multi-line string commence on same line Eric Sunshine
2018-08-07 8:21 ` [PATCH 4/5] chainlint: recognize multi-line quoted strings more robustly Eric Sunshine
2018-08-07 8:21 ` [PATCH 5/5] chainlint: add test of pathological case which triggered false positive Eric Sunshine
2018-08-08 22:53 ` [PATCH 0/5] chainlint: improve robustness against "unusual" shell coding Jeff King
2018-08-09 0:44 ` Junio C Hamano
2018-08-13 8:47 ` [PATCH v2 0/6] " Eric Sunshine
2018-08-13 8:47 ` [PATCH v2 1/6] chainlint: match arbitrary here-docs tags rather than hard-coded names Eric Sunshine
2018-08-13 8:47 ` [PATCH v2 2/6] chainlint: match 'quoted' here-doc tags Eric Sunshine
2018-08-13 19:27 ` Junio C Hamano
2018-08-13 20:12 ` Eric Sunshine
2018-08-13 8:47 ` [PATCH v2 3/6] chainlint: recognize multi-line $(...) when command cuddled with "$(" Eric Sunshine
2018-08-13 8:47 ` [PATCH v2 4/6] chainlint: let here-doc and multi-line string commence on same line Eric Sunshine
2018-08-13 8:47 ` [PATCH v2 5/6] chainlint: recognize multi-line quoted strings more robustly Eric Sunshine
2018-08-13 8:47 ` [PATCH v2 6/6] chainlint: add test of pathological case which triggered false positive Eric Sunshine
2018-08-15 18:45 ` [PATCH v3 0/6] chainlint: improve robustness against "unusual" shell coding Eric Sunshine
2018-08-15 18:45 ` [PATCH v3 1/6] chainlint: match arbitrary here-docs tags rather than hard-coded names Eric Sunshine
2018-08-15 18:45 ` [PATCH v3 2/6] chainlint: match quoted here-doc tags Eric Sunshine
2018-08-15 18:45 ` [PATCH v3 3/6] chainlint: recognize multi-line $(...) when command cuddled with "$(" Eric Sunshine
2018-08-15 18:45 ` [PATCH v3 4/6] chainlint: let here-doc and multi-line string commence on same line Eric Sunshine
2018-08-15 18:45 ` [PATCH v3 5/6] chainlint: recognize multi-line quoted strings more robustly Eric Sunshine
2018-08-15 18:45 ` [PATCH v3 6/6] chainlint: add test of pathological case which triggered false positive Eric Sunshine
2018-08-29 9:45 ` [PATCH] chainlint: match "quoted" here-doc tags Eric Sunshine
2018-08-29 17:57 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180711064642.6933-1-sunshine@sunshineco.com \
--to=sunshine@sunshineco.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=jonathantanmy@google.com \
--cc=jrnieder@gmail.com \
--cc=luke@diamand.org \
--cc=newren@gmail.com \
--cc=peff@peff.net \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).