git/parse-options.c, branch v2.43.2

Merge branch 'rs/parse-options-with-keep-unknown-abbrev-fix' into maint-2.43

2024-02-13T22:44:48Z

"git diff --no-rename A B" did not disable rename detection but did not trigger an error from the command line parser. * rs/parse-options-with-keep-unknown-abbrev-fix: parse-options: simplify positivation handling parse-options: fully disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN

Merge branch 'en/header-cleanup' into maint-2.43

2024-02-09T00:22:10Z

Remove unused header "#include". * en/header-cleanup: treewide: remove unnecessary includes in source files treewide: add direct includes currently only pulled in transitively trace2/tr2_tls.h: remove unnecessary include submodule-config.h: remove unnecessary include pkt-line.h: remove unnecessary include line-log.h: remove unnecessary include http.h: remove unnecessary include fsmonitor--daemon.h: remove unnecessary includes blame.h: remove unnecessary includes archive.h: remove unnecessary include treewide: remove unnecessary includes in source files treewide: remove unnecessary includes from header files

Merge branch 'rs/i18n-cannot-be-used-together' into maint-2.43

2024-02-09T00:22:05Z

Clean-up code that handles combinations of incompatible options. * rs/i18n-cannot-be-used-together: i18n: factorize even more 'incompatible options' messages

Merge branch 'jk/end-of-options' into maint-2.43

2024-02-09T00:22:02Z

"git $cmd --end-of-options --rev -- --path" for some $cmd failed to interpret "--rev" as a rev, and "--path" as a path. This was fixed for many programs like "reset" and "checkout". * jk/end-of-options: parse-options: decouple "--end-of-options" and "--"

parse-options: simplify positivation handling

2024-01-22T15:17:12Z

We accept the positive version of options whose long name starts with "no-" and are defined without the flag PARSE_OPT_NONEG. E.g. git clone has an explicitly defined --no-checkout option and also implicitly accepts --checkout to override it. parse_long_opt() handles that by restarting the option matching with the positive version when it finds that only the current option definition starts with "no-", but not the user-supplied argument. This code is located almost at the end of the matching logic. Avoid the need for a restart by moving the code up. We don't have to check the positive arg against the negative long_name at all -- the "no-" prefix of the latter makes a match impossible. Skip it and toggle OPT_UNSET right away to simplify the control flow. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

parse-options: fully disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN

2024-01-20T17:55:43Z

baa4adc66a (parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN, 2019-01-27) turned off support for abbreviated options when the flag PARSE_OPT_KEEP_UNKNOWN is given, as any shortened option could also be an abbreviation for one of the unknown options. The code for handling abbreviated options is guarded by an if, but it can also be reached via goto. baa4adc66a only blocked the first way. Add the condition to the other ones as well. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

treewide: remove unnecessary includes in source files

2023-12-26T20:04:31Z

Each of these were checked with gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE} to ensure that removing the direct inclusion of the header actually resulted in that header no longer being included at all (i.e. that no other header pulled it in transitively). ...except for a few cases where we verified that although the header was brought in transitively, nothing from it was directly used in that source file. These cases were: * builtin/credential-cache.c * builtin/pull.c * builtin/send-pack.c Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

parse-options: decouple "--end-of-options" and "--"

2023-12-08T23:21:02Z

When we added generic end-of-options support in 51b4594b40 (parse-options: allow --end-of-options as a synonym for "--", 2019-08-06), we made them true synonyms. They both stop option parsing, and they are both returned in the resulting argv if the KEEP_DASHDASH flag is used. The hope was that this would work for all callers: - most generic callers would not pass KEEP_DASHDASH, and so would just do the right thing (stop parsing there) without needing to know anything more. - callers with KEEP_DASHDASH were generally going to rely on setup_revisions(), which knew to handle --end-of-options specially But that turned out miss quite a few cases that pass KEEP_DASHDASH but do their own manual parsing. For example, "git reset", "git checkout", and so on want pass KEEP_DASHDASH so they can support: git reset $revs -- $paths but of course aren't going to actually do a traversal, so they don't call setup_revisions(). And those cases currently get confused by --end-of-options being left in place, like: $ git reset --end-of-options HEAD fatal: option '--end-of-options' must come before non-option arguments We could teach each of these callers to handle the leftover option explicitly. But let's try to be a bit more clever and see if we can solve it centrally in parse-options.c. The bogus assumption here is that KEEP_DASHDASH tells us the caller wants to see --end-of-options in the result. But really, the callers which need to know that --end-of-options was reached are those that may potentially parse more options from argv. In other words, those that pass the KEEP_UNKNOWN_OPT flag. If such a caller is aware of --end-of-options (e.g., because they call setup_revisions() with the result), then this will continue to do the right thing, treating anything after --end-of-options as a non-option. And if the caller is not aware of --end-of-options, they are better off keeping it intact, because either: 1. They are just passing the options along to somebody else anyway, in which case that somebody would need to know about the --end-of-options marker. 2. They are going to parse the remainder themselves, at which point choking on --end-of-options is much better than having it silently removed. The point is to avoid option injection from untrusted command line arguments, and bailing is better than quietly treating the untrusted argument as an option. This fixes bugs with --end-of-options across several commands, but I've focused on two in particular here: - t7102 confirms that "git reset --end-of-options --foo" now works. This checks two things. One, that we no longer barf on "--end-of-options" itself (which previously we did, even if the rev was something vanilla like "HEAD" instead of "--foo"). And two, that we correctly treat "--foo" as a revision rather than an option. This fix applies to any other cases which pass KEEP_DASHDASH but not KEEP_UNKNOWN_OPT, like "git checkout", "git check-attr", "git grep", etc, which would previously choke on "--end-of-options". - t9350 shows the opposite case: fast-export passed KEEP_UNKNOWN_OPT but not KEEP_DASHDASH, but then passed the result on to setup_revisions(). So it never saw --end-of-options, and would erroneously parse "fast-export --end-of-options --foo" as having a "--foo" option. This is now fixed. Note that this does shut the door for callers which want to know if we hit end-of-options, but don't otherwise need to keep unknown opts. The obvious thing here is feeding it to the DWIM verify_filename() machinery. And indeed, this is a problem even for commands which do understand --end-of-options already. For example, without this patch, you get: $ git log --end-of-options --foo fatal: option '--foo' must come before non-option arguments because we refuse to accept "--foo" as a filename (because it starts with a dash) even though we could know that we saw end-of-options. The verify_filename() function simply doesn't accept this extra information. So that is the status quo, and this patch doubles down further on that. Commands like "git reset" have the same problem, but they won't even know that parse-options saw --end-of-options! So even if we fixed verify_filename(), they wouldn't have anything to pass to it. But in practice I don't think this is a big deal. If you are being careful enough to use --end-of-options, then you should also be using "--" to disambiguate and avoid the DWIM behavior in the first place. In other words, doing: git log --end-of-options --this-is-a-rev -- --this-is-a-path works correctly, and will continue to do so. And likewise, with this patch now: git reset --end-of-options --this-is-a-rev -- --this-is-a-path will work, as well. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

