diff --git a/DESIGN.md b/DESIGN.md index c17307b8..7b2c60c5 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -899,8 +899,12 @@ staleness story. **What a scan may cover.** A scan runs from the rule's directory with the rest of the command stripped, so it may carry a word only from the invocation it reproduces, and it can reproduce an -invocation only when it and every invocation before it is a compile the scanner recognizes — one -scan per invocation of that leading prefix. Refusal is decidable from the token stream; anything +invocation only when it and every invocation before it is a compile the scanner recognizes or an +invocation it proves inert — one scan per compile of that leading prefix. **Reproduce or prove +inert — never silently drop**: what precedes a compile is kept in the scan (the wrapper, the +compile's own `NAME=VALUE` words) or shown to change nothing the compile reads (an invocation that +runs nothing, or only announces); a word dropped because it looked harmless is how a scan comes to +preprocess a tree the compile never saw. Refusal is decidable from the token stream; anything finer requires modeling the shell. A redirection ends what the scan may read from an invocation but divides no invocation — only a control operator starts a new one. Three consequences bind every future change here. A scan travels with the object it covers, because only the scanner's diff --git a/include/pup/graph/scanners/dep_words.hpp b/include/pup/graph/scanners/dep_words.hpp index aa7c09fe..d793b5ea 100644 --- a/include/pup/graph/scanners/dep_words.hpp +++ b/include/pup/graph/scanners/dep_words.hpp @@ -62,6 +62,20 @@ auto is_flag_barrier(std::string_view word) -> bool; [[nodiscard]] auto is_invocation_separator(std::string_view word) -> bool; +/// True for a leading `NAME=VALUE` word the scan keeps in front of the compiler, so the scan +/// preprocesses under the environment the compile did. A value carrying shell syntax is not one: +/// `FOO=$(id)` would be evaluated a second time, and `FOO=1;` ends the assignment where putup's +/// whitespace split did not. +[[nodiscard]] +auto is_env_assignment_word(std::string_view word) -> bool; + +/// True for an invocation a scan may step over: one that runs nothing, or one running a program +/// that only writes to its output stream, no word of which carries shell syntax putup's whitespace +/// split left unresolved. Such an invocation changes neither the directory nor the environment nor +/// any file a later compile reads, so it need not be reproduced. +[[nodiscard]] +auto is_scan_transparent(std::span invocation) -> bool; + /// The invocations a command's control operators divide its words into, in order. An invocation is /// empty where two operators meet or where the command begins with one; a word list with no /// operator is one invocation, and a trailing operator adds none. Which of them a scan may draw diff --git a/spec/requirements/dep-scan.ears.md b/spec/requirements/dep-scan.ears.md index bf7dfb9f..8cc2b6ec 100644 --- a/spec/requirements/dep-scan.ears.md +++ b/spec/requirements/dep-scan.ears.md @@ -16,10 +16,12 @@ accesses instead — so every requirement here is `putup-only` and no citation i Recognition is deliberately narrow. putup runs a scan from the rule's directory without the rest of the command, so it can only scan a command whose compile it can reproduce in that state; a prefix that changes the working directory or the environment makes the reproduction -false rather than incomplete. A recognized compiler wrapper is the one exception, because it -changes neither and the scan keeps it. Declining is therefore correct — but declining in -silence leaves a rule whose headers are never recorded, which is why the last requirement here -exists. +false rather than incomplete. What precedes a compile is therefore either reproduced or proven +inert, and never silently dropped: a recognized compiler wrapper and the compile's own leading +environment assignments are reproduced, because the scan keeps them; an invocation that runs +nothing, or only announces, is inert. Anything else is opaque. Declining is therefore correct — +but declining in silence leaves a rule whose headers are never recorded, which is why the last +requirement here exists. The unit that narrowness is measured in is the invocation: a command's control operators divide it into invocations, each gets its own scan, and a scan may draw a word only from the invocation it @@ -49,11 +51,38 @@ Which commands a scan is generated for. - discharge: test "A command whose first invocation is not a compile is scanned nowhere" - discharge: test "ClangClScanner scans the prefix before an invocation that is not a compile" -Where a command runs an invocation that is not a compile putup recognizes, whether a loop, a -directory change, an environment assignment, a link or any other program, putup shall generate no -dependency scan for that invocation or for any that follows it, because the scan runs from the -rule's directory with the rest of the command stripped and past such an invocation would -preprocess in a state the compile never had. +Where a command runs an invocation that is neither a compile putup recognizes nor one it proves +inert, whether a loop, a directory change, a standalone environment assignment, a link or any +other program, putup shall generate no dependency scan for that invocation or for any that +follows it, because the scan runs from the rule's directory with the rest of the command +stripped and past such an invocation would preprocess in a state the compile never had. + +### REQ-SCAN-KEEPS-ASSIGNMENT-PREFIX + +- conformance: putup-only +- discharge: test "GccScanner keeps a leading environment assignment in the scan" +- discharge: test "ClangClScanner reads the same prefix its sibling does" + +Where a compile's own invocation begins with `NAME=VALUE` words carrying no shell syntax putup's +word split left unresolved, putup shall keep those words in front of the compiler in the scan it builds, because the scan then preprocesses +under the environment the compile did, including a variable such as `CPATH` that moves the header +search and that a dropped word would resolve against a path the compile never read; an assignment +standing as its own invocation scopes to the rest of the command line rather than to one +invocation, so it is opaque instead. + +### REQ-SCAN-TRANSPARENT-INVOCATION + +- conformance: putup-only +- discharge: test "GccScanner scans past an invocation that changes nothing" +- discharge: test "ClangClScanner reads the same prefix its sibling does" +- discharge: test "A separator with nothing after it begins no invocation" + +Where an invocation before a compile runs nothing, or runs only a program that writes to its +output stream — `echo`, `true`, `:` — with no word that redirects and no shell syntax putup's word +split left unresolved, putup shall generate the scans for the compiles after it as if it were +absent, because such an invocation changes neither the directory, nor the environment, nor any +file the compile reads; where it redirects, the file it writes may be the very header the compile +includes, so it is opaque even though its program is one of these. ### REQ-SCAN-PER-INVOCATION diff --git a/src/graph/scanners/clang_cl.cpp b/src/graph/scanners/clang_cl.cpp index 3e0c94ab..b1b3f188 100644 --- a/src/graph/scanners/clang_cl.cpp +++ b/src/graph/scanners/clang_cl.cpp @@ -116,15 +116,19 @@ auto normalize_flag_path_into(Buf& out, std::string_view flag) -> void out += flag; } -/// Where the driver stands in one invocation -- first, or behind one recognized wrapper. +/// Where the driver stands in one invocation -- first, or behind leading environment assignments +/// and one recognized wrapper. The words before it are the scan's to keep. auto driver_index(std::span invocation) -> std::optional { - if (invocation.empty()) { + auto idx = std::size_t { 0 }; + while (idx < invocation.size() && is_env_assignment_word(invocation[idx])) { + ++idx; + } + if (idx >= invocation.size()) { return std::nullopt; } - auto idx = std::size_t { 0 }; - if (is_compiler_wrapper(program_basename(invocation[0])) && invocation.size() > 1) { - idx = 1; + if (is_compiler_wrapper(program_basename(invocation[idx])) && idx + 1 < invocation.size()) { + ++idx; } if (!is_clang_cl_name(program_basename(invocation[idx]))) { return std::nullopt; @@ -141,15 +145,22 @@ auto is_recognized_compile(std::span invocation) -> bool return std::ranges::any_of(invocation.subspan(*idx + 1), is_compile_flag); } -/// The leading invocations a scan can reproduce from the rule's directory: one that is not a -/// compile may change the directory or the environment, so it and everything after it are out of -/// reach (#356). -auto compile_prefix(std::span const> invocations) +/// The leading invocations a scan can draw from, ending at the last compile in them: an +/// invocation that is neither a compile nor scan-transparent may change the directory or the +/// environment, so it and everything after it are out of reach (#356, #352). Ending at the last +/// compile is what keeps a non-empty prefix and a non-empty scan set the same answer. +auto scannable_prefix(std::span const> invocations) -> std::span const> { auto length = std::size_t { 0 }; - while (length < invocations.size() && is_recognized_compile(invocations[length])) { - ++length; + for (auto i = std::size_t { 0 }; i < invocations.size(); ++i) { + if (is_recognized_compile(invocations[i])) { + length = i + 1; + continue; + } + if (!is_scan_transparent(invocations[i])) { + break; + } } return invocations.first(length); } @@ -159,12 +170,12 @@ auto compile_prefix(std::span const> invocatio auto matches_clang_cl_compile(std::string_view command) -> bool { auto tokens = tokenize_command(global_pool().intern(command)); - return !compile_prefix(tokens.invocations()).empty(); + return !scannable_prefix(tokens.invocations()).empty(); } auto ClangClScanner::matches(CommandInfo const& /*cmd*/, CommandTokens const& tokens) const -> bool { - return !compile_prefix(tokens.invocations()).empty(); + return !scannable_prefix(tokens.invocations()).empty(); } auto ClangClScanner::has_dep_flags(CommandTokens const& tokens) const -> bool @@ -182,7 +193,7 @@ auto ClangClScanner::build_dep_scans(CommandInfo const& /*cmd*/, CommandTokens c auto& pool = global_pool(); auto scans = Vec {}; - for (auto invocation : compile_prefix(tokens.invocations())) { + for (auto invocation : scannable_prefix(tokens.invocations())) { auto driver_idx = driver_index(invocation); if (!driver_idx) { continue; diff --git a/src/graph/scanners/dep_words.cpp b/src/graph/scanners/dep_words.cpp index cc0391ad..612ff794 100644 --- a/src/graph/scanners/dep_words.cpp +++ b/src/graph/scanners/dep_words.cpp @@ -69,6 +69,28 @@ auto quote_windows_into(Buf& out, std::string_view s) -> void out += '"'; } +auto is_name_start(char c) -> bool +{ + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_'; +} + +auto is_name_char(char c) -> bool +{ + return is_name_start(c) || (c >= '0' && c <= '9'); +} + +auto is_transparent_program(std::string_view name) -> bool +{ + return name == "echo" || name == "true" || name == ":"; +} + +// Words are split on whitespace, so one word can carry shell syntax that was never classified as +// such -- `FOO=1;` is an assignment the shell ends, `x>f.h` a redirection it never stood alone as. +auto hides_shell_syntax(std::string_view word) -> bool +{ + return has_shell_special(word) || word.find_first_of(";&|<>\n") != std::string_view::npos; +} + } // namespace auto is_compiler_wrapper(std::string_view name) -> bool @@ -132,6 +154,26 @@ auto is_invocation_separator(std::string_view word) -> bool return word == "&&" || word == "||" || word == ";" || word == "|" || word == "&"; } +auto is_env_assignment_word(std::string_view word) -> bool +{ + auto eq = word.find('='); + if (eq == std::string_view::npos || eq == 0 || !is_name_start(word[0])) { + return false; + } + return std::ranges::all_of(word.substr(0, eq), is_name_char) && !hides_shell_syntax(word); +} + +auto is_scan_transparent(std::span invocation) -> bool +{ + if (invocation.empty()) { + return true; + } + if (!is_transparent_program(program_basename(invocation[0]))) { + return false; + } + return std::ranges::none_of(invocation, hides_shell_syntax); +} + auto split_invocations(std::span words) -> Vec> { auto result = Vec> {}; @@ -143,7 +185,7 @@ auto split_invocations(std::span words) -> Vec bool return is_blank_word(flag) || has_shell_special(flag) || leads_any(hazard_flags, flag); } -/// Where the compiler stands in one invocation -- first, or behind one recognized wrapper. +/// Where the compiler stands in one invocation -- first, or behind leading environment +/// assignments and one recognized wrapper. The words before it are the scan's to keep. auto compiler_index(std::span invocation) -> std::optional { - if (invocation.empty()) { + auto idx = std::size_t { 0 }; + while (idx < invocation.size() && is_env_assignment_word(invocation[idx])) { + ++idx; + } + if (idx >= invocation.size()) { return std::nullopt; } - auto idx = std::size_t { 0 }; - if (is_compiler_wrapper(program_basename(invocation[0])) && invocation.size() > 1) { - idx = 1; + if (is_compiler_wrapper(program_basename(invocation[idx])) && idx + 1 < invocation.size()) { + ++idx; } if (!is_compiler_name(program_basename(invocation[idx]))) { return std::nullopt; @@ -130,15 +134,22 @@ auto is_recognized_compile(std::span invocation) -> bool return std::ranges::any_of(invocation.subspan(*idx + 1), [](auto w) { return w == "-c"; }); } -/// The leading invocations a scan can reproduce from the rule's directory: one that is not a -/// compile may change the directory or the environment, so it and everything after it are out of -/// reach (#356). -auto compile_prefix(std::span const> invocations) +/// The leading invocations a scan can draw from, ending at the last compile in them: an +/// invocation that is neither a compile nor scan-transparent may change the directory or the +/// environment, so it and everything after it are out of reach (#356, #352). Ending at the last +/// compile is what keeps a non-empty prefix and a non-empty scan set the same answer. +auto scannable_prefix(std::span const> invocations) -> std::span const> { auto length = std::size_t { 0 }; - while (length < invocations.size() && is_recognized_compile(invocations[length])) { - ++length; + for (auto i = std::size_t { 0 }; i < invocations.size(); ++i) { + if (is_recognized_compile(invocations[i])) { + length = i + 1; + continue; + } + if (!is_scan_transparent(invocations[i])) { + break; + } } return invocations.first(length); } @@ -148,12 +159,12 @@ auto compile_prefix(std::span const> invocatio auto matches_gcc_compile(std::string_view command) -> bool { auto tokens = tokenize_command(global_pool().intern(command)); - return !compile_prefix(tokens.invocations()).empty(); + return !scannable_prefix(tokens.invocations()).empty(); } auto GccScanner::matches(CommandInfo const& /*cmd*/, CommandTokens const& tokens) const -> bool { - return !compile_prefix(tokens.invocations()).empty(); + return !scannable_prefix(tokens.invocations()).empty(); } auto GccScanner::has_dep_flags(CommandTokens const& tokens) const -> bool @@ -185,7 +196,7 @@ auto GccScanner::build_dep_scans(CommandInfo const& /*cmd*/, CommandTokens const auto& pool = global_pool(); auto scans = Vec {}; - for (auto invocation : compile_prefix(tokens.invocations())) { + for (auto invocation : scannable_prefix(tokens.invocations())) { auto compiler_idx = compiler_index(invocation); if (!compiler_idx) { continue; diff --git a/test/e2e/fixtures/unscanned_compile/Tupfile.fixture b/test/e2e/fixtures/unscanned_compile/Tupfile.fixture index c7da9fb0..20a7c824 100644 --- a/test/e2e/fixtures/unscanned_compile/Tupfile.fixture +++ b/test/e2e/fixtures/unscanned_compile/Tupfile.fixture @@ -1,6 +1,8 @@ # scanned: an ordinary compile : plain.c |> gcc -c %f -o %o |> %B.o -# unscanned: the compile is not the command's first invocation -: hidden.c |> echo building %f && gcc -c %f -o %o |> %B.o +# scanned: the compile follows an invocation that changes nothing it reads +: shown.c |> echo building %f && gcc -c %f -o %o |> %B.o +# unscanned: the compile follows an invocation putup cannot reproduce +: hidden.c |> cd . && gcc -c %f -o %o |> %B.o # not a finding: the compile writes its own depfile : owndep.c |> gcc -MD -MF %o.d -c %f -o %o |> %B.o diff --git a/test/e2e/fixtures/unscanned_compile/shown.c b/test/e2e/fixtures/unscanned_compile/shown.c new file mode 100644 index 00000000..4afeba5b --- /dev/null +++ b/test/e2e/fixtures/unscanned_compile/shown.c @@ -0,0 +1,2 @@ +#include "shown.h" +int shown(void){return SHOWN_VALUE;} diff --git a/test/e2e/fixtures/unscanned_compile/shown.h b/test/e2e/fixtures/unscanned_compile/shown.h new file mode 100644 index 00000000..f69762cd --- /dev/null +++ b/test/e2e/fixtures/unscanned_compile/shown.h @@ -0,0 +1 @@ +#define SHOWN_VALUE 1 diff --git a/test/unit/test_dep_scanner.cpp b/test/unit/test_dep_scanner.cpp index 5b87fdcd..d3ccc7a9 100644 --- a/test/unit/test_dep_scanner.cpp +++ b/test/unit/test_dep_scanner.cpp @@ -1394,9 +1394,9 @@ TEST_CASE("A separator with nothing after it begins no invocation", "[dep_scanne REQUIRE(scanners::matches_gcc_compile("gcc -c a.c -o a.o &")); } - SECTION("a leading separator still forfeits the scan") + SECTION("a leading separator begins an invocation that runs nothing") { - REQUIRE(!scanners::matches_gcc_compile("; gcc -c a.c -o a.o")); + REQUIRE(scanners::matches_gcc_compile("; gcc -c a.c -o a.o")); } } @@ -1431,6 +1431,130 @@ TEST_CASE("ClangClScanner scans the prefix before an invocation that is not a co REQUIRE(pup::global_pool().get(scans[0].object) == "a.obj"); } +TEST_CASE("GccScanner keeps a leading environment assignment in the scan", "[dep_scanner][gcc]") +{ + auto scanner = scanners::GccScanner {}; + + SECTION("the assignment precedes the compiler in the scan") + { + auto scan = only_scan(scanner, gcc_compile(80, "FOO=1 gcc -c a.c -o a.o")); + REQUIRE(pup::global_pool().get(scan.command) == "FOO=1 gcc -M a.c"); + REQUIRE(pup::global_pool().get(scan.object) == "a.o"); + } + + SECTION("a variable that moves the header search is kept, not dropped") + { + // Keeping it is what makes the scan sound: dropping CPATH would search a path the + // compile never searched, which is #355's redirection in a new place. + auto scan = only_scan(scanner, gcc_compile(81, "CPATH=inc gcc -c a.c -o a.o")); + REQUIRE(pup::global_pool().get(scan.command) == "CPATH=inc gcc -M a.c"); + } + + SECTION("every assignment before a wrapper is kept") + { + auto scan = only_scan(scanner, gcc_compile(82, "A=1 B=2 ccache gcc -c a.c -o a.o")); + REQUIRE(pup::global_pool().get(scan.command) == "A=1 B=2 ccache gcc -M a.c"); + } + + SECTION("the matcher answers as the builder does") + { + REQUIRE(scanners::matches_gcc_compile("FOO=1 gcc -c a.c -o a.o")); + } + + SECTION("a standalone assignment is not absorbed") + { + // The prefix form scopes to one command; the standalone form persists into the rest of + // the line, so no scan of a later invocation reproduces it. + REQUIRE(!scanners::matches_gcc_compile("FOO=1; gcc -c a.c -o a.o")); + REQUIRE(scans_of(scanner, gcc_compile(83, "FOO=1; gcc -c a.c -o a.o")).empty()); + } + + SECTION("a value the scan's own shell would expand a second time is not absorbed") + { + REQUIRE(!scanners::matches_gcc_compile("FOO=$(date) gcc -c a.c -o a.o")); + } +} + +TEST_CASE("GccScanner scans past an invocation that changes nothing", "[dep_scanner][gcc]") +{ + auto scanner = scanners::GccScanner {}; + + SECTION("an invocation with no words") + { + auto scan = only_scan(scanner, gcc_compile(84, "; gcc -c a.c -o a.o")); + REQUIRE(pup::global_pool().get(scan.command) == "gcc -M a.c"); + } + + SECTION("an announcement") + { + auto scan = only_scan(scanner, gcc_compile(85, "echo building a.c && gcc -c a.c -o a.o")); + REQUIRE(pup::global_pool().get(scan.command) == "gcc -M a.c"); + } + + SECTION("the other programs that do nothing") + { + REQUIRE(scanners::matches_gcc_compile("true && gcc -c a.c -o a.o")); + REQUIRE(scanners::matches_gcc_compile(": && gcc -c a.c -o a.o")); + } + + SECTION("an announcement that writes a file is opaque") + { + // The write may create the very header the compile reads. + REQUIRE(!scanners::matches_gcc_compile("echo x > f.h && gcc -c f.c -o f.o")); + REQUIRE(scans_of(scanner, gcc_compile(86, "echo x >f.h && gcc -c f.c -o f.o")).empty()); + } + + SECTION("an announcement that runs a program is opaque") + { + REQUIRE(!scanners::matches_gcc_compile("echo $(gen) && gcc -c a.c -o a.o")); + } + + SECTION("a program that is neither compile nor announcement still ends the prefix") + { + REQUIRE(scans_of(scanner, gcc_compile(87, "cat x && gcc -c a.c -o a.o")).empty()); + } + + SECTION("a command that only announces is claimed by no scanner") + { + REQUIRE(!scanners::matches_gcc_compile("echo nothing to do")); + } + + SECTION("an announcement after the last compile adds no scan") + { + auto scans = scans_of(scanner, gcc_compile(88, "gcc -c a.c -o a.o && echo done")); + REQUIRE(scans.size() == 1); + REQUIRE(pup::global_pool().get(scans[0].command) == "gcc -M a.c"); + } +} + +TEST_CASE("ClangClScanner reads the same prefix its sibling does", "[dep_scanner][clang_cl]") +{ + auto scanner = scanners::ClangClScanner {}; + + SECTION("a leading assignment is kept in the scan") + { + auto scan = only_scan(scanner, clang_cl_compile(89, "INCLUDE=inc clang-cl -c a.cpp -o a.obj")); + REQUIRE(pup::global_pool().get(scan.command) == "INCLUDE=inc clang-cl /clang:-M a.cpp"); + REQUIRE(pup::global_pool().get(scan.object) == "a.obj"); + } + + SECTION("an announcement does not end the prefix") + { + auto scan = only_scan(scanner, clang_cl_compile(90, "echo building && clang-cl -c a.cpp -o a.obj")); + REQUIRE(pup::global_pool().get(scan.command) == "clang-cl /clang:-M a.cpp"); + } + + SECTION("an announcement that writes a file is opaque") + { + REQUIRE(!scanners::matches_clang_cl_compile("echo x > f.h && clang-cl -c f.cpp -o f.obj")); + } + + SECTION("a standalone assignment is not absorbed") + { + REQUIRE(!scanners::matches_clang_cl_compile("FOO=1; clang-cl -c a.cpp -o a.obj")); + } +} + TEST_CASE("a line continuation never reaches the scan command", "[dep_scanner]") { SECTION("a continuation the command text kept is not a word the scan carries") diff --git a/test/unit/test_e2e.cpp b/test/unit/test_e2e.cpp index 8f49393c..2593dd66 100644 --- a/test/unit/test_e2e.cpp +++ b/test/unit/test_e2e.cpp @@ -11244,7 +11244,7 @@ SCENARIO("A compile-shaped rule with no dependency scan is reported", "[e2e][str { // The scan is declined correctly — putup cannot reproduce the prefix's shell state — but // declining in silence leaves a rule whose headers are never recorded (#352). - GIVEN("a project with a scanned compile, an unscanned compile, and a self-depfiling compile") + GIVEN("a project with a scanned compile, an announced compile, an unscanned compile, and a self-depfiling compile") { auto f = E2EFixture { "unscanned_compile" }; REQUIRE(f.init().success()); @@ -11253,7 +11253,7 @@ SCENARIO("A compile-shaped rule with no dependency scan is reported", "[e2e][str { auto result = f.pup({ "parse" }); - THEN("the unscanned rule is named and the other two are not") + THEN("the unscanned rule is named and the other three are not") { INFO("stdout: " << result.stdout_output); INFO("stderr: " << result.stderr_output); @@ -11261,6 +11261,7 @@ SCENARIO("A compile-shaped rule with no dependency scan is reported", "[e2e][str REQUIRE(result.stderr_output.find("no dependency scan") != std::string::npos); REQUIRE(result.stderr_output.find("hidden.o") != std::string::npos); REQUIRE(result.stderr_output.find("plain.o") == std::string::npos); + REQUIRE(result.stderr_output.find("shown.o") == std::string::npos); REQUIRE(result.stderr_output.find("owndep.o") == std::string::npos); } }