diff --git a/DESIGN.md b/DESIGN.md index 1e9a3c42..f73fc106 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -480,7 +480,7 @@ struct EvalContext { | Flag | Meaning | Example | |------|---------|---------| | `%f` | All inputs | `main.c util.c` | -| `%i` | All inputs (alias) | `main.c util.c` | +| `%i` | Order-only inputs | `config.h` | | `%o` | All outputs | `main.o` | | `%O` | Output basename | `main` | | `%b` | Basename with ext, every input | `main.c util.c` | @@ -488,6 +488,7 @@ struct EvalContext { | `%e` | Extension (foreach, file must have one) | `c` | | `%d` | Directory | `src` | | `%Nf` | Nth input | `%1f` → first input | +| `%Ni` | Nth order-only input | `%1i` → first order-only input | **Expansion pipeline:** @@ -1037,7 +1038,7 @@ The index uses **instruction-based command storage** for significant space savin **Instruction deduplication**: Bang macros like `!cc = |> $(CC) -c %f -o %o |>` produce the same instruction for all source files. With 1000 C files, instead of storing 1000 nearly-identical command strings, v8 stores 1 instruction + 1000 operand records. -**Lazy reconstruction**: Full command strings are computed on demand via `expand_instruction()`, which substitutes operand paths into the instruction pattern. CommandNode stores `instruction_id` (the pattern) plus explicit `inputs`/`outputs` operand vectors. This keeps index loading fast and avoids storing redundant expanded strings. +**Lazy reconstruction**: Full command strings are computed on demand via `expand_instruction()`, which substitutes operand paths into the instruction pattern. CommandNode stores `instruction_id` (the pattern) plus explicit `inputs`/`order_only_inputs`/`outputs` operand vectors. This keeps index loading fast and avoids storing redundant expanded strings. Version history: - v1: Initial format with full path strings diff --git a/docs/reference.md b/docs/reference.md index 8d4d6bdd..3400ef4b 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -1006,7 +1006,7 @@ Pattern flags are placeholders expanded at build time. | Flag | Description | Example | |------|-------------|---------| | `%f` | All input files; the rule must have at least one | `gcc %f` → `gcc foo.c bar.c` | -| `%i` | All inputs (alias for %f) | `gcc %i` → `gcc foo.c bar.c` | +| `%i` | The order-only inputs written after `\|`; the rule must have at least one, and only a command or display string may spell it | `: a.c \| z.h y.h \|> cat %i` → `cat z.h y.h` | | `%o` | All output files; the rule must declare at least one | `ar rcs %o` → `ar rcs foo.a foo.map` | | `%O` | The output without its extension, keeping its directory — command string or extra outputs section, and only with exactly one output | `sub/foo.so` → `sub/foo` | | `%b` | Basename of every input, with extension; the rule must have at least one | `src/foo.c bar.c` → `foo.c bar.c` | @@ -1022,6 +1022,7 @@ Pattern flags are placeholders expanded at build time. | `%2f` | Second input file | | `%1b` | Basename of the first input | | `%1B` | Basename of the first input without its extension | +| `%1i` | First order-only input | | `%1o` | First output file | | `%2o` | Second output file | @@ -1029,7 +1030,6 @@ The number must be at least 1 and below 99 (tup's guard rejects 99 itself, despi naming the range 1-99), a letter must follow it, and that letter must be one of `f`, `b`, `B`, `o` or `i` — anything else is a parse error rather than literal text. A number that is in range but names an entry the rule does not have expands to nothing, as in tup. -`%Ni` (N-th order-only input) is not supported yet and is refused by name; see issue #426. **Examples:** @@ -2831,7 +2831,7 @@ CONFIG_RELEASE_LDFLAGS=-Wl,--gc-sections | Flag | Description | Example Input | Result | |------|-------------|---------------|--------| | `%f` | All inputs (rule must have one) | `foo.c bar.c` | `foo.c bar.c` | -| `%i` | All inputs (alias) | `foo.c bar.c` | `foo.c bar.c` | +| `%i` | Order-only inputs (rule must have one; command or display string only) | `a.c \| z.h y.h` | `z.h y.h` | | `%b` | Basename with ext, every input (rule must have one) | `src/foo.c bar.c` | `foo.c bar.c` | | `%B` | Basename no ext, every input (rule must have one) | `src/foo.c bar.c` | `foo bar` | | `%e` | Extension (foreach, file must have one) | `foo.c` | `c` | @@ -2852,6 +2852,7 @@ CONFIG_RELEASE_LDFLAGS=-Wl,--gc-sections | `%1f` | First input | | `%2f` | Second input | | `%3f` | Third input (etc.) | +| `%1i` | First order-only input | | `%1o` | First output | | `%2o` | Second output | | `%3o` | Third output (etc.) | diff --git a/include/pup/core/instruction.hpp b/include/pup/core/instruction.hpp index 93be69d5..f4cbd3ae 100644 --- a/include/pup/core/instruction.hpp +++ b/include/pup/core/instruction.hpp @@ -18,7 +18,7 @@ enum class AtomKind : std::uint8_t { Literal, GroupRef, AllInputs, - AllInputsAlias, + OrderOnlyInputs, InputBase, InputNoExt, InputExt, @@ -28,6 +28,7 @@ enum class AtomKind : std::uint8_t { NthInput, NthInputBase, NthInputNoExt, + NthOrderOnlyInput, NthOutput, }; @@ -119,9 +120,11 @@ auto fold_instruction(Instruction const& atoms, Site const& site) -> StringId site.append_group_ref(buf, atom.text()); break; case AtomKind::AllInputs: - case AtomKind::AllInputsAlias: site.append_all_inputs(buf); break; + case AtomKind::OrderOnlyInputs: + site.append_order_only_inputs(buf); + break; case AtomKind::InputBase: site.append_input_base(buf); break; @@ -149,6 +152,9 @@ auto fold_instruction(Instruction const& atoms, Site const& site) -> StringId case AtomKind::NthInputNoExt: site.append_nth_input_noext(buf, atom.operand()); break; + case AtomKind::NthOrderOnlyInput: + site.append_nth_order_only_input(buf, atom.operand()); + break; case AtomKind::NthOutput: site.append_nth_output(buf, atom.operand()); break; diff --git a/include/pup/graph/dag.hpp b/include/pup/graph/dag.hpp index 7711d22a..fded8a15 100644 --- a/include/pup/graph/dag.hpp +++ b/include/pup/graph/dag.hpp @@ -61,7 +61,7 @@ struct Guard { /// Command node - represents build commands /// Note: Type is determined by node_id::is_command(), not a stored field. /// Command string is reconstructed on demand via expand_instruction() from -/// instruction atoms + explicit operand NodeIds (inputs/outputs). +/// instruction atoms + explicit operand NodeIds (inputs/order-only inputs/outputs). struct CommandNode { NodeId id = 0; @@ -69,8 +69,9 @@ struct CommandNode { StringId source_dir = StringId::Empty; ///< Tupfile directory (relative to root, interned) Instruction instruction = {}; ///< Instruction atoms (e.g. "gcc -c %f -o %o") - TokenList inputs = {}; ///< Operand file NodeIds for %f, %b and %B expansion, grouped by written token - TokenList outputs = {}; ///< Operand file NodeIds for %o expansion, grouped by written token + TokenList inputs = {}; ///< Operand file NodeIds for %f, %b and %B expansion, grouped by written token + TokenList order_only_inputs = {}; ///< The rule's own order-only operands for %i expansion, grouped by written token + TokenList outputs = {}; ///< Operand file NodeIds for %o expansion, grouped by written token SortedIdVec exported_vars = {}; ///< Env vars to export to command (interned StringIds) @@ -205,6 +206,7 @@ auto get(Graph const& graph, NodeId id) -> typename get_storage::type; /// Role tags for composite-property access via view. struct Inputs { }; +struct OrderOnlyInputs { }; struct Outputs { }; struct ExportedVars { }; @@ -217,6 +219,10 @@ struct view_storage { using type = TokenList; }; template<> +struct view_storage { + using type = TokenList; +}; +template<> struct view_storage { using type = TokenList; }; diff --git a/include/pup/index/entry.hpp b/include/pup/index/entry.hpp index 530198f2..f00466ce 100644 --- a/include/pup/index/entry.hpp +++ b/include/pup/index/entry.hpp @@ -64,8 +64,9 @@ struct CommandEntry { /// carried this record past a change to one of its deps without re-running it. bool must_rerun = false; - TokenList inputs = {}; ///< Input file operands (for %f, %b and %B expansion), grouped by written token - TokenList outputs = {}; ///< Output file operands (for %o expansion), grouped by written token + TokenList inputs = {}; ///< Input file operands (for %f, %b and %B expansion), grouped by written token + TokenList order_only_inputs = {}; ///< The rule's own order-only operands (for %i expansion), grouped by written token + TokenList outputs = {}; ///< Output file operands (for %o expansion), grouped by written token /// Convert to raw format for serialization [[nodiscard]] @@ -85,6 +86,7 @@ struct CommandEntry { std::string_view display_str, std::string_view env_str, TokenList inputs, + TokenList order_only_inputs, TokenList outputs, std::size_t array_index ) -> Result; diff --git a/include/pup/index/format.hpp b/include/pup/index/format.hpp index af7de6e0..d5489297 100644 --- a/include/pup/index/format.hpp +++ b/include/pup/index/format.hpp @@ -70,6 +70,9 @@ inline constexpr auto INDEX_MAGIC = std::array { 'P', 'U', 'P', 'I' }; /// 255, so a rule with more operands recorded a prefix and the carried-forward record /// stopped seeing the rest change (issue #365). A v21 index parses at the wrong offsets /// under this layout, so it is not read. +/// 26 - A command's operand record carries a third list, the rule's own order-only operands +/// `%i` and `%Ni` spell, grouped by written token like the other two; before it `%i` +/// spelled the inputs (issue #461). A v25 record has no head words for it, so it is not read. /// Bump when a stale reader would misparse the bytes — any layout change — or wrong-join a key /// whose meaning changed; a change that only changes keys no-joins, and one re-run per affected /// command repairs it (#333, #335, #343). @@ -89,7 +92,7 @@ inline constexpr auto INDEX_MAGIC = std::array { 'P', 'U', 'P', 'I' }; /// `RawFileEntry::name_offset` is semantics-bearing on the same terms: `read_prior_paths` composes /// it into the paths `clean`/`distclean` delete and `reject_shadowed_sources` refuses a build over, /// so a name this reader cannot reproduce fails the record rather than reading as empty (#381). -inline constexpr auto INDEX_VERSION = std::uint32_t { 25 }; +inline constexpr auto INDEX_VERSION = std::uint32_t { 26 }; /// The oldest version whose `RawHeader` and `RawFileEntry` bytes mean what today's mean, so a /// record that old still says which paths it recorded as sources and which as generated even @@ -232,6 +235,8 @@ enum class OperandRecordWord : std::size_t { OutputCount = 1, InputTokenCount = 2, OutputTokenCount = 3, + OrderOnlyInputCount = 4, + OrderOnlyInputTokenCount = 5, }; [[nodiscard]] @@ -242,14 +247,17 @@ constexpr auto operand_word_category(OperandRecordWord word) -> std::string_view return "inputs"; case OperandRecordWord::OutputCount: return "outputs"; + case OperandRecordWord::OrderOnlyInputCount: + return "order-only-inputs"; case OperandRecordWord::InputTokenCount: case OperandRecordWord::OutputTokenCount: + case OperandRecordWord::OrderOnlyInputTokenCount: return "operand-tokens"; } return {}; } -inline constexpr auto OPERAND_RECORD_HEAD_WORDS = std::size_t { 4 }; +inline constexpr auto OPERAND_RECORD_HEAD_WORDS = std::size_t { 6 }; static_assert( !operand_word_category(static_cast(OPERAND_RECORD_HEAD_WORDS - 1)).empty(), diff --git a/include/pup/parser/eval.hpp b/include/pup/parser/eval.hpp index 719f36b6..09cdee17 100644 --- a/include/pup/parser/eval.hpp +++ b/include/pup/parser/eval.hpp @@ -206,12 +206,13 @@ enum class PatternSection { /// Pattern flags for command/output expansion struct PatternFlags { - std::optional input_ext = {}; ///< %e - the current foreach file's extension, absent outside foreach or when its name has no extension - std::string_view input_dir = {}; ///< %d - input directory - std::optional glob_match = {}; ///< %g - the text the input's glob matched, absent when no glob produced it - TokenList all_inputs = {}; ///< All inputs, for %f, %b, %B and their numbered forms, grouped by written token - TokenList all_outputs = {}; ///< All outputs, for %o, %No and %O, grouped by written token - PatternSection section = PatternSection::Command; ///< decides where %o and %O are legal + std::optional input_ext = {}; ///< %e - the current foreach file's extension, absent outside foreach or when its name has no extension + std::string_view input_dir = {}; ///< %d - input directory + std::optional glob_match = {}; ///< %g - the text the input's glob matched, absent when no glob produced it + TokenList all_inputs = {}; ///< All inputs, for %f, %b, %B and their numbered forms, grouped by written token + TokenList order_only_inputs = {}; ///< The rule's own order-only inputs, for %i and %Ni, grouped by written token + TokenList all_outputs = {}; ///< All outputs, for %o, %No and %O, grouped by written token + PatternSection section = PatternSection::Command; ///< decides where %o and %O are legal }; /// Expand an expression, replacing variable references with values diff --git a/spec/requirements/operand-flags.ears.md b/spec/requirements/operand-flags.ears.md index 0ec5a542..076733e0 100644 --- a/spec/requirements/operand-flags.ears.md +++ b/spec/requirements/operand-flags.ears.md @@ -89,6 +89,18 @@ number of the token it was written as rather than renumbering it for its own ite If a rule spells `%No`, then putup shall expand it to every operand of the N-th output token, joined by single spaces. +### REQ-OPERAND-ORDER-ONLY-INPUT-SPELLING + +- conformance: tup-conformant +- reference: upstream passes the rule's own order-only name list to `tup_printf` as its own argument for the command and display strings (`do_rule`, `&r->order_only_inputs`; a bang macro's `bang_oo_inputs` are kept apart and never passed), and the `%i` branch walks that list appending `nle->path`, joined by single spaces; the `%N`-flag branch selects from the same list by order id. The list is built by the tokenizer the input list is built by, so a group is one entry spelled as its reference, a bin is its members, a glob its sorted matches, and a path named twice is two entries. Measured against tup: `: a.c | y.h z.h |>` writes `i=[y.h z.h]`; `| *.h sub/*.h` writes `i=[y.h z.h sub/s.h]` and `%2i` writes `sub/s.h`; `| ` writes `i=[]` and `| sub/` writes `i=[sub/]`; `| {bn}` writes the bin's member; `| z.h z.h` writes `i=[z.h z.h]`; a rule in `sub/` writing `| ../z.h` writes `i=[../z.h]`; `!m = | y.h |> echo %i` called by `: a.c | z.h |> !m` writes `i=[z.h]`, and `!m = |> echo %2i` called by `: a.c | y.h z.h |> !m` writes `z.h`; `%2i` over one order-only input, and `%1i` over none, write nothing; a display string spells it too. putup spelled `%i` as the inputs, the same text as `%f`, at every expansion site, and refused `%Ni` as unsupported (#461) +- discharge: test "Scenario: A percent-i spells the rule's order-only inputs" +- discharge: test "Evaluator pattern expansion - %i spells the order-only inputs" + +If a rule spells `%i` in its command or display string, then putup shall expand it to every +order-only operand the rule itself wrote after `|`, spelled as `%f` spells an input and joined by +single spaces; and `%Ni` to every operand of the N-th such token, or to nothing when the token +does not exist. + ### REQ-OPERAND-DUPLICATE-INPUT-PRUNED - conformance: tup-conformant @@ -131,13 +143,25 @@ Tupfile. If a rule spells a `%N`-flag whose letter is not one of `f`, `b`, `B`, `o` or `i`, then putup shall reject the Tupfile. -### REQ-OPERAND-NUMBERED-ORDER-ONLY-UNSUPPORTED +### REQ-OPERAND-ORDER-ONLY-FLAG-COMMAND-ONLY -- conformance: deliberate-deviation -- reference: upstream expands `%Ni` to the N-th order-only input in a command string and refuses it elsewhere (`tup_printf`, the `%N`-flag branch, which is passed a null order-only list outside a command). putup does not bind that list yet and refuses `%Ni` everywhere, naming the issue; the alternative was to keep emitting it as literal text into a command line or a filename, which is the silent failure this area exists to end (#426) -- discharge: test "A numbered order-only input flag is refused as unsupported" +- conformance: tup-conformant +- reference: upstream passes a null order-only list to `tup_printf` for an output pattern and an input path (`parse_output_pattern`, `eval_path_list`), and both the `%i` branch and the `%N`-flag branch refuse on it with `%i is only valid in a command string`, the numbered one naming its number. Measured against tup: `|> %i.txt`, `|> out.txt | %i.txt` and `|> %1i.txt` are refused; `^ DISP[%i]^` is not (#461) +- discharge: test "Scenario: A percent-i in a rule with no order-only inputs or outside a command is refused" +- discharge: test "Evaluator pattern expansion - %i with no order-only inputs or outside a command is refused" + +If a rule spells `%i` or `%Ni` in its outputs or extra outputs, then putup shall reject the +Tupfile. + +### REQ-OPERAND-ORDER-ONLY-FLAG-NO-ORDER-ONLY-INPUTS + +- conformance: tup-conformant +- reference: upstream refuses `%i` when the order-only name list is empty with `%i used in rule pattern and no order-only input files were specified` (`tup_printf`, the `i` branch, guarded on `ooinput_nl->num_entries == 0`), in the same function as the `%f` guard REQ-OPERAND-INPUT-FLAGS-NO-INPUTS conforms to; the numbered form never refuses, `%1i` over no order-only inputs expanding to nothing like `%1f`. The list is the rule's own, so a bang macro's order-only inputs do not fill it, and neither does a group written in the inputs section, which upstream treats as order-only for dependencies but not for `%i`. Measured against tup: a rule with inputs and none order-only, a rule with no inputs at all, `!m = | z.h |> echo %i` called by `: a.c |> !m`, `: a.c |> echo %i`, and a display string in a rule with no order-only inputs, all refused with the same message; `%1i` over none builds. putup expanded `%i` to the inputs and built (#461) +- discharge: test "Scenario: A percent-i in a rule with no order-only inputs or outside a command is refused" +- discharge: test "Evaluator pattern expansion - %i with no order-only inputs or outside a command is refused" -If a rule spells `%Ni`, then putup shall reject the Tupfile naming the issue that tracks it. +If a rule spells `%i` in its command or display string and it wrote no order-only inputs after +`|`, then putup shall reject the Tupfile. ### REQ-OPERAND-EXTENSION-FOREACH-ONLY diff --git a/src/cli/cmd_build.cpp b/src/cli/cmd_build.cpp index 7d5ea722..f89546f4 100644 --- a/src/cli/cmd_build.cpp +++ b/src/cli/cmd_build.cpp @@ -816,6 +816,7 @@ auto serialize_command_nodes( } auto inputs = pup::graph::view(g, id); + auto order_only_inputs = pup::graph::view(g, id); auto outputs = pup::graph::view(g, id); auto& pool = pup::global_pool(); @@ -837,6 +838,7 @@ auto serialize_command_nodes( .signature = pup::graph::compute_command_signature(g, id, state.path_cache), .must_rerun = must_rerun_cmds.contains(id), .inputs = std::move(inputs), + .order_only_inputs = std::move(order_only_inputs), .outputs = std::move(outputs), }; index.add_command(std::move(entry)); @@ -1427,6 +1429,7 @@ auto merge_out_of_scope_commands( ); }; auto new_inputs = resolve_operands(cmd.inputs); + auto new_order_only_inputs = resolve_operands(cmd.order_only_inputs); auto new_outputs = resolve_operands(cmd.outputs); // An operand it could not carry means the record no longer describes what ran, so it // keeps its outputs but stops claiming they are current. @@ -1443,6 +1446,7 @@ auto merge_out_of_scope_commands( .signature = cmd.signature, .must_rerun = must_rerun, .inputs = std::move(new_inputs), + .order_only_inputs = std::move(new_order_only_inputs), .outputs = std::move(new_outputs), }); old_to_new_cmd.set(cmd.id, new_cmd_id); diff --git a/src/core/instruction.cpp b/src/core/instruction.cpp index 72cc66ea..97cde9ec 100644 --- a/src/core/instruction.cpp +++ b/src/core/instruction.cpp @@ -23,12 +23,13 @@ auto is_nth(AtomKind kind) -> bool case AtomKind::NthInput: case AtomKind::NthInputBase: case AtomKind::NthInputNoExt: + case AtomKind::NthOrderOnlyInput: case AtomKind::NthOutput: return true; case AtomKind::Literal: case AtomKind::GroupRef: case AtomKind::AllInputs: - case AtomKind::AllInputsAlias: + case AtomKind::OrderOnlyInputs: case AtomKind::InputBase: case AtomKind::InputNoExt: case AtomKind::InputExt: @@ -46,7 +47,8 @@ auto letter_of(AtomKind kind) -> char case AtomKind::AllInputs: case AtomKind::NthInput: return 'f'; - case AtomKind::AllInputsAlias: + case AtomKind::OrderOnlyInputs: + case AtomKind::NthOrderOnlyInput: return 'i'; case AtomKind::InputBase: case AtomKind::NthInputBase: @@ -80,6 +82,8 @@ auto kind_for_letter(char letter, bool numbered) -> AtomKind return AtomKind::NthInputBase; case 'B': return AtomKind::NthInputNoExt; + case 'i': + return AtomKind::NthOrderOnlyInput; default: return AtomKind::NthOutput; } @@ -88,7 +92,7 @@ auto kind_for_letter(char letter, bool numbered) -> AtomKind case 'f': return AtomKind::AllInputs; case 'i': - return AtomKind::AllInputsAlias; + return AtomKind::OrderOnlyInputs; case 'b': return AtomKind::InputBase; case 'B': @@ -183,7 +187,7 @@ auto render_instruction(Instruction const& atoms) -> StringId buf.append('>'); break; case AtomKind::AllInputs: - case AtomKind::AllInputsAlias: + case AtomKind::OrderOnlyInputs: case AtomKind::InputBase: case AtomKind::InputNoExt: case AtomKind::InputExt: @@ -193,6 +197,7 @@ auto render_instruction(Instruction const& atoms) -> StringId case AtomKind::NthInput: case AtomKind::NthInputBase: case AtomKind::NthInputNoExt: + case AtomKind::NthOrderOnlyInput: case AtomKind::NthOutput: buf.append('%'); if (is_nth(atom.kind())) { @@ -247,7 +252,7 @@ auto parse_instruction(std::string_view text) -> Result return make_error(ErrorCode::ParseError, msg.view()); } auto const letter = text[end]; - if (letter != 'f' && letter != 'b' && letter != 'B' && letter != 'o') { + if (letter != 'f' && letter != 'b' && letter != 'B' && letter != 'o' && letter != 'i') { auto msg = Buf {}; msg.fmt("Expected 'f', 'b', 'B', 'o', or 'i' after number in %{}-flag, but got '{}'", num, letter); return make_error(ErrorCode::ParseError, msg.view()); diff --git a/src/graph/builder.cpp b/src/graph/builder.cpp index 90b9b8d7..20fbf4ae 100644 --- a/src/graph/builder.cpp +++ b/src/graph/builder.cpp @@ -1949,6 +1949,26 @@ auto process_generated_rules( } } +/// How a command spells an operand: a file Tupfile-relative in the rule's one path space, a group +/// as its reference. The same spelling `%f` and `%i` render at exec time from the operand's node. +auto spell_operands( + BuilderContext& ctx, + PathTransformContext const& tc, + Vec const& operands +) -> Vec +{ + auto spelled = Vec {}; + spelled.reserve(operands.size()); + for (auto const& inp : operands) { + spelled.push_back(str( + inp.kind == RuleInput::Kind::GroupRef + ? group_operand_spelling(str(inp.value)) + : transform_input_path(*ctx.state, tc, str(inp.value)) + )); + } + return spelled; +} + auto expand_rule( BuilderContext& ctx, Builder& state, @@ -1979,30 +1999,25 @@ auto expand_rule( } } - auto tc = make_transform_context(ctx); - auto cmd_inputs = Vec {}; - cmd_inputs.reserve(operands.size()); - for (auto const& inp : operands) { - cmd_inputs.push_back( - inp.kind == RuleInput::Kind::GroupRef - ? group_operand_spelling(str(inp.value)) - : transform_input_path(*ctx.state, tc, str(inp.value)) - ); + auto rule_order_only = expand_inputs(ctx, rule.order_only_inputs); + if (!rule_order_only) { + return pup::unexpected(rule_order_only.error()); + } + auto order_only_operands = Vec {}; + auto order_only_tokens = Vec {}; + for (auto const& inp : rule_order_only->operands) { + if (inp.kind != RuleInput::Kind::Pattern) { + order_only_operands.push_back(inp); + order_only_tokens.push_back(inp.token); + } } - auto primary_input_sv = cmd_inputs.empty() ? std::string_view {} : str(cmd_inputs[0]); + auto tc = make_transform_context(ctx); + auto cmd_inputs = spell_operands(ctx, tc, operands); + auto const primary_input_sv = cmd_inputs.empty() ? std::string_view {} : cmd_inputs[0]; auto current_dir_name = is_empty(ctx.current_dir) ? std::string_view {} : pup::path::filename(str(ctx.current_dir)); - auto all_inputs_sv = Vec {}; - all_inputs_sv.reserve(cmd_inputs.size()); - for (auto id : cmd_inputs) { - all_inputs_sv.push_back(str(id)); - } - auto const input_tokens = TokenList::grouped( - std::move(all_inputs_sv), operand_tokens, token_count - ); - auto flags = parser::PatternFlags { .input_ext = rule.foreach_ && !pup::path::extension(primary_input_sv).empty() ? std::optional { parser::path_extension(primary_input_sv) } @@ -2011,7 +2026,10 @@ auto expand_rule( .glob_match = operands.empty() || is_empty(operands[0].glob) ? std::nullopt : std::optional { str(parser::glob_match_extract(str(operands[0].glob), primary_input_sv)) }, - .all_inputs = input_tokens, + .all_inputs = TokenList::grouped(std::move(cmd_inputs), operand_tokens, token_count), + .order_only_inputs = TokenList::grouped( + spell_operands(ctx, tc, order_only_operands), order_only_tokens, rule_order_only->token_count + ), }; // Early macro lookup - needed to process macro's order_only_inputs for demand-driven parsing @@ -2213,25 +2231,24 @@ auto expand_rule( display = *disp_result; } - // Expand non-group order-only inputs. Typed groups () are already handled - // by the pre-resolution loop above — their NodeIds are in deferred_group_vec. - // Expression-based group refs may still slip through expand_inputs (rare). - auto order_only_file_patterns = Vec {}; - for (auto const& p : all_order_only) { - if (!p.is_order_only_group) { - order_only_file_patterns.push_back(p); - } - } auto order_only_paths = Vec {}; - if (!order_only_file_patterns.empty()) { - auto order_inputs = expand_inputs(ctx, order_only_file_patterns); - if (order_inputs) { - order_only_paths.reserve(order_inputs->operands.size()); - for (auto const& inp : order_inputs->operands) { - order_only_paths.push_back(inp.value); + auto macro_order_only_paths = Vec {}; + auto collect_order_only_paths = [](Vec& into, TokenizedInputs const& expanded) { + for (auto const& inp : expanded.operands) { + if (inp.kind != RuleInput::Kind::GroupRef) { + into.push_back(inp.value); } } + }; + collect_order_only_paths(order_only_paths, *rule_order_only); + if (macro_ptr && !macro_ptr->order_only_inputs.empty()) { + auto macro_order_only = expand_inputs(ctx, macro_ptr->order_only_inputs); + if (!macro_order_only) { + return pup::unexpected(macro_order_only.error()); + } + collect_order_only_paths(macro_order_only_paths, *macro_order_only); } + order_only_paths.insert(order_only_paths.end(), macro_order_only_paths.begin(), macro_order_only_paths.end()); auto cmd_id = create_command_node(ctx, state, std::move(instruction_pattern), str(display)); if (!cmd_id) { @@ -2368,9 +2385,31 @@ auto expand_rule( } } + auto order_only_ids = Vec {}; + order_only_ids.reserve(order_only_operands.size()); + for (auto const& inp : order_only_operands) { + if (inp.kind == RuleInput::Kind::GroupRef) { + auto group_id = resolve_group_operand_node(ctx, state, str(inp.value)); + if (!group_id) { + return pup::unexpected(group_id.error()); + } + order_only_ids.push_back(*group_id); + continue; + } + auto file_id = resolve_input_node(ctx, str(inp.value)); + if (!file_id) { + return pup::unexpected(file_id.error()); + } + (void)add_edge(ctx.state->graph, *file_id, *cmd_id, LinkType::OrderOnly); + order_only_ids.push_back(*file_id); + } + // Store explicit operands on the command node for expand_instruction() if (auto* cmd = get_command_node(ctx.state->graph, *cmd_id)) { cmd->inputs = TokenList::grouped(std::move(input_ids), operand_tokens, token_count); + cmd->order_only_inputs = TokenList::grouped( + std::move(order_only_ids), order_only_tokens, rule_order_only->token_count + ); cmd->outputs = TokenList::grouped( std::move(output_ids), output_operand_tokens, outputs->token_count ); @@ -2378,7 +2417,7 @@ auto expand_rule( // Create order-only edges from the pre-expanded paths // Skip group references (deferred edge creation) and glob patterns (not valid paths) - for (auto oi : order_only_paths) { + for (auto oi : macro_order_only_paths) { auto oi_sv = str(oi); if (is_order_only_group_reference(oi_sv) || parser::has_glob_chars(oi_sv)) { continue; diff --git a/src/graph/dag.cpp b/src/graph/dag.cpp index 5610a4c0..a7730ecc 100644 --- a/src/graph/dag.cpp +++ b/src/graph/dag.cpp @@ -318,6 +318,14 @@ auto view(Graph const& graph, NodeId id) -> TokenList const& return node ? node->inputs : empty; } +template<> +auto view(Graph const& graph, NodeId id) -> TokenList const& +{ + static auto const empty = TokenList {}; + auto const* node = get_command_node(graph, id); + return node ? node->order_only_inputs : empty; +} + template<> auto view(Graph const& graph, NodeId id) -> TokenList const& { @@ -854,6 +862,11 @@ struct ExecSite { } } + auto append_order_only_inputs(Buf& buf) const -> void + { + append_token(buf, cmd->order_only_inputs.ids(), [this](NodeId id) { return operand_path(id); }); + } + auto append_input_base(Buf& buf) const -> void { append_token(buf, cmd->inputs.ids(), [this](NodeId id) { return basename_of(id); }); @@ -915,6 +928,11 @@ struct ExecSite { }); } + auto append_nth_order_only_input(Buf& buf, std::uint32_t token) const -> void + { + append_token(buf, cmd->order_only_inputs.token(token), [this](NodeId id) { return operand_path(id); }); + } + auto basename_of(NodeId id) const -> std::string_view { return pup::path::filename(operand_path(id)); } auto basename_without_extension_of(NodeId id) const -> std::string_view diff --git a/src/index/entry.cpp b/src/index/entry.cpp index 60425fc9..44f264d4 100644 --- a/src/index/entry.cpp +++ b/src/index/entry.cpp @@ -89,6 +89,7 @@ auto CommandEntry::from_raw( std::string_view display_str, std::string_view env_str, TokenList inputs, + TokenList order_only_inputs, TokenList outputs, std::size_t array_index ) -> Result @@ -106,6 +107,7 @@ auto CommandEntry::from_raw( .signature = raw.signature, .must_rerun = (raw.flags & to_underlying(CommandFlag::MustRerun)) != 0U, .inputs = std::move(inputs), + .order_only_inputs = std::move(order_only_inputs), .outputs = std::move(outputs), }; } @@ -332,6 +334,8 @@ class IndexSite final { auto append_all_inputs(Buf& buf) const -> void { append_all(buf, m_cmd.inputs); } + auto append_order_only_inputs(Buf& buf) const -> void { append_all(buf, m_cmd.order_only_inputs); } + auto append_input_base(Buf& buf) const -> void { append_token(buf, m_cmd.inputs.ids(), [this](NodeId id) { return basename_of(id); }); @@ -392,6 +396,11 @@ class IndexSite final { return pup::path::stem(operand_path(id)); } + auto append_nth_order_only_input(Buf& buf, std::uint32_t token) const -> void + { + append_token(buf, m_cmd.order_only_inputs.token(token), [this](NodeId id) { return operand_path(id); }); + } + auto append_nth_output(Buf& buf, std::uint32_t token) const -> void { append_token(buf, m_cmd.outputs.token(token), [this](NodeId id) { return operand_path(id); }); diff --git a/src/index/reader.cpp b/src/index/reader.cpp index 0f0127aa..7daa2c6f 100644 --- a/src/index/reader.cpp +++ b/src/index/reader.cpp @@ -29,8 +29,13 @@ constexpr auto UNREADABLE_DISPLAY = std::string_view { "[unreadable]" }; auto index_get_semantic_string(IndexFile const& f, std::uint32_t offset) -> Result; auto index_get_string(IndexFile const& f, std::uint32_t offset) -> std::string_view; -auto index_get_operands(IndexFile const& f, std::size_t cmd_index) - -> Result, TokenList>>; +struct Operands { + TokenList inputs; + TokenList order_only_inputs; + TokenList outputs; +}; + +auto index_get_operands(IndexFile const& f, std::size_t cmd_index) -> Result; template auto read_raw_entries( @@ -173,7 +178,14 @@ auto read_index(IndexFile const& f) -> Result return pup::unexpected(operands.error()); } auto command = CommandEntry::from_raw( - raw, instruction_pattern, display, *env, std::move(operands->first), std::move(operands->second), i + raw, + instruction_pattern, + display, + *env, + std::move(operands->inputs), + std::move(operands->order_only_inputs), + std::move(operands->outputs), + i ); if (!command) { return pup::unexpected(command.error()); @@ -384,10 +396,8 @@ auto index_get_string(IndexFile const& f, std::uint32_t offset) -> std::string_v return text ? *text : UNREADABLE_DISPLAY; } -auto index_get_operands(IndexFile const& f, std::size_t cmd_index) - -> Result, TokenList>> +auto index_get_operands(IndexFile const& f, std::size_t cmd_index) -> Result { - using Operands = std::pair, TokenList>; auto const* hdr = index_header(f); if (!hdr || cmd_index >= hdr->command_count) { @@ -427,12 +437,15 @@ auto index_get_operands(IndexFile const& f, std::size_t cmd_index) auto out_count = std::size_t { read_u32(record_pos + sizeof(std::uint32_t)) }; auto in_tokens = std::size_t { read_u32(record_pos + 2 * sizeof(std::uint32_t)) }; auto out_tokens = std::size_t { read_u32(record_pos + 3 * sizeof(std::uint32_t)) }; + auto oo_count = std::size_t { read_u32(record_pos + 4 * sizeof(std::uint32_t)) }; + auto oo_tokens = std::size_t { read_u32(record_pos + 5 * sizeof(std::uint32_t)) }; auto const in_boundary_words = in_tokens + 1; auto const out_boundary_words = out_tokens + 1; + auto const oo_boundary_words = oo_tokens + 1; auto expected_size = OPERAND_RECORD_HEAD_WORDS * sizeof(std::uint32_t) - + (in_count + out_count) * sizeof(NodeId) - + (in_boundary_words + out_boundary_words) * sizeof(std::uint32_t); + + (in_count + out_count + oo_count) * sizeof(NodeId) + + (in_boundary_words + out_boundary_words + oo_boundary_words) * sizeof(std::uint32_t); if (record_pos + expected_size > section_end) { return make_error(ErrorCode::IndexDamaged, "Operand record runs past the operand section"); } @@ -450,18 +463,21 @@ auto index_get_operands(IndexFile const& f, std::size_t cmd_index) auto input_ids = take(in_count); auto output_ids = take(out_count); + auto order_only_ids = take(oo_count); auto input_starts = take(in_boundary_words); auto output_starts = take(out_boundary_words); + auto order_only_starts = take(oo_boundary_words); auto inputs = TokenList::from_starts(std::move(input_ids), std::move(input_starts)); auto outputs = TokenList::from_starts(std::move(output_ids), std::move(output_starts)); - if (!inputs || !outputs) { + auto order_only_inputs = TokenList::from_starts(std::move(order_only_ids), std::move(order_only_starts)); + if (!inputs || !outputs || !order_only_inputs) { return make_error( ErrorCode::IndexDamaged, "Operand record groups its operands into tokens it does not span" ); } - return Operands { std::move(*inputs), std::move(*outputs) }; + return Operands { std::move(*inputs), std::move(*order_only_inputs), std::move(*outputs) }; } } // namespace diff --git a/src/index/writer.cpp b/src/index/writer.cpp index eae0264e..5cdce8fa 100644 --- a/src/index/writer.cpp +++ b/src/index/writer.cpp @@ -250,6 +250,8 @@ auto serialize_index(Index const& index) -> Result> push_u32(static_cast(cmd.outputs.size())); push_u32(cmd.inputs.token_count()); push_u32(cmd.outputs.token_count()); + push_u32(static_cast(cmd.order_only_inputs.size())); + push_u32(cmd.order_only_inputs.token_count()); for (auto id : cmd.inputs) { push_u32(id); @@ -257,12 +259,18 @@ auto serialize_index(Index const& index) -> Result> for (auto id : cmd.outputs) { push_u32(id); } + for (auto id : cmd.order_only_inputs) { + push_u32(id); + } for (auto start : cmd.inputs.starts()) { push_u32(start); } for (auto start : cmd.outputs.starts()) { push_u32(start); } + for (auto start : cmd.order_only_inputs.starts()) { + push_u32(start); + } } // Build edge entries diff --git a/src/parser/eval.cpp b/src/parser/eval.cpp index e96f5e2c..1206b18d 100644 --- a/src/parser/eval.cpp +++ b/src/parser/eval.cpp @@ -423,6 +423,11 @@ struct ParseSite { } } + auto append_order_only_inputs(Buf& buf) const -> void + { + append_token(buf, flags.order_only_inputs.ids(), [](std::string_view p) { return p; }); + } + auto append_input_base(Buf& buf) const -> void { append_token(buf, flags.all_inputs.ids(), basename_of); @@ -468,6 +473,11 @@ struct ParseSite { append_token(buf, flags.all_inputs.token(token), basename_without_extension_of); } + auto append_nth_order_only_input(Buf& buf, std::uint32_t token) const -> void + { + append_token(buf, flags.order_only_inputs.token(token), [](std::string_view p) { return p; }); + } + auto append_nth_output(Buf& buf, std::uint32_t token) const -> void { append_token(buf, flags.all_outputs.token(token), [](std::string_view p) { return p; }); @@ -550,29 +560,28 @@ auto expand_pattern_atoms( auto const letter = text[end]; - if (letter == 'f' || letter == 'b' || letter == 'B' || letter == 'o') { + if (letter == 'f' || letter == 'b' || letter == 'B' || letter == 'o' || letter == 'i') { if (letter == 'o' && flags.section == PatternSection::Outputs) { return make_error( ErrorCode::ParseError, "%o can only be used in a command string or extra outputs section" ); } + if (letter == 'i' && flags.section != PatternSection::Command) { + auto msg = Buf {}; + msg.fmt("%{}i is only valid in a command string", num); + return make_error(ErrorCode::ParseError, msg.view()); + } auto const kind = letter == 'f' ? AtomKind::NthInput : letter == 'b' ? AtomKind::NthInputBase : letter == 'B' ? AtomKind::NthInputNoExt + : letter == 'i' ? AtomKind::NthOrderOnlyInput : AtomKind::NthOutput; builder.nth(kind, num); pos = end + 1; continue; } - if (letter == 'i') { - return make_error( - ErrorCode::ParseError, - "%Ni is not supported yet (#426)" - ); - } - auto msg = Buf {}; msg.fmt("Expected 'f', 'b', 'B', 'o', or 'i' after number in %{}-flag, but got '{}'", num, letter); return make_error(ErrorCode::ParseError, msg.view()); @@ -609,7 +618,16 @@ auto expand_pattern_atoms( ); break; case 'i': - builder.flag(AtomKind::AllInputsAlias); + if (flags.section != PatternSection::Command) { + return make_error(ErrorCode::ParseError, "%i is only valid in a command string"); + } + if (flags.order_only_inputs.empty()) { + return make_error( + ErrorCode::ParseError, + "%i used in rule pattern and no order-only input files were specified" + ); + } + builder.flag(AtomKind::OrderOnlyInputs); break; case 'e': if (!flags.input_ext) { diff --git a/test/unit/test_e2e.cpp b/test/unit/test_e2e.cpp index 56f33b63..0d721ab1 100644 --- a/test/unit/test_e2e.cpp +++ b/test/unit/test_e2e.cpp @@ -15119,6 +15119,394 @@ SCENARIO("A percent-f, percent-b or percent-B in a rule with no inputs is refuse } } +SCENARIO("A percent-i spells the rule's order-only inputs", "[e2e][build]") +{ + GIVEN("a rule naming two order-only inputs") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("z.h", ""); + f.write_file("y.h", ""); + f.write_file("Tupfile", ": a.c | z.h y.h |> echo 'i=[%i]' > %o |> out.txt\n"); + + REQUIRE(f.init().success()); + + WHEN("the project is built") + { + auto result = f.build(); + + THEN("the flag spells them in written order") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE(f.read_file("out.txt") == "i=[z.h y.h]\n"); + } + } + + WHEN("the project is built twice") + { + REQUIRE(f.build().success()); + auto result = f.build(); + + THEN("the second build has nothing to do") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.is_noop()); + } + } + } + + GIVEN("a glob and a group in the order-only list") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("b.c", ""); + f.write_file("z.h", ""); + f.write_file("y.h", ""); + f.write_file("Tupfile", ": b.c |> cp %f %o |> b.out \n: a.c | *.h |> echo 'i=[%i]' > %o |> out.txt\n"); + + REQUIRE(f.init().success()); + + WHEN("the project is built") + { + auto result = f.build(); + + THEN("the flag spells the order-only inputs") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE(f.read_file("out.txt") == "i=[y.h z.h ]\n"); + } + } + } + + GIVEN("a group another directory produces") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.mkdir("sub"); + f.write_file("sub/s.h", ""); + f.write_file("Tupfile", ": a.c | sub/ |> echo 'i=[%i]' > %o |> out.txt\n"); + f.write_file("sub/Tupfile", ": foreach s.h |> cp %f %o |> %B.out \n"); + + REQUIRE(f.init().success()); + + WHEN("the project is built") + { + auto result = f.build(); + + THEN("the flag spells the order-only inputs") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE(f.read_file("out.txt") == "i=[sub/]\n"); + } + } + } + + GIVEN("a foreach rule") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("b.c", ""); + f.write_file("z.h", ""); + f.write_file("Tupfile", ": foreach a.c b.c | z.h |> echo 'i=[%i] f=[%f]' > %o |> %B.out\n"); + + REQUIRE(f.init().success()); + + WHEN("the project is built") + { + auto result = f.build(); + + THEN("the flag spells the order-only inputs") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE(f.read_file("b.out") == "i=[z.h] f=[b.c]\n"); + } + } + } + + GIVEN("a bang macro with its own order-only inputs, called by a rule with its own") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("z.h", ""); + f.write_file("y.h", ""); + f.write_file("Tupfile", "!m = | y.h |> echo 'i=[%i]' > %o |>\n: a.c | z.h |> !m |> out.txt\n"); + + REQUIRE(f.init().success()); + + WHEN("the project is built") + { + auto result = f.build(); + + THEN("the flag spells the order-only inputs") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE(f.read_file("out.txt") == "i=[z.h]\n"); + } + } + } + + GIVEN("a numbered flag over glob tokens") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("z.h", ""); + f.write_file("y.h", ""); + f.mkdir("sub"); + f.write_file("sub/s.h", ""); + f.write_file("Tupfile", ": a.c | *.h sub/*.h |> echo 'i=[%2i]' > %o |> out.txt\n"); + + REQUIRE(f.init().success()); + + WHEN("the project is built") + { + auto result = f.build(); + + THEN("the flag spells the order-only inputs") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE(f.read_file("out.txt") == "i=[sub/s.h]\n"); + } + } + } + + GIVEN("an order-only input another rule generates") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("b.c", ""); + f.write_file("Tupfile", ": b.c |> cp %f %o |> gen.h\n: a.c | gen.h |> echo 'i=[%i]' > %o |> out.txt\n"); + + REQUIRE(f.init().success()); + + WHEN("the project is built") + { + auto result = f.build(); + + THEN("the flag spells the order-only inputs") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE(f.read_file("out.txt") == "i=[gen.h]\n"); + } + } + } + + GIVEN("a rule in a subdirectory naming its parent's file") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("z.h", ""); + f.mkdir("sub"); + f.write_file("sub/a.c", ""); + f.write_file("Tupfile", ""); + f.write_file("sub/Tupfile", ": a.c | ../z.h |> echo 'i=[%i]' > %o |> out.txt\n"); + + REQUIRE(f.init().success()); + + WHEN("the project is built") + { + auto result = f.build(); + + THEN("the flag spells the order-only inputs") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE(f.read_file("sub/out.txt") == "i=[../z.h]\n"); + } + } + } +} + +SCENARIO("A percent-i in a rule with no order-only inputs or outside a command is refused", "[e2e][build]") +{ + GIVEN("a rule with inputs but no order-only inputs whose command spells %i") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("Tupfile", ": a.c |> echo 'i=[%i]' > %o |> out.txt\n"); + + WHEN("the project is configured") + { + auto result = f.init(); + + THEN("the Tupfile is rejected") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE_FALSE(result.success()); + auto const combined = result.stdout_output + result.stderr_output; + REQUIRE(combined.find("%i used in rule pattern and no order-only input files were specified") != std::string::npos); + } + } + } + + GIVEN("a rule with no inputs at all whose command spells %i") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("Tupfile", ": |> echo 'i=[%i]' > %o |> out.txt\n"); + + WHEN("the project is configured") + { + auto result = f.init(); + + THEN("the Tupfile is rejected") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE_FALSE(result.success()); + auto const combined = result.stdout_output + result.stderr_output; + REQUIRE(combined.find("%i used in rule pattern and no order-only input files were specified") != std::string::npos); + } + } + } + + GIVEN("a bang macro whose own order-only inputs are the only ones") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("z.h", ""); + f.write_file("Tupfile", "!m = | z.h |> echo 'i=[%i]' > %o |>\n: a.c |> !m |> out.txt\n"); + + WHEN("the project is configured") + { + auto result = f.init(); + + THEN("the Tupfile is rejected") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE_FALSE(result.success()); + auto const combined = result.stdout_output + result.stderr_output; + REQUIRE(combined.find("%i used in rule pattern and no order-only input files were specified") != std::string::npos); + } + } + } + + GIVEN("a group written in the inputs section") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("b.c", ""); + f.write_file("Tupfile", ": b.c |> cp %f %o |> b.out \n: a.c |> echo 'i=[%i]' > %o |> out.txt\n"); + + WHEN("the project is configured") + { + auto result = f.init(); + + THEN("the Tupfile is rejected") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE_FALSE(result.success()); + auto const combined = result.stdout_output + result.stderr_output; + REQUIRE(combined.find("%i used in rule pattern and no order-only input files were specified") != std::string::npos); + } + } + } + + GIVEN("a display string spelling %i in a rule with no order-only inputs") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("Tupfile", ": a.c |> ^ DISP[%i]^ echo x > %o |> out.txt\n"); + + WHEN("the project is configured") + { + auto result = f.init(); + + THEN("the Tupfile is rejected") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE_FALSE(result.success()); + auto const combined = result.stdout_output + result.stderr_output; + REQUIRE(combined.find("%i used in rule pattern and no order-only input files were specified") != std::string::npos); + } + } + } + + GIVEN("an output name spelling %i") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("z.h", ""); + f.write_file("Tupfile", ": a.c | z.h |> echo x > %o |> %i.txt\n"); + + WHEN("the project is configured") + { + auto result = f.init(); + + THEN("the Tupfile is rejected") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE_FALSE(result.success()); + auto const combined = result.stdout_output + result.stderr_output; + REQUIRE(combined.find("%i is only valid in a command string") != std::string::npos); + } + } + } + + GIVEN("an extra output name spelling %i") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("z.h", ""); + f.write_file("Tupfile", ": a.c | z.h |> echo x > %o; touch %i.txt |> out.txt | %i.txt\n"); + + WHEN("the project is configured") + { + auto result = f.init(); + + THEN("the Tupfile is rejected") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE_FALSE(result.success()); + auto const combined = result.stdout_output + result.stderr_output; + REQUIRE(combined.find("%i is only valid in a command string") != std::string::npos); + } + } + } + + GIVEN("an output name spelling %1i") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", ""); + f.write_file("z.h", ""); + f.write_file("Tupfile", ": a.c | z.h |> echo x > %o |> %1i.txt\n"); + + WHEN("the project is configured") + { + auto result = f.init(); + + THEN("the Tupfile is rejected") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE_FALSE(result.success()); + auto const combined = result.stdout_output + result.stderr_output; + REQUIRE(combined.find("%1i is only valid in a command string") != std::string::npos); + } + } + } +} + SCENARIO("A percent-g in a rule with no inputs or several is refused", "[e2e][build]") { GIVEN("a rule with no inputs whose command spells %g") diff --git a/test/unit/test_eval.cpp b/test/unit/test_eval.cpp index f9098791..44d37a2f 100644 --- a/test/unit/test_eval.cpp +++ b/test/unit/test_eval.cpp @@ -376,7 +376,6 @@ TEST_CASE("Evaluator pattern expansion", "[eval]") REQUIRE(sv(*result) == "foo"); } - // Note: %i is for order-only inputs, not yet implemented } TEST_CASE("A numbered flag outside one to ninety-nine is refused", "[eval]") @@ -424,15 +423,94 @@ TEST_CASE("A numbered flag with an unknown letter is refused", "[eval]") REQUIRE_FALSE(expand_pattern(ctx, "%1t", flags).has_value()); } -TEST_CASE("A numbered order-only input flag is refused as unsupported", "[eval]") +TEST_CASE("Evaluator pattern expansion - %i spells the order-only inputs", "[eval]") { auto vars = VarDb {}; auto ctx = EvalContext { .vars = &vars }; - auto flags = PatternFlags { .all_inputs = { "a.c" } }; + auto flags = PatternFlags { + .all_inputs = { "a.c" }, + .order_only_inputs = pup::TokenList::grouped({ "z.h", "y.h", "sub/s.h" }, { 1, 2, 2 }, 2), + }; - auto const refused = expand_pattern(ctx, "%1i", flags); - REQUIRE_FALSE(refused.has_value()); - REQUIRE(sv(refused.error().message).find("#426") != std::string_view::npos); + SECTION("%i - every order-only input, in written order") + { + auto result = expand_pattern(ctx, "i=[%i] f=[%f]", flags); + REQUIRE(result.has_value()); + REQUIRE(sv(*result) == "i=[z.h y.h sub/s.h] f=[a.c]"); + } + + SECTION("%Ni - the order-only inputs of the N-th written token") + { + auto result = expand_pattern(ctx, "%1i|%2i", flags); + REQUIRE(result.has_value()); + REQUIRE(sv(*result) == "z.h|y.h sub/s.h"); + } + + SECTION("a numbered order-only flag past the last written token expands to nothing") + { + auto result = expand_pattern(ctx, "i=[%3i]", flags); + REQUIRE(result.has_value()); + REQUIRE(sv(*result) == "i=[]"); + } +} + +TEST_CASE("Evaluator pattern expansion - %i with no order-only inputs or outside a command is refused", "[eval]") +{ + auto vars = VarDb {}; + auto ctx = EvalContext { .vars = &vars }; + + SECTION("%i over no order-only inputs") + { + auto flags = PatternFlags { .all_inputs = { "a.c" }, .all_outputs = { "out.txt" } }; + auto result = expand_pattern(ctx, "echo %i > %o", flags); + REQUIRE_FALSE(result.has_value()); + REQUIRE(sv(result.error().message).find("%i used in rule pattern and no order-only input files were specified") != std::string_view::npos); + } + + SECTION("a numbered order-only flag over no order-only inputs still expands to nothing") + { + auto flags = PatternFlags { .all_inputs = { "a.c" }, .all_outputs = { "out.txt" } }; + auto result = expand_pattern(ctx, "i=[%1i]", flags); + REQUIRE(result.has_value()); + REQUIRE(sv(*result) == "i=[]"); + } + + SECTION("%i in the outputs section") + { + auto flags = PatternFlags { + .all_inputs = { "a.c" }, + .order_only_inputs = { "z.h" }, + .section = PatternSection::Outputs, + }; + auto result = expand_pattern(ctx, "%i.txt", flags); + REQUIRE_FALSE(result.has_value()); + REQUIRE(sv(result.error().message).find("%i is only valid in a command string") != std::string_view::npos); + } + + SECTION("%Ni in the outputs section") + { + auto flags = PatternFlags { + .all_inputs = { "a.c" }, + .order_only_inputs = { "z.h" }, + .section = PatternSection::Outputs, + }; + auto result = expand_pattern(ctx, "%1i.txt", flags); + REQUIRE_FALSE(result.has_value()); + REQUIRE(sv(result.error().message).find("%1i is only valid in a command string") != std::string_view::npos); + } + + SECTION("%i in the extra outputs section") + { + auto flags = PatternFlags { + .all_inputs = { "a.c" }, + .order_only_inputs = { "z.h" }, + .all_outputs = { "out.txt" }, + .section = PatternSection::ExtraOutputs, + }; + auto result = expand_pattern(ctx, "%i.txt", flags); + REQUIRE_FALSE(result.has_value()); + REQUIRE(sv(result.error().message).find("%i is only valid in a command string") != std::string_view::npos); + } } TEST_CASE("Evaluator pattern expansion - multiple inputs", "[eval]") @@ -469,7 +547,6 @@ TEST_CASE("Evaluator pattern expansion - multiple inputs", "[eval]") REQUIRE(sv(*result) == "a.c b.c c.c|a b c"); } - // Note: %i is for order-only inputs, not yet implemented } TEST_CASE("Evaluator pattern expansion - %e with no extension bound is refused", "[eval]") diff --git a/test/unit/test_index.cpp b/test/unit/test_index.cpp index 7f2e7916..ec301ef8 100644 --- a/test/unit/test_index.cpp +++ b/test/unit/test_index.cpp @@ -187,6 +187,7 @@ TEST_CASE("CommandEntry conversion", "[index]") .signature = signature, .must_rerun = true, .inputs = { 10 }, + .order_only_inputs = { 30 }, .outputs = { 20 }, }; @@ -204,7 +205,7 @@ TEST_CASE("CommandEntry conversion", "[index]") auto& pool = global_pool(); auto restored = CommandEntry::from_raw( raw, pool.get(cmd.instruction_pattern), pool.get(cmd.display), pool.get(cmd.env), - pup::Vec { 10 }, pup::Vec { 20 }, 4 + pup::Vec { 10 }, pup::Vec { 30 }, pup::Vec { 20 }, 4 ); REQUIRE(restored.has_value()); @@ -218,6 +219,7 @@ TEST_CASE("CommandEntry conversion", "[index]") REQUIRE(restored->signature == signature); REQUIRE(restored->must_rerun); REQUIRE(restored->inputs == cmd.inputs); + REQUIRE(restored->order_only_inputs == cmd.order_only_inputs); REQUIRE(restored->outputs == cmd.outputs); } @@ -905,11 +907,16 @@ TEST_CASE("A command's operand tokens survive the record", "[index]") pup::Vec { 4, 5 }, pup::Vec { 2, 2 }, 2 ); + auto const order_only_inputs = pup::TokenList::grouped( + pup::Vec { 5, 1 }, pup::Vec { 1, 3 }, 3 + ); + auto cmd_id = node_id::make_command(1); index.add_command(CommandEntry { .id = cmd_id, - .instruction_pattern = intern("cc %2f -o %2o"), + .instruction_pattern = intern("cc %2f -o %2o %i"), .inputs = inputs, + .order_only_inputs = order_only_inputs, .outputs = outputs, }); @@ -930,6 +937,10 @@ TEST_CASE("A command's operand tokens survive the record", "[index]") REQUIRE(cmd->inputs.token(2).size() == 2); REQUIRE(cmd->inputs.token(3).empty()); REQUIRE(cmd->outputs.token(2).size() == 2); + REQUIRE(cmd->order_only_inputs == order_only_inputs); + REQUIRE(cmd->order_only_inputs.token(1).size() == 1); + REQUIRE(cmd->order_only_inputs.token(2).empty()); + REQUIRE(cmd->order_only_inputs.token(3).size() == 1); opened->file.close(); std::filesystem::remove(temp_path); @@ -2477,6 +2488,10 @@ SCENARIO("A recorded command and a graph command expand a template the same way" std::uint32_t input_token_count = 0; std::vector output_tokens = {}; std::uint32_t output_token_count = 0; + std::vector graph_order_only = {}; + std::vector index_order_only = {}; + std::vector order_only_tokens = {}; + std::uint32_t order_only_token_count = 0; }; auto grouped = [](std::vector const& ids, @@ -2516,6 +2531,12 @@ SCENARIO("A recorded command and a graph command expand a template the same way" { "one token owning two outputs", { *foo_c }, { *foo_o, *foo_d }, { 3 }, { 6, 7 }, intern("src"), 1, {}, 0, { 2, 2 }, 2 }, + { "an order-only input beside the inputs", + { *foo_c }, { *foo_o }, { 3 }, { 6 }, intern("src"), 1, + {}, 0, {}, 0, { *bar_c }, { 4 } }, + { "one token owning two order-only inputs and a trailing token owning none", + { *foo_c }, { *foo_o }, { 3 }, { 6 }, intern("src"), 1, + {}, 0, {}, 0, { *bar_c, *dotfile }, { 4, 5 }, { 1, 1 }, 2 }, }; auto const templates = std::vector { @@ -2525,6 +2546,7 @@ SCENARIO("A recorded command and a graph command expand a template the same way" "echo %O", "echo %d", "echo %i", + "echo %1i %2i", "echo %f%%literal", "echo a%1bb%2Bc", "link %f -o %O", @@ -2549,6 +2571,9 @@ SCENARIO("A recorded command and a graph command expand a template the same way" node.outputs = grouped( operands.graph_outputs, operands.output_tokens, operands.output_token_count ); + node.order_only_inputs = grouped( + operands.graph_order_only, operands.order_only_tokens, operands.order_only_token_count + ); auto cmd_id = graph::add_command_node(g, std::move(node)); REQUIRE(cmd_id.has_value()); @@ -2563,6 +2588,9 @@ SCENARIO("A recorded command and a graph command expand a template the same way" record.outputs = grouped( operands.index_outputs, operands.output_tokens, operands.output_token_count ); + record.order_only_inputs = grouped( + operands.index_order_only, operands.order_only_tokens, operands.order_only_token_count + ); index.add_command(record); auto const from_graph diff --git a/test/unit/test_instruction.cpp b/test/unit/test_instruction.cpp index 4c89e9aa..0722fa5b 100644 --- a/test/unit/test_instruction.cpp +++ b/test/unit/test_instruction.cpp @@ -44,6 +44,7 @@ struct RecordingSite { buf.append(">"); } auto append_all_inputs(Buf& buf) const -> void { note(buf, "all_inputs"); } + auto append_order_only_inputs(Buf& buf) const -> void { note(buf, "order_only_inputs"); } auto append_input_base(Buf& buf) const -> void { note(buf, "input_base"); } auto append_input_noext(Buf& buf) const -> void { note(buf, "input_noext"); } auto append_input_ext(Buf& buf) const -> void { note(buf, "input_ext"); } @@ -53,6 +54,7 @@ struct RecordingSite { auto append_nth_input(Buf& buf, std::size_t i) const -> void { note_nth(buf, "nth_input", i); } auto append_nth_input_base(Buf& buf, std::size_t i) const -> void { note_nth(buf, "nth_input_base", i); } auto append_nth_input_noext(Buf& buf, std::size_t i) const -> void { note_nth(buf, "nth_input_noext", i); } + auto append_nth_order_only_input(Buf& buf, std::size_t i) const -> void { note_nth(buf, "nth_order_only_input", i); } auto append_nth_output(Buf& buf, std::size_t i) const -> void { note_nth(buf, "nth_output", i); } private: @@ -155,12 +157,13 @@ SCENARIO("A rendered instruction re-parses to the same atoms", "[instruction][pr "gcc -c ", " -o ", "%", "%%", "a%1fb.c", "echo ", " > ", "", "üñ", "% { - AtomKind::AllInputs, AtomKind::AllInputsAlias, AtomKind::InputBase, + AtomKind::AllInputs, AtomKind::OrderOnlyInputs, AtomKind::InputBase, AtomKind::InputNoExt, AtomKind::InputExt, AtomKind::AllOutputs, AtomKind::OutputNoExt, AtomKind::InputDir, }; auto const numbered = std::vector { - AtomKind::NthInput, AtomKind::NthInputBase, AtomKind::NthInputNoExt, AtomKind::NthOutput, + AtomKind::NthInput, AtomKind::NthInputBase, AtomKind::NthInputNoExt, AtomKind::NthOrderOnlyInput, + AtomKind::NthOutput, }; auto const groups = std::vector { "gen", "gen-headers", "objs" };