From 93fafb54c6f1a1b78b7eb5b285b7e7f44be86535 Mon Sep 17 00:00:00 2001 From: helly25 <6420169+helly25@users.noreply.github.com> Date: Fri, 11 Sep 2026 13:51:28 +0100 Subject: [PATCH] Report config-local option overrides --- CHANGELOG.md | 7 ++ XFF.md | 2 + xff/cli/BUILD.bazel | 29 +++++++ xff/cli/config_validation.cc | 134 ++++++++++++++++++++++++++++++ xff/cli/config_validation.h | 22 +++++ xff/cli/config_validation_test.cc | 73 ++++++++++++++++ xff/cli/explain_test.sh | 10 +++ xff/cli/globals.cc | 69 ++++++++------- xff/cli/globals.h | 11 +++ xff/cli/globals_test.cc | 24 ++++++ xff/cli/help_build.cc | 7 ++ xff/cli/main.cc | 4 + 12 files changed, 363 insertions(+), 29 deletions(-) create mode 100644 xff/cli/config_validation.cc create mode 100644 xff/cli/config_validation.h create mode 100644 xff/cli/config_validation_test.cc diff --git a/CHANGELOG.md b/CHANGELOG.md index 6046a16e2e..d72629e0f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ explicitly selected `--xffrc` files. Each permission pair governs only its matching skip flag. - Add config-selectable `--allow-xffrc` / `--no-allow-xffrc` controls for explicitly named config files, subject to the system policy gate and unavailable to the file being admitted. +- Warn about overriding the same setting more than once within one logical config section while + preserving last-value-wins behavior. Deliberately accumulating settings, expression primaries, + separate sections and tiers, and all command-line repetitions remain unaffected. - Apply named configuration and explicitly loaded config files at each command-line selector's exact position instead of flattening them ahead of all CLI options. - Defer regex matcher binding until the final configuration has selected grammar @@ -23,6 +26,10 @@ while `--sort=global` sorts the operands and walks each tree in deterministic depth-first order. Clarify the ordering, directory read-ahead, result buffering, post-order, and score-ranking contracts throughout the generated reference and parallel-walk design. +- Honor the selected path encoding in tree-comparison status records, including non-UTF-8 path + bytes, instead of emitting raw relative paths independently of the output configuration. +- Apply every matching system-policy safety-class denial to mixed config lines, so a destructive + primary cannot be hidden from an `@destructive` rule by a sensitive primary on the same line. - Clarify concurrency and matching controls, including worker defaults and `-exec` timing, filesystem-native versus explicit case folding, block-size units, and regex grammar scope. diff --git a/XFF.md b/XFF.md index f7198e94af..cbd3b4f9d8 100644 --- a/XFF.md +++ b/XFF.md @@ -83,6 +83,8 @@ Every `--config=NAME` remains active, so multiple named blocks can apply. Among Configuration expands in application order. Automatic system/user defaults and the invocation selector come first; each command-line `--config` then activates newly matching lines at that exact position, and each `--xffrc` loads its currently matching lines where it appears. Because conflicting options usually use the last value, moving a selector can intentionally change the result. A config line is applied at most once. +Within one logical config section, repeating an overriding setting keeps the normal last-value-wins result but emits a warning: the earlier value is locally redundant and is usually a copy/paste mistake. Options that deliberately accumulate (such as `--exclude`, `--summary`, and `--define`) may repeat without warning, as may expression primaries. Separate config sections and tiers remain independent. Command-line repetition is never warned about, so a pasted command can be adjusted by appending an override. + ### Arming dangerous directives A dangerous directive (the exec family `-exec` / `-execdir` / `-ok` / `-capture`, or `-delete`) carried by an `--xffrc` file is inert unless `--allow-exec` is set from a trusted tier (the command line or the system/user config, never an `--xffrc` file itself). Unarmed lines are dropped with a warning; the root system `[policy]` can hard-deny even `--allow-exec`. diff --git a/xff/cli/BUILD.bazel b/xff/cli/BUILD.bazel index 67384e8dbe..d36dd62038 100644 --- a/xff/cli/BUILD.bazel +++ b/xff/cli/BUILD.bazel @@ -41,6 +41,7 @@ cc_library( name = "main_cc", srcs = ["main.cc"], deps = [ + ":config_validation_cc", ":globals_cc", ":help_backend_cc", ":help_build_cc", @@ -73,6 +74,34 @@ cc_library( ], ) +cc_library( + name = "config_validation_cc", + srcs = ["config_validation.cc"], + hdrs = ["config_validation.h"], + deps = [ + ":globals_cc", + "//xff/config:config_cc", + "//xff/config:xffrc_cc", + "//xff/registry:registry_cc", + "@abseil-cpp//absl/container:flat_hash_set", + "@abseil-cpp//absl/strings", + "@mboworks_mbo//mbo/types:optional_ref_cc", + ], +) + +cc_test( + name = "config_validation_test", + size = "small", + srcs = ["config_validation_test.cc"], + deps = [ + ":config_validation_cc", + "//xff/config:config_cc", + "//xff/config:xffrc_cc", + "@googletest//:gtest", + "@googletest//:gtest_main", + ], +) + # The stock lean binary: core only, no composable extras. This is the target every test and golden # runs against, and the one built by `//...`. cc_binary( diff --git a/xff/cli/config_validation.cc b/xff/cli/config_validation.cc new file mode 100644 index 0000000000..cedfbdda7e --- /dev/null +++ b/xff/cli/config_validation.cc @@ -0,0 +1,134 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +#include "xff/cli/config_validation.h" + +#include +#include +#include +#include + +#include "absl/container/flat_hash_set.h" +#include "absl/strings/str_cat.h" +#include "mbo/types/optional_ref.h" +#include "xff/cli/globals.h" +#include "xff/config/config.h" +#include "xff/config/xffrc.h" +#include "xff/registry/descriptor.h" +#include "xff/registry/registry.h" + +namespace xff::cli { +namespace { + +struct SectionSettings { + std::string key; + absl::flat_hash_set names; +}; + +std::string CanonicalName(const GlobalFlag& flag) { + constexpr std::string_view kNo = "--no-"; + if (!flag.name.starts_with(kNo)) { + return std::string(flag.name); + } + const std::string positive = absl::StrCat("--", flag.name.substr(kNo.size())); + const mbo::types::OptionalRef counterpart = LookupGlobal(positive); + return std::string( + counterpart.has_value() && counterpart->repetition == GlobalFlag::Repetition::kOverride ? counterpart->name + : flag.name); +} + +std::string SettingName(std::string_view token, const GlobalFlag& flag) { + std::string name = CanonicalName(flag); + if (flag.repetition != GlobalFlag::Repetition::kKeyed) { + return name; + } + const std::string_view::size_type first = token.find('='); + if (first == std::string_view::npos) { + return name; + } + const std::string_view value = token.substr(first + 1); + const std::string_view key = value.substr(0, value.find('=')); + absl::StrAppend(&name, "=", key); + return name; +} + +void RecordSetting( + std::string_view token, + std::string_view location, + absl::flat_hash_set& seen, + std::vector& notices) { + const mbo::types::OptionalRef flag = LookupGlobalArgument(token); + if (!flag.has_value() || flag->repetition == GlobalFlag::Repetition::kAccumulate) { + return; + } + const std::string name = SettingName(token, *flag); + if (!seen.insert(name).second) { + notices.push_back(absl::StrCat("setting ", name, " is overridden within ", location)); + } +} + +std::string_view PrimaryName(std::string_view token) { + return token.substr(0, token.find(':')); +} + +void FindOverrides( + const std::vector& tokens, + std::string_view location, + absl::flat_hash_set& seen, + std::vector& notices) { + for (std::size_t pos = 0; pos < tokens.size(); ++pos) { + if (tokens[pos] == "--") { + break; + } + if (LookupGlobalArgument(tokens[pos]).has_value()) { + RecordSetting(tokens[pos], location, seen, notices); + continue; + } + const auto primary = registry::Lookup(PrimaryName(tokens[pos])); + if (!primary.has_value()) { + continue; + } + if (primary->arity < 0) { + while (++pos < tokens.size() && tokens[pos] != ";" && tokens[pos] != "+") {} + } else { + pos += static_cast(primary->arity); + } + } +} + +void FindRcOverrides( + const std::vector& lines, + std::string_view file, + std::vector& notices) { + std::vector sections; + for (const config::RcLine& line : lines) { + const std::string key = absl::StrCat(line.base, ":", line.config); + auto section = sections.begin(); + while (section != sections.end() && section->key != key) { + ++section; + } + if (section == sections.end()) { + sections.push_back({.key = key}); + section = sections.end() - 1; + } + const std::string location = absl::StrCat(file, " section '", key, "'"); + FindOverrides(line.flags, location, section->names, notices); + } +} + +} // namespace + +std::vector ConfigOverrideNotices(const config::ConfigInputs& inputs) { + std::vector notices; + absl::flat_hash_set system_globals; + FindOverrides(inputs.system.globals, "system config globals", system_globals, notices); + absl::flat_hash_set system_defaults; + FindOverrides(inputs.system.defaults, "system config [defaults]", system_defaults, notices); + FindRcOverrides(inputs.user, "user config", notices); + for (const config::ExplicitConfig& file : inputs.xffrc) { + FindRcOverrides(file.lines, absl::StrCat("--xffrc file ", file.path), notices); + } + return notices; +} + +} // namespace xff::cli diff --git a/xff/cli/config_validation.h b/xff/cli/config_validation.h new file mode 100644 index 0000000000..93464bea96 --- /dev/null +++ b/xff/cli/config_validation.h @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +#ifndef XFF_CLI_CONFIG_VALIDATION_H_ +#define XFF_CLI_CONFIG_VALIDATION_H_ + +#include +#include + +#include "xff/config/config.h" + +namespace xff::cli { + +// Reports overriding global settings that occur more than once in one logical config section. +// Positive/negative forms are one setting; aliases and valued forms use their canonical global +// identity. Accumulating settings, expression primaries, separate selectors, and separate files +// remain independent. Structurally invalid config controls are rejected separately by policy. +std::vector ConfigOverrideNotices(const config::ConfigInputs& inputs); + +} // namespace xff::cli + +#endif // XFF_CLI_CONFIG_VALIDATION_H_ diff --git a/xff/cli/config_validation_test.cc b/xff/cli/config_validation_test.cc new file mode 100644 index 0000000000..0fd2171ba5 --- /dev/null +++ b/xff/cli/config_validation_test.cc @@ -0,0 +1,73 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +#include "xff/cli/config_validation.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "xff/config/config.h" +#include "xff/config/xffrc.h" + +namespace xff::cli { +namespace { + +using ::testing::ElementsAre; +using ::testing::HasSubstr; +using ::testing::IsEmpty; + +struct ConfigValidationTest : ::testing::Test {}; + +TEST_F(ConfigValidationTest, ReportsCanonicalAliasAndNegatedOverrides) { + config::ConfigInputs inputs; + inputs.system.defaults = {"--timezone=utc", "--tz=local"}; + EXPECT_THAT(ConfigOverrideNotices(inputs), ElementsAre(HasSubstr("setting --timezone is overridden"))); + + inputs.system.defaults = {"--hidden", "--no-hidden"}; + EXPECT_THAT(ConfigOverrideNotices(inputs), ElementsAre(HasSubstr("setting --hidden is overridden"))); +} + +TEST_F(ConfigValidationTest, PermitsAccumulatingSettings) { + config::ConfigInputs inputs; + inputs.system.defaults = {"--exclude=one", "--exclude=two", "--config=one", "--config=two"}; + EXPECT_THAT(ConfigOverrideNotices(inputs), IsEmpty()); +} + +TEST_F(ConfigValidationTest, KeyedSettingsAccumulateByNameAndReportSameNameOverrides) { + config::ConfigInputs inputs; + inputs.system.defaults = {"--define=A=one", "--define=B=two"}; + EXPECT_THAT(ConfigOverrideNotices(inputs), IsEmpty()); + + inputs.system.defaults.emplace_back("--define=A=three"); + EXPECT_THAT(ConfigOverrideNotices(inputs), ElementsAre(HasSubstr("setting --define=A is overridden"))); +} + +TEST_F(ConfigValidationTest, CombinesRepeatedLogicalUserSections) { + config::ConfigInputs inputs; + inputs.user = config::ParseXffrc("debug: --sort=tree\nother: --sort=global\ndebug: --sort=none"); + EXPECT_THAT(ConfigOverrideNotices(inputs), ElementsAre(HasSubstr("user config section 'debug:'"))); +} + +TEST_F(ConfigValidationTest, KeepsDifferentSectionsAndFilesIndependent) { + config::ConfigInputs inputs; + inputs.user = config::ParseXffrc("debug: --sort=tree\nother: --sort=global"); + inputs.xffrc = { + {.path = "/one", .lines = config::ParseXffrc("common: --color=always")}, + {.path = "/two", .lines = config::ParseXffrc("common: --color=never")}, + }; + EXPECT_THAT(ConfigOverrideNotices(inputs), IsEmpty()); +} + +TEST_F(ConfigValidationTest, DoesNotTreatPrimaryArgumentsAsGlobalSettings) { + config::ConfigInputs inputs; + inputs.user = config::ParseXffrc("common: -exec echo --sort=tree ; --sort=global"); + EXPECT_THAT(ConfigOverrideNotices(inputs), IsEmpty()); +} + +TEST_F(ConfigValidationTest, StopsRecognizingSettingsAfterDoubleDash) { + config::ConfigInputs inputs; + inputs.user = config::ParseXffrc("common: --sort=tree -- --sort=global"); + EXPECT_THAT(ConfigOverrideNotices(inputs), IsEmpty()); +} + +} // namespace +} // namespace xff::cli diff --git a/xff/cli/explain_test.sh b/xff/cli/explain_test.sh index 8ce2b9fa91..23c0df022c 100755 --- a/xff/cli/explain_test.sh +++ b/xff/cli/explain_test.sh @@ -150,6 +150,16 @@ test::authorized_no_user_config_suppresses_user_defaults_but_keeps_explicit_xffr expect_matches 'xffrc[[:space:]]+--color=never' "${out}" } +test::config_local_override_warns_but_last_value_still_wins() { + local cfg="${TEST_TMPDIR}/local_override" + printf 'common: --color=always\ncommon: --color=never\n' >"${cfg}" + local out + out="$(XFF_CONFIG="${cfg}" "$(_xff_bin)" --explain 2>&1)" + expect_output_contains 'warning: setting --color is overridden within user config section' "${out}" + expect_matches 'user[[:space:]]+--color=always' "${out}" + expect_matches 'user[[:space:]]+--color=never' "${out}" +} + test::xffrc_dangerous_line_is_inert_unless_armed() { # The --xffrc tier is non-arming: a sensitive -exec carried by the file is dropped (inert) with a # "needs --allow-exec" note unless --allow-exec is passed from a trusted tier (here, the CLI). A diff --git a/xff/cli/globals.cc b/xff/cli/globals.cc index 3c1db9612b..c15ed818b0 100644 --- a/xff/cli/globals.cc +++ b/xff/cli/globals.cc @@ -271,6 +271,7 @@ constexpr std::array kGlobals = std::to_array({ "A `STYLE:EPOCH` spelling such as `xff:2` selects `STYLE` while retaining the full name as a config " "selector. See `--help=styles` for the per-style defaults and `--help=config` for layering.", .topic = "config", + .repetition = GlobalFlag::Repetition::kAccumulate, }, { .name = "--no-config", @@ -326,6 +327,7 @@ constexpr std::array kGlobals = std::to_array({ "a one-line warning. Repeatable; later files win.", .affects = "--allow-exec", .topic = "config", + .repetition = GlobalFlag::Repetition::kAccumulate, }, { .name = "--allow-exec", @@ -792,6 +794,7 @@ constexpr std::array kGlobals = std::to_array({ .header = "Path filters", .summary = "skip paths matching a gitignore-style glob (repeatable; a matched directory is pruned)", .topic = "ignore", + .repetition = GlobalFlag::Repetition::kAccumulate, }, { .name = "--include", @@ -800,6 +803,7 @@ constexpr std::array kGlobals = std::to_array({ .header = "Path filters", .summary = "re-include paths a --exclude would skip, matching a gitignore-style glob (repeatable)", .topic = "ignore", + .repetition = GlobalFlag::Repetition::kAccumulate, }, { .name = "--lang-db", @@ -814,6 +818,7 @@ constexpr std::array kGlobals = std::to_array({ "between two languages in ONE file follow `--lang-conflicts`.", .affects = "-lang", .topic = "content", + .repetition = GlobalFlag::Repetition::kAccumulate, }, { .name = "--lang-conflicts", @@ -841,6 +846,7 @@ constexpr std::array kGlobals = std::to_array({ "folds case. Conflicts between two types in ONE file follow `--mime-conflicts`.", .affects = "-mime", .topic = "content", + .repetition = GlobalFlag::Repetition::kAccumulate, }, { .name = "--mime-conflicts", @@ -887,6 +893,7 @@ constexpr std::array kGlobals = std::to_array({ .header = "Filter & Ignore", .summary = "read an extra gitignore-format file, rooted at its own directory (repeatable)", .topic = "ignore", + .repetition = GlobalFlag::Repetition::kAccumulate, }, { .name = "--no-ignore", @@ -1174,6 +1181,7 @@ constexpr std::array kGlobals = std::to_array({ .affects = "--pack", .topic = "archive", .extra = "archive", + .repetition = GlobalFlag::Repetition::kKeyed, }, { .name = "--pack-level", @@ -1216,6 +1224,7 @@ constexpr std::array kGlobals = std::to_array({ "for scripts.", .values = kSummaryValues, .topic = "stats", + .repetition = GlobalFlag::Repetition::kAccumulate, .value_check = GlobalFlag::ValueCheck::kEnumOrTemplate, }, { @@ -1236,6 +1245,7 @@ constexpr std::array kGlobals = std::to_array({ "--unicode) or ASCII '#' otherwise; --top=N keeps the N tallest and --format=jsonl emits one " "object per bar for scripts.", .topic = "stats", + .repetition = GlobalFlag::Repetition::kAccumulate, }, { .name = "--shards", @@ -1293,6 +1303,7 @@ constexpr std::array kGlobals = std::to_array({ "`(?P...)` and `(?P...)` are optional. Repeatable; the patterns are tried in " "order, before the built-in schemes.", .topic = "stats", + .repetition = GlobalFlag::Repetition::kAccumulate, }, { .name = "--count", @@ -1571,6 +1582,7 @@ constexpr std::array kGlobals = std::to_array({ .group = "fields", .header = "Fields & Exec", .summary = "define a value referenced as {def.NAME}", + .repetition = GlobalFlag::Repetition::kKeyed, }, { .name = "--time-format", @@ -1626,6 +1638,33 @@ mbo::types::OptionalRef LookupGlobal(std::string_view name) { return std::nullopt; } +mbo::types::OptionalRef LookupGlobalArgument(std::string_view arg) { + if (const mbo::types::OptionalRef exact = LookupGlobal(arg); exact.has_value()) { + return exact; + } + if (arg == "-0") { + return LookupGlobal("--format"); + } + if (arg == "-i") { + return LookupGlobal("--case"); + } + if (arg.starts_with("-j") && arg.size() > 2 && !arg.starts_with("-j=")) { + return LookupGlobal("--jobs"); + } + for (const GlobalFlag& flag : Globals()) { + if (absl::c_contains(flag.sign_forms, arg)) { + return flag; + } + } + if (const std::string_view::size_type equals = arg.find('='); equals != std::string_view::npos) { + const mbo::types::OptionalRef valued = LookupGlobal(arg.substr(0, equals)); + if (valued.has_value() && absl::StrContains(valued->display, '=')) { + return valued; + } + } + return std::nullopt; +} + absl::Status ValidateGlobalValue(std::string_view arg) { const std::string_view::size_type equals = arg.find('='); if (equals == std::string_view::npos) { @@ -1682,35 +1721,7 @@ absl::Status ValidateGlobalValue(std::string_view arg) { } bool IsKnownGlobal(std::string_view arg) { - // The sign ladders come from the flags themselves (GlobalFlag::sign_forms), so a new one is - // recognised by declaring it rather than by also editing a list here. - for (const GlobalFlag& flag : Globals()) { - if (absl::c_contains(flag.sign_forms, arg)) { - return true; - } - } - // What is left are the compat aliases that carry no sign: -0 (= --format=nul) and -i - // (= --case=insensitive). - if (arg == "-0" || arg == "-i") { - return true; - } - // Conventional attached short-option argument: -j4 / -jall. The help leads - // with the more readable -j N and -j=N forms, but build-tool users expect this. - if (arg.starts_with("-j") && arg.size() > 2 && !arg.starts_with("-j=")) { - return true; - } - // An exact name or alias (bare flags, and valued flags used without a value). - if (LookupGlobal(arg).has_value()) { - return true; - } - // A valued form name=VALUE / alias=VALUE: the key must resolve to a flag that - // advertises a value (its display contains '='), so `--safe=x` stays unknown while - // `--sort=tree` / `--define=A=B` are accepted (only the key before the first '='). - if (const std::string_view::size_type eq = arg.find('='); eq != std::string_view::npos) { - const mbo::types::OptionalRef flag = LookupGlobal(arg.substr(0, eq)); - return flag.has_value() && absl::StrContains(flag->display, '='); - } - return false; + return LookupGlobalArgument(arg).has_value(); } bool ExtraEnabled(std::string_view key) { diff --git a/xff/cli/globals.h b/xff/cli/globals.h index 2ef7f81e96..c551a73a4f 100644 --- a/xff/cli/globals.h +++ b/xff/cli/globals.h @@ -88,6 +88,12 @@ struct GlobalFlag { // extra step); what each level MEANS is the flag's own business, and the families differ: `-g` // is a tri-state, `-z` is a nested ladder, `-Z` is that ladder with a capability added. absl::Span sign_forms; + // How multiple occurrences combine. This is parser metadata, not merely documentation: config + // diagnostics distinguish deliberate stacking from a likely-unnecessary override inside one + // logical section. kKeyed accumulates different NAMEs but overrides the same NAME, as in + // `--define=NAME=VALUE` and inline `--pack-option=NAME=VALUE`. + enum class Repetition : std::uint8_t { kOverride, kAccumulate, kKeyed }; + Repetition repetition = Repetition::kOverride; // How a `name=VALUE` form is CHECKED, so a typo is a usage error rather than a silent default // (`--case=insensitve` used to match case-sensitively and look like it worked). Only kNone // accepts anything: it is the default because most valued flags take free text (a path, a @@ -132,6 +138,11 @@ absl::Span Globals(); // none. `name` carries its leading dashes (e.g. "--sort", "-j"). mbo::types::OptionalRef LookupGlobal(std::string_view name); +// The global option represented by the complete argument `arg`, normalized to its registry entry. +// Accepts the same exact, valued, sign-suffixed, and compatibility forms as IsKnownGlobal. This is +// useful when callers need the canonical option identity rather than only a yes/no classification. +mbo::types::OptionalRef LookupGlobalArgument(std::string_view arg); + // Whether `arg` is a recognized whole-run global token, so `main` can reject an // unknown leading option instead of silently ignoring it. Accepts: an exact name or // alias; a valued `name=VALUE` / `alias=VALUE` form when the flag advertises a value diff --git a/xff/cli/globals_test.cc b/xff/cli/globals_test.cc index 792f7522b2..8727295196 100644 --- a/xff/cli/globals_test.cc +++ b/xff/cli/globals_test.cc @@ -32,8 +32,10 @@ using ::mbo::testing::IsOk; using ::mbo::testing::StatusIs; using ::testing::_; using ::testing::Contains; +using ::testing::ElementsAre; using ::testing::ElementsAreArray; using ::testing::Eq; +using ::testing::Field; using ::testing::HasSubstr; using ::testing::IsEmpty; using ::testing::IsFalse; @@ -88,6 +90,15 @@ TEST_F(GlobalsTest, LookupResolvesNameAndAlias) { EXPECT_THAT(LookupGlobal("--nonesuch"), Eq(std::nullopt)); } +TEST_F(GlobalsTest, LookupArgumentCanonicalizesEveryAcceptedForm) { + EXPECT_THAT(LookupGlobalArgument("--sort=global"), Optional(Field("name", &GlobalFlag::name, Eq("--sort")))); + EXPECT_THAT(LookupGlobalArgument("--tz=utc"), Optional(Field("name", &GlobalFlag::name, Eq("--timezone")))); + EXPECT_THAT(LookupGlobalArgument("-j4"), Optional(Field("name", &GlobalFlag::name, Eq("--jobs")))); + EXPECT_THAT(LookupGlobalArgument("-g+"), Optional(Field("name", &GlobalFlag::name, Eq("--gitignore")))); + EXPECT_THAT(LookupGlobalArgument("-0"), Optional(Field("name", &GlobalFlag::name, Eq("--format")))); + EXPECT_THAT(LookupGlobalArgument("--unknown"), Eq(std::nullopt)); +} + TEST_F(GlobalsTest, StringifiesAsCanonicalName) { const mbo::types::OptionalRef jobs = LookupGlobal("--jobs"); ASSERT_THAT(jobs, Optional(_)); @@ -138,6 +149,19 @@ TEST_F(GlobalsTest, EveryGlobalResolvesByItsOwnName) { } } +TEST_F(GlobalsTest, NonOverridingGlobalsDeclareTheirRepetitionSemantics) { + std::vector names; + for (const GlobalFlag& flag : Globals()) { + if (flag.repetition != GlobalFlag::Repetition::kOverride) { + names.push_back(flag.name); + } + } + EXPECT_THAT( + names, ElementsAre( + "--config", "--xffrc", "--exclude", "--include", "--lang-db", "--mime-vocabulary", "--ignore-file", + "--pack-option", "--summary", "--histogram", "--shard-pattern", "--define")); +} + TEST_F(GlobalsTest, IsKnownGlobalAcceptsEveryTableNameAndAlias) { for (const GlobalFlag& flag : Globals()) { EXPECT_THAT(IsKnownGlobal(flag.name), IsTrue()) << flag.name; diff --git a/xff/cli/help_build.cc b/xff/cli/help_build.cc index 6a75e6b4e7..e4a0cb6136 100644 --- a/xff/cli/help_build.cc +++ b/xff/cli/help_build.cc @@ -917,6 +917,13 @@ Section ConfigSection(bool in_full) { "each `--xffrc` loads its currently matching lines where it appears. Because conflicting options usually " "use the last value, moving a selector can intentionally change the result. A config line is applied at " "most once.")); + style.children.push_back(ProseOf( + "Within one logical config section, repeating an overriding setting keeps the normal last-value-wins " + "result but emits a warning: the earlier value is locally redundant and is usually a copy/paste mistake. " + "Options that deliberately accumulate (such as `--exclude`, `--summary`, and `--define`) may repeat " + "without warning, as may expression primaries. Separate config sections and tiers remain independent. " + "Command-line repetition is never warned about, so a pasted command can be adjusted by appending an " + "override.")); section.children.push_back(Content{.node = std::move(style)}); Subsection arming{.title = "Arming dangerous directives"}; diff --git a/xff/cli/main.cc b/xff/cli/main.cc index eec77a6b0f..c4cacfe96d 100644 --- a/xff/cli/main.cc +++ b/xff/cli/main.cc @@ -32,6 +32,7 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_join.h" #include "absl/types/span.h" +#include "xff/cli/config_validation.h" #include "xff/cli/globals.h" #include "xff/cli/help.h" #include "xff/cli/help_backend.h" @@ -658,6 +659,9 @@ int RunMain(int argc, char** argv) { // Config is system + user + explicit --xffrc only; there is no auto-discovered project layer // (Option B, 2026-07-06), so the search roots do not feed config discovery. const xff::config::ConfigInputs inputs = xff::config::Discover(opts, ReadFile); + for (const std::string& notice : xff::cli::ConfigOverrideNotices(inputs)) { + std::cerr << "xff: warning: " << notice << "\n"; + } if (const absl::Status status = xff::config::ValidateConfigSkips(inputs); !status.ok()) { std::cerr << "xff: " << status.message() << "\n"; return 2;