For thoroughness when checking for one-shot environment variable
assignments at shell function call sites, check-non-portable-shell
stitches together incomplete lines (those ending with backslash). This
allows it to correctly flag such undesirable usage even when the
variable assignment and function call are split across lines, for
example:
FOO=bar \
func
where 'func' is a shell function.
The stitching is accomplished like this:
while (<>) {
chomp;
# stitch together incomplete lines (those ending with "\")
while (s/\\$//) {
$_ .= readline;
chomp;
}
# detect unportable/undesirable shell constructs
...
}
Although this implementation is well supported in reasonably modern Perl
versions (5.10 and later), it fails with older versions (such as Perl
5.8 shipped with ancient Mac OS 10.5). In particular, in older Perl
versions, 'readline' is not connected to the file handle associated with
the "magic" while (<>) {...} construct, so 'readline' throws a
"readline() on unopened filehandle" error. Work around this problem by
dropping readline() and instead incorporating the stitching of
incomplete lines directly into the existing while (<>) {...} loop.
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The test lint learned to catch non-portable "sed" options.
* tb/test-lint-sed-options:
test-lint: only use only sed [-n] [-e command] [-f command_file]
From `man sed` (on a Mac OS X box):
The -E, -a and -i options are non-standard FreeBSD extensions and may not be available
on other operating systems.
From `man sed` on a Linux box:
REGULAR EXPRESSIONS
POSIX.2 BREs should be supported, but they aren't completely because of
performance problems. The \n sequence in a regular expression matches the newline
character, and similarly for \a, \t, and other sequences.
The -E option switches to using extended regular expressions instead; the -E option
has been supported for years by GNU sed, and is now included in POSIX.
Well, there are still a lot of systems out there, which don't support it.
Beside that, IEEE Std 1003.1TM-2017, see
http://pubs.opengroup.org/onlinepubs/9699919799/
does not mention -E either.
To be on the safe side, don't allow -E (or -r, which is GNU).
Change check-non-portable-shell.pl to only accept the portable options:
sed [-n] [-e command] [-f command_file]
Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
cp -a, while a common flag isn't in POSIX and will therefore fail
on systems that don't have GNUish tools (like OpenBSD, AIX or Solaris)
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --file option to grep isn't in POSIX[1], but -f is[1]. Let's check
for that in the future, and fix the portability regression in
f237c8b6fe ("commit-graph: implement git-commit-graph write",
2018-04-02) that broke e.g. AIX.
1. http://pubs.opengroup.org/onlinepubs/009695399/utilities/grep.html
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The seq command is not in POSIX, and doesn't exist on
e.g. OpenBSD. We've had the test_seq wrapper since d17cf5f3a3 ("tests:
Introduce test_seq", 2012-08-04), but use of it keeps coming back,
e.g. in the recently added "fetch negotiator" tests being added here.
So let's also add a check to "make test-lint". The regex is aiming to
capture the likes of $(seq ..) and "seq" as a stand-alone command,
without capturing some existing cases where we e.g. have files called
"seq", as \bseq\b would do.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "head -c BYTES" option is non-portable (not in POSIX[1]). Change
such invocations to use the test_copy_bytes wrapper added in
48860819e8 ("t9300: factor out portable "head -c" replacement",
2016-06-30).
This fixes a test added in 9d2e330b17 ("ewah_read_mmap: bounds-check
mmap reads", 2018-06-14), which has been breaking
t5310-pack-bitmaps.sh on OpenBSD since 2.18.0. The OpenBSD ports
already have a similar workaround after their upgrade to 2.18.0[2].
I have not tested this on IRIX, but according to 4de0bbd898 ("t9300:
use perl "head -c" clone in place of "dd bs=1 count=16000" kluge",
2010-12-13) this invocation would have broken things there too.
Also, change a valgrind-specific codepath in test-lib.sh to use this
wrapper. Given where valgrind runs I don't think this would ever
become a portability issue in practice, but it's easier to just use
the wrapper than introduce some exception for the "make test-lint"
check being added here.
1. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/head.html
2. 08d5d82eae (diff-f7d3c4fabeed1691620d608f1534f5e5)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
One-shot environment variable assignments, such as 'FOO' in
"FOO=bar cmd", exist only during the invocation of 'cmd'. However, if
'cmd' happens to be a shell function, then 'FOO' is assigned in the
executing shell itself, and that assignment remains until the process
exits (unless explicitly unset). Since this side-effect of
"FOO=bar shell_func" is unlikely to be intentional, detect and report
such usage.
To distinguish shell functions from other commands, perform a pre-scan
of shell scripts named as input, gleaning a list of function names by
recognizing lines of the form (loosely matching whitespace):
shell_func () {
and later report suspect lines of the form (loosely matching quoted
values):
FOO=bar [BAR=foo ...] shell_func
Also take care to stitch together incomplete lines (those ending with
"\") since suspect invocations may be split over multiple lines:
FOO=bar BAR=foo \
shell_func
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Error messages emitted by this linting script are long and noisy,
consisting of several sections:
<test-script>:<line#>: error: <explanation>: <failed-shell-text>
The line of failed shell text, usually coming from within a test body,
is often indented by one or two TABs, with the result that the actual
(important) text is separated from <explanation> by a good deal of empty
space. This can make for a difficult read, especially on typical
80-column terminals.
Make the messages more compact and perhaps a bit easier to digest by
folding out the leading whitespace from <failed-shell-text>.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Error messages emitted by this linting script are long and noisy,
consisting of several sections:
<test-script>:<line#>: error: <explanation>: <failed-shell-text>
Many problem explanations ask the reader to "please" use a suggested
alternative, however, such politeness is unnecessary and just adds to
the noise and length of the line, so drop "please" to make the message a
bit more concise.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test scripts count number of lines in an output and check it againt
its expectation. fb3340a6 ("test-lib: introduce test_line_count to
measure files", 2010-10-31) introduced a helper to show a failure in
such a test in a more readable way than comparing `wc -l` output with
a number.
Besides, on some platforms, "$(wc -l <file)" is padded with leading
whitespace on the left, so
test "$(wc -l <file)" = 4
would not work (most notably on macosX); the users of test_line_count
helper would not suffer from such a portability glitch.
Add a check in check-non-portable-shell.pl to find '"' between
`wc -l` and '=' and hint the user about test_line_count().
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some implementations of `echo` support the '-e' option to enable
backslash interpretation of the following string.
As an addition, they support '-E' to turn it off.
However, none of these are portable, POSIX doesn't even mention them,
and many implementations don't support them.
A check for '-n' is already done in check-non-portable-shell.pl,
extend it to cover '-n', '-e' or '-E'.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of anchoring these checks with "^\s*", just check that the
usage is preceded by a word boundary. So now we can catch
test $cond && export foo=bar
just like we already catch
test $cond &&
export foo=bar
As a side effect, this will detect usage of "sed -i", "echo -n", "test
a == b", and "export a=b" in comments. That is not ideal but it's
potentially useful because people sometimes copy code from comments so
it can be good to also avoid nonportable patterns there.
To avoid false positives, keep the checks for 'declare' and 'which'
anchored. Those are frequently used words in normal English-language
comments.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some shells do not understand the one-line construct, and instead need
FOO=bar &&
export FOO
Detect this in the test-lint target.
Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add the perl script "check-non-portable-shell.pl" to detect
non-portable shell syntax.
"echo -n" is an example of a shell command working on Linux, but not
on Mac OS X.
These shell commands are checked and reported as error:
- "echo -n" (printf should be used)
- "sed -i" (GNUism; use a temp file instead)
- "declare" (bashism, often used with arrays)
- "which" (unreliable exit status and output; use type instead)
- "test a == b" (bashism for "test a = b")
"make test-lint-shell-syntax" can be used to run only the check.
Helped-By: Jeff King <peff@peff.net>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
