From patchwork Thu Sep 1 00:29:39 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961580 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EBCFDECAAD4 for ; Thu, 1 Sep 2022 00:30:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231700AbiIAAaF (ORCPT ); Wed, 31 Aug 2022 20:30:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46260 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229607AbiIAAaD (ORCPT ); Wed, 31 Aug 2022 20:30:03 -0400 Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C7B5125C4C for ; Wed, 31 Aug 2022 17:30:01 -0700 (PDT) Received: by mail-wr1-x435.google.com with SMTP id w5so4092468wrn.12 for ; Wed, 31 Aug 2022 17:30:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=qzKTWlJoINKQudyGiXVS/3T0blznJqjVaOqJ1HdYyfk=; b=F642mROJV4TYjqZnZQz00thpEq+uRs74AUok4sxRvtlXSzD2VhpHH2QL1ydFdDPjYW VGit6kcrlqkdrfAe4GSRWHup8N6k4zZm3eJlUCCVTRdY2kHsI3rx7RRSeoqvJOC/8LDL IuudfIQEi1gfC4GlzJ411GirOycvIJd2rCmyxMQv6EGZKhaZm+8Ffsi93fkvOrJw9rdB nZCk16S3/QaRGuo+/OgHkZBRwj7o7xHvwhkQtmUfIs3WUb+tjzedGFHHpEpVE6ac9TlU AlxM+5fwT+t9FhrEkCs0sHTkwRE0xVzXzpkjZntpCFIA97k0nn8+xVOdzTYlj9lDIHKC 1pJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=qzKTWlJoINKQudyGiXVS/3T0blznJqjVaOqJ1HdYyfk=; b=PZXDLGsuPgLNqW73LirjSSdDVn2icigqsWjA802gGfaoUJe2Ovq10bY4G4RuP6GrCY efBY9DqdtumsZNZ+vabHE29pXPERCFzMi62MG7EXOwI/wJt79N1m6mlTHXUPbMrMBwZV +t12VydAzwVdPy91Vm4k+d93EaiYVPKJB/mUapkgmIaLjHZiGFkCyWOvirva3W/nagbT sQlKoxEwM71F+17c6Z0IKsH93dXmgf7XWPfN4AeCM9+TiLwvxCbcf0f66CTiYOv7zEiz rHMa9+kIO8xxhoLMMSlGNhmrQzj+FDutWfWjoBXDTpdnOQC/LJXDnObzzM4O2AXUnZRR G8SA== X-Gm-Message-State: ACgBeo1x04qlogWcQmNnQth7Tkc28lsRvNQkaihsWvMgOOBcnSbrOxT1 BYdXm1vwGqDgCP6oyqm6Gngy27jmyd0= X-Google-Smtp-Source: AA6agR767JjMPgT+lvC4e79oydVgzU4QQLlp9LP2yg4Sls9lQ6VfYg3zsMdawaKDijfnaUXVNRMaGQ== X-Received: by 2002:a5d:6d85:0:b0:225:760f:1d50 with SMTP id l5-20020a5d6d85000000b00225760f1d50mr12659884wrs.608.1661992200032; Wed, 31 Aug 2022 17:30:00 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id z18-20020adff752000000b0021e5adb92desm13121125wrp.60.2022.08.31.17.29.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:29:59 -0700 (PDT) Message-Id: <3423df94bd6035640828a2508968cf8e1f5b4dda.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:39 +0000 Subject: [PATCH 01/18] t: add skeleton chainlint.pl Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine Although chainlint.sed usefully identifies broken &&-chains in tests, it has several shortcomings which include: * only detects &&-chain breakage in subshells (one-level deep) * does not check for broken top-level &&-chains; that task is left to the "magic exit code 117" checker built into test-lib.sh, however, that detection does not extend to `{...}` blocks, `$(...)` expressions, or compound statements such as `if...fi`, `while...done`, `case...esac` * uses heuristics, which makes it (potentially) fallible and difficult to tweak to handle additional real-world cases * written in `sed` and employs advanced `sed` operators which are probably not well-known to many programmers, thus the pool of people who can maintain it is likely small * manually simulates recursion into subshells which makes it much more difficult to reason about than, say, a traditional top-down parser * checks each test as the test is run, which can get expensive for tests which are run repeatedly by functions or loops since their bodies will be checked over and over (tens or hundreds of times) unnecessarily To address these shortcomings, begin implementing a more functional and precise test linter which understands shell syntax and semantics rather than employing heuristics, thus is able to recognize structural problems with tests beyond broken &&-chains. The new linter is written in Perl, thus should be more accessible to a wider audience, and is structured as a traditional top-down parser which makes it much easier to reason about, and allows it to inspect compound statements within test bodies to any depth. Furthermore, it can check all test definitions in the entire project in a single invocation rather than having to be invoked once per test, and each test definition is checked only once no matter how many times the test is actually run. At this stage, the new linter is just a skeleton containing boilerplate which handles command-line options, collects and reports statistics, and feeds its arguments -- paths of test scripts -- to a (presently) do-nothing script parser for validation. Subsequent changes will flesh out the functionality. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 115 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100755 t/chainlint.pl diff --git a/t/chainlint.pl b/t/chainlint.pl new file mode 100755 index 00000000000..e8ab95c7858 --- /dev/null +++ b/t/chainlint.pl @@ -0,0 +1,115 @@ +#!/usr/bin/env perl +# +# Copyright (c) 2021-2022 Eric Sunshine +# +# This tool scans shell scripts for test definitions and checks those tests for +# problems, such as broken &&-chains, which might hide bugs in the tests +# themselves or in behaviors being exercised by the tests. +# +# Input arguments are pathnames of shell scripts containing test definitions, +# or globs referencing a collection of scripts. For each problem discovered, +# the pathname of the script containing the test is printed along with the test +# name and the test body with a `?!FOO?!` annotation at the location of each +# detected problem, where "FOO" is a tag such as "AMP" which indicates a broken +# &&-chain. Returns zero if no problems are discovered, otherwise non-zero. + +use warnings; +use strict; +use File::Glob; +use Getopt::Long; + +my $show_stats; +my $emit_all; + +package ScriptParser; + +sub new { + my $class = shift @_; + my $self = bless {} => $class; + $self->{output} = []; + $self->{ntests} = 0; + return $self; +} + +sub parse_cmd { + return undef; +} + +# main contains high-level functionality for processing command-line switches, +# feeding input test scripts to ScriptParser, and reporting results. +package main; + +my $getnow = sub { return time(); }; +my $interval = sub { return time() - shift; }; +if (eval {require Time::HiRes; Time::HiRes->import(); 1;}) { + $getnow = sub { return [Time::HiRes::gettimeofday()]; }; + $interval = sub { return Time::HiRes::tv_interval(shift); }; +} + +sub show_stats { + my ($start_time, $stats) = @_; + my $walltime = $interval->($start_time); + my ($usertime) = times(); + my ($total_workers, $total_scripts, $total_tests, $total_errs) = (0, 0, 0, 0); + for (@$stats) { + my ($worker, $nscripts, $ntests, $nerrs) = @$_; + print(STDERR "worker $worker: $nscripts scripts, $ntests tests, $nerrs errors\n"); + $total_workers++; + $total_scripts += $nscripts; + $total_tests += $ntests; + $total_errs += $nerrs; + } + printf(STDERR "total: %d workers, %d scripts, %d tests, %d errors, %.2fs/%.2fs (wall/user)\n", $total_workers, $total_scripts, $total_tests, $total_errs, $walltime, $usertime); +} + +sub check_script { + my ($id, $next_script, $emit) = @_; + my ($nscripts, $ntests, $nerrs) = (0, 0, 0); + while (my $path = $next_script->()) { + $nscripts++; + my $fh; + unless (open($fh, "<", $path)) { + $emit->("?!ERR?! $path: $!\n"); + next; + } + my $s = do { local $/; <$fh> }; + close($fh); + my $parser = ScriptParser->new(\$s); + 1 while $parser->parse_cmd(); + if (@{$parser->{output}}) { + my $s = join('', @{$parser->{output}}); + $emit->("# chainlint: $path\n" . $s); + $nerrs += () = $s =~ /\?![^?]+\?!/g; + } + $ntests += $parser->{ntests}; + } + return [$id, $nscripts, $ntests, $nerrs]; +} + +sub exit_code { + my $stats = shift @_; + for (@$stats) { + my ($worker, $nscripts, $ntests, $nerrs) = @$_; + return 1 if $nerrs; + } + return 0; +} + +Getopt::Long::Configure(qw{bundling}); +GetOptions( + "emit-all!" => \$emit_all, + "stats|show-stats!" => \$show_stats) or die("option error\n"); + +my $start_time = $getnow->(); +my @stats; + +my @scripts; +push(@scripts, File::Glob::bsd_glob($_)) for (@ARGV); +unless (@scripts) { + show_stats($start_time, \@stats) if $show_stats; + exit; +} + +push(@stats, check_script(1, sub { shift(@scripts); }, sub { print(@_); })); +show_stats($start_time, \@stats) if $show_stats; +exit(exit_code(\@stats)); From patchwork Thu Sep 1 00:29:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961581 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 837D8ECAAD4 for ; Thu, 1 Sep 2022 00:30:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231863AbiIAAaH (ORCPT ); Wed, 31 Aug 2022 20:30:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46148 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231522AbiIAAaE (ORCPT ); Wed, 31 Aug 2022 20:30:04 -0400 Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D7EC21D0D3 for ; Wed, 31 Aug 2022 17:30:02 -0700 (PDT) Received: by mail-wr1-x429.google.com with SMTP id b5so20260253wrr.5 for ; Wed, 31 Aug 2022 17:30:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=ZOcsp5p/DsGMowTf24vGQJhP/4vb/MNWyPi+xOhSNE4=; b=TszPlNen7X55y+GNhbFdf5DTjp0yAmqTcEIRT+8dZ+Vj0UIoW2ib14UYLsnETUwtVw sR5rHTsFRYoL1jWvEnKvudou+j8+pbglqMEn/IqL5Qq2tAla+RHdOSTw3ww2TvMqd5d6 reuTC6vwtvIhz0ARvQ++pBdBPy0nHguYIE7xT+iQTrxeNj26OlD7fgfyz5J8x8Lil20q 87ZCC64zbXtvGdVB6g5RIuFcrHNJAA9/hxIMrrpG40SrsO9mrhM+Gss7ODvnJ36k39EZ hek5D4ZJ7om8ve56HiRgygMFgt+bFoHWKGIwEAJ89H8oEYjL+0Bq3pqqyyqi0yrLUm7E Vw4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=ZOcsp5p/DsGMowTf24vGQJhP/4vb/MNWyPi+xOhSNE4=; b=ShzwT60Wmx+b3gYvL+mMC8ZzZcnk9FfUsVOP9LyWTZu5K3a+ZYHrmOEZQhzJ1dCO3S LkkJmWl/wkhR0qamxUiWgz1clBVOTZHNwkw3RFx1XszglNuEFZ8KNBWObZsbuOh/QU6q 01fAheExFpAGnimJW5HUuBMfk+fNGHTp6dz3KUtzn/vjbJAfqtGn/S2UxKMW+e/zg9lF BFZY+ZrxqouWBJGx8MUAUQWdehGlnUg0Rkk1v7l3ds+Em9x63Url+3mh/jLr05FVNQOE f47HDHId3N3XgA9qyeHLNf6OspJTcUuDRM6WdOJ8CH3BWZ0YObdoe4LV87AYqKgkXcN7 GYhQ== X-Gm-Message-State: ACgBeo0dsEmg54oPqsoHbaxCNxp3OiUtgzMubZMG896497LmyZluZXfx EtS9dY0dWZuctSXVJJ9h+YZ5XURA/Y8= X-Google-Smtp-Source: AA6agR6cc/GXzGnBO8DO8E/LWHncF/5rG+Qk1KrjWbNXdrhM1JU1FQ7gR9IZ/FnSJOOKupBVK3ipPg== X-Received: by 2002:adf:ee50:0:b0:225:7508:9d88 with SMTP id w16-20020adfee50000000b0022575089d88mr13125194wro.320.1661992201025; Wed, 31 Aug 2022 17:30:01 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id o5-20020a05600c510500b003a31fd05e0fsm10048008wms.2.2022.08.31.17.30.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:00 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:40 +0000 Subject: [PATCH 02/18] chainlint.pl: add POSIX shell lexical analyzer Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine Begin fleshing out chainlint.pl by adding a lexical analyzer for the POSIX shell command language. The sole entry point Lexer::scan_token() returns the next token from the input. It will be called by the upcoming shell language parser. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 177 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) diff --git a/t/chainlint.pl b/t/chainlint.pl index e8ab95c7858..81ffbf28bf3 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -21,6 +21,183 @@ use Getopt::Long; my $show_stats; my $emit_all; +# Lexer tokenizes POSIX shell scripts. It is roughly modeled after section 2.3 +# "Token Recognition" of POSIX chapter 2 "Shell Command Language". Although +# similar to lexical analyzers for other languages, this one differs in a few +# substantial ways due to quirks of the shell command language. +# +# For instance, in many languages, newline is just whitespace like space or +# TAB, but in shell a newline is a command separator, thus a distinct lexical +# token. A newline is significant and returned as a distinct token even at the +# end of a shell comment. +# +# In other languages, `1+2` would typically be scanned as three tokens +# (`1`, `+`, and `2`), but in shell it is a single token. However, the similar +# `1 + 2`, which embeds whitepace, is scanned as three token in shell, as well. +# In shell, several characters with special meaning lose that meaning when not +# surrounded by whitespace. For instance, the negation operator `!` is special +# when standing alone surrounded by whitespace; whereas in `foo!uucp` it is +# just a plain character in the longer token "foo!uucp". In many other +# languages, `"string"/foo:'string'` might be scanned as five tokens ("string", +# `/`, `foo`, `:`, and 'string'), but in shell, it is just a single token. +# +# The lexical analyzer for the shell command language is also somewhat unusual +# in that it recursively invokes the parser to handle the body of `$(...)` +# expressions which can contain arbitrary shell code. Such expressions may be +# encountered both inside and outside of double-quoted strings. +# +# The lexical analyzer is responsible for consuming shell here-doc bodies which +# extend from the line following a `< $parser, + buff => $s, + heretags => [] + } => $class; +} + +sub scan_heredoc_tag { + my $self = shift @_; + ${$self->{buff}} =~ /\G(-?)/gc; + my $indented = $1; + my $tag = $self->scan_token(); + $tag =~ s/['"\\]//g; + push(@{$self->{heretags}}, $indented ? "\t$tag" : "$tag"); + return "<<$indented$tag"; +} + +sub scan_op { + my ($self, $c) = @_; + my $b = $self->{buff}; + return $c unless $$b =~ /\G(.)/sgc; + my $cc = $c . $1; + return scan_heredoc_tag($self) if $cc eq '<<'; + return $cc if $cc =~ /^(?:&&|\|\||>>|;;|<&|>&|<>|>\|)$/; + pos($$b)--; + return $c; +} + +sub scan_sqstring { + my $self = shift @_; + ${$self->{buff}} =~ /\G([^']*'|.*\z)/sgc; + return "'" . $1; +} + +sub scan_dqstring { + my $self = shift @_; + my $b = $self->{buff}; + my $s = '"'; + while (1) { + # slurp up non-special characters + $s .= $1 if $$b =~ /\G([^"\$\\]+)/gc; + # handle special characters + last unless $$b =~ /\G(.)/sgc; + my $c = $1; + $s .= '"', last if $c eq '"'; + $s .= '$' . $self->scan_dollar(), next if $c eq '$'; + if ($c eq '\\') { + $s .= '\\', last unless $$b =~ /\G(.)/sgc; + $c = $1; + next if $c eq "\n"; # line splice + # backslash escapes only $, `, ", \ in dq-string + $s .= '\\' unless $c =~ /^[\$`"\\]$/; + $s .= $c; + next; + } + die("internal error scanning dq-string '$c'\n"); + } + return $s; +} + +sub scan_balanced { + my ($self, $c1, $c2) = @_; + my $b = $self->{buff}; + my $depth = 1; + my $s = $c1; + while ($$b =~ /\G([^\Q$c1$c2\E]*(?:[\Q$c1$c2\E]|\z))/gc) { + $s .= $1; + $depth++, next if $s =~ /\Q$c1\E$/; + $depth--; + last if $depth == 0; + } + return $s; +} + +sub scan_subst { + my $self = shift @_; + my @tokens = $self->{parser}->parse(qr/^\)$/); + $self->{parser}->next_token(); # closing ")" + return @tokens; +} + +sub scan_dollar { + my $self = shift @_; + my $b = $self->{buff}; + return $self->scan_balanced('(', ')') if $$b =~ /\G\((?=\()/gc; # $((...)) + return '(' . join(' ', $self->scan_subst()) . ')' if $$b =~ /\G\(/gc; # $(...) + return $self->scan_balanced('{', '}') if $$b =~ /\G\{/gc; # ${...} + return $1 if $$b =~ /\G(\w+)/gc; # $var + return $1 if $$b =~ /\G([@*#?$!0-9-])/gc; # $*, $1, $$, etc. + return ''; +} + +sub swallow_heredocs { + my $self = shift @_; + my $b = $self->{buff}; + my $tags = $self->{heretags}; + while (my $tag = shift @$tags) { + my $indent = $tag =~ s/^\t// ? '\\s*' : ''; + $$b =~ /(?:\G|\n)$indent\Q$tag\E(?:\n|\z)/gc; + } +} + +sub scan_token { + my $self = shift @_; + my $b = $self->{buff}; + my $token = ''; +RESTART: + $$b =~ /\G[ \t]+/gc; # skip whitespace (but not newline) + return "\n" if $$b =~ /\G#[^\n]*(?:\n|\z)/gc; # comment + while (1) { + # slurp up non-special characters + $token .= $1 if $$b =~ /\G([^\\;&|<>(){}'"\$\s]+)/gc; + # handle special characters + last unless $$b =~ /\G(.)/sgc; + my $c = $1; + last if $c =~ /^[ \t]$/; # whitespace ends token + pos($$b)--, last if length($token) && $c =~ /^[;&|<>(){}\n]$/; + $token .= $self->scan_sqstring(), next if $c eq "'"; + $token .= $self->scan_dqstring(), next if $c eq '"'; + $token .= $c . $self->scan_dollar(), next if $c eq '$'; + $self->swallow_heredocs(), $token = $c, last if $c eq "\n"; + $token = $self->scan_op($c), last if $c =~ /^[;&|<>]$/; + $token = $c, last if $c =~ /^[(){}]$/; + if ($c eq '\\') { + $token .= '\\', last unless $$b =~ /\G(.)/sgc; + $c = $1; + next if $c eq "\n" && length($token); # line splice + goto RESTART if $c eq "\n"; # line splice + $token .= '\\' . $c; + next; + } + die("internal error scanning character '$c'\n"); + } + return length($token) ? $token : undef; +} + package ScriptParser; sub new { From patchwork Thu Sep 1 00:29:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961583 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C587BECAAD1 for ; Thu, 1 Sep 2022 00:30:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232112AbiIAAaX (ORCPT ); Wed, 31 Aug 2022 20:30:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46454 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231802AbiIAAaG (ORCPT ); Wed, 31 Aug 2022 20:30:06 -0400 Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A669B32D8C for ; Wed, 31 Aug 2022 17:30:04 -0700 (PDT) Received: by mail-wr1-x42d.google.com with SMTP id w5so4092571wrn.12 for ; Wed, 31 Aug 2022 17:30:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=GuSd5IyRy70lw70LBC+oyW8iHa8Qji6Pv61pMWXCrpM=; b=dFx2ehS7EP1ZJqGOxV5lfGgMgVti928+UnXMwT9EE0pd0R5K8QYoeMQ3D3bi9jA9VS /R+cjyL0HoxMjUl3h/dle6A7xul93GrewcBgRAN9F0E/YvnJ/Op1oZh5yW2ej9OFjKzt QfxIPX8rFzxX60Chu48ajas2bZTOwQtfiZSvaUVk+yMp1eTmoZpjL9MZTbYzAiQYHaOG 1z8UO0KVFq6uvNzhxvI/L1NWNyszRM1N0JBr39dhqXYrVPpJnpdebkLIUuDgfUdj07bq r7dPCvMD54slao+CJahQM900eIKvkqwFZH7csqjWqXR08vuXocnEYBd1QmfuPzHZMvmc CaAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=GuSd5IyRy70lw70LBC+oyW8iHa8Qji6Pv61pMWXCrpM=; b=8ELb5SeIJpFBcsTL1OdtNjDTRSK73J3ditqhkhrEH5P9i7PhyHJtOeNyzYFOdlSyHK FhMp+E0pvQGhbbpCS+JrVefOEWseWj9aG4IXv0PN/90OYatt0322I71q0OD/sqsyySkG HL+H63khDul6lrXMSCu0+2XkL+JDpj7oPcWPoexWjL0LWB67vhpJsB6wSJ7jTdktvYbW zYd1xEiqmoVAQQOZGF7ZdGjifRh7ybgsXBfEiQhVw3m47P9Ue6J8S6xaN/WDCmiPZ0gZ 58W1Zzrbq9mrQaLsATJOr+O1UgMGxAXc2R1Gdq2qU7OHYTT8UNXae8enSEjDRkY4rlY1 kE7g== X-Gm-Message-State: ACgBeo2D2zx/+GsAXoNBWfY9HiFUVZTBTYPzVAgjRLiVwZ7MfbtY3LrL fDSFGQ2zXGJiaAeDA57EB04l0466Rps= X-Google-Smtp-Source: AA6agR6QjDCSdbsGmwbMeX/MdgB8Bk1wQfqenR9Jd0mnuud/1LH9ZBhbNxr7Fo62kdaBKecJe7r5eA== X-Received: by 2002:adf:ec49:0:b0:225:4a49:1f9f with SMTP id w9-20020adfec49000000b002254a491f9fmr13007549wrn.502.1661992202582; Wed, 31 Aug 2022 17:30:02 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id l23-20020adfa397000000b002254b912727sm13254906wrb.26.2022.08.31.17.30.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:01 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:41 +0000 Subject: [PATCH 03/18] chainlint.pl: add POSIX shell parser Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine Continue fleshing out chainlint.pl by adding a general purpose recursive descent parser for the POSIX shell command language. Although never invoked directly, upcoming parser subclasses will extend its functionality for specific purposes, such as plucking test definitions from input scripts and applying domain-specific knowledge to perform test validation. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 243 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 243 insertions(+) diff --git a/t/chainlint.pl b/t/chainlint.pl index 81ffbf28bf3..cdf136896be 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -198,6 +198,249 @@ RESTART: return length($token) ? $token : undef; } +# ShellParser parses POSIX shell scripts (with minor extensions for Bash). It +# is a recursive descent parser very roughly modeled after section 2.10 "Shell +# Grammar" of POSIX chapter 2 "Shell Command Language". +package ShellParser; + +sub new { + my ($class, $s) = @_; + my $self = bless { + buff => [], + stop => [], + output => [] + } => $class; + $self->{lexer} = Lexer->new($self, $s); + return $self; +} + +sub next_token { + my $self = shift @_; + return pop(@{$self->{buff}}) if @{$self->{buff}}; + return $self->{lexer}->scan_token(); +} + +sub untoken { + my $self = shift @_; + push(@{$self->{buff}}, @_); +} + +sub peek { + my $self = shift @_; + my $token = $self->next_token(); + return undef unless defined($token); + $self->untoken($token); + return $token; +} + +sub stop_at { + my ($self, $token) = @_; + return 1 unless defined($token); + my $stop = ${$self->{stop}}[-1] if @{$self->{stop}}; + return defined($stop) && $token =~ $stop; +} + +sub expect { + my ($self, $expect) = @_; + my $token = $self->next_token(); + return $token if defined($token) && $token eq $expect; + push(@{$self->{output}}, "?!ERR?! expected '$expect' but found '" . (defined($token) ? $token : "") . "'\n"); + $self->untoken($token) if defined($token); + return (); +} + +sub optional_newlines { + my $self = shift @_; + my @tokens; + while (my $token = $self->peek()) { + last unless $token eq "\n"; + push(@tokens, $self->next_token()); + } + return @tokens; +} + +sub parse_group { + my $self = shift @_; + return ($self->parse(qr/^}$/), + $self->expect('}')); +} + +sub parse_subshell { + my $self = shift @_; + return ($self->parse(qr/^\)$/), + $self->expect(')')); +} + +sub parse_case_pattern { + my $self = shift @_; + my @tokens; + while (defined(my $token = $self->next_token())) { + push(@tokens, $token); + last if $token eq ')'; + } + return @tokens; +} + +sub parse_case { + my $self = shift @_; + my @tokens; + push(@tokens, + $self->next_token(), # subject + $self->optional_newlines(), + $self->expect('in'), + $self->optional_newlines()); + while (1) { + my $token = $self->peek(); + last unless defined($token) && $token ne 'esac'; + push(@tokens, + $self->parse_case_pattern(), + $self->optional_newlines(), + $self->parse(qr/^(?:;;|esac)$/)); # item body + $token = $self->peek(); + last unless defined($token) && $token ne 'esac'; + push(@tokens, + $self->expect(';;'), + $self->optional_newlines()); + } + push(@tokens, $self->expect('esac')); + return @tokens; +} + +sub parse_for { + my $self = shift @_; + my @tokens; + push(@tokens, + $self->next_token(), # variable + $self->optional_newlines()); + my $token = $self->peek(); + if (defined($token) && $token eq 'in') { + push(@tokens, + $self->expect('in'), + $self->optional_newlines()); + } + push(@tokens, + $self->parse(qr/^do$/), # items + $self->expect('do'), + $self->optional_newlines(), + $self->parse_loop_body(), + $self->expect('done')); + return @tokens; +} + +sub parse_if { + my $self = shift @_; + my @tokens; + while (1) { + push(@tokens, + $self->parse(qr/^then$/), # if/elif condition + $self->expect('then'), + $self->optional_newlines(), + $self->parse(qr/^(?:elif|else|fi)$/)); # if/elif body + my $token = $self->peek(); + last unless defined($token) && $token eq 'elif'; + push(@tokens, $self->expect('elif')); + } + my $token = $self->peek(); + if (defined($token) && $token eq 'else') { + push(@tokens, + $self->expect('else'), + $self->optional_newlines(), + $self->parse(qr/^fi$/)); # else body + } + push(@tokens, $self->expect('fi')); + return @tokens; +} + +sub parse_loop_body { + my $self = shift @_; + return $self->parse(qr/^done$/); +} + +sub parse_loop { + my $self = shift @_; + return ($self->parse(qr/^do$/), # condition + $self->expect('do'), + $self->optional_newlines(), + $self->parse_loop_body(), + $self->expect('done')); +} + +sub parse_func { + my $self = shift @_; + return ($self->expect('('), + $self->expect(')'), + $self->optional_newlines(), + $self->parse_cmd()); # body +} + +sub parse_bash_array_assignment { + my $self = shift @_; + my @tokens = $self->expect('('); + while (defined(my $token = $self->next_token())) { + push(@tokens, $token); + last if $token eq ')'; + } + return @tokens; +} + +my %compound = ( + '{' => \&parse_group, + '(' => \&parse_subshell, + 'case' => \&parse_case, + 'for' => \&parse_for, + 'if' => \&parse_if, + 'until' => \&parse_loop, + 'while' => \&parse_loop); + +sub parse_cmd { + my $self = shift @_; + my $cmd = $self->next_token(); + return () unless defined($cmd); + return $cmd if $cmd eq "\n"; + + my $token; + my @tokens = $cmd; + if ($cmd eq '!') { + push(@tokens, $self->parse_cmd()); + return @tokens; + } elsif (my $f = $compound{$cmd}) { + push(@tokens, $self->$f()); + } elsif (defined($token = $self->peek()) && $token eq '(') { + if ($cmd !~ /\w=$/) { + push(@tokens, $self->parse_func()); + return @tokens; + } + $tokens[-1] .= join(' ', $self->parse_bash_array_assignment()); + } + + while (defined(my $token = $self->next_token())) { + $self->untoken($token), last if $self->stop_at($token); + push(@tokens, $token); + last if $token =~ /^(?:[;&\n|]|&&|\|\|)$/; + } + push(@tokens, $self->next_token()) if $tokens[-1] ne "\n" && defined($token = $self->peek()) && $token eq "\n"; + return @tokens; +} + +sub accumulate { + my ($self, $tokens, $cmd) = @_; + push(@$tokens, @$cmd); +} + +sub parse { + my ($self, $stop) = @_; + push(@{$self->{stop}}, $stop); + goto DONE if $self->stop_at($self->peek()); + my @tokens; + while (my @cmd = $self->parse_cmd()) { + $self->accumulate(\@tokens, \@cmd); + last if $self->stop_at($self->peek()); + } +DONE: + pop(@{$self->{stop}}); + return @tokens; +} + package ScriptParser; sub new { From patchwork Thu Sep 1 00:29:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961582 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 80E29ECAAD4 for ; Thu, 1 Sep 2022 00:30:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231901AbiIAAaW (ORCPT ); Wed, 31 Aug 2022 20:30:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46452 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231755AbiIAAaG (ORCPT ); Wed, 31 Aug 2022 20:30:06 -0400 Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 799E832D88 for ; Wed, 31 Aug 2022 17:30:04 -0700 (PDT) Received: by mail-wm1-x331.google.com with SMTP id i188-20020a1c3bc5000000b003a7b6ae4eb2so443623wma.4 for ; Wed, 31 Aug 2022 17:30:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=26YUjmv3YfoBiahlEeoZe8AJZ67CvGYEVs/aobgiE7s=; b=ADFZT7fUabyWKAnh0iK3d8EPxLDoGFpbIRAj92JFwKQWGZOo5yF3O7MnDv2zSrp8gL e1RH/aFmhpYd5f2BgWDKc+b0c4j0BDUtvfY5nHRMBPEtvP5mqtQIK7Qpn+TW06zLZAp6 3YzSBfbipESRA6L8wK4uGzXL4nu/LMUMcGDWG5ZR4XVNRs85xP8qIe0fykaSTHVXa8dq DWXETaOvIHMD+W80p5ChjiCBCKyqU0ZFiGpCV4xpSb3fVwGUlmWs6S7B8H37ee/Z0BQP p2QzWVW7YPsAqd+QZr8b9QwCChK0W0gEfwtnEgyMl8Yebqb8jTedDu6VYFIJgFBXPkMI u8VQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=26YUjmv3YfoBiahlEeoZe8AJZ67CvGYEVs/aobgiE7s=; b=SoZhqFWLxu0VLvij0QJ/c6XtxKdhpv8FgLGriEihufNyI3JUY/D1vPh/bKxPqKVzuJ TtxHEov2cfVnCTozpDyKeF1aFNArZu8748YgmiskOcTUT57Amj7hd+aX1llWaLZjuojt SEj1YgBREEjAUI8b6L8BRMNdnhptzKAo056QW3EJpyEyjcIgvKdpPjj0+Km4WNCcMhJR 5IgrAiELksVV4VVD+NUvblE3GqswxtMIesLjAqFXxWFUMmB7a87t1t5WX+93qKJtE7Oe qYBfGyaPwgOM29vj3EN64tg58NF7Kq1EvhINF5Bl2TsXUcTjWDJw9fT6s+iELkI6ns4A qQcg== X-Gm-Message-State: ACgBeo2onzsFYR8fMLAbt0Nut2EL168S15IfR/6q2BVJETKUfDnjf/LR 3DKuOYhXtsdnadQuaXBePbCRiZKB8Qw= X-Google-Smtp-Source: AA6agR71l8eMJ2mHrk/+P77Tb7IgIZ6zoNDaEatTdI/UM/5Okq0SeiYHN1Bem78OQO5L+AUEP9JK5w== X-Received: by 2002:a7b:c2a9:0:b0:3a6:2400:722c with SMTP id c9-20020a7bc2a9000000b003a62400722cmr3402082wmk.108.1661992203882; Wed, 31 Aug 2022 17:30:03 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id r7-20020a05600c35c700b003a5b6086381sm4243050wmq.48.2022.08.31.17.30.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:03 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:42 +0000 Subject: [PATCH 04/18] chainlint.pl: add parser to validate tests Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine Continue fleshing out chainlint.pl by adding TestParser, a parser with special knowledge about how Git tests should be written; for instance, it knows that commands within a test body should be chained together with `&&`. An upcoming parser which plucks test definitions from test scripts will invoke TestParser for each test body it encounters. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/t/chainlint.pl b/t/chainlint.pl index cdf136896be..ad257106e56 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -441,6 +441,52 @@ DONE: return @tokens; } +# TestParser is a subclass of ShellParser which, beyond parsing shell script +# code, is also imbued with semantic knowledge of test construction, and checks +# tests for common problems (such as broken &&-chains) which might hide bugs in +# the tests themselves or in behaviors being exercised by the tests. As such, +# TestParser is only called upon to parse test bodies, not the top-level +# scripts in which the tests are defined. +package TestParser; + +use base 'ShellParser'; + +sub find_non_nl { + my $tokens = shift @_; + my $n = shift @_; + $n = $#$tokens if !defined($n); + $n-- while $n >= 0 && $$tokens[$n] eq "\n"; + return $n; +} + +sub ends_with { + my ($tokens, $needles) = @_; + my $n = find_non_nl($tokens); + for my $needle (reverse(@$needles)) { + return undef if $n < 0; + $n = find_non_nl($tokens, $n), next if $needle eq "\n"; + return undef if $$tokens[$n] !~ $needle; + $n--; + } + return 1; +} + +sub accumulate { + my ($self, $tokens, $cmd) = @_; + goto DONE unless @$tokens; + goto DONE if @$cmd == 1 && $$cmd[0] eq "\n"; + + # did previous command end with "&&", "||", "|"? + goto DONE if ends_with($tokens, [qr/^(?:&&|\|\||\|)$/]); + + # flag missing "&&" at end of previous command + my $n = find_non_nl($tokens); + splice(@$tokens, $n + 1, 0, '?!AMP?!') unless $n < 0; + +DONE: + $self->SUPER::accumulate($tokens, $cmd); +} + package ScriptParser; sub new { From patchwork Thu Sep 1 00:29:43 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961585 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 75D0CECAAD1 for ; Thu, 1 Sep 2022 00:30:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232132AbiIAAaa (ORCPT ); Wed, 31 Aug 2022 20:30:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46574 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231891AbiIAAaI (ORCPT ); Wed, 31 Aug 2022 20:30:08 -0400 Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F2230422CA for ; Wed, 31 Aug 2022 17:30:06 -0700 (PDT) Received: by mail-wm1-x32c.google.com with SMTP id z14-20020a7bc7ce000000b003a5db0388a8so2420357wmk.1 for ; Wed, 31 Aug 2022 17:30:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=g5sz2lNydeA2+NtaZNWjc4Wf0QYQKS+84cFK0G1aZg8=; b=PuyX7RcU1PhkN77LIOCx0uj0cRAy17fRYQSZ2C98rUKPEIdqjhZuFFeHsejQVcmH/P sqvQBvwjgboNurbD3DKmWoDenF8FFYUNReO2JC8T8Qm9zSwbTSwbM7hf1zTtmTuQWUMn atMKg8Rlojp+f2tHn5HajP/pHovafybpyVEcCto3Yzu5sszTCfMKzX0Cigs95sMCD7sl P8md7G41rWPpcD3VfwxnRIyyj+8aEKrf9LBG0xn4PUbAoLP9TH4JnKa10CMy+0lhusJD Y3IykPwtfAcnfCYaTCygq8Hwe79GWmIDz6BLB1Swx0gry9ivdy7ZpwJUNSwI0GR1ubWd h3BQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=g5sz2lNydeA2+NtaZNWjc4Wf0QYQKS+84cFK0G1aZg8=; b=T0GeuKijjulH/yqAkwXWwSD/lnD3a8tz+eBjgYUD0UhJhkGupxCNyKX0PBB7Se7pqm oQMDaD2pGpDKnLUT/0JwjSl4WfQ/xBDRZhWYcrr3awSG2WAQGeTd328zxxJhfxOMjRBq DvzigBlCWGBJ+RdHFgUOaYWEQ7KcninAWSGZZZvVKNyrhNFZGvjzd5GtFw3tZII8vR49 ZSGV6K5jTEHXrxthZScIqK05qKDYneflFwQlvSrTTiACDdzpNUgL9iNCQv5inE4Y5t20 XbXpgdI7jzi3dVtOuJIGa98ToakJoxhvuTddwyUhQ3ENUM7BzJg25URJS1mzqKLxu2mB CclA== X-Gm-Message-State: ACgBeo1tbBR/1713h+OMwUA3OxH2kmQvRtDPxLFs9RAVju4Bz6JNYqjb q8t90CxOUCObXsP8t3LTfKTfJCk4CGI= X-Google-Smtp-Source: AA6agR42F2XM9uJICIiFySSUZbNnQJ0kFL4DqPi2+kUFImf25cL93sNddfFNGDT/2jlOSrvripjFog== X-Received: by 2002:a05:600c:4e8b:b0:3a5:f5bf:9c5a with SMTP id f11-20020a05600c4e8b00b003a5f5bf9c5amr3479048wmq.85.1661992205276; Wed, 31 Aug 2022 17:30:05 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id ay21-20020a05600c1e1500b003a536d5aa2esm3657937wmb.11.2022.08.31.17.30.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:04 -0700 (PDT) Message-Id: <0de14477a42f2c18efb4b1e0ba52155645a7f0e2.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:43 +0000 Subject: [PATCH 05/18] chainlint.pl: add parser to identify test definitions Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine Finish fleshing out chainlint.pl by adding ScriptParser, a parser which scans shell scripts for tests defined by test_expect_success() and test_expect_failure(), plucks the test body from each definition, and passes it to TestParser for validation. It recognizes test definitions not only at the top-level of test scripts but also tests synthesized within compound commands such as loops and function. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 63 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/t/chainlint.pl b/t/chainlint.pl index ad257106e56..d526723ac00 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -487,18 +487,75 @@ DONE: $self->SUPER::accumulate($tokens, $cmd); } +# ScriptParser is a subclass of ShellParser which identifies individual test +# definitions within test scripts, and passes each test body through TestParser +# to identify possible problems. ShellParser detects test definitions not only +# at the top-level of test scripts but also within compound commands such as +# loops and function definitions. package ScriptParser; +use base 'ShellParser'; + sub new { my $class = shift @_; - my $self = bless {} => $class; - $self->{output} = []; + my $self = $class->SUPER::new(@_); $self->{ntests} = 0; return $self; } +# extract the raw content of a token, which may be a single string or a +# composition of multiple strings and non-string character runs; for instance, +# `"test body"` unwraps to `test body`; `word"a b"42'c d'` to `worda b42c d` +sub unwrap { + my $token = @_ ? shift @_ : $_; + # simple case: 'sqstring' or "dqstring" + return $token if $token =~ s/^'([^']*)'$/$1/; + return $token if $token =~ s/^"([^"]*)"$/$1/; + + # composite case + my ($s, $q, $escaped); + while (1) { + # slurp up non-special characters + $s .= $1 if $token =~ /\G([^\\'"]*)/gc; + # handle special characters + last unless $token =~ /\G(.)/sgc; + my $c = $1; + $q = undef, next if defined($q) && $c eq $q; + $q = $c, next if !defined($q) && $c =~ /^['"]$/; + if ($c eq '\\') { + last unless $token =~ /\G(.)/sgc; + $c = $1; + $s .= '\\' if $c eq "\n"; # preserve line splice + } + $s .= $c; + } + return $s +} + +sub check_test { + my $self = shift @_; + my ($title, $body) = map(unwrap, @_); + $self->{ntests}++; + my $parser = TestParser->new(\$body); + my @tokens = $parser->parse(); + return unless $emit_all || grep(/\?![^?]+\?!/, @tokens); + my $checked = join(' ', @tokens); + $checked =~ s/^\n//; + $checked =~ s/^ //mg; + $checked =~ s/ $//mg; + $checked .= "\n" unless $checked =~ /\n$/; + push(@{$self->{output}}, "# chainlint: $title\n$checked"); +} + sub parse_cmd { - return undef; + my $self = shift @_; + my @tokens = $self->SUPER::parse_cmd(); + return @tokens unless @tokens && $tokens[0] =~ /^test_expect_(?:success|failure)$/; + my $n = $#tokens; + $n-- while $n >= 0 && $tokens[$n] =~ /^(?:[;&\n|]|&&|\|\|)$/; + $self->check_test($tokens[1], $tokens[2]) if $n == 2; # title body + $self->check_test($tokens[2], $tokens[3]) if $n > 2; # prereq title body + return @tokens; } # main contains high-level functionality for processing command-line switches, From patchwork Thu Sep 1 00:29:44 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961584 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DF004ECAAD1 for ; Thu, 1 Sep 2022 00:30:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232065AbiIAAa1 (ORCPT ); Wed, 31 Aug 2022 20:30:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46740 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231522AbiIAAaK (ORCPT ); Wed, 31 Aug 2022 20:30:10 -0400 Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0867946DB4 for ; Wed, 31 Aug 2022 17:30:08 -0700 (PDT) Received: by mail-wm1-x32b.google.com with SMTP id d5so8169634wms.5 for ; Wed, 31 Aug 2022 17:30:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=SHNMajoaPTP2MJWJ6AkRDSQv92Srq80YyiWtJHQHFVs=; b=okfIMDnT9YePMX4MQIt8ZxoSlecEG9WyjyVqbNasAGECVPWYFQSnifTKW/uc8W0Jmw uNciu1czvbLXZ0FaKfc+yxm18kHKI2LGzEikqwoIqEQD+1Je69IYcMDApMvQ6/GdHYxO K48d2g9WsVjoc6FMJEo2hWq98rE3Qe9Kh/UgmTu6zyuYd0/E+I1pSkxaT4hTCHnk4vaN F71EvgZe3/wfrR7HTvT+ZxnOlxtV2L+FbhERyaF4Gq9fZ4cZRMu/0G0EvOM3u9RFjXHw VLBqhwHPU59ciL6nTD0YbM7o7PyPqf4wekl+/jXxY+addPFkXgxgHeTLU4vvUxXP+9/G REvQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=SHNMajoaPTP2MJWJ6AkRDSQv92Srq80YyiWtJHQHFVs=; b=2DTi3wPt98J3943ODVaQ3JMWNGkCuA/vFaQSSQnQbcGP6il3ptWHufKnWAahlDZNFf +8LbMZa/DO3uuz11bpi3EjXuM5Pkz27TFpHsRihN42u35+8eHmb5MH/qdXa/eVCGifXV RsNyhymqoS6s6ChYWAZOXYgpu2aAcFNCYkr298qXLpzQUkbMwfbfaoTU2R/MKxfE3FEQ 8HWqsgNn7+wvZU5EfRwC/+1uOrFYMSG6ecGWroLf29Ju/EOuiKCezgINAqIhI01CozAu CD84d7H7WRrdv0OjxF6RaQj+F9NRVMg1FxCerTSTxx/RyTjPhid65m+Ye65+aQpVpLo9 hCXA== X-Gm-Message-State: ACgBeo0PLMFBuAiAm5J2UEvMEOgxWom4TTuw4BKIS3njwXKztx3NHUoW e5H8pTUqkkAE0AsKau1eO4FHaZc1HUc= X-Google-Smtp-Source: AA6agR5lLTRNapFn+lZizQphsFVdZxnewpDfGjeHV3jKb9LNU4kWql9wGQZaHTqWzhIBkGfVfvtF5w== X-Received: by 2002:a05:600c:410d:b0:3a6:1db8:b419 with SMTP id j13-20020a05600c410d00b003a61db8b419mr3376202wmi.119.1661992206302; Wed, 31 Aug 2022 17:30:06 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id n8-20020a5d4848000000b00226d01a4635sm13182910wrs.35.2022.08.31.17.30.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:05 -0700 (PDT) Message-Id: <62fc652eb47a4df83d88a197e376f28dbbab3b52.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:44 +0000 Subject: [PATCH 06/18] chainlint.pl: validate test scripts in parallel Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine Although chainlint.pl has undergone a good deal of optimization during its development -- increasing in speed significantly -- parsing and validating 1050+ scripts and 16500+ tests via Perl is not exactly instantaneous. However, perceived performance can be improved by taking advantage of the fact that there is no interdependence between test scripts or test definitions, thus parsing and validating can be done in parallel. The number of available cores is determined automatically but can be overridden via the --jobs option. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/t/chainlint.pl b/t/chainlint.pl index d526723ac00..898573a9100 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -15,9 +15,11 @@ use warnings; use strict; +use Config; use File::Glob; use Getopt::Long; +my $jobs = -1; my $show_stats; my $emit_all; @@ -569,6 +571,16 @@ if (eval {require Time::HiRes; Time::HiRes->import(); 1;}) { $interval = sub { return Time::HiRes::tv_interval(shift); }; } +sub ncores { + # Windows + return $ENV{NUMBER_OF_PROCESSORS} if exists($ENV{NUMBER_OF_PROCESSORS}); + # Linux / MSYS2 / Cygwin / WSL + do { local @ARGV='/proc/cpuinfo'; return scalar(grep(/^processor\s*:/, <>)); } if -r '/proc/cpuinfo'; + # macOS & BSD + return qx/sysctl -n hw.ncpu/ if $^O =~ /(?:^darwin$|bsd)/; + return 1; +} + sub show_stats { my ($start_time, $stats) = @_; my $walltime = $interval->($start_time); @@ -621,7 +633,9 @@ sub exit_code { Getopt::Long::Configure(qw{bundling}); GetOptions( "emit-all!" => \$emit_all, + "jobs|j=i" => \$jobs, "stats|show-stats!" => \$show_stats) or die("option error\n"); +$jobs = ncores() if $jobs < 1; my $start_time = $getnow->(); my @stats; @@ -633,6 +647,40 @@ unless (@scripts) { exit; } -push(@stats, check_script(1, sub { shift(@scripts); }, sub { print(@_); })); +unless ($Config{useithreads} && eval { + require threads; threads->import(); + require Thread::Queue; Thread::Queue->import(); + 1; + }) { + push(@stats, check_script(1, sub { shift(@scripts); }, sub { print(@_); })); + show_stats($start_time, \@stats) if $show_stats; + exit(exit_code(\@stats)); +} + +my $script_queue = Thread::Queue->new(); +my $output_queue = Thread::Queue->new(); + +sub next_script { return $script_queue->dequeue(); } +sub emit { $output_queue->enqueue(@_); } + +sub monitor { + while (my $s = $output_queue->dequeue()) { + print($s); + } +} + +my $mon = threads->create({'context' => 'void'}, \&monitor); +threads->create({'context' => 'list'}, \&check_script, $_, \&next_script, \&emit) for 1..$jobs; + +$script_queue->enqueue(@scripts); +$script_queue->end(); + +for (threads->list()) { + push(@stats, $_->join()) unless $_ == $mon; +} + +$output_queue->end(); +$mon->join(); + show_stats($start_time, \@stats) if $show_stats; exit(exit_code(\@stats)); From patchwork Thu Sep 1 00:29:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961586 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 644C1ECAAD4 for ; Thu, 1 Sep 2022 00:30:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232154AbiIAAad (ORCPT ); Wed, 31 Aug 2022 20:30:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47494 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231941AbiIAAaU (ORCPT ); Wed, 31 Aug 2022 20:30:20 -0400 Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0ABDD56BB2 for ; Wed, 31 Aug 2022 17:30:09 -0700 (PDT) Received: by mail-wr1-x42f.google.com with SMTP id k9so20313530wri.0 for ; Wed, 31 Aug 2022 17:30:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=5+ckIDZAAoQ96Z/lwz0pSCfyVgJt+I5FQnr5SSG0NHg=; b=hUeg2BmrhMpL88pdVZ7jEA9eg4aQTSJAGgfHrvh620FdIj3OrD3StGiQT6Nh1nNRJl H3YdbnsEtmMryMOoFgmGFhm8/Z+I7/9KiIUugmx/e1+zNnMsPYqEuKyfZGQaWHxXLAJp cpxI8W54RmYZ6qhQVgI/aLb3vjs7z2RRwOHXnKf08SGHV3/d0oX0xjQE3cdl23QUz0aB 7FZlBu/7J7TTcksQqPvHAfeUjeWrTuFd6tzyfVG4XNxkVcLFt0jqAZHOQ9ObVqHXYSH3 BB+UJNQJF7jJ1A16tpuZGKUlg+soxH/MPDIM+w7qwmDBAq9ca3IHrfi/cqcod07rcoHd jILQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=5+ckIDZAAoQ96Z/lwz0pSCfyVgJt+I5FQnr5SSG0NHg=; b=Rge3NH4Q1y8qWwlxsUhBV5IR4WWajve0m5IdQ0tpn1y8AYsVqpXkPK5iNTU4xjTaNX c7a4uHCkLggAw+UJPOM7ypzatLcyYrqwL/bfV+NVTNML7ieU+BtSkFS3jyx2vKu9UZDQ ArY1VoeDMT74Q6kZQBIzuZNVjw5wwVA96Ubg2vAZTifkpskB9ouw0HDLV12BfwI1WBTP OcBlK4aEIF/SrWxEFBgjryLmiZk8V+di6EdvZB5nUvPnlbRDVwVxfoQsr3iiCyLBx+Bt 0IXjTEeijGtjQ+bNoGXK/0kZZXxAPT3uJhp31W8OmPOJxcqkWuJjuL38PSVYewMhcUdq qDsQ== X-Gm-Message-State: ACgBeo0Hajf1rb1NXQ506RW+8JK3fijL6owOC6Ay10RxSbRDgnSqAdBp e7t7ZbVMk2Sj2pmJ8L9STWdG/KAnk6w= X-Google-Smtp-Source: AA6agR7uPxziZCcXHDZpC1XRiek31QKoSYsHARVdzIBdldb9vuuBeWRCPtbmySzqS8sgNhtyN/3GtQ== X-Received: by 2002:a05:6000:616:b0:226:d80b:76ab with SMTP id bn22-20020a056000061600b00226d80b76abmr10325560wrb.547.1661992207119; Wed, 31 Aug 2022 17:30:07 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id w6-20020a1cf606000000b003a5fa79007fsm3428898wmc.7.2022.08.31.17.30.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:06 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:45 +0000 Subject: [PATCH 07/18] chainlint.pl: don't require `return|exit|continue` to end with `&&` Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine In order to check for &&-chain breakage, each time TestParser encounters a new command, it checks whether the previous command ends with `&&`, and -- with a couple exceptions -- signals breakage if it does not. The first exception is that a command may validly end with `||`, which is commonly employed as `command || return 1` at the very end of a loop body to terminate the loop early. The second is that piping one command's output with `|` to another command does not constitute a &&-chain break (the exit status of the pipe is the exit status of the final command in the pipe). However, it turns out that there are a few additional cases found in the wild in which it is likely safe for `&&` to be missing even when other commands follow. For instance: while {condition-1} do test {condition-2} || return 1 # or `exit 1` within a subshell more-commands done while {condition-1} do test {condition-2} || continue more-commands done Such cases indicate deliberate thought about failure modes by the test author, thus flagging them as breaking the &&-chain is not helpful. Therefore, take these special cases into consideration when checking for &&-chain breakage. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 20 ++++++++++++++++++-- t/chainlint/chain-break-continue.expect | 12 ++++++++++++ t/chainlint/chain-break-continue.test | 13 +++++++++++++ t/chainlint/chain-break-return-exit.expect | 4 ++++ t/chainlint/chain-break-return-exit.test | 5 +++++ t/chainlint/return-loop.expect | 5 +++++ t/chainlint/return-loop.test | 6 ++++++ 7 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 t/chainlint/chain-break-continue.expect create mode 100644 t/chainlint/chain-break-continue.test create mode 100644 t/chainlint/chain-break-return-exit.expect create mode 100644 t/chainlint/chain-break-return-exit.test create mode 100644 t/chainlint/return-loop.expect create mode 100644 t/chainlint/return-loop.test diff --git a/t/chainlint.pl b/t/chainlint.pl index 898573a9100..31c444067ce 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -473,13 +473,29 @@ sub ends_with { return 1; } +sub match_ending { + my ($tokens, $endings) = @_; + for my $needles (@$endings) { + next if @$tokens < scalar(grep {$_ ne "\n"} @$needles); + return 1 if ends_with($tokens, $needles); + } + return undef; +} + +my @safe_endings = ( + [qr/^(?:&&|\|\||\|)$/], + [qr/^(?:exit|return)$/, qr/^(?:\d+|\$\?)$/], + [qr/^(?:exit|return)$/, qr/^(?:\d+|\$\?)$/, qr/^;$/], + [qr/^(?:exit|return|continue)$/], + [qr/^(?:exit|return|continue)$/, qr/^;$/]); + sub accumulate { my ($self, $tokens, $cmd) = @_; goto DONE unless @$tokens; goto DONE if @$cmd == 1 && $$cmd[0] eq "\n"; - # did previous command end with "&&", "||", "|"? - goto DONE if ends_with($tokens, [qr/^(?:&&|\|\||\|)$/]); + # did previous command end with "&&", "|", "|| return" or similar? + goto DONE if match_ending($tokens, \@safe_endings); # flag missing "&&" at end of previous command my $n = find_non_nl($tokens); diff --git a/t/chainlint/chain-break-continue.expect b/t/chainlint/chain-break-continue.expect new file mode 100644 index 00000000000..47a34577100 --- /dev/null +++ b/t/chainlint/chain-break-continue.expect @@ -0,0 +1,12 @@ +git ls-tree --name-only -r refs/notes/many_notes | +while read path +do + test "$path" = "foobar/non-note.txt" && continue + test "$path" = "deadbeef" && continue + test "$path" = "de/adbeef" && continue + + if test $(expr length "$path") -ne $hexsz + then + return 1 + fi +done diff --git a/t/chainlint/chain-break-continue.test b/t/chainlint/chain-break-continue.test new file mode 100644 index 00000000000..f0af71d8bd9 --- /dev/null +++ b/t/chainlint/chain-break-continue.test @@ -0,0 +1,13 @@ +git ls-tree --name-only -r refs/notes/many_notes | +while read path +do +# LINT: broken &&-chain okay if explicit "continue" + test "$path" = "foobar/non-note.txt" && continue + test "$path" = "deadbeef" && continue + test "$path" = "de/adbeef" && continue + + if test $(expr length "$path") -ne $hexsz + then + return 1 + fi +done diff --git a/t/chainlint/chain-break-return-exit.expect b/t/chainlint/chain-break-return-exit.expect new file mode 100644 index 00000000000..dba292ee89b --- /dev/null +++ b/t/chainlint/chain-break-return-exit.expect @@ -0,0 +1,4 @@ +for i in 1 2 3 4 ; do + git checkout main -b $i || return $? + test_commit $i $i $i tag$i || return $? +done diff --git a/t/chainlint/chain-break-return-exit.test b/t/chainlint/chain-break-return-exit.test new file mode 100644 index 00000000000..e2b059933aa --- /dev/null +++ b/t/chainlint/chain-break-return-exit.test @@ -0,0 +1,5 @@ +for i in 1 2 3 4 ; do +# LINT: broken &&-chain okay if explicit "return $?" signals failure + git checkout main -b $i || return $? + test_commit $i $i $i tag$i || return $? +done diff --git a/t/chainlint/return-loop.expect b/t/chainlint/return-loop.expect new file mode 100644 index 00000000000..cfc0549befe --- /dev/null +++ b/t/chainlint/return-loop.expect @@ -0,0 +1,5 @@ +while test $i -lt $((num - 5)) +do + git notes add -m "notes for commit$i" HEAD~$i || return 1 + i=$((i + 1)) +done diff --git a/t/chainlint/return-loop.test b/t/chainlint/return-loop.test new file mode 100644 index 00000000000..f90b1713005 --- /dev/null +++ b/t/chainlint/return-loop.test @@ -0,0 +1,6 @@ +while test $i -lt $((num - 5)) +do +# LINT: "|| return {n}" valid loop escape outside subshell; no "&&" needed + git notes add -m "notes for commit$i" HEAD~$i || return 1 + i=$((i + 1)) +done From patchwork Thu Sep 1 00:29:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961590 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 46891ECAAD1 for ; Thu, 1 Sep 2022 00:30:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232178AbiIAAam (ORCPT ); Wed, 31 Aug 2022 20:30:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47546 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232003AbiIAAaV (ORCPT ); Wed, 31 Aug 2022 20:30:21 -0400 Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6C85E6AA3E for ; Wed, 31 Aug 2022 17:30:09 -0700 (PDT) Received: by mail-wm1-x329.google.com with SMTP id k17so8180524wmr.2 for ; Wed, 31 Aug 2022 17:30:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=q86GMcjBtEEokqSMLg6tH3ZO+WReq/RUwXBUegY1+6U=; b=TcpCbHfHLuAP07jYqpLsgSvO3AbPaEmF0EAhH9oKCj55LdpV1tcNsq94CuuunG5b2C oMi4RIbraogmKFXzCyxrl5Xwy8npxu6iZBF9BR3Vt9twQaI8gqZlDr4D/QDt3QyHqaQ/ +Ayk68TpSPhjH5Xsu92z1Z2XyQL2D9V8slvEAbvAxyiSeAgAcQwW2JsxhVb1y0RmQjn8 imD7uyCt32EF+rIeCn0qeSEw6WgKYHvk42yuIGHTgYIpFRIrbtVTDxlZk7Rn4NkxUfLH iuPlJ2iWl3vhf7PGzobGawXKXK+qXKceAsayxasfa9/5qxM+vTA4kdIaNX5yaLb82dat g4Og== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=q86GMcjBtEEokqSMLg6tH3ZO+WReq/RUwXBUegY1+6U=; b=I9RP8z4NJzE/UqoATiVKaPCbdbplZNofCa9mslBmGXyQaYkh+4xS99h59bwsGSRi1U 4DSnXQ6BadxdqrYpzS0Ap1eFEkYr9ucm8sXH1kDC/BuZ34F2G6ufvhQcgo/NGBKe2ETo l0q11q+R5bUDPHl/l1aH5U3NdJ1r7VfOm2oAkLNmCSPmLWmaGWu3aqyy8IblO7RXpUac 7wQyuKrCLaS5qXL9nTrgnapc8KXOm3eaJlTmbUrPr/SXafsd3FxULTT7haahpT1hJJ7r 4SPrVmz8S1JhxPQEEUcD0ErXCGf3z7Lu3su7FnzP2u/TPPO1qpxLFgkdb+yf3Wc2Fn41 XaKw== X-Gm-Message-State: ACgBeo3MWvHd3kCi6uIQl/NeYod5/lmuoAxyQjjFhDGQKsBgaEeRMEhM aYK2L9jyb2jUriIyUh3+vPxU2wOF1ho= X-Google-Smtp-Source: AA6agR4xTqlCCqYaYylyrja0ZLpwo5FiRuphbEcnzfB2CtrSHI0Ozr9L2GxQHxwwv5HXRDXoKvwcaw== X-Received: by 2002:a05:600c:3c84:b0:3a6:9596:fa1f with SMTP id bg4-20020a05600c3c8400b003a69596fa1fmr3412767wmb.162.1661992208118; Wed, 31 Aug 2022 17:30:08 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id j5-20020a05600c42c500b003a54fffa809sm3253093wme.17.2022.08.31.17.30.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:07 -0700 (PDT) Message-Id: <737d666bf9e309686f95e4909998c33200c1e0a4.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:46 +0000 Subject: [PATCH 08/18] t/Makefile: apply chainlint.pl to existing self-tests Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine Now that chainlint.pl is functional, take advantage of the existing chainlint self-tests to validate its operation. (While at it, stop validating chainlint.sed against the self-tests since it will soon be retired.) Due to chainlint.sed implementation limitations leaking into the self-test "expect" files, a few of them require minor adjustment to make them compatible with chainlint.pl which does not share those limitations. First, because `sed` does not provide any sort of real recursion, chainlint.sed only emulates recursion into subshells, and each level of recursion leads to a multiplicative increase in complexity of the `sed` rules. To avoid substantial complexity, chainlint.sed, therefore, only emulates subshell recursion one level deep. Any subshell deeper than that is passed through as-is, which means that &&-chains are not checked in deeper subshells. chainlint.pl, on the other hand, employs a proper recursive descent parser, thus checks subshells to any depth and correctly flags broken &&-chains in deep subshells. Second, due to sed's line-oriented nature, chainlint.sed, by necessity, folds multi-line quoted strings into a single line. chainlint.pl, on the other hand, employs a proper lexical analyzer which preserves quoted strings as-is, including embedded newlines. Furthermore, the output of chainlint.sed and chainlint.pl do not match precisely in terms of whitespace. However, since the purpose of the self-checks is to verify that the ?!AMP?! annotations are being correctly added, minor whitespace differences are immaterial. For this reason, rather than adjusting whitespace in all existing self-test "expect" files to match the new linter's output, the `check-chainlint` target ignores whitespace differences. Since `diff -w` is not POSIX, `check-chainlint` attempts to employ `git diff -w`, and only falls back to non-POSIX `diff -w` (and `-u`) if `git diff` is not available. Signed-off-by: Eric Sunshine --- t/Makefile | 29 +++++++++++++++---- t/chainlint/block.expect | 2 +- t/chainlint/here-doc-multi-line-string.expect | 3 +- t/chainlint/multi-line-string.expect | 11 +++++-- t/chainlint/nested-subshell.expect | 2 +- t/chainlint/t7900-subtree.expect | 13 +++++++-- 6 files changed, 46 insertions(+), 14 deletions(-) diff --git a/t/Makefile b/t/Makefile index 1c80c0c79a0..11f276774ea 100644 --- a/t/Makefile +++ b/t/Makefile @@ -38,7 +38,7 @@ T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)) THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh))) TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh)) CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test))) -CHAINLINT = sed -f chainlint.sed +CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl all: $(DEFAULT_TEST_TARGET) @@ -73,10 +73,29 @@ clean-chainlint: check-chainlint: @mkdir -p '$(CHAINLINTTMP_SQ)' && \ - sed -e '/^# LINT: /d' $(patsubst %,chainlint/%.test,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/tests && \ - sed -e '/^[ ]*$$/d' $(patsubst %,chainlint/%.expect,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/expect && \ - $(CHAINLINT) '$(CHAINLINTTMP_SQ)'/tests | grep -v '^[ ]*$$' >'$(CHAINLINTTMP_SQ)'/actual && \ - diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual + for i in $(CHAINLINTTESTS); do \ + echo "test_expect_success '$$i' '" && \ + sed -e '/^# LINT: /d' chainlint/$$i.test && \ + echo "'"; \ + done >'$(CHAINLINTTMP_SQ)'/tests && \ + { \ + echo "# chainlint: $(CHAINLINTTMP_SQ)/tests" && \ + for i in $(CHAINLINTTESTS); do \ + echo "# chainlint: $$i" && \ + sed -e '/^[ ]*$$/d' chainlint/$$i.expect; \ + done \ + } >'$(CHAINLINTTMP_SQ)'/expect && \ + $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests | \ + grep -v '^[ ]*$$' >'$(CHAINLINTTMP_SQ)'/actual && \ + if test -f ../GIT-BUILD-OPTIONS; then \ + . ../GIT-BUILD-OPTIONS; \ + fi && \ + if test -x ../git$$X; then \ + DIFFW="../git$$X --no-pager diff -w --no-index"; \ + else \ + DIFFW="diff -w -u"; \ + fi && \ + $$DIFFW '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \ test-lint-filenames diff --git a/t/chainlint/block.expect b/t/chainlint/block.expect index da60257ebc4..37dbf7d95fa 100644 --- a/t/chainlint/block.expect +++ b/t/chainlint/block.expect @@ -1,7 +1,7 @@ ( foo && { - echo a + echo a ?!AMP?! echo b } && bar && diff --git a/t/chainlint/here-doc-multi-line-string.expect b/t/chainlint/here-doc-multi-line-string.expect index 2578191ca8a..be64b26869a 100644 --- a/t/chainlint/here-doc-multi-line-string.expect +++ b/t/chainlint/here-doc-multi-line-string.expect @@ -1,4 +1,5 @@ ( - cat <<-TXT && echo "multi-line string" ?!AMP?! + cat <<-TXT && echo "multi-line + string" ?!AMP?! bap ) diff --git a/t/chainlint/multi-line-string.expect b/t/chainlint/multi-line-string.expect index ab0dadf748e..27ff95218e7 100644 --- a/t/chainlint/multi-line-string.expect +++ b/t/chainlint/multi-line-string.expect @@ -1,9 +1,14 @@ ( - x="line 1 line 2 line 3" && - y="line 1 line2" ?!AMP?! + x="line 1 + line 2 + line 3" && + y="line 1 + line2" ?!AMP?! foobar ) && ( - echo "xyz" "abc def ghi" && + echo "xyz" "abc + def + ghi" && barfoo ) diff --git a/t/chainlint/nested-subshell.expect b/t/chainlint/nested-subshell.expect index 41a48adaa2b..02e0a9f1bb5 100644 --- a/t/chainlint/nested-subshell.expect +++ b/t/chainlint/nested-subshell.expect @@ -6,7 +6,7 @@ ) >file && cd foo && ( - echo a + echo a ?!AMP?! echo b ) >file ) diff --git a/t/chainlint/t7900-subtree.expect b/t/chainlint/t7900-subtree.expect index 1cccc7bf7e1..69167da2f27 100644 --- a/t/chainlint/t7900-subtree.expect +++ b/t/chainlint/t7900-subtree.expect @@ -1,10 +1,17 @@ ( - chks="sub1sub2sub3sub4" && + chks="sub1 +sub2 +sub3 +sub4" && chks_sub=$(cat < X-Patchwork-Id: 12961589 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 273E2ECAAD4 for ; Thu, 1 Sep 2022 00:30:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232176AbiIAAal (ORCPT ); Wed, 31 Aug 2022 20:30:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47556 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232025AbiIAAaV (ORCPT ); Wed, 31 Aug 2022 20:30:21 -0400 Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F1BA67FE41 for ; Wed, 31 Aug 2022 17:30:11 -0700 (PDT) Received: by mail-wr1-x435.google.com with SMTP id k9so20313662wri.0 for ; Wed, 31 Aug 2022 17:30:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=oqUh3cjpNmKm4FAaZzlli3XUOmHrKMdNuCJ7vI/P/a4=; b=FF14YGpiaYO1Op6FiX7laXOMSGKfPFJB5s1P/rp16sbvgXai7J6RwQeLhEtOhHJSOs fLYCkeKh0dz80yRgyCNAXg6RhmlXhWTwePikwP6DPmRRyJtWu+1GXzqP8b3ZUqAHIkTF x9rJcW8+/29QNk50jRaLa0rEbt4n1SOML/Ua1nvbNNIBKv2+aJF3+9jQyuEkS7Bg0Xfd fyTNDWS5o/KQZVV/5T/iTP83nWyW6x1daZh6KiJUmnElUS5saRxjhu60TKY41FfzuKFS nlB9OzF4pFib8XAObFs0nkyhXCcHPoCyvAvhltGUlgltbsoyhUrHSpYfo0pQv+ARLJ1W 5JgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=oqUh3cjpNmKm4FAaZzlli3XUOmHrKMdNuCJ7vI/P/a4=; b=fkFQRnBMRJoo3nQNO7WbR8EC8LjAyOF6oKwn/xuOrXMhBP9MVvbXlulV1reOAgnWed TBeSREq05t49/y6c6ysAAHz1UtRVAMypaw7IM/LfkCbil2JUCdaXGiU1D1wBEYhBCQ2I vO+42MLb8xes0aOHme/g6aPhmLREDOInfKYpgXfdI7Py1ZznL0/ibta7y/6GLAkkALOz blJNp0IZrwv9m7sfzU49i/aMAb/3hD+eNHd/nxWIgkBZvjy+RgaoFN/LiI5bNKbMd7di spDcfVFgtdKTKtNsJIEc8lYci32I8YKk8893xWCIstciZevc177T3L3YEYa/ZOHQHNi4 M/DA== X-Gm-Message-State: ACgBeo1+7WeG8teIpYeRSCfnHMB68qeM/xpR+0YEkV1AikWJJFiCurXT ONQq9yVl/DKZ86+L9YkEbkbwdFgVgK4= X-Google-Smtp-Source: AA6agR5XdVlNksn+dhhQ+ZyULjpxXPOCsPzV4bTscZvcJLHwHDQwgWUfBJJ2pRPwuYV1ySX9NXklAA== X-Received: by 2002:a5d:6a01:0:b0:21a:338c:4862 with SMTP id m1-20020a5d6a01000000b0021a338c4862mr12561239wru.631.1661992209853; Wed, 31 Aug 2022 17:30:09 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id r6-20020a5d4946000000b00226d2462b32sm12290086wrs.52.2022.08.31.17.30.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:08 -0700 (PDT) Message-Id: <86a718bfa5585c826fc98f8844a52faabb45fa55.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:47 +0000 Subject: [PATCH 09/18] chainlint.pl: don't require `&` background command to end with `&&` Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine The exit status of the `&` asynchronous operator which starts a command in the background is unconditionally zero, and the few places in the test scripts which launch commands asynchronously are not interested in the exit status of the `&` operator (though they often capture the background command's PID). As such, there is little value in complaining about broken &&-chain for a command launched in the background, and doing so would only make busy-work for test authors. Therefore, take this special case into account when checking for &&-chain breakage. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 2 +- t/chainlint/chain-break-background.expect | 9 +++++++++ t/chainlint/chain-break-background.test | 10 ++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 t/chainlint/chain-break-background.expect create mode 100644 t/chainlint/chain-break-background.test diff --git a/t/chainlint.pl b/t/chainlint.pl index 31c444067ce..ba3fcb0c8e6 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -483,7 +483,7 @@ sub match_ending { } my @safe_endings = ( - [qr/^(?:&&|\|\||\|)$/], + [qr/^(?:&&|\|\||\||&)$/], [qr/^(?:exit|return)$/, qr/^(?:\d+|\$\?)$/], [qr/^(?:exit|return)$/, qr/^(?:\d+|\$\?)$/, qr/^;$/], [qr/^(?:exit|return|continue)$/], diff --git a/t/chainlint/chain-break-background.expect b/t/chainlint/chain-break-background.expect new file mode 100644 index 00000000000..28f9114f42d --- /dev/null +++ b/t/chainlint/chain-break-background.expect @@ -0,0 +1,9 @@ +JGIT_DAEMON_PID= && +git init --bare empty.git && +> empty.git/git-daemon-export-ok && +mkfifo jgit_daemon_output && +{ + jgit daemon --port="$JGIT_DAEMON_PORT" . > jgit_daemon_output & + JGIT_DAEMON_PID=$! +} && +test_expect_code 2 git ls-remote --exit-code git://localhost:$JGIT_DAEMON_PORT/empty.git diff --git a/t/chainlint/chain-break-background.test b/t/chainlint/chain-break-background.test new file mode 100644 index 00000000000..e10f656b055 --- /dev/null +++ b/t/chainlint/chain-break-background.test @@ -0,0 +1,10 @@ +JGIT_DAEMON_PID= && +git init --bare empty.git && +>empty.git/git-daemon-export-ok && +mkfifo jgit_daemon_output && +{ +# LINT: exit status of "&" is always 0 so &&-chaining immaterial + jgit daemon --port="$JGIT_DAEMON_PORT" . >jgit_daemon_output & + JGIT_DAEMON_PID=$! +} && +test_expect_code 2 git ls-remote --exit-code git://localhost:$JGIT_DAEMON_PORT/empty.git From patchwork Thu Sep 1 00:29:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961587 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 419C8ECAAD1 for ; Thu, 1 Sep 2022 00:30:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232156AbiIAAaf (ORCPT ); Wed, 31 Aug 2022 20:30:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47548 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232007AbiIAAaV (ORCPT ); Wed, 31 Aug 2022 20:30:21 -0400 Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7516C491CC for ; Wed, 31 Aug 2022 17:30:11 -0700 (PDT) Received: by mail-wr1-x429.google.com with SMTP id b5so20260634wrr.5 for ; Wed, 31 Aug 2022 17:30:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=Lkq7/33y5N60KZxpCX9WI6SuWmoIYHLJZakile9msBA=; b=b9NkuUwOjshd3Ygla3KOBZwim47Pm0TfyuvvPogyoZkmoMMm4d/DrtTGDjetrthjYu 2m3NbSE98n/L9ZuTayltcnb47nacmMUnwwlNpULuVVtZzXlJp7lJmoHtRh+sV3DehlOa ZSSfMSKbIwGqyiJOYJcvtrdOPZ7r2BZ1socdssNQsou0XdymHDGOGSTNDEzAtKxaHDQ0 fLpTuxZ9BbslUFm3XxUkl0ti9RduW74OqyZ9VUyjm4Amq6Yc8KLsuLZXB2dHCINyjn6F 1+dhtnqdCJAc5ASd1KoBZf2suuWh4zcXMPAQJmvfKPtOtfhluhXYhWaHrVfqbs6TKvxZ VZyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=Lkq7/33y5N60KZxpCX9WI6SuWmoIYHLJZakile9msBA=; b=RKbffJnIZdfn+KZXjgBzFai18iGvJ6i8hlCLGOpakeiD9TXIPClUJPkHiDLdi7ns6v BemOu1nCpTEYowk4en0zFSQTKxLlZj6NCSnn1JXZ5dALUritQ17+cO/lIpfS6W1FG2MA TBjVnnlijEMBnkhhfzZk3R1MGDM+3M0AWjxQ6rmtTxanMVAHSh69s5YKUh31uiERiRsG ieTTU+B+0oyZTi9EGatd20ZryEGAnki6nXkvibrJNqNPR9fHZ050GPR9OYpJBPn0m2Op CqIsQ5oYiDm67wsaMCsoro8rdN6n1Qv3L8jaOHDZbXOHfZ1Jec6cEUM+6/UnjEg8XFP4 JUyw== X-Gm-Message-State: ACgBeo1YAP8D4il0YKp/q69d7YwYNdlD0UIS4lHPPwwC8gwLfIYZPdEX RhbC1qTBkmw7O0WljdvkUoVdH+dHNZM= X-Google-Smtp-Source: AA6agR5LS4yF4X+/2vSGwe/ZAiFVZsnJssDhiYPlfYSkvg5cVfyx2wQXmlkBD0yrgcl47AUIVdltsA== X-Received: by 2002:a05:6000:1a42:b0:225:8b5e:e0f8 with SMTP id t2-20020a0560001a4200b002258b5ee0f8mr13909881wry.710.1661992210853; Wed, 31 Aug 2022 17:30:10 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id e20-20020a05600c4e5400b003a5bd5ea215sm3497419wmq.37.2022.08.31.17.30.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:10 -0700 (PDT) Message-Id: <7df396ddea4cdaf9d014bb90a38da010676c1ce8.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:48 +0000 Subject: [PATCH 10/18] chainlint.pl: don't flag broken &&-chain if `$?` handled explicitly Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine There are cases in which tests capture and check a command's exit code explicitly without employing test_expect_code(). They do so by intentionally breaking the &&-chain since it would be impossible to capture "$?" in the failing case if the `status=$?` assignment was part of the &&-chain. Since such constructs are manually checking the exit code, their &&-chain breakage is legitimate and safe, thus should not be flagged. Therefore, stop flagging &&-chain breakage in such cases. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 6 ++++++ t/chainlint/chain-break-status.expect | 9 +++++++++ t/chainlint/chain-break-status.test | 11 +++++++++++ 3 files changed, 26 insertions(+) create mode 100644 t/chainlint/chain-break-status.expect create mode 100644 t/chainlint/chain-break-status.test diff --git a/t/chainlint.pl b/t/chainlint.pl index ba3fcb0c8e6..14e1db3519a 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -497,6 +497,12 @@ sub accumulate { # did previous command end with "&&", "|", "|| return" or similar? goto DONE if match_ending($tokens, \@safe_endings); + # if this command handles "$?" specially, then okay for previous + # command to be missing "&&" + for my $token (@$cmd) { + goto DONE if $token =~ /\$\?/; + } + # flag missing "&&" at end of previous command my $n = find_non_nl($tokens); splice(@$tokens, $n + 1, 0, '?!AMP?!') unless $n < 0; diff --git a/t/chainlint/chain-break-status.expect b/t/chainlint/chain-break-status.expect new file mode 100644 index 00000000000..f4bada94632 --- /dev/null +++ b/t/chainlint/chain-break-status.expect @@ -0,0 +1,9 @@ +OUT=$(( ( large_git ; echo $? 1 >& 3 ) | : ) 3 >& 1) && +test_match_signal 13 "$OUT" && + +{ test-tool sigchain > actual ; ret=$? ; } && +{ + test_match_signal 15 "$ret" || + test "$ret" = 3 +} && +test_cmp expect actual diff --git a/t/chainlint/chain-break-status.test b/t/chainlint/chain-break-status.test new file mode 100644 index 00000000000..a6602a7b99c --- /dev/null +++ b/t/chainlint/chain-break-status.test @@ -0,0 +1,11 @@ +# LINT: broken &&-chain okay if next command handles "$?" explicitly +OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) && +test_match_signal 13 "$OUT" && + +# LINT: broken &&-chain okay if next command handles "$?" explicitly +{ test-tool sigchain >actual; ret=$?; } && +{ + test_match_signal 15 "$ret" || + test "$ret" = 3 +} && +test_cmp expect actual From patchwork Thu Sep 1 00:29:49 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961591 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 19A7EECAAD5 for ; Thu, 1 Sep 2022 00:30:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232185AbiIAAao (ORCPT ); Wed, 31 Aug 2022 20:30:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47574 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232052AbiIAAaV (ORCPT ); Wed, 31 Aug 2022 20:30:21 -0400 Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AE6898D3D5 for ; Wed, 31 Aug 2022 17:30:13 -0700 (PDT) Received: by mail-wm1-x330.google.com with SMTP id n23-20020a7bc5d7000000b003a62f19b453so446809wmk.3 for ; Wed, 31 Aug 2022 17:30:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=ylS7PjWkZoP2NA4NykpbUEsjOmimC7ie5JIkYd4cs3s=; b=BXXsESIQN80j84TNjAEQDbRGbdSMXEA6+tQ4wbtZHxjD4IytQifJXO3Fw44KwZOkwE KbYJRGf7CsNvaN+w+ngbSu4DY4+ZpRL0IbIoB6ZHa7G7SDonP1YE6o0iuBSN8zFjquwF zrAfCncHn2IUts1O/2OAr8uSQAdLebkOCJLJqgEEL8CLjLwJlCW/M2biFQQxA0oQkdZZ 6RXp+is9eaAqmW5ZQ+DggP3BpZiPVtF3sLPr5L+J9RVb1EtIPjzSycnE0r7XR/WN8D1b Z0Z3ay+okoNtMZSvlnQ1suxb1sRABBi7cms7gWLUUCBDj4uSz+dfU+6KCy9J9pcm1r3w vnHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=ylS7PjWkZoP2NA4NykpbUEsjOmimC7ie5JIkYd4cs3s=; b=QxHkU+wm1GugI3EBehSyHU4xv0e5KlEWj4JS1wcT41tg78+UcPNLlEb0s69ITcyEPb DTEpA/z4OwBq4jsGS/J1tTKan3BMmbOY6o95IgydimVwjXwBnjwK31H7HCiZTGKn622n yzkputzgTUEQm2eJhBrtC7miWayjXAA0rmLidmEMoj5O1TKKksUG8r0RVElNBYxVrlNx A7QA+nTl775puFiFQ6NRKGi4+pqI8lD0pzZWcSHySsRBee7bcET+wX5SeySAhiJCoUDG cAtIMNXKZre0CCsIvjt4tyX0nhixi3Q2CRjBAg5UuhBPtYs1Cq5aDONOqnMUK0zcLbzs J5AQ== X-Gm-Message-State: ACgBeo3YqpHtBLROpgIjXQNZvz4P7Z3Y/kUM++wFbQtcsJa4+YPBhhO0 5EucaVuH220VG/B+Ymb/7id2JoQ53yA= X-Google-Smtp-Source: AA6agR5rILCPW4jyHXtztyEfzqqGEgWg44ge/xm9IFur6NS7K0RfwmZtiC+oKJO8xl52A5dXjhS8Rw== X-Received: by 2002:a7b:c844:0:b0:3a9:70d2:bf23 with SMTP id c4-20020a7bc844000000b003a970d2bf23mr392526wml.165.1661992211994; Wed, 31 Aug 2022 17:30:11 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id az19-20020a05600c601300b003a342933727sm3749201wmb.3.2022.08.31.17.30.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:11 -0700 (PDT) Message-Id: <11bd449766dd6854466b214cf0d144feb01d4fd2.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:49 +0000 Subject: [PATCH 11/18] chainlint.pl: don't flag broken &&-chain if failure indicated explicitly Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine There are quite a few tests which print an error messages and then explicitly signal failure with `false`, `return 1`, or `exit 1` as the final command in an `if` branch. In these cases, the tests don't bother maintaining the &&-chain between `echo` and the explicit "test failed" indicator. Since such constructs are manually signaling failure, their &&-chain breakage is legitimate and safe -- both for the command immediately preceding `false`, `return`, or `exit`, as well as for all preceding commands in the `if` branch. Therefore, stop flagging &&-chain breakage in these sorts of cases. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 8 ++++++++ t/chainlint/chain-break-false.expect | 9 +++++++++ t/chainlint/chain-break-false.test | 10 ++++++++++ t/chainlint/chain-break-return-exit.expect | 15 +++++++++++++++ t/chainlint/chain-break-return-exit.test | 18 ++++++++++++++++++ t/chainlint/if-in-loop.expect | 2 +- t/chainlint/if-in-loop.test | 2 +- 7 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 t/chainlint/chain-break-false.expect create mode 100644 t/chainlint/chain-break-false.test diff --git a/t/chainlint.pl b/t/chainlint.pl index 14e1db3519a..a76a09ecf5e 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -503,6 +503,14 @@ sub accumulate { goto DONE if $token =~ /\$\?/; } + # if this command is "false", "return 1", or "exit 1" (which signal + # failure explicitly), then okay for all preceding commands to be + # missing "&&" + if ($$cmd[0] =~ /^(?:false|return|exit)$/) { + @$tokens = grep(!/^\?!AMP\?!$/, @$tokens); + goto DONE; + } + # flag missing "&&" at end of previous command my $n = find_non_nl($tokens); splice(@$tokens, $n + 1, 0, '?!AMP?!') unless $n < 0; diff --git a/t/chainlint/chain-break-false.expect b/t/chainlint/chain-break-false.expect new file mode 100644 index 00000000000..989766fb856 --- /dev/null +++ b/t/chainlint/chain-break-false.expect @@ -0,0 +1,9 @@ +if condition not satisified +then + echo it did not work... + echo failed! + false +else + echo it went okay ?!AMP?! + congratulate user +fi diff --git a/t/chainlint/chain-break-false.test b/t/chainlint/chain-break-false.test new file mode 100644 index 00000000000..a5aaff8c8a4 --- /dev/null +++ b/t/chainlint/chain-break-false.test @@ -0,0 +1,10 @@ +# LINT: broken &&-chain okay if explicit "false" signals failure +if condition not satisified +then + echo it did not work... + echo failed! + false +else + echo it went okay + congratulate user +fi diff --git a/t/chainlint/chain-break-return-exit.expect b/t/chainlint/chain-break-return-exit.expect index dba292ee89b..1732d221c32 100644 --- a/t/chainlint/chain-break-return-exit.expect +++ b/t/chainlint/chain-break-return-exit.expect @@ -1,3 +1,18 @@ +case "$(git ls-files)" in +one ) echo pass one ;; +* ) echo bad one ; return 1 ;; +esac && +( + case "$(git ls-files)" in + two ) echo pass two ;; + * ) echo bad two ; exit 1 ;; +esac +) && +case "$(git ls-files)" in +dir/two"$LF"one ) echo pass both ;; +* ) echo bad ; return 1 ;; +esac && + for i in 1 2 3 4 ; do git checkout main -b $i || return $? test_commit $i $i $i tag$i || return $? diff --git a/t/chainlint/chain-break-return-exit.test b/t/chainlint/chain-break-return-exit.test index e2b059933aa..46542edf881 100644 --- a/t/chainlint/chain-break-return-exit.test +++ b/t/chainlint/chain-break-return-exit.test @@ -1,3 +1,21 @@ +case "$(git ls-files)" in +one) echo pass one ;; +# LINT: broken &&-chain okay if explicit "return 1" signals failuire +*) echo bad one; return 1 ;; +esac && +( + case "$(git ls-files)" in + two) echo pass two ;; +# LINT: broken &&-chain okay if explicit "exit 1" signals failuire + *) echo bad two; exit 1 ;; + esac +) && +case "$(git ls-files)" in +dir/two"$LF"one) echo pass both ;; +# LINT: broken &&-chain okay if explicit "return 1" signals failuire +*) echo bad; return 1 ;; +esac && + for i in 1 2 3 4 ; do # LINT: broken &&-chain okay if explicit "return $?" signals failure git checkout main -b $i || return $? diff --git a/t/chainlint/if-in-loop.expect b/t/chainlint/if-in-loop.expect index 03b82a3e58c..d6514ae7492 100644 --- a/t/chainlint/if-in-loop.expect +++ b/t/chainlint/if-in-loop.expect @@ -3,7 +3,7 @@ do if false then - echo "err" ?!AMP?! + echo "err" exit 1 fi ?!AMP?! foo diff --git a/t/chainlint/if-in-loop.test b/t/chainlint/if-in-loop.test index f0cf19cfada..90c23976fec 100644 --- a/t/chainlint/if-in-loop.test +++ b/t/chainlint/if-in-loop.test @@ -3,7 +3,7 @@ do if false then -# LINT: missing "&&" on "echo" +# LINT: missing "&&" on "echo" okay since "exit 1" signals error explicitly echo "err" exit 1 # LINT: missing "&&" on "fi" From patchwork Thu Sep 1 00:29:50 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961588 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5113AECAAD4 for ; Thu, 1 Sep 2022 00:30:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232170AbiIAAai (ORCPT ); Wed, 31 Aug 2022 20:30:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47554 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232008AbiIAAaV (ORCPT ); Wed, 31 Aug 2022 20:30:21 -0400 Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9BEC58B2C7 for ; Wed, 31 Aug 2022 17:30:13 -0700 (PDT) Received: by mail-wm1-x331.google.com with SMTP id i188-20020a1c3bc5000000b003a7b6ae4eb2so443786wma.4 for ; Wed, 31 Aug 2022 17:30:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=6vHtuz5wCziYtfU5pjHHqiPxIhRGBAGTH4/xxkFWUSg=; b=DN2xPkAAmEJB8b4zul7SmoxyGgj31fjBK2Z2ZaMEhWU+jSyPzbJWIl5SQTDpcHkLnB RbnP7gMetYTtR/kAMHN3Py4hLLXBWsasYI80Nqy6siVs8F9I2Rnxoxl2AsyqG+2J9iUg VL0P6M/iZPKp+9sMjB+VRobOPxEaFdR1isXQpntzY3WMMbEYqqSLBohvTKQAsaxh7UbI rrm9h89WgfPSZEy+8MWE9BQfN+7pZA962FvEdedIn6ApmYEglScOfy5+Ksr+ZbIn++Rm RQWDzXFmyNUdoXNywvMzmxmXqCo1+xTxdwVNth5jQCOeLozhrputo522eE2AnbKnKnIG Espw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=6vHtuz5wCziYtfU5pjHHqiPxIhRGBAGTH4/xxkFWUSg=; b=zNWjE1M/+ZX1jdLwEtHUqtKBaLeaGWl6jm0WQagIwMasyLfOg3UqcI/LBgBw/zmh7A 8LX94wDezhS8wvdsvgHQKZDF7Bpkku/RiDFVSqSYufHsJDEfXVxk0z5uLZ1h57Ecth1H 7/HaVpZjgZIekEafV5+xHl0KrXVwkbyjJan5S+FrBp+Yuz5C6VzlHet+Ovg/oUEe8mpa QFbf+m1bVy+n7J0jdRjFD1+0gPOVjZutq+Llhs2Z/dgvsYEqEW7pdLNAryiuNA/x4odg rpiqw1lTw4Ig+7dwPrnYbP2T+Gc5omqyKOW+ESGSn+VRYdhQAHnQmWSGeAlPM+aj6O40 FZzA== X-Gm-Message-State: ACgBeo01rKPVUj+biYWI4yIfJjCN4BMMCgT7H2yJdx9PUekIESgcv3bs it8v8MDGF8jtFsT4vomvWs9ibqLC+lY= X-Google-Smtp-Source: AA6agR6cfDlRqCSIOgco1gEcPYqdzgL14w8A6Cg+QBN0DWiDgNi7XV1T36gINMuJLOWUySKm5HJW9w== X-Received: by 2002:a05:600c:1e8d:b0:3a5:e37f:6fd2 with SMTP id be13-20020a05600c1e8d00b003a5e37f6fd2mr3414343wmb.33.1661992212914; Wed, 31 Aug 2022 17:30:12 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id az19-20020a05600c601300b003a342933727sm3749243wmb.3.2022.08.31.17.30.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:12 -0700 (PDT) Message-Id: <1049172aaca8e13f64201dc0ebf04ba5c655c0ce.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:50 +0000 Subject: [PATCH 12/18] chainlint.pl: complain about loops lacking explicit failure handling Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine Shell `for` and `while` loops do not terminate automatically just because a command fails within the loop body. Instead, the loop continues to iterate and eventually returns the exit status of the final command of the final iteration, which may not be the command which failed, thus it is possible for failures to go undetected. Consequently, it is important for test authors to explicitly handle failure within the loop body by terminating the loop manually upon failure. This can be done by returning a non-zero exit code from within the loop body (i.e. `|| return 1`) or exiting (i.e. `|| exit 1`) if the loop is within a subshell, or by manually checking `$?` and taking some appropriate action. Therefore, add logic to detect and complain about loops which lack explicit `return` or `exit`, or `$?` check. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 11 ++++++ t/chainlint/complex-if-in-cuddled-loop.expect | 2 +- t/chainlint/for-loop.expect | 4 +-- t/chainlint/loop-detect-failure.expect | 15 ++++++++ t/chainlint/loop-detect-failure.test | 17 +++++++++ t/chainlint/loop-detect-status.expect | 18 ++++++++++ t/chainlint/loop-detect-status.test | 19 ++++++++++ t/chainlint/loop-in-if.expect | 2 +- t/chainlint/nested-loop-detect-failure.expect | 31 ++++++++++++++++ t/chainlint/nested-loop-detect-failure.test | 35 +++++++++++++++++++ t/chainlint/semicolon.expect | 2 +- t/chainlint/while-loop.expect | 4 +-- 12 files changed, 153 insertions(+), 7 deletions(-) create mode 100644 t/chainlint/loop-detect-failure.expect create mode 100644 t/chainlint/loop-detect-failure.test create mode 100644 t/chainlint/loop-detect-status.expect create mode 100644 t/chainlint/loop-detect-status.test create mode 100644 t/chainlint/nested-loop-detect-failure.expect create mode 100644 t/chainlint/nested-loop-detect-failure.test diff --git a/t/chainlint.pl b/t/chainlint.pl index a76a09ecf5e..674b3ddf696 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -482,6 +482,17 @@ sub match_ending { return undef; } +sub parse_loop_body { + my $self = shift @_; + my @tokens = $self->SUPER::parse_loop_body(@_); + # did loop signal failure via "|| return" or "|| exit"? + return @tokens if !@tokens || grep(/^(?:return|exit|\$\?)$/, @tokens); + # flag missing "return/exit" handling explicit failure in loop body + my $n = find_non_nl(\@tokens); + splice(@tokens, $n + 1, 0, '?!LOOP?!'); + return @tokens; +} + my @safe_endings = ( [qr/^(?:&&|\|\||\||&)$/], [qr/^(?:exit|return)$/, qr/^(?:\d+|\$\?)$/], diff --git a/t/chainlint/complex-if-in-cuddled-loop.expect b/t/chainlint/complex-if-in-cuddled-loop.expect index 2fca1834095..dac2d0fd1d9 100644 --- a/t/chainlint/complex-if-in-cuddled-loop.expect +++ b/t/chainlint/complex-if-in-cuddled-loop.expect @@ -4,6 +4,6 @@ : else echo >file - fi + fi ?!LOOP?! done) && test ! -f file diff --git a/t/chainlint/for-loop.expect b/t/chainlint/for-loop.expect index 6671b8cd842..a5810c9bddd 100644 --- a/t/chainlint/for-loop.expect +++ b/t/chainlint/for-loop.expect @@ -2,10 +2,10 @@ for i in a b c do echo $i ?!AMP?! - cat <<-EOF + cat <<-EOF ?!LOOP?! done ?!AMP?! for i in a b c; do echo $i && - cat $i + cat $i ?!LOOP?! done ) diff --git a/t/chainlint/loop-detect-failure.expect b/t/chainlint/loop-detect-failure.expect new file mode 100644 index 00000000000..a66025c39d4 --- /dev/null +++ b/t/chainlint/loop-detect-failure.expect @@ -0,0 +1,15 @@ +git init r1 && +for n in 1 2 3 4 5 +do + echo "This is file: $n" > r1/file.$n && + git -C r1 add file.$n && + git -C r1 commit -m "$n" || return 1 +done && + +git init r2 && +for n in 1000 10000 +do + printf "%"$n"s" X > r2/large.$n && + git -C r2 add large.$n && + git -C r2 commit -m "$n" ?!LOOP?! +done diff --git a/t/chainlint/loop-detect-failure.test b/t/chainlint/loop-detect-failure.test new file mode 100644 index 00000000000..b9791cc802e --- /dev/null +++ b/t/chainlint/loop-detect-failure.test @@ -0,0 +1,17 @@ +git init r1 && +# LINT: loop handles failure explicitly with "|| return 1" +for n in 1 2 3 4 5 +do + echo "This is file: $n" > r1/file.$n && + git -C r1 add file.$n && + git -C r1 commit -m "$n" || return 1 +done && + +git init r2 && +# LINT: loop fails to handle failure explicitly with "|| return 1" +for n in 1000 10000 +do + printf "%"$n"s" X > r2/large.$n && + git -C r2 add large.$n && + git -C r2 commit -m "$n" +done diff --git a/t/chainlint/loop-detect-status.expect b/t/chainlint/loop-detect-status.expect new file mode 100644 index 00000000000..0ad23bb35e4 --- /dev/null +++ b/t/chainlint/loop-detect-status.expect @@ -0,0 +1,18 @@ +( while test $i -le $blobcount +do + printf "Generating blob $i/$blobcount\r" >& 2 && + printf "blob\nmark :$i\ndata $blobsize\n" && + + printf "%-${blobsize}s" $i && + echo "M 100644 :$i $i" >> commit && + i=$(($i+1)) || + echo $? > exit-status +done && +echo "commit refs/heads/main" && +echo "author A U Thor 123456789 +0000" && +echo "committer C O Mitter 123456789 +0000" && +echo "data 5" && +echo ">2gb" && +cat commit ) | +git fast-import --big-file-threshold=2 && +test ! -f exit-status diff --git a/t/chainlint/loop-detect-status.test b/t/chainlint/loop-detect-status.test new file mode 100644 index 00000000000..1c6c23cfc9e --- /dev/null +++ b/t/chainlint/loop-detect-status.test @@ -0,0 +1,19 @@ +# LINT: "$?" handled explicitly within loop body +(while test $i -le $blobcount + do + printf "Generating blob $i/$blobcount\r" >&2 && + printf "blob\nmark :$i\ndata $blobsize\n" && + #test-tool genrandom $i $blobsize && + printf "%-${blobsize}s" $i && + echo "M 100644 :$i $i" >> commit && + i=$(($i+1)) || + echo $? > exit-status + done && + echo "commit refs/heads/main" && + echo "author A U Thor 123456789 +0000" && + echo "committer C O Mitter 123456789 +0000" && + echo "data 5" && + echo ">2gb" && + cat commit) | +git fast-import --big-file-threshold=2 && +test ! -f exit-status diff --git a/t/chainlint/loop-in-if.expect b/t/chainlint/loop-in-if.expect index e1be42376c5..6c5d6e5b243 100644 --- a/t/chainlint/loop-in-if.expect +++ b/t/chainlint/loop-in-if.expect @@ -4,7 +4,7 @@ while true do echo "pop" ?!AMP?! - echo "glup" + echo "glup" ?!LOOP?! done ?!AMP?! foo fi ?!AMP?! diff --git a/t/chainlint/nested-loop-detect-failure.expect b/t/chainlint/nested-loop-detect-failure.expect new file mode 100644 index 00000000000..4793a0e8e12 --- /dev/null +++ b/t/chainlint/nested-loop-detect-failure.expect @@ -0,0 +1,31 @@ +for i in 0 1 2 3 4 5 6 7 8 9 ; +do + for j in 0 1 2 3 4 5 6 7 8 9 ; + do + echo "$i$j" > "path$i$j" ?!LOOP?! + done ?!LOOP?! +done && + +for i in 0 1 2 3 4 5 6 7 8 9 ; +do + for j in 0 1 2 3 4 5 6 7 8 9 ; + do + echo "$i$j" > "path$i$j" || return 1 + done +done && + +for i in 0 1 2 3 4 5 6 7 8 9 ; +do + for j in 0 1 2 3 4 5 6 7 8 9 ; + do + echo "$i$j" > "path$i$j" ?!LOOP?! + done || return 1 +done && + +for i in 0 1 2 3 4 5 6 7 8 9 ; +do + for j in 0 1 2 3 4 5 6 7 8 9 ; + do + echo "$i$j" > "path$i$j" || return 1 + done || return 1 +done diff --git a/t/chainlint/nested-loop-detect-failure.test b/t/chainlint/nested-loop-detect-failure.test new file mode 100644 index 00000000000..e6f0c1acfb8 --- /dev/null +++ b/t/chainlint/nested-loop-detect-failure.test @@ -0,0 +1,35 @@ +# LINT: neither loop handles failure explicitly with "|| return 1" +for i in 0 1 2 3 4 5 6 7 8 9; +do + for j in 0 1 2 3 4 5 6 7 8 9; + do + echo "$i$j" >"path$i$j" + done +done && + +# LINT: inner loop handles failure explicitly with "|| return 1" +for i in 0 1 2 3 4 5 6 7 8 9; +do + for j in 0 1 2 3 4 5 6 7 8 9; + do + echo "$i$j" >"path$i$j" || return 1 + done +done && + +# LINT: outer loop handles failure explicitly with "|| return 1" +for i in 0 1 2 3 4 5 6 7 8 9; +do + for j in 0 1 2 3 4 5 6 7 8 9; + do + echo "$i$j" >"path$i$j" + done || return 1 +done && + +# LINT: inner & outer loops handles failure explicitly with "|| return 1" +for i in 0 1 2 3 4 5 6 7 8 9; +do + for j in 0 1 2 3 4 5 6 7 8 9; + do + echo "$i$j" >"path$i$j" || return 1 + done || return 1 +done diff --git a/t/chainlint/semicolon.expect b/t/chainlint/semicolon.expect index ed0b3707ae9..3aa2259f36c 100644 --- a/t/chainlint/semicolon.expect +++ b/t/chainlint/semicolon.expect @@ -15,5 +15,5 @@ ) && (cd foo && for i in a b c; do - echo; + echo; ?!LOOP?! done) diff --git a/t/chainlint/while-loop.expect b/t/chainlint/while-loop.expect index 0d3a9b3d128..f272aa21fee 100644 --- a/t/chainlint/while-loop.expect +++ b/t/chainlint/while-loop.expect @@ -2,10 +2,10 @@ while true do echo foo ?!AMP?! - cat <<-EOF + cat <<-EOF ?!LOOP?! done ?!AMP?! while true; do echo foo && - cat bar + cat bar ?!LOOP?! done ) From patchwork Thu Sep 1 00:29:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961592 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8AEFDECAAD1 for ; Thu, 1 Sep 2022 00:30:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232194AbiIAAap (ORCPT ); Wed, 31 Aug 2022 20:30:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47494 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231874AbiIAAaW (ORCPT ); Wed, 31 Aug 2022 20:30:22 -0400 Received: from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com [IPv6:2a00:1450:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B25193F33C for ; Wed, 31 Aug 2022 17:30:15 -0700 (PDT) Received: by mail-wr1-x42b.google.com with SMTP id v16so17540049wrm.8 for ; Wed, 31 Aug 2022 17:30:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=h5RsgfjcFiVXikZi8QggWlCtjp9Xr85P6jHdbE2KQtY=; b=po7ShVkiU2PeAYC2wBpKf1ayaAp0/x2qjNwUGvMxXHozubBj0IYwhGa8BvAmpY09Bw rXByU3ArIXzuHZzF1QI3/W11NIvC3Jd7seOvzOVj0v8eb5iV8tDqbq54KBDRXlOgeVFb C2sykul4tcQSbzKJzrQcTSWkeKWR52bUbaw5Eb3EnBBBqjfVpnOowqG2EeB5ldnST3aJ EA3YipfgZsiHay0WSYgsfv7ipMTHZXRCINrnGkQ2oLbQ4FnzY+W1r2rFwEFMXYj/V6K7 OK/A5M5esLLWBklPp+iUXNVOWF8Upg5Cda2gfUVw/Gz4N3V9tofqnSji9J2oQMNLiT7+ 0sew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=h5RsgfjcFiVXikZi8QggWlCtjp9Xr85P6jHdbE2KQtY=; b=gkYNQUVai5mnl3PtgJCsO4F238EdqGGGScz81pPP8pbIakOGpoO1kvqwxMbGMDKAID LSN3tI3prfn/PMZ0ZI8ie6azIFNwuwzUhUEAbw5haIMMsVKci3hA1Sm1GtjjcfVu+Hvp N09cxAegdlaQhypKsZF++E22gETpn0T7WU7v5qchOPJuU0y6S8AJCTIepeMcJaVku4Nm L+bOGteET5cZr7AI0k5KB/uYxlgMiYbSU0WRpKBK00hZHYBTFX+mVU34za125ZFJg6J/ rMIAd2LwWHBtuSSytigmpDlxh4ez8XepEdYTRZv1uYxVxDN5b47l2Hpjtk5d0Gy/l4VD lclg== X-Gm-Message-State: ACgBeo2My5hsizJm8BaOznkL41Egft15+Y3BaF8VzHPErbzDZV4rMBfC MQl2H6sgyQDGBGy2kSqnYjgIlWVpGFY= X-Google-Smtp-Source: AA6agR4FPTUAA01NgmiP+JgVXJL+bdzJew8Hf/gs9fyeDsRrcCjj2hUk9STX274F7XlTEDf1HvI1sA== X-Received: by 2002:a5d:64a4:0:b0:226:df78:5db7 with SMTP id m4-20020a5d64a4000000b00226df785db7mr8160654wrp.591.1661992214092; Wed, 31 Aug 2022 17:30:14 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id f28-20020a5d58fc000000b0021e4829d359sm12756109wrd.39.2022.08.31.17.30.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:13 -0700 (PDT) Message-Id: <7179972013825de50bddb6a68a17f0b83d5f35a8.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:51 +0000 Subject: [PATCH 13/18] chainlint.pl: allow `|| echo` to signal failure upstream of a pipe Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine The use of `|| return` (or `|| exit`) to signal failure within a loop isn't effective when the loop is upstream of a pipe since the pipe swallows all upstream exit codes and returns only the exit code of the final command in the pipeline. To work around this limitation, tests may adopt an alternative strategy of signaling failure by emitting text which would never be emitted in the non-failing case. For instance: while condition do command1 && command2 || echo "impossible text" done | sort >actual && Such usage indicates deliberate thought about failure cases by the test author, thus flagging them as missing `|| return` (or `|| exit`) is not helpful. Therefore, take this case into consideration when checking for explicit loop termination. Signed-off-by: Eric Sunshine --- t/chainlint.pl | 3 +++ t/chainlint/loop-upstream-pipe.expect | 10 ++++++++++ t/chainlint/loop-upstream-pipe.test | 11 +++++++++++ 3 files changed, 24 insertions(+) create mode 100644 t/chainlint/loop-upstream-pipe.expect create mode 100644 t/chainlint/loop-upstream-pipe.test diff --git a/t/chainlint.pl b/t/chainlint.pl index 674b3ddf696..386999ce65d 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -487,6 +487,9 @@ sub parse_loop_body { my @tokens = $self->SUPER::parse_loop_body(@_); # did loop signal failure via "|| return" or "|| exit"? return @tokens if !@tokens || grep(/^(?:return|exit|\$\?)$/, @tokens); + # did loop upstream of a pipe signal failure via "|| echo 'impossible + # text'" as the final command in the loop body? + return @tokens if ends_with(\@tokens, [qr/^\|\|$/, "\n", qr/^echo$/, qr/^.+$/]); # flag missing "return/exit" handling explicit failure in loop body my $n = find_non_nl(\@tokens); splice(@tokens, $n + 1, 0, '?!LOOP?!'); diff --git a/t/chainlint/loop-upstream-pipe.expect b/t/chainlint/loop-upstream-pipe.expect new file mode 100644 index 00000000000..0b82ecc4b96 --- /dev/null +++ b/t/chainlint/loop-upstream-pipe.expect @@ -0,0 +1,10 @@ +( + git rev-list --objects --no-object-names base..loose | + while read oid + do + path="$objdir/$(test_oid_to_path "$oid")" && + printf "%s %d\n" "$oid" "$(test-tool chmtime --get "$path")" || + echo "object list generation failed for $oid" + done | + sort -k1 +) >expect && diff --git a/t/chainlint/loop-upstream-pipe.test b/t/chainlint/loop-upstream-pipe.test new file mode 100644 index 00000000000..efb77da897c --- /dev/null +++ b/t/chainlint/loop-upstream-pipe.test @@ -0,0 +1,11 @@ +( + git rev-list --objects --no-object-names base..loose | + while read oid + do +# LINT: "|| echo" signals failure in loop upstream of a pipe + path="$objdir/$(test_oid_to_path "$oid")" && + printf "%s %d\n" "$oid" "$(test-tool chmtime --get "$path")" || + echo "object list generation failed for $oid" + done | + sort -k1 +) >expect && From patchwork Thu Sep 1 00:29:52 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961593 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DE0ABECAAD4 for ; Thu, 1 Sep 2022 00:30:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232201AbiIAAar (ORCPT ); Wed, 31 Aug 2022 20:30:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47606 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232075AbiIAAaW (ORCPT ); Wed, 31 Aug 2022 20:30:22 -0400 Received: from mail-wm1-x333.google.com (mail-wm1-x333.google.com [IPv6:2a00:1450:4864:20::333]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 04F568E9AF for ; Wed, 31 Aug 2022 17:30:17 -0700 (PDT) Received: by mail-wm1-x333.google.com with SMTP id r204-20020a1c44d5000000b003a84b160addso459446wma.2 for ; Wed, 31 Aug 2022 17:30:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=8nXOFB8iiBeRF/i/hK7RJVYSnBTXC+aXlO0UIqySsxM=; b=ZZUJaREAwrji8ocTDI6Ih0KUCSEnB129yiAoSXpTjBqUX+MKGnb3MeuRCzS6TxVyZp MsnCemIBAxAEPGo9KRkvim6reDNL8PNaNbPfSRP/iQLrfSdt0snKNzzA2QYJCoJ2dvFA xLp2w5UmtR7zdSjTOcGiZSpJOc1MLnbF4QT3QEflFz0ZvJxF4PTSUqo3zow1g01crXF9 gX9zlL+0+S8WWa/flRL+i8GcQfrPshSxXvqO2RYePkgH2FO2IZX7HU1NsONoAGxzkCbb 7LNn1BmAtqa7ihbCVt5/Q2vwSg/f6UjojKCOC4LjdPrr3bip/vxt4iOaUcYhTzKai5z3 qy5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=8nXOFB8iiBeRF/i/hK7RJVYSnBTXC+aXlO0UIqySsxM=; b=k5/yDOEitcva8bm3XzPuol0G3Bzou3laLa6ZzTWrXos/MOzcf8TR4mM2flFHx56SYJ pzcJiUtNrdkaDYiei3DdF7IlfoNQ37lTXnzvtOOa9iKcZIaAL2N8KcB7ulu7tq2LbUlW A2lEZRAF/W9I3rBeLmr1W27OWzr69Y7iNr1Bwg56/+qmsJkMgFZpTfcUk4qH4Ny6kspC XpmtyRTgYtexIgPH7/qC52OC+apW8AoNvGu5fJALlV3kB4ScvsUXiJGIy1MOJdiw9iyg s9fLmhmlBfi/9RSsu8qgw4d1mk8OKaQZ2K9P8w4r9w8llhTU9ILb5IDI3i5RSJUrCkOC XzMw== X-Gm-Message-State: ACgBeo0s10AEv/5+XU3HbB40JFNCVr74siHNPa0iLKxLaPKZqOQhwSzq vjZ9XvW1ru80Trl9zgaSsiDuq56cT5g= X-Google-Smtp-Source: AA6agR4S2n8eS5Q1zAZMhWuZzYOwaf3MQiU3+TLxDPLpjuMYZ5wiaxyypwtobB8bC9QR+XP4gpn29Q== X-Received: by 2002:a7b:c051:0:b0:3a6:36fc:8429 with SMTP id u17-20020a7bc051000000b003a636fc8429mr3309124wmc.78.1661992215059; Wed, 31 Aug 2022 17:30:15 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id f9-20020a05600c154900b003a60ff7c082sm3896750wmg.15.2022.08.31.17.30.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:14 -0700 (PDT) Message-Id: <7814d6a51c47ace670db6e185f71e1640d53aab1.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:52 +0000 Subject: [PATCH 14/18] t/chainlint: add more chainlint.pl self-tests Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine During the development of chainlint.pl, numerous new self-tests were created to verify correct functioning beyond the checks already represented by the existing self-tests. The new checks fall into several categories: * behavior of the lexical analyzer for complex cases, such as line splicing, token pasting, entering and exiting string contexts inside and outside of test script bodies; for instance: test_expect_success 'title' ' x=$(echo "something" | sed -e '\''s/\\/\\\\/g'\'' -e '\''s/[[/.*^$]/\\&/g'\'' ' * behavior of the parser for all compound grammatical constructs, such as `if...fi`, `case...esac`, `while...done`, `{...}`, etc., and for other legal shell grammatical constructs not covered by existing chainlint.sed self-tests, as well as complex cases, such as: OUT=$( ((large_git 1>&3) | :) 3>&1 ) && * detection of problems, such as &&-chain breakage, from top-level to any depth since the existing self-tests do not cover any top-level context and only cover subshells one level deep due to limitations of chainlint.sed * address blind spots in chainlint.sed (such as not detecting a broken &&-chain on a one-line for-loop in a subshell[1]) which chainlint.pl correctly detects * real-world cases which tripped up chainlint.pl during its development [1]: https://lore.kernel.org/git/dce35a47012fecc6edc11c68e91dbb485c5bc36f.1661663880.git.gitgitgadget@gmail.com/ Signed-off-by: Eric Sunshine --- t/chainlint/blank-line-before-esac.expect | 18 +++++++++++ t/chainlint/blank-line-before-esac.test | 19 +++++++++++ t/chainlint/block.expect | 13 +++++++- t/chainlint/block.test | 15 ++++++++- t/chainlint/chained-block.expect | 9 ++++++ t/chainlint/chained-block.test | 11 +++++++ t/chainlint/chained-subshell.expect | 10 ++++++ t/chainlint/chained-subshell.test | 13 ++++++++ .../command-substitution-subsubshell.expect | 2 ++ .../command-substitution-subsubshell.test | 3 ++ t/chainlint/double-here-doc.expect | 2 ++ t/chainlint/double-here-doc.test | 12 +++++++ t/chainlint/dqstring-line-splice.expect | 3 ++ t/chainlint/dqstring-line-splice.test | 7 ++++ t/chainlint/dqstring-no-interpolate.expect | 11 +++++++ t/chainlint/dqstring-no-interpolate.test | 15 +++++++++ t/chainlint/empty-here-doc.expect | 3 ++ t/chainlint/empty-here-doc.test | 5 +++ t/chainlint/exclamation.expect | 4 +++ t/chainlint/exclamation.test | 8 +++++ t/chainlint/for-loop-abbreviated.expect | 5 +++ t/chainlint/for-loop-abbreviated.test | 6 ++++ t/chainlint/function.expect | 11 +++++++ t/chainlint/function.test | 13 ++++++++ t/chainlint/here-doc-indent-operator.expect | 5 +++ t/chainlint/here-doc-indent-operator.test | 13 ++++++++ t/chainlint/if-condition-split.expect | 7 ++++ t/chainlint/if-condition-split.test | 8 +++++ t/chainlint/one-liner-for-loop.expect | 9 ++++++ t/chainlint/one-liner-for-loop.test | 10 ++++++ t/chainlint/sqstring-in-sqstring.expect | 4 +++ t/chainlint/sqstring-in-sqstring.test | 5 +++ t/chainlint/token-pasting.expect | 27 ++++++++++++++++ t/chainlint/token-pasting.test | 32 +++++++++++++++++++ 34 files changed, 336 insertions(+), 2 deletions(-) create mode 100644 t/chainlint/blank-line-before-esac.expect create mode 100644 t/chainlint/blank-line-before-esac.test create mode 100644 t/chainlint/chained-block.expect create mode 100644 t/chainlint/chained-block.test create mode 100644 t/chainlint/chained-subshell.expect create mode 100644 t/chainlint/chained-subshell.test create mode 100644 t/chainlint/command-substitution-subsubshell.expect create mode 100644 t/chainlint/command-substitution-subsubshell.test create mode 100644 t/chainlint/double-here-doc.expect create mode 100644 t/chainlint/double-here-doc.test create mode 100644 t/chainlint/dqstring-line-splice.expect create mode 100644 t/chainlint/dqstring-line-splice.test create mode 100644 t/chainlint/dqstring-no-interpolate.expect create mode 100644 t/chainlint/dqstring-no-interpolate.test create mode 100644 t/chainlint/empty-here-doc.expect create mode 100644 t/chainlint/empty-here-doc.test create mode 100644 t/chainlint/exclamation.expect create mode 100644 t/chainlint/exclamation.test create mode 100644 t/chainlint/for-loop-abbreviated.expect create mode 100644 t/chainlint/for-loop-abbreviated.test create mode 100644 t/chainlint/function.expect create mode 100644 t/chainlint/function.test create mode 100644 t/chainlint/here-doc-indent-operator.expect create mode 100644 t/chainlint/here-doc-indent-operator.test create mode 100644 t/chainlint/if-condition-split.expect create mode 100644 t/chainlint/if-condition-split.test create mode 100644 t/chainlint/one-liner-for-loop.expect create mode 100644 t/chainlint/one-liner-for-loop.test create mode 100644 t/chainlint/sqstring-in-sqstring.expect create mode 100644 t/chainlint/sqstring-in-sqstring.test create mode 100644 t/chainlint/token-pasting.expect create mode 100644 t/chainlint/token-pasting.test diff --git a/t/chainlint/blank-line-before-esac.expect b/t/chainlint/blank-line-before-esac.expect new file mode 100644 index 00000000000..48ed4eb1246 --- /dev/null +++ b/t/chainlint/blank-line-before-esac.expect @@ -0,0 +1,18 @@ +test_done ( ) { + case "$test_failure" in + 0 ) + test_at_end_hook_ + + exit 0 ;; + + * ) + if test $test_external_has_tap -eq 0 + then + say_color error "# failed $test_failure among $msg" + say "1..$test_count" + fi + + exit 1 ;; + + esac +} diff --git a/t/chainlint/blank-line-before-esac.test b/t/chainlint/blank-line-before-esac.test new file mode 100644 index 00000000000..cecccad19f5 --- /dev/null +++ b/t/chainlint/blank-line-before-esac.test @@ -0,0 +1,19 @@ +# LINT: blank line before "esac" +test_done () { + case "$test_failure" in + 0) + test_at_end_hook_ + + exit 0 ;; + + *) + if test $test_external_has_tap -eq 0 + then + say_color error "# failed $test_failure among $msg" + say "1..$test_count" + fi + + exit 1 ;; + + esac +} diff --git a/t/chainlint/block.expect b/t/chainlint/block.expect index 37dbf7d95fa..a3bcea492a9 100644 --- a/t/chainlint/block.expect +++ b/t/chainlint/block.expect @@ -9,4 +9,15 @@ echo c } ?!AMP?! baz -) +) && + +{ + echo a ; ?!AMP?! echo b +} && +{ echo a ; ?!AMP?! echo b ; } && + +{ + echo "${var}9" && + echo "done" +} && +finis diff --git a/t/chainlint/block.test b/t/chainlint/block.test index 0a82fd579f6..4ab69a4afc4 100644 --- a/t/chainlint/block.test +++ b/t/chainlint/block.test @@ -11,4 +11,17 @@ echo c } baz -) +) && + +# LINT: ";" not allowed in place of "&&" +{ + echo a; echo b +} && +{ echo a; echo b; } && + +# LINT: "}" inside string not mistaken as end of block +{ + echo "${var}9" && + echo "done" +} && +finis diff --git a/t/chainlint/chained-block.expect b/t/chainlint/chained-block.expect new file mode 100644 index 00000000000..574cdceb071 --- /dev/null +++ b/t/chainlint/chained-block.expect @@ -0,0 +1,9 @@ +echo nobody home && { + test the doohicky ?!AMP?! + right now +} && + +GIT_EXTERNAL_DIFF=echo git diff | { + read path oldfile oldhex oldmode newfile newhex newmode && + test "z$oh" = "z$oldhex" +} diff --git a/t/chainlint/chained-block.test b/t/chainlint/chained-block.test new file mode 100644 index 00000000000..86f81ece639 --- /dev/null +++ b/t/chainlint/chained-block.test @@ -0,0 +1,11 @@ +# LINT: start of block chained to preceding command +echo nobody home && { + test the doohicky + right now +} && + +# LINT: preceding command pipes to block on same line +GIT_EXTERNAL_DIFF=echo git diff | { + read path oldfile oldhex oldmode newfile newhex newmode && + test "z$oh" = "z$oldhex" +} diff --git a/t/chainlint/chained-subshell.expect b/t/chainlint/chained-subshell.expect new file mode 100644 index 00000000000..af0369d3285 --- /dev/null +++ b/t/chainlint/chained-subshell.expect @@ -0,0 +1,10 @@ +mkdir sub && ( + cd sub && + foo the bar ?!AMP?! + nuff said +) && + +cut "-d " -f actual | ( read s1 s2 s3 && +test -f $s1 ?!AMP?! +test $(cat $s2) = tree2path1 && +test $(cat $s3) = tree3path1 ) diff --git a/t/chainlint/chained-subshell.test b/t/chainlint/chained-subshell.test new file mode 100644 index 00000000000..4ff6ddd8cbd --- /dev/null +++ b/t/chainlint/chained-subshell.test @@ -0,0 +1,13 @@ +# LINT: start of subshell chained to preceding command +mkdir sub && ( + cd sub && + foo the bar + nuff said +) && + +# LINT: preceding command pipes to subshell on same line +cut "-d " -f actual | (read s1 s2 s3 && +test -f $s1 +test $(cat $s2) = tree2path1 && +# LINT: closing subshell ")" correctly detected on same line as "$(...)" +test $(cat $s3) = tree3path1) diff --git a/t/chainlint/command-substitution-subsubshell.expect b/t/chainlint/command-substitution-subsubshell.expect new file mode 100644 index 00000000000..ab2f79e8457 --- /dev/null +++ b/t/chainlint/command-substitution-subsubshell.expect @@ -0,0 +1,2 @@ +OUT=$(( ( large_git 1 >& 3 ) | : ) 3 >& 1) && +test_match_signal 13 "$OUT" diff --git a/t/chainlint/command-substitution-subsubshell.test b/t/chainlint/command-substitution-subsubshell.test new file mode 100644 index 00000000000..321de2951ce --- /dev/null +++ b/t/chainlint/command-substitution-subsubshell.test @@ -0,0 +1,3 @@ +# LINT: subshell nested in subshell nested in command substitution +OUT=$( ((large_git 1>&3) | :) 3>&1 ) && +test_match_signal 13 "$OUT" diff --git a/t/chainlint/double-here-doc.expect b/t/chainlint/double-here-doc.expect new file mode 100644 index 00000000000..75477bb1add --- /dev/null +++ b/t/chainlint/double-here-doc.expect @@ -0,0 +1,2 @@ +run_sub_test_lib_test_err run-inv-range-start "--run invalid range start" --run="a-5" <<-EOF && +check_sub_test_lib_test_err run-inv-range-start <<-EOF_OUT 3 <<-EOF_ERR diff --git a/t/chainlint/double-here-doc.test b/t/chainlint/double-here-doc.test new file mode 100644 index 00000000000..cd584a43573 --- /dev/null +++ b/t/chainlint/double-here-doc.test @@ -0,0 +1,12 @@ +run_sub_test_lib_test_err run-inv-range-start \ + "--run invalid range start" \ + --run="a-5" <<-\EOF && +test_expect_success "passing test #1" "true" +test_done +EOF +check_sub_test_lib_test_err run-inv-range-start \ + <<-\EOF_OUT 3<<-EOF_ERR +> FATAL: Unexpected exit with code 1 +EOF_OUT +> error: --run: invalid non-numeric in range start: ${SQ}a-5${SQ} +EOF_ERR diff --git a/t/chainlint/dqstring-line-splice.expect b/t/chainlint/dqstring-line-splice.expect new file mode 100644 index 00000000000..bf9ced60d4c --- /dev/null +++ b/t/chainlint/dqstring-line-splice.expect @@ -0,0 +1,3 @@ +echo 'fatal: reword option of --fixup is mutually exclusive with' '--patch/--interactive/--all/--include/--only' > expect && +test_must_fail git commit --fixup=reword:HEAD~ $1 2 > actual && +test_cmp expect actual diff --git a/t/chainlint/dqstring-line-splice.test b/t/chainlint/dqstring-line-splice.test new file mode 100644 index 00000000000..b40714439f6 --- /dev/null +++ b/t/chainlint/dqstring-line-splice.test @@ -0,0 +1,7 @@ +# LINT: line-splice within DQ-string +'" +echo 'fatal: reword option of --fixup is mutually exclusive with'\ + '--patch/--interactive/--all/--include/--only' >expect && +test_must_fail git commit --fixup=reword:HEAD~ $1 2>actual && +test_cmp expect actual +"' diff --git a/t/chainlint/dqstring-no-interpolate.expect b/t/chainlint/dqstring-no-interpolate.expect new file mode 100644 index 00000000000..10724987a5f --- /dev/null +++ b/t/chainlint/dqstring-no-interpolate.expect @@ -0,0 +1,11 @@ +grep "^ ! [rejected][ ]*$BRANCH -> $BRANCH (non-fast-forward)$" out && + +grep "^\.git$" output.txt && + + +( + cd client$version && + GIT_TEST_PROTOCOL_VERSION=$version git fetch-pack --no-progress .. $(cat ../input) +) > output && + cut -d ' ' -f 2 < output | sort > actual && + test_cmp expect actual diff --git a/t/chainlint/dqstring-no-interpolate.test b/t/chainlint/dqstring-no-interpolate.test new file mode 100644 index 00000000000..d2f4219cbbb --- /dev/null +++ b/t/chainlint/dqstring-no-interpolate.test @@ -0,0 +1,15 @@ +# LINT: regex dollar-sign eol anchor in double-quoted string not special +grep "^ ! \[rejected\][ ]*$BRANCH -> $BRANCH (non-fast-forward)$" out && + +# LINT: escaped "$" not mistaken for variable expansion +grep "^\\.git\$" output.txt && + +'" +( + cd client$version && +# LINT: escaped dollar-sign in double-quoted test body + GIT_TEST_PROTOCOL_VERSION=$version git fetch-pack --no-progress .. \$(cat ../input) +) >output && + cut -d ' ' -f 2 actual && + test_cmp expect actual +"' diff --git a/t/chainlint/empty-here-doc.expect b/t/chainlint/empty-here-doc.expect new file mode 100644 index 00000000000..f42f2d41ba8 --- /dev/null +++ b/t/chainlint/empty-here-doc.expect @@ -0,0 +1,3 @@ +git ls-tree $tree path > current && +cat > expected <current && +# LINT: empty here-doc +cat >expected <<\EOF && +EOF +test_output diff --git a/t/chainlint/exclamation.expect b/t/chainlint/exclamation.expect new file mode 100644 index 00000000000..2d961a58c66 --- /dev/null +++ b/t/chainlint/exclamation.expect @@ -0,0 +1,4 @@ +if ! condition ; then echo nope ; else yep ; fi && +test_prerequisite !MINGW && +mail uucp!address && +echo !whatever! diff --git a/t/chainlint/exclamation.test b/t/chainlint/exclamation.test new file mode 100644 index 00000000000..323595b5bd8 --- /dev/null +++ b/t/chainlint/exclamation.test @@ -0,0 +1,8 @@ +# LINT: "! word" is two tokens +if ! condition; then echo nope; else yep; fi && +# LINT: "!word" is single token, not two tokens "!" and "word" +test_prerequisite !MINGW && +# LINT: "word!word" is single token, not three tokens "word", "!", and "word" +mail uucp!address && +# LINT: "!word!" is single token, not three tokens "!", "word", and "!" +echo !whatever! diff --git a/t/chainlint/for-loop-abbreviated.expect b/t/chainlint/for-loop-abbreviated.expect new file mode 100644 index 00000000000..a21007a63f1 --- /dev/null +++ b/t/chainlint/for-loop-abbreviated.expect @@ -0,0 +1,5 @@ +for it +do + path=$(expr "$it" : ( [^:]*) ) && + git update-index --add "$path" || exit +done diff --git a/t/chainlint/for-loop-abbreviated.test b/t/chainlint/for-loop-abbreviated.test new file mode 100644 index 00000000000..1084eccb89c --- /dev/null +++ b/t/chainlint/for-loop-abbreviated.test @@ -0,0 +1,6 @@ +# LINT: for-loop lacking optional "in [word...]" before "do" +for it +do + path=$(expr "$it" : '\([^:]*\)') && + git update-index --add "$path" || exit +done diff --git a/t/chainlint/function.expect b/t/chainlint/function.expect new file mode 100644 index 00000000000..a14388e6b9f --- /dev/null +++ b/t/chainlint/function.expect @@ -0,0 +1,11 @@ +sha1_file ( ) { + echo "$*" | sed "s#..#.git/objects/&/#" +} && + +remove_object ( ) { + file=$(sha1_file "$*") && + test -e "$file" ?!AMP?! + rm -f "$file" +} ?!AMP?! + +sha1_file arg && remove_object arg diff --git a/t/chainlint/function.test b/t/chainlint/function.test new file mode 100644 index 00000000000..5ee59562c93 --- /dev/null +++ b/t/chainlint/function.test @@ -0,0 +1,13 @@ +# LINT: "()" in function definition not mistaken for subshell +sha1_file() { + echo "$*" | sed "s#..#.git/objects/&/#" +} && + +# LINT: broken &&-chain in function and after function +remove_object() { + file=$(sha1_file "$*") && + test -e "$file" + rm -f "$file" +} + +sha1_file arg && remove_object arg diff --git a/t/chainlint/here-doc-indent-operator.expect b/t/chainlint/here-doc-indent-operator.expect new file mode 100644 index 00000000000..fb6cf7285d0 --- /dev/null +++ b/t/chainlint/here-doc-indent-operator.expect @@ -0,0 +1,5 @@ +cat > expect <<-EOF && + +cat > expect <<-EOF ?!AMP?! + +cleanup diff --git a/t/chainlint/here-doc-indent-operator.test b/t/chainlint/here-doc-indent-operator.test new file mode 100644 index 00000000000..c8a6f18eb45 --- /dev/null +++ b/t/chainlint/here-doc-indent-operator.test @@ -0,0 +1,13 @@ +# LINT: whitespace between operator "<<-" and tag legal +cat >expect <<- EOF && +header: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0 +num_commits: $1 +chunks: oid_fanout oid_lookup commit_metadata generation_data bloom_indexes bloom_data +EOF + +# LINT: not an indented here-doc; just a plain here-doc with tag named "-EOF" +cat >expect << -EOF +this is not indented +-EOF + +cleanup diff --git a/t/chainlint/if-condition-split.expect b/t/chainlint/if-condition-split.expect new file mode 100644 index 00000000000..ee745ef8d7f --- /dev/null +++ b/t/chainlint/if-condition-split.expect @@ -0,0 +1,7 @@ +if bob && + marcia || + kevin +then + echo "nomads" ?!AMP?! + echo "for sure" +fi diff --git a/t/chainlint/if-condition-split.test b/t/chainlint/if-condition-split.test new file mode 100644 index 00000000000..240daa9fd5d --- /dev/null +++ b/t/chainlint/if-condition-split.test @@ -0,0 +1,8 @@ +# LINT: "if" condition split across multiple lines at "&&" or "||" +if bob && + marcia || + kevin +then + echo "nomads" + echo "for sure" +fi diff --git a/t/chainlint/one-liner-for-loop.expect b/t/chainlint/one-liner-for-loop.expect new file mode 100644 index 00000000000..51a3dc7c544 --- /dev/null +++ b/t/chainlint/one-liner-for-loop.expect @@ -0,0 +1,9 @@ +git init dir-rename-and-content && +( + cd dir-rename-and-content && + test_write_lines 1 2 3 4 5 >foo && + mkdir olddir && + for i in a b c; do echo $i >olddir/$i; ?!LOOP?! done ?!AMP?! + git add foo olddir && + git commit -m "original" && +) diff --git a/t/chainlint/one-liner-for-loop.test b/t/chainlint/one-liner-for-loop.test new file mode 100644 index 00000000000..4bd8c066c79 --- /dev/null +++ b/t/chainlint/one-liner-for-loop.test @@ -0,0 +1,10 @@ +git init dir-rename-and-content && +( + cd dir-rename-and-content && + test_write_lines 1 2 3 4 5 >foo && + mkdir olddir && +# LINT: one-liner for-loop missing "|| exit"; also broken &&-chain + for i in a b c; do echo $i >olddir/$i; done + git add foo olddir && + git commit -m "original" && +) diff --git a/t/chainlint/sqstring-in-sqstring.expect b/t/chainlint/sqstring-in-sqstring.expect new file mode 100644 index 00000000000..cf0b591cf7d --- /dev/null +++ b/t/chainlint/sqstring-in-sqstring.expect @@ -0,0 +1,4 @@ +perl -e ' + defined($_ = -s $_) or die for @ARGV; + exit 1 if $ARGV[0] <= $ARGV[1]; +' test-2-$packname_2.pack test-3-$packname_3.pack diff --git a/t/chainlint/sqstring-in-sqstring.test b/t/chainlint/sqstring-in-sqstring.test new file mode 100644 index 00000000000..77a425e0c79 --- /dev/null +++ b/t/chainlint/sqstring-in-sqstring.test @@ -0,0 +1,5 @@ +# LINT: SQ-string Perl code fragment within SQ-string +perl -e '\'' + defined($_ = -s $_) or die for @ARGV; + exit 1 if $ARGV[0] <= $ARGV[1]; +'\'' test-2-$packname_2.pack test-3-$packname_3.pack diff --git a/t/chainlint/token-pasting.expect b/t/chainlint/token-pasting.expect new file mode 100644 index 00000000000..342360bcd05 --- /dev/null +++ b/t/chainlint/token-pasting.expect @@ -0,0 +1,27 @@ +git config filter.rot13.smudge ./rot13.sh && +git config filter.rot13.clean ./rot13.sh && + +{ + echo "*.t filter=rot13" ?!AMP?! + echo "*.i ident" +} > .gitattributes && + +{ + echo a b c d e f g h i j k l m ?!AMP?! + echo n o p q r s t u v w x y z ?!AMP?! + echo '$Id$' +} > test && +cat test > test.t && +cat test > test.o && +cat test > test.i && +git add test test.t test.i && +rm -f test test.t test.i && +git checkout -- test test.t test.i && + +echo "content-test2" > test2.o && +echo "content-test3 - filename with special characters" > "test3 'sq',$x=.o" ?!AMP?! + +downstream_url_for_sed=$( + printf "%sn" "$downstream_url" | + sed -e 's/\/\\/g' -e 's/[[/.*^$]/\&/g' +) diff --git a/t/chainlint/token-pasting.test b/t/chainlint/token-pasting.test new file mode 100644 index 00000000000..b4610ce815a --- /dev/null +++ b/t/chainlint/token-pasting.test @@ -0,0 +1,32 @@ +# LINT: single token; composite of multiple strings +git config filter.rot13.smudge ./rot13.sh && +git config filter.rot13.clean ./rot13.sh && + +{ + echo "*.t filter=rot13" + echo "*.i ident" +} >.gitattributes && + +{ + echo a b c d e f g h i j k l m + echo n o p q r s t u v w x y z +# LINT: exit/enter string context and escaped-quote outside of string + echo '\''$Id$'\'' +} >test && +cat test >test.t && +cat test >test.o && +cat test >test.i && +git add test test.t test.i && +rm -f test test.t test.i && +git checkout -- test test.t test.i && + +echo "content-test2" >test2.o && +# LINT: exit/enter string context and escaped-quote outside of string +echo "content-test3 - filename with special characters" >"test3 '\''sq'\'',\$x=.o" + +# LINT: single token; composite of multiple strings +downstream_url_for_sed=$( + printf "%s\n" "$downstream_url" | +# LINT: exit/enter string context; "&" inside string not command terminator + sed -e '\''s/\\/\\\\/g'\'' -e '\''s/[[/.*^$]/\\&/g'\'' +) From patchwork Thu Sep 1 00:29:53 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961594 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A7982ECAAD1 for ; Thu, 1 Sep 2022 00:30:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232208AbiIAAas (ORCPT ); Wed, 31 Aug 2022 20:30:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47528 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231755AbiIAAaW (ORCPT ); Wed, 31 Aug 2022 20:30:22 -0400 Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 270F9AA3D1 for ; Wed, 31 Aug 2022 17:30:17 -0700 (PDT) Received: by mail-wr1-x42c.google.com with SMTP id u18so7604107wrq.10 for ; Wed, 31 Aug 2022 17:30:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=vd2juGQY/66EKxbSOpTUM3J+R1Bw2qLH0cP0aKCrEhw=; b=MBkxHuIsO7KeYlYD3h9/KGzNR8Mrcl9K+BoPuLNUmR8h3JiBXbu+yylcGPGqnuI7Ob Jh0M5U6o0STd9XyJiYqvomrV8V2CP7plN0X3bsMb1GNwriGbcs9Y3auUMDJ8VGALVuIu 6j0Kg+MkEITNW40jy+hv+XChrqpwPSb/hSFCAqsUKyFrEV8j12YoGni1AUfV0SOZQqb/ w4FUH68ZxcXIrFhRljwQ2PtUKvj2+3SJjnbLvb5TP1ss6TJa94XqV+GuKCns7gsyCLk+ KJDbkhHLQoOiofHqShJDZVDbUTNMvszkVRHEH5t+LfZlcCRN0ZV2LmVYkd5syXUqkstp 3X9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=vd2juGQY/66EKxbSOpTUM3J+R1Bw2qLH0cP0aKCrEhw=; b=6h+P1pLxLupKarS1+49DdEmWDzs5bCWByNxp7iVbZwgdwpvHhaJbjsP719n704iS7U /EIftBmVZeu63cqtO1kEe4oiKAjYDPHnIoVOGauBlubOgNKAjecmIATLtWFRULEi4fzW 6cW/n67fGCnY7DiuVF1Yo2TTtcHNOxjCT75Tsr+qivUkjEZmIkzzU0xlJc8QmAqSn1tV WFk6tFzVF/GOTI6RwWccKY7bWlYKi6K2/zzl4nlaFIVzFCyP/2lZYYxgDRTOMcoTM0u3 ApmL8bHvRPwAWGxQ/snYF+aKZx6bPBFcWdo/EtcwnFVhbtlKYFAb+XOERXUq22u5BkXV gabQ== X-Gm-Message-State: ACgBeo2jWKvmKzpnDGfYjjBe7awv3DGI42PPGtFqroDgRWNMrOSy/LVR Evu47eWYK0aDjayEzJ1CP/SOTwNhmYs= X-Google-Smtp-Source: AA6agR701I6AKR3IxZ2pEUvdYkC+CB/fdFwvkbR3nYH8XnjcgdGVdxLAmKSfCszcLQx2MXvAUcrOzA== X-Received: by 2002:a5d:6b10:0:b0:21e:4bbd:e893 with SMTP id v16-20020a5d6b10000000b0021e4bbde893mr13148874wrw.613.1661992216178; Wed, 31 Aug 2022 17:30:16 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id n18-20020a05600c501200b003a32251c3f9sm4250866wmr.5.2022.08.31.17.30.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:15 -0700 (PDT) Message-Id: <32992926ba2f3e8682f6dd91819ec66b82365c3b.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:53 +0000 Subject: [PATCH 15/18] test-lib: retire "lint harder" optimization hack Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine `test_run_` in test-lib.sh "lints" the body of a test by sending it down a `sed chainlint.sed | grep` pipeline; this happens once for each test run by a test script. Although this pipeline may seem relatively cheap in isolation, it can become expensive when invoked 26800+ times by `make test`, once for each test run, despite the existence of only 16500+ test definitions across all tests scripts. This difference in the number of tests defined in the scripts (16500+) and the number of tests actually run by `make test` (26800+) is explained by the fact that some test scripts run a very large number of small tests, all driven by a series of functions/loops which fill in the test bodies. This means that certain test definitions are being linted repeatedly (tens or hundreds of times) unnecessarily. To avoid such unnecessary work, 2d86a96220 (t: avoid sed-based chain-linting in some expensive cases, 2021-05-13) added an optimization hack which allows individual scripts to manually suppress the unnecessary repeated linting of the same test definition. However, unlike chainlint.sed which checks a test body as the test is run, chainlint.pl checks each test definition just once, no matter how many times the test is run, thus the sort of optimization hack introduced by 2d86a96220 is no longer needed and can be retired. Therefore, revert 2d86a96220. Signed-off-by: Eric Sunshine --- t/README | 5 ----- t/t0027-auto-crlf.sh | 7 +------ t/t3070-wildmatch.sh | 5 ----- t/test-lib.sh | 7 ++----- 4 files changed, 3 insertions(+), 21 deletions(-) diff --git a/t/README b/t/README index 2f439f96589..979b2d4833d 100644 --- a/t/README +++ b/t/README @@ -196,11 +196,6 @@ appropriately before running "make". Short options can be bundled, i.e. this feature by setting the GIT_TEST_CHAIN_LINT environment variable to "1" or "0", respectively. - A few test scripts disable some of the more advanced - chain-linting detection in the name of efficiency. You can - override this by setting the GIT_TEST_CHAIN_LINT_HARDER - environment variable to "1". - --stress:: Run the test script repeatedly in multiple parallel jobs until one of them fails. Useful for reproducing rare failures in diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh index a22e0e1382c..a94ac1eae37 100755 --- a/t/t0027-auto-crlf.sh +++ b/t/t0027-auto-crlf.sh @@ -387,9 +387,7 @@ test_expect_success 'setup main' ' test_tick ' -# Disable extra chain-linting for the next set of tests. There are many -# auto-generated ones that are not worth checking over and over. -GIT_TEST_CHAIN_LINT_HARDER_DEFAULT=0 + warn_LF_CRLF="LF will be replaced by CRLF" warn_CRLF_LF="CRLF will be replaced by LF" @@ -606,9 +604,6 @@ do checkout_files "" "$id" "crlf" true "" CRLF CRLF CRLF CRLF_mix_CR CRLF_nul done -# The rest of the tests are unique; do the usual linting. -unset GIT_TEST_CHAIN_LINT_HARDER_DEFAULT - # Should be the last test case: remove some files from the worktree test_expect_success 'ls-files --eol -d -z' ' rm crlf_false_attr__CRLF.txt crlf_false_attr__CRLF_mix_LF.txt crlf_false_attr__LF.txt .gitattributes && diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh index f9539968e4c..5d871fde960 100755 --- a/t/t3070-wildmatch.sh +++ b/t/t3070-wildmatch.sh @@ -5,11 +5,6 @@ test_description='wildmatch tests' TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh -# Disable expensive chain-lint tests; all of the tests in this script -# are variants of a few trivial test-tool invocations, and there are a lot of -# them. -GIT_TEST_CHAIN_LINT_HARDER_DEFAULT=0 - should_create_test_file() { file=$1 diff --git a/t/test-lib.sh b/t/test-lib.sh index 377cc1c1203..dc0d0591095 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1091,11 +1091,8 @@ test_run_ () { trace= # 117 is magic because it is unlikely to match the exit # code of other programs - if test "OK-117" != "$(test_eval_ "(exit 117) && $1${LF}${LF}echo OK-\$?" 3>&1)" || - { - test "${GIT_TEST_CHAIN_LINT_HARDER:-${GIT_TEST_CHAIN_LINT_HARDER_DEFAULT:-1}}" != 0 && - $(printf '%s\n' "$1" | sed -f "$GIT_BUILD_DIR/t/chainlint.sed" | grep -q '?![A-Z][A-Z]*?!') - } + if $(printf '%s\n' "$1" | sed -f "$GIT_BUILD_DIR/t/chainlint.sed" | grep -q '?![A-Z][A-Z]*?!') || + test "OK-117" != "$(test_eval_ "(exit 117) && $1${LF}${LF}echo OK-\$?" 3>&1)" then BUG "broken &&-chain or run-away HERE-DOC: $1" fi From patchwork Thu Sep 1 00:29:54 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961595 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1384ECAAD5 for ; Thu, 1 Sep 2022 00:30:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232209AbiIAAau (ORCPT ); Wed, 31 Aug 2022 20:30:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47550 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231955AbiIAAaW (ORCPT ); Wed, 31 Aug 2022 20:30:22 -0400 Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 09B4732D88 for ; Wed, 31 Aug 2022 17:30:17 -0700 (PDT) Received: by mail-wr1-x435.google.com with SMTP id w5so4093098wrn.12 for ; Wed, 31 Aug 2022 17:30:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=bDLQc4w7rxzNO22p/rRmIVIciPKBABfTRvM2eAKkfP0=; b=TEECpXSc/HKkcOLcqTYT3XH6iaSMDWeh1EgIrLhGXkFZtDMnDmeVx3I2ruuSXl6M1P fnaCKK0Z0aLyRhmP7yOONBqhs69fMk7xUv4JxH8EulBEEU55zzuw/IpCLVvi2jqhFUKU 2ITTke6ribdpZZeKCyx0xsJc4RxV5nNmkl3vF+9xPZsVautLjHVdfiJWWmQAB6EnPKF9 QRG4d8whodF+7DOXdTlPAS/WbOp/hczmqP5PXUbtP4S8+udlrqDROcdSh/ZWUqX87n/Y cBmNF8fHy6ege1yLCB9NR82/vuBRMb/BlMrtAyZKqmJoPpcfcywD5oeKqsUwsFUTL29x b7CQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=bDLQc4w7rxzNO22p/rRmIVIciPKBABfTRvM2eAKkfP0=; b=H7O8QaluwNDUvZFmSUg6vr0momrn7ZyjBA5kxILYr4o1+7g8ddkGbRqh2mxU3M08wL 6h3j8KA5xgrLwKE1VG6H+mxwG6TV8HQAlKQ3Ni4HsDg+ss7RxVENt6a6Qjpir/8XVgeU m7vz+gamY3PYdutVczBsEpgwY12kwez6WY7MS8x6Tl9Piyd4blvdsibmrPgvn2yEV6Pg ES3MwFgEnfWdI20FzV32ooBhpArPK+XQMgVzcYDpMjYv9fYjN4XIFKuWHHAkT3rz3W3j v170TpDEBOxs4lukyrktYSpPg28HuC4BbryQEJqUGdaHCbJCzdB7bxbK3sgJIgJJoGQw dPSw== X-Gm-Message-State: ACgBeo3pTKa1Sz0I6HvNm08nXPX4wUYTXrzvy+myZtRwKMshy/I+4z5n 18ABTgR+yuacY0hFmVPiDGVlI4CpwDU= X-Google-Smtp-Source: AA6agR4k3BrMQpowgrD7KxYFpshTsnKh/pjEI9532X1ONKzG//6jqTToQLEKQbuu4dAie4J2wTRKVw== X-Received: by 2002:a05:6000:381:b0:221:7542:61bb with SMTP id u1-20020a056000038100b00221754261bbmr12986950wrf.305.1661992217316; Wed, 31 Aug 2022 17:30:17 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id w6-20020a1cf606000000b003a5fa79007fsm3429183wmc.7.2022.08.31.17.30.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:16 -0700 (PDT) Message-Id: <9589f2a6e495034cc4f45bd0bce80dedfcd30f16.1661992197.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:54 +0000 Subject: [PATCH 16/18] test-lib: replace chainlint.sed with chainlint.pl Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine By automatically invoking chainlint.sed upon each test it runs, `test_run_` in test-lib.sh ensures that broken &&-chains will be detected early as tests are modified or new are tests created since it is typical to run a test script manually (i.e. `./t1234-test-script.sh`) during test development. Now that the implementation of chainlint.pl is complete, modify test-lib.sh to invoke it automatically instead of chainlint.sed each time a test script is run. This change reduces the number of "linter" invocations from 26800+ (once per test run) down to 1050+ (once per test script), however, a subsequent change will drop the number of invocations to 1 per `make test`, thus fully realizing the benefit of the new linter. Note that the "magic exit code 117" &&-chain checker added by bb79af9d09 (t/test-lib: introduce --chain-lint option, 2015-03-20) which is built into t/test-lib.sh is retained since it has near zero-cost and (theoretically) may catch a broken &&-chain not caught by chainlint.pl. Signed-off-by: Eric Sunshine --- contrib/buildsystems/CMakeLists.txt | 2 +- t/test-lib.sh | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 2237109b57f..ca358a21a5f 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -1076,7 +1076,7 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH}) "string(REPLACE \"\${GIT_BUILD_DIR_REPL}\" \"GIT_BUILD_DIR=\\\"$TEST_DIRECTORY/../${BUILD_DIR_RELATIVE}\\\"\" content \"\${content}\")\n" "file(WRITE ${CMAKE_SOURCE_DIR}/t/test-lib.sh \${content})") #misc copies - file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.sed DESTINATION ${CMAKE_BINARY_DIR}/t/) + file(COPY ${CMAKE_SOURCE_DIR}/t/chainlint.pl DESTINATION ${CMAKE_BINARY_DIR}/t/) file(COPY ${CMAKE_SOURCE_DIR}/po/is.po DESTINATION ${CMAKE_BINARY_DIR}/po/) file(COPY ${CMAKE_SOURCE_DIR}/mergetools/tkdiff DESTINATION ${CMAKE_BINARY_DIR}/mergetools/) file(COPY ${CMAKE_SOURCE_DIR}/contrib/completion/git-prompt.sh DESTINATION ${CMAKE_BINARY_DIR}/contrib/completion/) diff --git a/t/test-lib.sh b/t/test-lib.sh index dc0d0591095..a65df2fd220 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1091,8 +1091,7 @@ test_run_ () { trace= # 117 is magic because it is unlikely to match the exit # code of other programs - if $(printf '%s\n' "$1" | sed -f "$GIT_BUILD_DIR/t/chainlint.sed" | grep -q '?![A-Z][A-Z]*?!') || - test "OK-117" != "$(test_eval_ "(exit 117) && $1${LF}${LF}echo OK-\$?" 3>&1)" + if test "OK-117" != "$(test_eval_ "(exit 117) && $1${LF}${LF}echo OK-\$?" 3>&1)" then BUG "broken &&-chain or run-away HERE-DOC: $1" fi @@ -1588,6 +1587,12 @@ then BAIL_OUT_ENV_NEEDS_SANITIZE_LEAK "GIT_TEST_SANITIZE_LEAK_LOG=true" fi +if test "${GIT_TEST_CHAIN_LINT:-1}" != 0 +then + "$PERL_PATH" "$TEST_DIRECTORY/chainlint.pl" "$0" || + BUG "lint error (see '?!...!? annotations above)" +fi + # Last-minute variable setup USER_HOME="$HOME" HOME="$TRASH_DIRECTORY" From patchwork Thu Sep 1 00:29:55 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961597 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1548CECAAD1 for ; Thu, 1 Sep 2022 00:31:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232244AbiIAAbF (ORCPT ); Wed, 31 Aug 2022 20:31:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47730 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232098AbiIAAaX (ORCPT ); Wed, 31 Aug 2022 20:30:23 -0400 Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2535EAA4E5 for ; Wed, 31 Aug 2022 17:30:19 -0700 (PDT) Received: by mail-wr1-x42e.google.com with SMTP id s7so5364909wro.2 for ; Wed, 31 Aug 2022 17:30:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=ErgQMUsG7c28yKpl9jLIaLFXZ1NiyhHYBFR/p41F6Cw=; b=bQ25yg89kMP8uFhpAlQb623k7bcWkf++kn/xE0Y7AH9p1QuYzQdA07lor0GLbxTZBN O7kuUdibrqPmH0voPHxOBCjIHSpAN5lVENvZ1vXLf+F4F0Jl7ytRhCI+3ILnEeuTc0Rn LculC68xmnhpkijV//OM4FoAT5zI23wDpnbOgf76ZErMmhMNVZDcQMvn22VAwY7eoT3I tO0wUsXDauxC8NUCy/OsYhnsqZRpzL03SJgGWzNXTulCnjmkn766sl6veiuhkawDaR/X 6VHy2zPwYwQoXbfzSLMk0BhTNDbiKio5XAl4MHFFz3GmnpOIH0CReqQB+9o235cyxuWE HEAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=ErgQMUsG7c28yKpl9jLIaLFXZ1NiyhHYBFR/p41F6Cw=; b=RrSZzlpaW5Abq4d2NLvyDbqxyAKkOYHh7H3T8InV4whli04go8E+jYuEuxnA7ga4H7 Z0JmF+42i4b4MZrM9rUo8IlCci9Az5IehBQGSv4saVTznVaDLkRjbnemVmH/IwdXU7i8 /P40/Ry7gjUj3b1wYCHYo0kNRHYqIfNsF3ZWhLDUghX6f94hHABZN0M7DDJ6Kj9ukyrf BQgN3wbDYg4LClEfcc+zEOt/W45kOXIM9nPjQHyW4opC/YOz2zISh7hJ82ypUtFc0wmg ivqFSW8+nB5YbKUcEdItVspMmBIsQ6Llng47AiQLL4XRS6pZaAG3Jv/Q5ikiGhVpJtfy wovA== X-Gm-Message-State: ACgBeo3w/olhjaWiSQ9BFfT/N1Yv1wzB3iv9PXNWi7GLkO7Zq4Uq6WQl X5MpOL5m6xFKjqfW+565HyvYTMZdJ4s= X-Google-Smtp-Source: AA6agR54jfo1qqNZhcZ3FBMZv53THXAmmFYj/Xh/HVOjurq3EPZc3gWOJdYoZ6nBRZL1Th5EJqC3aw== X-Received: by 2002:a5d:5943:0:b0:226:d08e:fe11 with SMTP id e3-20020a5d5943000000b00226d08efe11mr13302184wri.78.1661992218195; Wed, 31 Aug 2022 17:30:18 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id i14-20020a05600c354e00b003a5dde32e4bsm3601087wmq.37.2022.08.31.17.30.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:17 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:55 +0000 Subject: [PATCH 17/18] t/Makefile: teach `make test` and `make prove` to run chainlint.pl Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine Unlike chainlint.sed which "lints" a single test body at a time, thus is invoked once per test, chainlint.pl can check all test bodies in all test scripts with a single invocation. As such, it is akin to other bulk "linters" run by the Makefile, such as `test-lint-shell-syntax`, `test-lint-duplicates`, etc. Therefore, teach `make test` and `make prove` to invoke chainlint.pl along with the other bulk linters. Also, since the single chainlint.pl invocation by `make test` or `make prove` has already checked all tests in all scripts, instruct the individual test scripts not to run chainlint.pl on themselves unnecessarily. Signed-off-by: Eric Sunshine --- t/Makefile | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/t/Makefile b/t/Makefile index 11f276774ea..3db48c0cb64 100644 --- a/t/Makefile +++ b/t/Makefile @@ -36,14 +36,21 @@ CHAINLINTTMP_SQ = $(subst ','\'',$(CHAINLINTTMP)) T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)) THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh))) +TLIBS = $(sort $(wildcard lib-*.sh)) annotate-tests.sh TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh)) +TINTEROP = $(sort $(wildcard interop/i[0-9][0-9][0-9][0-9]-*.sh)) CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test))) CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl +# `test-chainlint` (which is a dependency of `test-lint`, `test` and `prove`) +# checks all tests in all scripts via a single invocation, so tell individual +# scripts not to "chainlint" themselves +CHAINLINTSUPPRESS = GIT_TEST_CHAIN_LINT=0 && export GIT_TEST_CHAIN_LINT && + all: $(DEFAULT_TEST_TARGET) test: pre-clean check-chainlint $(TEST_LINT) - $(MAKE) aggregate-results-and-cleanup + $(CHAINLINTSUPPRESS) $(MAKE) aggregate-results-and-cleanup failed: @failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \ @@ -52,7 +59,7 @@ failed: test -z "$$failed" || $(MAKE) $$failed prove: pre-clean check-chainlint $(TEST_LINT) - @echo "*** prove ***"; $(PROVE) --exec '$(TEST_SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS) + @echo "*** prove ***"; $(CHAINLINTSUPPRESS) $(PROVE) --exec '$(TEST_SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS) $(MAKE) clean-except-prove-cache $(T): @@ -99,6 +106,9 @@ check-chainlint: test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \ test-lint-filenames +ifneq ($(GIT_TEST_CHAIN_LINT),0) +test-lint: test-chainlint +endif test-lint-duplicates: @dups=`echo $(T) $(TPERF) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \ @@ -121,6 +131,9 @@ test-lint-filenames: test -z "$$bad" || { \ echo >&2 "non-portable file name(s): $$bad"; exit 1; } +test-chainlint: + @$(CHAINLINT) $(T) $(TLIBS) $(TPERF) $(TINTEROP) + aggregate-results-and-cleanup: $(T) $(MAKE) aggregate-results $(MAKE) clean @@ -136,4 +149,5 @@ valgrind: perf: $(MAKE) -C perf/ all -.PHONY: pre-clean $(T) aggregate-results clean valgrind perf check-chainlint clean-chainlint +.PHONY: pre-clean $(T) aggregate-results clean valgrind perf \ + check-chainlint clean-chainlint test-chainlint From patchwork Thu Sep 1 00:29:56 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sunshine X-Patchwork-Id: 12961596 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F287CECAAD4 for ; Thu, 1 Sep 2022 00:31:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232117AbiIAAbC (ORCPT ); Wed, 31 Aug 2022 20:31:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47546 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231992AbiIAAaX (ORCPT ); Wed, 31 Aug 2022 20:30:23 -0400 Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 254A8AB411 for ; Wed, 31 Aug 2022 17:30:20 -0700 (PDT) Received: by mail-wr1-x429.google.com with SMTP id b5so20260907wrr.5 for ; Wed, 31 Aug 2022 17:30:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc; bh=EcdrArJbkoIin3rbgtMSYGVs6oL8Z+0m0NcLO+turgs=; b=JIMzJVDick4DYet5ExWjkq/+t+6fFjFtWdZjpTqyd4ccXo/dt9AUkO6jshA55D2Xaa WZxQ7a/0gXALgfMSNx2qeZthP/G9zKnKQGs0oTsdNT197IZiE06IoNORPsYSDMnsq1xc CuYckHnirYlndC6swQV39NSvuOTP3F8FeS31adtmi/U9ALpRx56oJhQg00qZNjjchg+3 oQF/IABv0ntMj0mw0gVrM3810LPRhTiB3BBel+nNvAPR5onupldehgrWQlJvZ7DS2e9x UPl0PPnVpJPZexsmydUgD9Pz46VsseF3yd1D8nfTMQotO1Odb4sW4AFAcrj4epooE8Ql lwzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc; bh=EcdrArJbkoIin3rbgtMSYGVs6oL8Z+0m0NcLO+turgs=; b=ONkZT+KWVK6Y/ydRwVste4OrN5T4LyHPD7xJKLyA7F3JMRREhGx6c0iHJ4rhvW8624 NHbkXTzSoFNTT4T+Doj/vVIbAfURbXQwQunQcO4ijTH8XsDW0uwfl2MSxnlzXHFsGKN7 tb2qRPHppcyWMuDhDd/i7+26nClF6krQrmbwTwYHX3Ozd97vQNoKHA0IMvedIRpdhz6H OMisGKVSlsVOVofj3ay0TB26lJo+GPyw/gBiFTvq7Mxdvc8u3Mo8kwOleN3EKb/cfMzo O/GVlFFHxkUMMbWnVH4BDT3NMBgSWpWPWuBjP19ta72swkVUX6rV9O8ljggjuaGYkBia E05g== X-Gm-Message-State: ACgBeo3onEXbRYzFf/uuRnkebTvsk2kOHR6yfBSbQKmEPUwWryF24k4Q hk635Df+PFl8MVOqb8rxdXjqVFUp9KQ= X-Google-Smtp-Source: AA6agR7d+hs1rjTBuo0ng26M2s4vjf4MJ+0MxAGfMX5aAYb4Lj2HKAXTXrXqg8S7mARNkFyZIostJw== X-Received: by 2002:a5d:5047:0:b0:226:eb0a:d2d5 with SMTP id h7-20020a5d5047000000b00226eb0ad2d5mr3195035wrt.558.1661992219294; Wed, 31 Aug 2022 17:30:19 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id l11-20020adff48b000000b0021e6277bc50sm15668521wro.36.2022.08.31.17.30.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 17:30:18 -0700 (PDT) Message-Id: In-Reply-To: References: Date: Thu, 01 Sep 2022 00:29:56 +0000 Subject: [PATCH 18/18] t: retire unused chainlint.sed Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Fabian Stelzer , Johannes Schindelin , Eric Sunshine , Eric Sunshine Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Eric Sunshine From: Eric Sunshine Retire chainlint.sed since it has been replaced by a more accurate and functional &&-chain "linter", thus is no longer used. Signed-off-by: Eric Sunshine --- t/chainlint.sed | 399 ------------------------------------------------ 1 file changed, 399 deletions(-) delete mode 100644 t/chainlint.sed diff --git a/t/chainlint.sed b/t/chainlint.sed deleted file mode 100644 index dc4ce37cb51..00000000000 --- a/t/chainlint.sed +++ /dev/null @@ -1,399 +0,0 @@ -#------------------------------------------------------------------------------ -# Detect broken &&-chains in tests. -# -# At present, only &&-chains in subshells are examined by this linter; -# top-level &&-chains are instead checked directly by the test framework. Like -# the top-level &&-chain linter, the subshell linter (intentionally) does not -# check &&-chains within {...} blocks. -# -# Checking for &&-chain breakage is done line-by-line by pure textual -# inspection. -# -# Incomplete lines (those ending with "\") are stitched together with following -# lines to simplify processing, particularly of "one-liner" statements. -# Top-level here-docs are swallowed to avoid false positives within the -# here-doc body, although the statement to which the here-doc is attached is -# retained. -# -# Heuristics are used to detect end-of-subshell when the closing ")" is cuddled -# with the final subshell statement on the same line: -# -# (cd foo && -# bar) -# -# in order to avoid misinterpreting the ")" in constructs such as "x=$(...)" -# and "case $x in *)" as ending the subshell. -# -# Lines missing a final "&&" are flagged with "?!AMP?!", as are lines which -# chain commands with ";" internally rather than "&&". A line may be flagged -# for both violations. -# -# Detection of a missing &&-link in a multi-line subshell is complicated by the -# fact that the last statement before the closing ")" must not end with "&&". -# Since processing is line-by-line, it is not known whether a missing "&&" is -# legitimate or not until the _next_ line is seen. To accommodate this, within -# multi-line subshells, each line is stored in sed's "hold" area until after -# the next line is seen and processed. If the next line is a stand-alone ")", -# then a missing "&&" on the previous line is legitimate; otherwise a missing -# "&&" is a break in the &&-chain. -# -# ( -# cd foo && -# bar -# ) -# -# In practical terms, when "bar" is encountered, it is flagged with "?!AMP?!", -# but when the stand-alone ")" line is seen which closes the subshell, the -# "?!AMP?!" violation is removed from the "bar" line (retrieved from the "hold" -# area) since the final statement of a subshell must not end with "&&". The -# final line of a subshell may still break the &&-chain by using ";" internally -# to chain commands together rather than "&&", but an internal "?!AMP?!" is -# never removed from a line even though a line-ending "?!AMP?!" might be. -# -# Care is taken to recognize the last _statement_ of a multi-line subshell, not -# necessarily the last textual _line_ within the subshell, since &&-chaining -# applies to statements, not to lines. Consequently, blank lines, comment -# lines, and here-docs are swallowed (but not the command to which the here-doc -# is attached), leaving the last statement in the "hold" area, not the last -# line, thus simplifying &&-link checking. -# -# The final statement before "done" in for- and while-loops, and before "elif", -# "else", and "fi" in if-then-else likewise must not end with "&&", thus -# receives similar treatment. -# -# Swallowing here-docs with arbitrary tags requires a bit of finesse. When a -# line such as "cat <cat <\n\1$/ is attempted to see if -# the content inside "<...>" matches the entirety of the newly-read line. For -# instance, if the next line read is "some data", when concatenated with the -# target line, it becomes "cat <cat <" does match the text following the -# newline, thus the closing here-doc tag has been found. The closing tag line -# and the "<...>" prefix on the target line are then discarded, leaving just -# the target line "cat <\1\2/ - :hered - N - /^<\([^>]*\)>.*\n[ ]*\1[ ]*$/!{ - s/\n.*$// - bhered - } - s/^<[^>]*>// - s/\n.*$// -} -:notdoc - -# one-liner "(...) &&" -/^[ ]*!*[ ]*(..*)[ ]*&&[ ]*$/boneline - -# same as above but without trailing "&&" -/^[ ]*!*[ ]*(..*)[ ]*$/boneline - -# one-liner "(...) >x" (or "2>x" or "|&]/boneline - -# multi-line "(...\n...)" -/^[ ]*(/bsubsh - -# innocuous line -- print it and advance to next line -b - -# found one-liner "(...)" -- mark suspect if it uses ";" internally rather than -# "&&" (but not ";" in a string) -:oneline -/;/{ - /"[^"]*;[^"]*"/!s/;/; ?!AMP?!/ -} -b - -:subsh -# bare "(" line? -- stash for later printing -/^[ ]*([ ]*$/ { - h - bnextln -} -# "(..." line -- "(" opening subshell cuddled with command; temporarily replace -# "(" with sentinel "^" and process the line as if "(" had been seen solo on -# the preceding line; this temporary replacement prevents several rules from -# accidentally thinking "(" introduces a nested subshell; "^" is changed back -# to "(" at output time -x -s/.*// -x -s/(/^/ -bslurp - -:nextln -N -s/.*\n// - -:slurp -# incomplete line "...\" -/\\$/bicmplte -# multi-line quoted string "...\n..."? -/"/bdqstr -# multi-line quoted string '...\n...'? (but not contraction in string "it's") -/'/{ - /"[^'"]*'[^'"]*"/!bsqstr -} -:folded -# here-doc -- swallow it (but not "<<" in a string) -/<<-*[ ]*[\\'"]*[A-Za-z0-9_]/{ - /"[^"]*<<[^"]*"/!bheredoc -} -# comment or empty line -- discard since final non-comment, non-empty line -# before closing ")", "done", "elsif", "else", or "fi" will need to be -# re-visited to drop "suspect" marking since final line of those constructs -# legitimately lacks "&&", so "suspect" mark must be removed -/^[ ]*#/bnextln -/^[ ]*$/bnextln -# in-line comment -- strip it (but not "#" in a string, Bash ${#...} array -# length, or Perforce "//depot/path#42" revision in filespec) -/[ ]#/{ - /"[^"]*#[^"]*"/!s/[ ]#.*$// -} -# one-liner "case ... esac" -/^[ ^]*case[ ]*..*esac/bchkchn -# multi-line "case ... esac" -/^[ ^]*case[ ]..*[ ]in/bcase -# multi-line "for ... done" or "while ... done" -/^[ ^]*for[ ]..*[ ]in/bcont -/^[ ^]*while[ ]/bcont -/^[ ]*do[ ]/bcont -/^[ ]*do[ ]*$/bcont -/;[ ]*do/bcont -/^[ ]*done[ ]*&&[ ]*$/bdone -/^[ ]*done[ ]*$/bdone -/^[ ]*done[ ]*[<>|]/bdone -/^[ ]*done[ ]*)/bdone -/||[ ]*exit[ ]/bcont -/||[ ]*exit[ ]*$/bcont -# multi-line "if...elsif...else...fi" -/^[ ^]*if[ ]/bcont -/^[ ]*then[ ]/bcont -/^[ ]*then[ ]*$/bcont -/;[ ]*then/bcont -/^[ ]*elif[ ]/belse -/^[ ]*elif[ ]*$/belse -/^[ ]*else[ ]/belse -/^[ ]*else[ ]*$/belse -/^[ ]*fi[ ]*&&[ ]*$/bdone -/^[ ]*fi[ ]*$/bdone -/^[ ]*fi[ ]*[<>|]/bdone -/^[ ]*fi[ ]*)/bdone -# nested one-liner "(...) &&" -/^[ ^]*(.*)[ ]*&&[ ]*$/bchkchn -# nested one-liner "(...)" -/^[ ^]*(.*)[ ]*$/bchkchn -# nested one-liner "(...) >x" (or "2>x" or "|]/bchkchn -# nested multi-line "(...\n...)" -/^[ ^]*(/bnest -# multi-line "{...\n...}" -/^[ ^]*{/bblock -# closing ")" on own line -- exit subshell -/^[ ]*)/bclssolo -# "$((...))" -- arithmetic expansion; not closing ")" -/\$(([^)][^)]*))[^)]*$/bchkchn -# "$(...)" -- command substitution; not closing ")" -/\$([^)][^)]*)[^)]*$/bchkchn -# multi-line "$(...\n...)" -- command substitution; treat as nested subshell -/\$([^)]*$/bnest -# "=(...)" -- Bash array assignment; not closing ")" -/=(/bchkchn -# closing "...) &&" -/)[ ]*&&[ ]*$/bclose -# closing "...)" -/)[ ]*$/bclose -# closing "...) >x" (or "2>x" or "|]/bclose -:chkchn -# mark suspect if line uses ";" internally rather than "&&" (but not ";" in a -# string and not ";;" in one-liner "case...esac") -/;/{ - /;;/!{ - /"[^"]*;[^"]*"/!s/;/; ?!AMP?!/ - } -} -# line ends with pipe "...|" -- valid; not missing "&&" -/|[ ]*$/bcont -# missing end-of-line "&&" -- mark suspect -/&&[ ]*$/!s/$/ ?!AMP?!/ -:cont -# retrieve and print previous line -x -s/^\([ ]*\)^/\1(/ -s/?!HERE?!/<\1?!HERE?!\2\3/ -:hdocsub -N -/^<\([^>]*\)>.*\n[ ]*\1[ ]*$/!{ - s/\n.*$// - bhdocsub -} -s/^<[^>]*>// -s/\n.*$// -bfolded - -# found "case ... in" -- pass through untouched -:case -x -s/^\([ ]*\)^/\1(/ -s/?!HERE?!/<