This extract from contrib/subtree/t7900 triggered a false positive due
to three chainlint limitations:
* recognizing only a "blessed" set of here-doc tag names in a subshell
("EOF", "EOT", "INPUT_END"), of which "TXT" is not a member
* inability to recognize multi-line $(...) when the first statement of
the body is cuddled with the opening "$("
* inability to recognize multiple constructs on a single line, such as
opening a multi-line $(...) and starting a here-doc
Now that all of these shortcomings have been addressed, turn this rather
pathological bit of shell coding into a chainlint test case.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
chainlint.sed recognizes multi-line quoted strings within subshells:
echo "abc
def" >out &&
so it can avoid incorrectly classifying lines internal to the string as
breaking the &&-chain. To identify the first line of a multi-line
string, it checks if the line contains a single quote. However, this is
fragile and can be easily fooled by a line containing multiple strings:
echo "xyz" "abc
def" >out &&
Make detection more robust by checking for an odd number of quotes
rather than only a single one.
(Escaped quotes are not handled, but support may be added later.)
The original multi-line string recognizer rather cavalierly threw away
all but the final quote, whereas the new one is careful to retain all
quotes, so the "expected" output of a couple existing chainlint tests is
updated to account for this new behavior.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After swallowing a here-doc, chainlint.sed assumes that no other
processing needs to be done on the line aside from checking for &&-chain
breakage; likewise, after folding a multi-line quoted string. However,
it's conceivable (even if unlikely in practice) that both a here-doc and
a multi-line quoted string might commence on the same line:
cat <<\EOF && echo "foo
bar"
data
EOF
Support this case by sending the line (after swallowing and folding)
through the normal processing sequence rather than jumping directly to
the check for broken &&-chain.
This change also allows other somewhat pathological cases to be handled,
such as closing a subshell on the same line starting a here-doc:
(
cat <<-\INPUT)
data
INPUT
or, for instance, opening a multi-line $(...) expression on the same
line starting a here-doc:
x=$(cat <<-\END &&
data
END
echo "x")
among others.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For multi-line $(...) expressions nested within subshells, chainlint.sed
only recognizes:
x=$(
echo foo &&
...
but it is not unlikely that test authors may also cuddle the command
with the opening "$(", so support that style, as well:
x=$(echo foo &&
...
The closing ")" is already correctly recognized when cuddled or not.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A here-doc tag can be quoted ('EOF') or escaped (\EOF) to suppress
interpolation within the body. Although, chainlint recognizes escaped
tags, it does not know about quoted tags. For completeness, teach it to
recognize quoted tags, as well.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
chainlint.sed swallows top-level here-docs to avoid being fooled by
content which might look like start-of-subshell. It likewise swallows
here-docs in subshells to avoid marking content lines as breaking the
&&-chain, and to avoid being fooled by content which might look like
end-of-subshell, start-of-nested-subshell, or other specially-recognized
constructs.
At the time of implementation, it was believed that it was not possible
to support arbitrary here-doc tag names since 'sed' provides no way to
stash the opening tag name in a variable for later comparison against a
line signaling end-of-here-doc. Consequently, tag names are hard-coded,
with "EOF" being the only tag recognized at the top-level, and only
"EOF", "EOT", and "INPUT_END" being recognized within subshells. Also,
special care was taken to avoid being confused by here-docs nested
within other here-docs.
In practice, this limited number of hard-coded tag names has been "good
enough" for the 13000+ existing Git test, despite many of those tests
using tags other than the recognized ones, since the bodies of those
here-docs do not contain content which would fool the linter.
Nevertheless, the situation is not ideal since someone writing new
tests, and choosing a name not in the "blessed" set could potentially
trigger a false-positive.
To address this shortcoming, upgrade chainlint.sed to handle arbitrary
here-doc tag names, both at the top-level and within subshells.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This character class, like many others in this script, matches
horizontal whitespace consisting of spaces and tabs, however, a few
extra, entirely harmless, spaces somehow slipped into the expression.
Removing them is purely a cosmetic fix.
While at it, re-indent three lines with a single TAB each which were
incorrectly indented with six spaces. Also, a purely cosmetic fix.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --chain-lint option uses heuristics and knowledge of shell syntax to
detect broken &&-chains in subshells by pure textual inspection. The
heuristics handle a range of stylistic variations in existing tests
(evolved over the years), however, they are still best-guesses. As such,
it is possible for future changes to accidentally break assumptions upon
which the heuristics are based. Protect against this possibility by
adding tests which check the linter itself for correctness.
In addition to protecting against regressions, these tests help document
(for humans) expected behavior, which is important since the linter's
implementation language ('sed') does not necessarily lend itself to easy
comprehension.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --chain-lint option uses heuristics and knowledge of shell syntax to
detect broken &&-chains in subshells by pure textual inspection. The
heuristics handle a range of stylistic variations in existing tests
(evolved over the years), however, they are still best-guesses. As such,
it is possible for future changes to accidentally break assumptions upon
which the heuristics are based. Protect against this possibility by
adding tests which check the linter itself for correctness.
In addition to protecting against regressions, these tests help document
(for humans) expected behavior, which is important since the linter's
implementation language ('sed') does not necessarily lend itself to easy
comprehension.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --chain-lint option uses heuristics and knowledge of shell syntax to
detect broken &&-chains in subshells by pure textual inspection. The
heuristics handle a range of stylistic variations in existing tests
(evolved over the years), however, they are still best-guesses. As such,
it is possible for future changes to accidentally break assumptions upon
which the heuristics are based. Protect against this possibility by
adding tests which check the linter itself for correctness.
In addition to protecting against regressions, these tests help document
(for humans) expected behavior, which is important since the linter's
implementation language ('sed') does not necessarily lend itself to easy
comprehension.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --chain-lint option uses heuristics and knowledge of shell syntax to
detect broken &&-chains in subshells by pure textual inspection. The
heuristics handle a range of stylistic variations in existing tests
(evolved over the years), however, they are still best-guesses. As such,
it is possible for future changes to accidentally break assumptions upon
which the heuristics are based. Protect against this possibility by
adding tests which check the linter itself for correctness.
In addition to protecting against regressions, these tests help document
(for humans) expected behavior, which is important since the linter's
implementation language ('sed') does not necessarily lend itself to easy
comprehension.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --chain-lint option uses heuristics and knowledge of shell syntax to
detect broken &&-chains in subshells by pure textual inspection. The
heuristics handle a range of stylistic variations in existing tests
(evolved over the years), however, they are still best-guesses. As such,
it is possible for future changes to accidentally break assumptions upon
which the heuristics are based. Protect against this possibility by
adding tests which check the linter itself for correctness.
In addition to protecting against regressions, these tests help document
(for humans) expected behavior, which is important since the linter's
implementation language ('sed') does not necessarily lend itself to easy
comprehension.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --chain-lint option uses heuristics and knowledge of shell syntax to
detect broken &&-chains in subshells by pure textual inspection. The
heuristics handle a range of stylistic variations in existing tests
(evolved over the years), however, they are still best-guesses. As such,
it is possible for future changes to accidentally break assumptions upon
which the heuristics are based. Protect against this possibility by
adding tests which check the linter itself for correctness.
In addition to protecting against regressions, these tests help document
(for humans) expected behavior, which is important since the linter's
implementation language ('sed') does not necessarily lend itself to easy
comprehension.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --chain-lint option uses heuristics and knowledge of shell syntax to
detect broken &&-chains in subshells by pure textual inspection. The
heuristics handle a range of stylistic variations in existing tests
(evolved over the years), however, they are still best-guesses. As such,
it is possible for future changes to accidentally break assumptions upon
which the heuristics are based. Protect against this possibility by
adding tests which check the linter itself for correctness.
In addition to protecting against regressions, these tests help document
(for humans) expected behavior, which is important since the linter's
implementation language ('sed') does not necessarily lend itself to easy
comprehension.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --chain-lint option uses heuristics and knowledge of shell syntax to
detect broken &&-chains in subshells by pure textual inspection. The
heuristics handle a range of stylistic variations in existing tests
(evolved over the years), however, they are still best-guesses. As such,
it is possible for future changes to accidentally break assumptions upon
which the heuristics are based. Protect against this possibility by
adding tests which check the linter itself for correctness.
In addition to protecting against regressions, these tests help document
(for humans) expected behavior, which is important since the linter's
implementation language ('sed') does not necessarily lend itself to easy
comprehension.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --chain-lint option uses heuristics and knowledge of shell syntax to
detect broken &&-chains in subshells by pure textual inspection.
Although the heuristics work well, they are still best-guesses and
future changes could accidentally break assumptions upon which they are
based. To protect against this possibility, tests checking correctness
of the linter itself will be added. As preparation, add a new makefile
"check-chainlint" target and associated machinery.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --chain-lint option detects broken &&-chains by forcing the test to
exit early (as the very first step) with a sentinel value. If that
sentinel is the test's overall exit code, then the &&-chain is intact;
if not, then the chain is broken. Unfortunately, this detection does not
extend to &&-chains within subshells even when the subshell itself is
properly linked into the outer &&-chain.
Address this shortcoming by feeding the body of the test to a
lightweight "linter" which can peer inside subshells and identify broken
&&-chains by pure textual inspection. Although the linter does not
actually parse shell scripts, it has enough knowledge of shell syntax to
reliably deal with formatting style variations (as evolved over the
years) and to avoid being fooled by non-shell content (such as inside
here-docs and multi-line strings). It recognizes modern subshell
formatting:
statement1 &&
(
statement2 &&
statement3
) &&
statement4
as well as old-style:
statement1 &&
(statement2 &&
statement3) &&
statement4
Heuristics are employed to properly identify the extent of a subshell
formatted in the old-style since a number of legitimate constructs may
superficially appear to close the subshell even though they don't. For
example, it understands that neither "x=$(command)" nor "case $x in *)"
end a subshell, despite the ")" at the end of line.
Due to limitations of the tool used ('sed') and its inherent
line-by-line processing, only subshells one level deep are handled, as
well as one-liner subshells one level below that. Subshells deeper than
that or multi-line subshells at level two are passed through as-is, thus
&&-chains in their bodies are not checked.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test uses a convoluted method to verify that "p4 help" errors
out when asked for help about an unknown command. In doing so, it
intentionally breaks the &&-chain. Simplify by employing the typical
"! command" idiom and a normal &&-chain instead.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test has been dysfunctional since it was added by 6489660b4b
(send-email: support validate hook, 2017-05-12), however, the problem
went unnoticed due to a broken &&-chain late in the test.
The test wants to verify that a non-zero exit code from the
'sendemail-validate' hook causes git-send-email to abort with a
particular error message. A command which is expected to fail should be
run with 'test_must_fail', however, the test neglects to do so.
Fix this problem, as well as the broken &&-chain behind which the
problem hid.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test manually checks the exit code of git-grep for a particular
value. In doing so, it intentionally breaks the &&-chain. Modernize the
test by taking advantage of test_expect_code() and a normal &&-chain.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test has been dysfunctional since it was added by 619acfc78c
(submodule add: extend force flag to add existing repos, 2016-10-06),
however, two problems early in the test went unnoticed due to a broken
&&-chain later in the test.
First, it tries configuring the submodule with repository "bogus-url",
however, "git submodule add" insists that the repository be either an
absolute URL or a relative pathname requiring prefix "./" or "../" (this
is true even with --force), but "bogus-url" does not meet those
criteria, thus the command fails.
Second, it then tries configuring a submodule with a path which is
.gitignore'd, which is disallowed. This restriction can be overridden
with --force, but the test neglects to use that option.
Fix both problems, as well as the broken &&-chain behind which they hid.
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test employs a for-loop inside a subshell and correctly aborts the
loop and fails the test overall (via "exit 1") if any iteration of the
for-loop fails. Otherwise, it exits the subshell with an explicit but
entirely unnecessary "exit 0", presumably to indicate that all
iterations of the loop succeeded. The &&-chain is broken between the
for-loop and the "exit 0". Rather than fixing the &&-chain, just drop
the pointless "exit 0".
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These tests reference non-existent object "c" when they really mean to
be referencing "C", however, these errors went unnoticed due to a broken
&&-chain later in the tests. Fix these errors, as well as the broken
&&-chains behind which they hid.
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test uses a subshell within a subshell but is formatted in such a
way as to suggests that the inner subshell is a sibling rather than a
child, which makes it difficult to digest the test's structure and
intent.
Worse, the inner subshell performs cleanup of actions from earlier in
the test, however, a failure between the initial actions and the cleanup
will prevent the cleanup from taking place.
Fix these problems by modernizing and simplifying the test and by using
test_when_finished() for the cleanup action.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Take advantage of write_script() to abstract-away details of shell
script creation, thus allowing the reader to focus on script content.
Readability benefits, particularly in this case, since the script body
was buried in a noisy one-liner subshell responsible for emitting
boilerplate and body.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test expects "git push" to fail, thus it manually inverts that
local expected failure into a successful exit code for the test overall.
In doing so, it intentionally breaks the &&-chain. Modernize by
replacing manual exit code management with test_must_fail() and a normal
&&-chain.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test has been dysfunctional since it was added by 259f3ee296
(lib-submodule-update.sh: define tests for recursing into submodules,
2017-03-14), however, the problem went unnoticed due to a broken
&&-chain.
The test wants to verify that replacing a submodule containing a .git
directory will absorb the .git directory into the .git/modules/ of the
superproject, and then replace the working tree content appropriate to
the superproject. It is, therefore, incorrect to check if the
submodule content still exists since the submodule will have been
replaced by the content of the superproject.
Fix this by removing the submodule content check, which also happens
to be the line that broke the &&-chain.
While at it, fix broken &&-chains in a couple neighboring tests.
Helped-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These tests intentionally break the &&-chain after using 'unset' since
they don't know if 'unset' will succeed or fail and don't want a local
'unset' failure to fail the test overall. We can do better by using
sane_unset(), which can be linked into the &&-chain as usual.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These tests employ a noisy subshell (with missing &&-chain) to feed
input into Git commands or files:
(echo a; echo b; echo c) | git some-command ...
Simplify by taking advantage of test_write_lines():
test_write_lines a b c | git some-command ...
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These tests manually coerce the exit code of invoked commands to
"success" when they don't care if the command succeeds or fails since
failure of those commands should not cause the test to fail overall.
In doing so, they intentionally break the &&-chain. Modernize by
replacing manual exit code management with test_might_fail() and a
normal &&-chain.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The code to try seeing if a fetch is necessary in a submodule
during a fetch with --recurse-submodules got confused when the path
to the submodule was changed in the range of commits in the
superproject, sometimes showing "(null)". This has been corrected.
* sb/fix-fetching-moved-submodules:
t5526: test recursive submodules when fetching moved submodules
submodule: fix NULL correctness in renamed broken submodules
Build and test procedure for netrc credential helper (in contrib/)
has been updated.
* tz/cred-netrc-cleanup:
git-credential-netrc: make "all" default target of Makefile
git-credential-netrc: fix exit status when tests fail
git-credential-netrc: use in-tree Git.pm for tests
git-credential-netrc: minor whitespace cleanup in test script
Continuing with the idea to programmatically enumerate various
pieces of data required for command line completion, the codebase
has been taught to enumerate options prefixed with "--no-" to
negate them.
* nd/completion-negation:
completion: collapse extra --no-.. options
completion: suppress some -no- options
parse-options: option to let --git-completion-helper show negative form
When user edits the patch in "git add -p" and the user's editor is
set to strip trailing whitespaces indiscriminately, an empty line
that is unchanged in the patch would become completely empty
(instead of a line with a sole SP on it). The code introduced in
Git 2.17 timeframe failed to parse such a patch, but now it learned
to notice the situation and cope with it.
* pw/add-p-recount:
add -p: fix counting empty context lines in edited patches
"git fetch-pack --all" used to unnecessarily fail upon seeing an
annotated tag that points at an object other than a commit.
* jk/fetch-all-peeled-fix:
fetch-pack: test explicitly that --all can fetch tag references pointing to non-commits
fetch-pack: don't try to fetch peel values with --all
Eric Sunshine