summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2023-04-03credential/wincred: store password_expiry_utcM Hickford1-2/+23
This attribute is important when storing OAuth credentials which may expire after as little as one hour. d208bfdf (credential: new attribute password_expiry_utc, 2023-02-18) added support for this attribute in general so that individual credential backend like wincred can use it. Signed-off-by: M Hickford <mirth.hickford@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-03format-patch: correct documentation of --thread without an argumentAlex Henrie2-2/+6
In Git, almost all command line flags unconditionally override the corresponding config option.[1] Add a test to confirm that this is the case for `git format-patch --thread`. [1] https://lore.kernel.org/git/CAMMLpeS3+NUQa2oqpHKVo3yWQNVMgkEXrs4U5_ggvk31yQbezQ@mail.gmail.com/ Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-03object-info: init request_info before reading argJiang Xin1-1/+1
When retrieving object info via capability "object-info", we store the command args into a requested_info variable, but forget to initialize it. Initialize the variable before use to prevent unexpected output. Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-03e-mail workflow: Message-ID is spelled with ID in both capital lettersJunio C Hamano15-75/+75
We used to write "Message-Id:" and "Message-ID:" pretty much interchangeably, and the header name is defined to be case insensitive by the RFCs, but the canonical form "Message-ID:" is used throughout the RFC documents, so let's imitate it ourselves. Signed-off-by: Junio C Hamano <gitster@pobox.com> Reviewed-by: Elijah Newren <newren@gmail.com>
2023-04-03l10n: TEAMS: Update pt_PT repo linkDaniel Santos1-1/+1
Signed-off-by: Daniel Santos <dacs.git@brilhante.top>
2023-03-31The sixth batchJunio C Hamano1-0/+4
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-31Merge branch 'ss/hashmap-typofix'Junio C Hamano1-1/+1
Typofix. * ss/hashmap-typofix: hashmap.h: fix minor typo
2023-03-31Merge branch 'ds/p2000-fix-grep-sparse'Junio C Hamano1-1/+1
Fix perf test. * ds/p2000-fix-grep-sparse: p2000: remove stray '--sparse' flag from test
2023-03-31Merge branch 'kh/commentchar-config-error-message'Junio C Hamano1-1/+1
Doc update. * kh/commentchar-config-error-message: config: tell the user that we expect an ASCII character
2023-03-31Merge branch 'ab/retire-scripted-add-p'Junio C Hamano1-6/+0
Test fix. * ab/retire-scripted-add-p: t3701: we don't need no Perl for `add -i` anymore
2023-03-31Merge branch 'js/t5563-portability-fix'Junio C Hamano1-14/+12
Test portability fix. * js/t5563-portability-fix: t5563: prevent "ambiguous redirect"
2023-03-31Merge branch 'bb/unicode-width-table-15'Junio C Hamano1-12/+21
Update width table for the latest edition of Unicode. * bb/unicode-width-table-15: unicode: update the width tables to Unicode 15
2023-03-31t2107: fix mention of the_index.cache_changedAndrei Rybak1-1/+1
Commit [1] added a test to t2107-update-index-basic.sh with a comment that mentions macro "active_cache_changed". Later in [2], the macro was removed and its usage in function cmd_update_index in file builtin/update-index.c was replaced with "the_index.cache_changed". Fix the outdated comment in file t2107-update-index-basic.sh. [1] fa137f67a4 (lockfile.c: store absolute path, 2014-11-02) [2] dc594180d9 (cocci & cache.h: apply variable section of "pending" index-compatibility, 2022-11-19) Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-31t3060: fix mention of function prune_indexAndrei Rybak1-2/+2
Commit [1] added tests which trigger function prune_cache. The comments in these tests, however, incorrectly call it "prune_path". Since then, function "prune_cache" has been renamed to "prune_index" in commit [2]. Later still in commit [3], the_index singleton, which is also mentioned in a comment, stopped being used directly with function "prune_index". Fix mentions of function "prune_index" and the struct it changes in comments in file "t3060-ls-files-with-tree.sh". [1] 54e1abce90 (Add test case for ls-files --with-tree, 2007-10-03) [2] 6510ae173a (ls-files: convert prune_cache to take an index, 2017-06-12) [3] 188dce131f (ls-files: use repository object, 2017-06-22) Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-31fetch: download bundles once, even with --allDerrick Stolee3-1/+49
When fetch.bundleURI is set, 'git fetch' downloads bundles from the given bundle URI before fetching from the specified remote. However, when using non-file remotes, 'git fetch --all' will launch 'git fetch' subprocesses which then read fetch.bundleURI and fetch the bundle list again. We do not expect the bundle list to have new information during these multiple runs, so avoid these extra calls by un-setting fetch.bundleURI in the subprocess arguments. Be careful to skip fetching bundles for the empty bundle string. Fetching bundles from the empty list presents some interesting test failures. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-31t5563: prevent "ambiguous redirect"Johannes Schindelin1-14/+12
When I ran this test using `TEST_SHELL_PATH=/bin/bash` in my Ubuntu setup (where Bash is at version 5.0.17(1)-release), I was greeted with this error message: ./test-lib.sh: line 1072: $CHALLENGE: ambiguous redirect This commit fixes that error by quoting the `CHALLENGE` variable (which has as value a path containing spaces), and by avoiding to cuddle the empty string parameter in the `printf` call with the redirect character (in fact, the `printf ''>$CHALLENGE` is removed because the next line overwrites the file anyway because it _also_ uses a single `>` to redirect the output). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-30The fifth batchJunio C Hamano1-0/+13
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-30Merge branch 'mk/workaround-pcre-jit-ucp-bug'Junio C Hamano2-0/+12
A recent-ish change to allow unicode character classes to be used with "grep -P" triggered a JIT bug in older pcre2 libraries. The problematic change in Git built with these older libraries has been disabled to work around the bug. * mk/workaround-pcre-jit-ucp-bug: grep: work around UTF-8 related JIT bug in PCRE2 <= 10.34
2023-03-30Merge branch 'jc/am-doc-refer-to-format-patch'Junio C Hamano1-2/+5
Doc update. * jc/am-doc-refer-to-format-patch: am: refer to format-patch in the documentation
2023-03-30Merge branch 'sg/parse-options-h-initializers'Junio C Hamano1-67/+244
Code clean-up to use designated initializers in parse-options API. * sg/parse-options-h-initializers: parse-options.h: use designated initializers in OPT_* macros parse-options.h: rename _OPT_CONTAINS_OR_WITH()'s parameters parse-options.h: use consistent name for the callback parameters
2023-03-30Merge branch 'sg/parse-options-h-users'Junio C Hamano15-5/+18
Code clean-up to include and/or uninclude parse-options.h file as needed. * sg/parse-options-h-users: treewide: remove unnecessary inclusions of parse-options.h from headers treewide: include parse-options.h in source files
2023-03-30tests: skip test_eval_ in internal chain-lintJeff King1-11/+6
To check for broken &&-chains, we run "fail_117 && $1" as a test snippet, and check the exit code. We use test_eval_ to do so, because that's the way we run the actual test. But we don't need any of its niceties, like "set -x" tracing. In fact, they hinder us, because we have to explicitly disable them. So let's skip that and use "eval" more directly, which is simpler. I had hoped it would also be faster, but it doesn't seem to produce a measurable improvement (probably because it's just running internal shell commands, with no subshells or forks). Note that there is one gotcha: even though we don't intend to run any of the commands if the &&-chain is intact, an error like this: test_expect_success 'broken' ' # this next line breaks the &&-chain true # and then this one is executed even by the linter return 1 ' means we'll "return 1" from the eval, and thus from test_run_(). We actually do notice this in test_expect_success, but only by saying "hey, this test didn't say it was OK, so it must have failed", which is not right (it should say "broken &&-chain"). We can handle this by calling test_eval_inner_() instead, which is our trick for wrapping "return" in a test snippet. But to do that, we have to push the trace code out of that inner function and into test_eval_(). This is arguably where it belonged in the first place, but it never mattered because the "inner_" function had only one caller. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-30tests: drop here-doc check from internal chain-linterJeff King1-2/+3
Commit 99a64e4b73c (tests: lint for run-away here-doc, 2017-03-22) tweaked the chain-lint test to catch unclosed here-docs. It works by adding an extra "echo" command after the test snippet, and checking that it is run (if it gets swallowed by a here-doc, naturally it is not run). The downside here is that we introduced an extra $() substitution, which happens in a subshell. This has a measurable performance impact when run for many tests. The tradeoff in safety was undoubtedly worth it when 99a64e4b73c was written. But since the external chainlint.pl learned to find these recently, we can just rely on it. By switching back to a simpler chain-lint, hyperfine reports a measurable speedup on t3070 (which has 1800 tests): 'HEAD' ran 1.12 ± 0.01 times faster than 'HEAD~1' Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-30tests: diagnose unclosed here-doc in chainlint.plEric Sunshine5-3/+34
An unclosed here-doc in a test is a problem, because it silently gobbles up any remaining commands. Since 99a64e4b73c (tests: lint for run-away here-doc, 2017-03-22) we detect this by piggy-backing on the internal chainlint checker in test-lib.sh. However, it would be nice to detect it in chainlint.pl, for a few reasons: - the output from chainlint.pl is much nicer; it can show the exact spot of the error, rather than a vague "somewhere in this test you broke the &&-chain or had a bad here-doc" message. - the implementation in test-lib.sh runs for each test snippet. And since it requires a subshell, the extra cost is small but not zero. If chainlint.pl can reliably find the problem, we can optimize the test-lib.sh code. The chainlint.pl code never intended to find here-doc problems. But since it has to parse them anyway (to avoid reporting problems inside here-docs), most of what we need is already there. We can detect the problem when we fail to find the missing end-tag in swallow_heredocs(). The extra change in scan_heredoc_tag() stores the location of the start of the here-doc, which lets us mark it as the source of the error in the output (see the new tests for examples). [jk: added commit message and tests] Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-30tests: replace chainlint subshell with a functionJeff King1-1/+5
To test that we don't break the &&-chain, test-lib.sh does something like: (exit 117) && $test_commands and checks that the result is exit code 117. We don't care what that initial command is, as long as it exits with a unique code. Using "exit" works and is simple, but is a bit expensive since it requires a subshell (to avoid exiting the whole script!). This isn't usually very noticeable, but it can add up for scripts which have a large number of tests. Using "return" naively won't work here, because we'd return from the function eval-ing the snippet (and it wouldn't find &&-chain breakages). But if we further push that into its own function, it does exactly what we want, without extra subshell overhead. According to hyperfine, this produces a measurable improvement when running t3070 (which has 1800 tests, all of them quite short): 'HEAD' ran 1.09 ± 0.01 times faster than 'HEAD~1' Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-30tests: run internal chain-linter under "make test"Jeff King2-3/+4
Since 69b9924b875 (t/Makefile: teach `make test` and `make prove` to run chainlint.pl, 2022-09-01), we run a single chainlint.pl process for all scripts, and then instruct each individual script to run with the equivalent of --no-chain-lint, which tells them not to redundantly run the chainlint script themselves. However, this also disables the internal linter run within the shell by eval-ing "(exit 117) && $1" and confirming we get code 117. In theory the external linter produces a superset of complaints, and we don't need the internal one anymore. However, we know there is at least one case where they differ. A test like: test_expect_success 'should fail linter' ' false && sleep 2 & pid=$! && kill $pid ' is buggy (it ignores the failure from "false", because it is backgrounded along with the sleep). The internal linter catches this, but the external one doesn't (and teaching it to do so is complicated[1]). So not only does "make test" miss this problem, but it's doubly confusing because running the script standalone does complain. Let's teach the suppression in the Makefile to only turn off the external linter (which we know is redundant, as it was already run) and leave the internal one intact. I've used a new environment variable to do this here, and intentionally did not add a "--no-ext-chain-lint" option. This is an internal optimization used by the Makefile, and not something that ordinary users would need to tweak. [1] For discussion of chainlint.pl and this case, see: https://lore.kernel.org/git/CAPig+cQtLFX4PgXyyK_AAkCvg4Aw2RAC5MmLbib-aHHgTBcDuw@mail.gmail.com/ Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-30unicode: update the width tables to Unicode 15Beat Bolli1-12/+21
Unicode version 15 was released in September 2022 [1], and we have so far neglected to update our width tables. Do this now. [1] http://blog.unicode.org/2022/09/announcing-unicode-standard-version-150.html Signed-off-by: Beat Bolli <dev+git@drbeat.li> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-30hashmap.h: fix minor typoSiddharth Singh1-1/+1
The word "no" should be "not". Signed-off-by: Siddharth Singh <siddhartth@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-30ref-filter: remove unused ref_format memberØystein Walle2-2/+0
use_rest was added in b9dee075eb (ref-filter: add %(rest) atom, 2021-07-26) but was never used. As far as I can tell it was used in a later patch that was submitted to the mailing list but never applied. Signed-off-by: Øystein Walle <oystwa@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-30pack-redundant: document deprecationJeff King1-0/+14
Running the command itself has generated a warning for several versions, which has recently been upgraded to an error. Let's also make sure the documentation mentions what is going on. This also gives us a good spot to explain the reasoning and recommend alternatives. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28parse-options: drop parse_opt_unknown_cb()Jeff King2-18/+0
This low-level callback was introduced in ce564eb1bd5 (parse-options: add parse_opt_unknown_cb(), 2016-09-05) so that we could advertise --indent-heuristic in git-blame's "-h" output, even though the option is actually handled in parse_revision_opt(). We later stopped doing so in 44ae131e384 (builtin/blame.c: remove '--indent-heuristic' from usage string, 2019-10-28). This is a weird thing to do, and in the intervening years, we've never used it again. Let's drop the helper in the name of simplicity. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28t/helper: mark unused argv/argc argumentsJeff King27-32/+33
Many test helper programs do not bother to look at argc or argv, because they don't take any options. In a user-facing program, it's a good idea to check for unexpected arguments and complain. But for a test helper, it's not worth the trouble to enforce this. But we do want to tell the compiler we're OK with ignoring them, to silence -Wunused-parameter (and obviously we can't get rid of them, since we have to conform to the usual cmd__foo() interface). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28mark "argv" as unused when we check argcJeff King3-5/+5
A few commands don't take any options at all, and confirm this by checking argc. After that they have no need to look at argv, but we're still stuck with it by convention. Let's annotate these cases so that the compiler doesn't complain with -Wunused-parameter. Note that in scalar and get-tar-commit-id, we're forced to keep argv by calling convention (the functions must match cmd_main() and builtin cmd_foo() conventions, respectively). In diff, these are subcommand modes that we call individually, so we _could_ just drop the argv parameters entirely. But it's weird to pass argc without argv, and it implies that the caller knows that the subcommands aren't interested in further arguments. It's less confusing to just keep them and silence the compiler warning. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28builtins: mark unused prefix parametersJeff King12-12/+12
All builtins receive a "prefix" parameter, but it is only useful if they need to adjust filenames given by the user on the command line. For builtins that do not even call parse_options(), they often don't look at the prefix at all, and -Wunused-parameter complains. Let's annotate those to silence the compiler warning. I gave a quick scan of each of these cases, and it seems like they don't have anything they _should_ be using the prefix for (i.e., there is no hidden bug that we are missing). The only questionable cases I saw were: - in git-unpack-file, we create a tempfile which will always be at the root of the repository, even if the command is run from a subdir. Arguably this should be created in the subdir from which we're run (as we report the path only as a relative name). However, nobody has complained, and I'm hesitant to change something that is deep plumbing going back to April 2005 (though I think within our scripts, the sole caller in git-merge-one-file would be OK, as it moves to the toplevel itself). - in fetch-pack, local-filesystem remotes are taken as relative to the project root, not the current directory. So: git init server.git [...put stuff in server.git...] git init client.git cd client.git mkdir subdir cd subdir git fetch-pack ../../server.git ... won't work, as we quietly move to the top of the repository before interpreting the path (so "../server.git" would work). This is weird, but again, nobody has complained and this is how it has always worked. And this is how "git fetch" works, too. Plus it raises questions about how a configured remote like: git config remote.origin.url ../server.git should behave. I can certainly come up with a reasonable set of behavior, but it may not be worth stirring up complications in a plumbing tool. So I've left the behavior untouched in both of those cases. If anybody really wants to revisit them, it's easy enough to drop the UNUSED marker. This commit is just about removing them as obstacles to turning on -Wunused-parameter all the time. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28builtins: annotate always-empty prefix parametersJeff King7-0/+22
It's usually a bad idea for a builtin's cmd_foo() to ignore the "prefix" argument it gets, as it needs to prepend that string when accessing any paths given by the user. But if a builtin does not ask for the git wrapper to run repository setup (via the RUN_SETUP or RUN_SETUP_GENTLY flags), then we know the prefix will always be NULL (it is adjusting for the chdir() done during repo setup, but there cannot be one if we did not set up the repo). In those cases it's OK to ignore "prefix", but it's worth annotating for a few reasons: 1. It serves as documentation to somebody reading the code about what we expect. 2. If the flags in git.c ever change, the run-time assertion may help detect the problem (though only if the command is run from a subdirectory of the repository). 3. It notes to the compiler that we are OK ignoring "prefix". In particular, this silences -Wunused-parameter. It _could_ also help the compiler generate better code (because it will know the prefix is NULL), but in practice this is quite unlikely to matter. Note that I've only added this annotation to commands which triggered -Wunused-parameter. It would be correct to add it to any builtin which doesn't ask for RUN_SETUP, but most of the rest of them do the sensible thing with "prefix" by passing it to parse_options(). So they're much more likely to just work if they ever switched to RUN_SETUP, and aren't worth annotating. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28builtins: always pass prefix to parse_options()Jeff King2-5/+6
Our builtins receive a "prefix" argument as part of their cmd_foo() function. We should always pass this to parse_options() if we're calling it, as it may be used for OPT_FILENAME() options. In the cases here, there's no option that would use it, so we're not fixing any bug. This is just future-proofing and setting a good example (plus quelling some -Wunused-parameter warnings). Note in the case of revert/cherry-pick, that we plumb the prefix through to run_sequencer(), as those builtins are just thin wrappers around it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28fast-import: fix file access when run from subdirJeff King2-2/+37
In cmd_fast_import(), we ignore the "prefix" argument entirely, even though it tells us how we may have changed directory to the root of the repository earlier in the process. Which means that if you run it from a subdir and point to paths in the filesystem, like: cd subdir git fast-import --import-marks=foo <dump then it will look for "foo" in the root of the repository, not the current directory ("subdir/") which the user would have expected. We can fix this by recording the prefix and using it as appropriate whenever we open a file for reading or writing. I found each of these by looking for cases where we call fopen() within fast-import.c, so this should cover all cases. The new test triggers each one, as well as making sure we don't accidentally apply the prefix when --relative-marks is in use (since that option interprets some paths as relative to a specific directory). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28p2000: remove stray '--sparse' flag from testDerrick Stolee1-1/+1
This argument was added in 7cae7627c45 (builtin/grep.c: integrate with sparse index, 2022-09-22), but it was a carry-over from an earlier version where the --sparse flag was added to the 'git grep' builtin. This argument does not exist, so currently the p2000-sparse-operations.sh performance test script fails when reaching this step. With this fix, the script works with these numbers for my copy of the Git source code repository: Test HEAD ------------------------------------------------------------ 2000.30: git grep --cached ... (full-v3) 0.34(1.20+0.14) 2000.31: git grep --cached ... (full-v4) 0.31(1.15+0.13) 2000.32: git grep --cached ... (sparse-v3) 0.26(1.13+0.12) 2000.33: git grep --cached ... (sparse-v4) 0.27(1.13+0.12) Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28config.c: rename "struct config_source cf"Glen Choo1-131/+131
The "cf" name is a holdover from before 4d8dd1494e (config: make parsing stack struct independent from actual data source, 2013-07-12), when the struct was named config_file. Since that acronym no longer makes sense, rename "cf" to "cs". In some places, we have "struct config_set cs", so to avoid conflict, rename those "cs" to "set" ("config_set" would be more descriptive, but it's much longer and would require us to rewrap several lines). Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28config: report cached filenames in die_bad_number()Glen Choo4-20/+72
If, when parsing numbers from config, die_bad_number() is called, it reports the filename and config source type if we were parsing a config file, but not if we were iterating a config_set (it defaults to a less specific error message). Most call sites don't parse config files because config is typically read once and cached, so we only report filename and config source type in "git config --type" (since "git config" always parses config files). This could have been fixed when we taught the current_config_* functions to respect config_set values (0d44a2dacc (config: return configset value for current_config_ functions, 2016-05-26), but it was hard to spot then and we might have just missed it (I didn't find mention of die_bad_number() in the original ML discussion [1].) Fix this by refactoring the current_config_* functions into variants that don't BUG() when we aren't reading config, and using the resulting functions in die_bad_number(). "git config --get[-regexp] --type=int" cannot use the non-refactored version because it parses the int value _after_ parsing the config file, which would run into the BUG(). Since the refactored functions aren't public, they use "struct config_reader". 1. https://lore.kernel.org/git/20160518223712.GA18317@sigill.intra.peff.net/ Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28config.c: remove current_parsing_scopeGlen Choo1-26/+37
Add ".parsing_scope" to "struct config_reader" and replace "current_parsing_scope" with "the_reader.parsing_scope. Adjust the comment slightly to make it clearer that the scope applies to the config source (not the current value), and should only be set when parsing a config source. As such, ".parsing_scope" (only set when parsing config sources) and ".config_kvi" (only set when iterating a config set) should not be set together, so enforce this with a setter function. Unlike previous commits, "populate_remote_urls()" still needs to store and restore the 'scope' value because it could have touched "current_parsing_scope" ("config_with_options()" can set the scope). Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28config.c: remove current_config_kviGlen Choo1-39/+43
Add ".config_kvi" to "struct config_reader" and replace "current_config_kvi" with "the_reader.config_kvi", plumbing "struct config_reader" where necesssary. Also, introduce a setter function for ".config_kvi", which allows us to enforce the contraint that only one of ".source" and ".config_kvi" can be set at a time (as documented in the comments). Because of this constraint, we know that "populate_remote_urls()" was never touching "current_config_kvi" when iterating through config files, so it doesn't need to store and restore that value. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28config.c: plumb the_reader through callbacksGlen Choo1-31/+43
The remaining references to "cf_global" are in config callback functions. Remove them by plumbing "struct config_reader" via the "*data" arg. In both of the callbacks here, we are only reading from "reader->source". So in the long run, if we had a way to expose readonly information from "reader->source" (probably in the form of "struct key_value_info"), we could undo this patch (i.e. remove "struct config_reader" fom "*data"). Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28config.c: create config_reader and the_readerGlen Choo1-31/+51
Create "struct config_reader" to hold the state of the config source currently being read. Then, create a static instance of it, "the_reader", and use "the_reader.source" to replace references to "cf_global" in public functions. This doesn't create much immediate benefit (since we're mostly replacing static variables with a bigger static variable), but it prepares us for a future where this state doesn't have to be global; "struct config_reader" (or a similar struct) could be provided by the caller, or constructed internally by a function like "do_config_from()". A more typical approach would be to put this struct on "the_repository", but that's a worse fit for this use case since config reading is not scoped to a repository. E.g. we can read config before the repository is known ("read_very_early_config()"), blatantly ignore the repo ("read_protected_config()"), or read only from a file ("git_config_from_file()"). This is especially evident in t5318 and t9210, where test-tool and scalar parse config but don't fully initialize "the_repository". We could have also replaced the references to "cf_global" in callback functions (which are the only ones left), but we'll eventually plumb "the_reader" through the callback "*data" arg, so that would be unnecessary churn. Until we remove "cf_global" altogether, add logic to "config_reader_*_source()" to keep "cf_global" and "the_reader.source" in sync. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28config.c: don't assign to "cf_global" directlyGlen Choo1-13/+24
To make "cf_global" easier to remove, replace all direct assignments to it with function calls. This refactor has an additional maintainability benefit: all of these functions were manually implementing stack pop/push semantics on "struct config_source", so replacing them with function calls allows us to only implement this logic once. In this process, perform some now-obvious clean ups: - Drop some unnecessary "cf_global" assignments in populate_remote_urls(). Since it was introduced in 399b198489 (config: include file if remote URL matches a glob, 2022-01-18), it has stored and restored the value of "cf_global" to ensure that it doesn't get accidentally mutated. However, this was never necessary since "do_config_from()" already pushes/pops "cf_global" further down the call chain. - Zero out every "struct config_source" with a dedicated initializer. This matters because the "struct config_source" is assigned to "cf_global" and we later 'pop the stack' by assigning "cf_global = cf_global->prev", but "cf_global->prev" could be pointing to uninitialized garbage. Fortunately, this has never bothered us since we never try to read "cf_global" except while iterating through config, in which case, "cf_global" is either set to a sensible value (when parsing a file), or it is ignored (when iterating a configset). Later in the series, zero-ing out memory will also let us enforce the constraint that "cf_global" and "current_config_kvi" are never non-NULL together. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28config.c: plumb config_source through static fnsGlen Choo1-69/+84
This reduces the direct dependence on the global "struct config_source", which will make it easier to remove in a later commit. To minimize the changes we need to make, we rename the current variable from "cf" to "cf_global", and the plumbed arg uses the old name "cf". This is a little unfortunate, since we now have the confusingly named "struct config_source cf" everywhere (which is a holdover from before 4d8dd1494e (config: make parsing stack struct independent from actual data source, 2013-07-12), when the struct used to be called "config_file"), but we will rename "cf" to "cs" by the end of the series. In some cases (public functions and config callback functions), there isn't an obvious way to plumb "struct config_source" through function args. As a workaround, add references to "cf_global" that we'll address in later commits. The remaining references to "cf_global" are direct assignments to "cf_global", which we'll also address in a later commit. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28docs: document caveats of rev-list's object-name outputJeff King1-2/+7
At first glance, the names given by "rev-list --objects" seem like a good way to see which paths are present in a set of commits. But there are some subtle gotchas there. We do not document the format of the names at all, so let's do so, along with warning of these problems. I intentionally did not document the exact format of the names here, as I don't think it's something we want people to rely on (though I doubt in practice that we'd change it at this point). Though all of this is historically tied to "--objects", these days we have a separate "--object-names" flag which can turn the names off or on. So I put the detailed documentation there, but added a note from --objects (which did not otherwise mention the names at all, even though they are on by default). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28The fourth batchJunio C Hamano1-0/+22
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-28Merge branch 'fc/docbook-remove-groff-workaround'Junio C Hamano3-28/+0
Remove workaround for ancient versions of DocBook to make it work correctly with groff, which has not been necessary since docbook 1.76 from 2010. * fc/docbook-remove-groff-workaround: doc: remove GNU troff workaround
2023-03-28Merge branch 'pe/time-use-gettimeofday'Junio C Hamano1-0/+19
time(2) on glib 2.31+, especially on Linux, goes out of sync with higher resolution timers used for gettimeofday(2) and by the filesystem. Replace all calls to it with a git_time() wrapper and use gettimeofday(2) in its implementation. * pe/time-use-gettimeofday: git-compat-util: use gettimeofday(2) for time(2)