i18n: factorize even more 'incompatible options' messages

2023-11-27T01:01:45Z

Continue the work of 12909b6b8a (i18n: turn "options are incompatible" into "cannot be used together", 2022-01-05) and a699367bb8 (i18n: factorize more 'incompatible options' messages, 2022-01-31) to use the same parameterized error message for reporting incompatible command line options. This reduces the number of strings to translate and makes the UI slightly more consistent. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

parse-options: make CMDMODE errors more precise

2023-10-29T00:15:18Z

Only a single PARSE_OPT_CMDMODE option can be specified for the same variable at the same time. This is enforced by get_value(), but the error messages are imprecise in three ways: 1. If a non-PARSE_OPT_CMDMODE option changes the value variable of a PARSE_OPT_CMDMODE option then an ominously vague message is shown: $ t/helper/test-tool parse-options --set23 --mode1 error: option `mode1' : incompatible with something else Worse: If the order of options is reversed then no error is reported at all: $ t/helper/test-tool parse-options --mode1 --set23 boolean: 0 integer: 23 magnitude: 0 timestamp: 0 string: (not set) abbrev: 7 verbose: -1 quiet: 0 dry run: no file: (not set) Fortunately this can currently only happen in the test helper; actual Git commands don't share the same variable for the value of options with and without the flag PARSE_OPT_CMDMODE. 2. If there are multiple options with the same value (synonyms), then the one that is defined first is shown rather than the one actually given on the command line, which is confusing: $ git am --resolved --quit error: option `quit' is incompatible with --continue 3. Arguments of PARSE_OPT_CMDMODE options are not handled by the parse-option machinery. This is left to the callback function. We currently only have a single affected option, --show-current-patch of git am. Errors for it can show an argument that was not actually given on the command line: $ git am --show-current-patch --show-current-patch=diff error: options '--show-current-patch=diff' and '--show-current-patch=raw' cannot be used together The options --show-current-patch and --show-current-patch=raw are synonyms, but the error accuses the user of input they did not actually made. Or it can awkwardly print a NULL pointer: $ git am --show-current-patch=diff --show-current-patch error: options '--show-current-patch=(null)' and '--show-current-patch=diff' cannot be used together The reasons for these shortcomings is that the current code checks incompatibility only when encountering a PARSE_OPT_CMDMODE option at the command line, and that it searches the previous incompatible option by value. Fix the first two points by checking all PARSE_OPT_CMDMODE variables after parsing each option and by storing all relevant details if their value changed. Do that whether or not the changing options has the flag PARSE_OPT_CMDMODE set. Report an incompatibility only if two options change the variable to different values and at least one of them is a PARSE_OPT_CMDMODE option. This changes the output of the first three examples above to: $ t/helper/test-tool parse-options --set23 --mode1 error: --mode1 is incompatible with --set23 $ t/helper/test-tool parse-options --mode1 --set23 error: --set23 is incompatible with --mode1 $ git am --resolved --quit error: --quit is incompatible with --resolved Store the argument of PARSE_OPT_CMDMODE options of type OPTION_CALLBACK as well to allow taking over the responsibility for compatibility checking from the callback function. The next patch will use this capability to fix the messages for git am --show-current-patch. Use a linked list for storing the PARSE_OPT_CMDMODE variables. This somewhat outdated data structure is simple and suffices, as the number of elements per command is currently only zero or one. We do support multiple different command modes variables per command, but I don't expect that we'd ever use a significant number of them. Once we do we can switch to a hashmap. Since we no longer need to search the conflicting option, the all_opts parameter of get_value() is no longer used. Remove it. Extend the tests to check for both conflicting option names, but don't insist on a particular order. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano