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>
This argument was added in 7cae7627c4 (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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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
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)
Transports that do not support protocol v2 did not correctly fall
back to protocol v0 under certain conditions, which has been
corrected.
* jk/fix-proto-downgrade-to-v0:
git_connect(): fix corner cases in downgrading v2 to v0
"git rev-parse --quiet foo@{u}", or anything that asks @{u} to be
parsed with GET_OID_QUIETLY option, did not quietly fail, which has
been corrected.
* fc/oid-quietly-parse-upstream:
object-name: fix quiet @{u} parsing
Lift the limitation that colored prompts can only be used with
PROMPT_COMMAND mode.
* fc/completion-colors-do-not-need-prompt-command:
completion: prompt: use generic colors
Fix a logic error in 4950b2a2b5 (for-each-repo: run subcommands on
configured repos, 2020-09-11). Due to assuming that elements returned
from the repo_config_get_value_multi() call wouldn't be "NULL" we'd
conflate the <path> and <command> part of the argument list when
running commands.
As noted in the preceding commit the fix is to move to a safer
"*_string_multi()" version of the *_multi() API. This change is
separated from the rest because those all segfaulted. In this change
we ended up with different behavior.
When using the "--config=<config>" form we take each element of the
list as a path to a repository. E.g. with a configuration like:
[repo] list = /some/repo
We would, with this command:
git for-each-repo --config=repo.list status builtin
Run a "git status" in /some/repo, as:
git -C /some/repo status builtin
I.e. ask "status" to report on the "builtin" directory. But since a
configuration such as this would result in a "struct string_list *"
with one element, whose "string" member is "NULL":
[repo] list
We would, when constructing our command-line in
"builtin/for-each-repo.c"...
strvec_pushl(&child.args, "-C", path, NULL);
for (i = 0; i < argc; i++)
strvec_push(&child.args, argv[i]);
...have that "path" be "NULL", and as strvec_pushl() stops when it
sees NULL we'd end with the first "argv" element as the argument to
the "-C" option, e.g.:
git -C status builtin
I.e. we'd run the command "builtin" in the "status" directory.
In another context this might be an interesting security
vulnerability, but I think that this amounts to a nothingburger on
that front.
A hypothetical attacker would need to be able to write config for the
victim to run, if they're able to do that there's more interesting
attack vectors. See the "safe.directory" facility added in
8d1a744820 (setup.c: create `safe.bareRepository`, 2022-07-14).
An even more unlikely possibility would be an attacker able to
generate the config used for "for-each-repo --config=<key>", but
nothing else (e.g. an automated system producing that list).
Even in that case the attack vector is limited to the user running
commands whose name matches a directory that's interesting to the
attacker (e.g. a "log" directory in a repository). The second
argument (if any) of the command is likely to make git die without
doing anything interesting (e.g. "-p" to "log", there being no "-p"
built-in command to run).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix numerous and mostly long-standing segfaults in consumers of
the *_config_*value_multi() API. As discussed in the preceding commit
an empty key in the config syntax yields a "NULL" string, which these
users would give to strcmp() (or similar), resulting in segfaults.
As this change shows, most users users of the *_config_*value_multi()
API didn't really want such an an unsafe and low-level API, let's give
them something with the safety of git_config_get_string() instead.
This fix is similar to what the *_string() functions and others
acquired in[1] and [2]. Namely introducing and using a safer
"*_get_string_multi()" variant of the low-level "_*value_multi()"
function.
This fixes segfaults in code introduced in:
- d811c8e17c (versionsort: support reorder prerelease suffixes, 2015-02-26)
- c026557a37 (versioncmp: generalize version sort suffix reordering, 2016-12-08)
- a086f921a7 (submodule: decouple url and submodule interest, 2017-03-17)
- a6be5e6764 (log: add log.excludeDecoration config option, 2020-04-16)
- 92156291ca (log: add default decoration filter, 2022-08-05)
- 50a044f1e4 (gc: replace config subprocesses with API calls, 2022-09-27)
There are now two users ofthe low-level API:
- One in "builtin/for-each-repo.c", which we'll convert in a
subsequent commit.
- The "t/helper/test-config.c" code added in [3].
As seen in the preceding commit we need to give the
"t/helper/test-config.c" caller these "NULL" entries.
We could also alter the underlying git_configset_get_value_multi()
function to be "string safe", but doing so would leave no room for
other variants of "*_get_value_multi()" that coerce to other types.
Such coercion can't be built on the string version, since as we've
established "NULL" is a true value in the boolean context, but if we
coerced it to "" for use in a list of strings it'll be subsequently
coerced to "false" as a boolean.
The callback pattern being used here will make it easy to introduce
e.g. a "multi" variant which coerces its values to "bool", "int",
"path" etc.
1. 40ea4ed903 (Add config_error_nonbool() helper function,
2008-02-11)
2. 6c47d0e8f3 (config.c: guard config parser from value=NULL,
2008-02-11).
3. 4c715ebb96 (test-config: add tests for the config_set API,
2014-07-28)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As we'll discuss in the subsequent commit these tests all
show *_get_value_multi() API users unable to handle there being a
value-less key in the config, which is represented with a "NULL" for
that entry in the "string" member of the returned "struct
string_list", causing a segfault.
These added tests exhaustively test for that issue, as we'll see in a
subsequent commit we'll need to change all of the API users
of *_get_value_multi(). These cases were discovered by triggering each
one individually, and then adding these tests.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As noted in 6c62f01552 (for-each-repo: do nothing on empty config,
2021-01-08) this command wants to ignore a non-existing config key,
but let's not conflate that with bad config.
Before this, all these added tests would pass with an exit code of 0.
We could preserve the comment added in 6c62f01552, but now that we're
directly using the documented repo_config_get_value_multi() value it's
just narrating something that should be obvious from the API use, so
let's drop it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Have the "git_configset_get_value_multi()" function and its siblings
return an "int" and populate a "**dest" parameter like every other
git_configset_get_*()" in the API.
As we'll take advantage of in subsequent commits, this fixes a blind
spot in the API where it wasn't possible to tell whether a list was
empty from whether a config key existed. For now we don't make use of
those new return values, but faithfully convert existing API users.
Most of this is straightforward, commentary on cases that stand out:
- To ensure that we'll properly use the return values of this function
in the future we're using the "RESULT_MUST_BE_USED" macro introduced
in [1].
As git_die_config() now has to handle this return value let's have
it BUG() if it can't find the config entry. As tested for in a
preceding commit we can rely on getting the config list in
git_die_config().
- The loops after getting the "list" value in "builtin/gc.c" could
also make use of "unsorted_string_list_has_string()" instead of using
that loop, but let's leave that for now.
- In "versioncmp.c" we now use the return value of the functions,
instead of checking if the lists are still non-NULL.
1. 1e8697b5c4 (submodule--helper: check repo{_submodule,}_init()
return values, 2022-09-01),
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor the reading of the versionSort.suffix and
versionSort.prereleaseSuffix configuration variables to stay within
the bounds of our CodingGuidelines when it comes to line length, and
to avoid repeating ourselves.
Renaming "deprecated_prereleases" to "oldl" doesn't help us to avoid
line wrapping now, but it will in a subsequent commit.
Let's also split out the names of the config variables into variables
of our own, and refactor the nested if/else to avoid indenting it, and
the existing bracing style issue.
This all helps with the subsequent commit, where we'll need to start
checking different git_config_get_value_multi() return value. See
c026557a37 (versioncmp: generalize version sort suffix reordering,
2016-12-08) for the original implementation of most of this.
Moving the "initialized = 1" assignment allows us to move some of this
to the variable declarations in the subsequent commit.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We already have the basic "git_config_get_value()" function and its
"repo_*" and "configset" siblings to get a given "key" and assign the
last key found to a provided "value".
But some callers don't care about that value, but just want to use the
return value of the "get_value()" function to check whether the key
exist (or another non-zero return value).
The immediate motivation for this is that a subsequent commit will
need to change all callers of the "*_get_value_multi()" family of
functions. In two cases here we (ab)used it to check whether we had
any values for the given key, but didn't care about the return value.
The rest of the callers here used various other config API functions
to do the same, all of which resolved to the same underlying functions
to provide the answer.
Some of these were using either git_config_get_string() or
git_config_get_string_tmp(), see fe4c750fb1 (submodule--helper: fix a
configure_added_submodule() leak, 2022-09-01) for a recent example. We
can now use a helper function that doesn't require a throwaway
variable.
We could have changed git_configset_get_value_multi() (and then
git_config_get_value() etc.) to accept a "NULL" as a "dest" for all
callers, but let's avoid changing the behavior of existing API
users. Having an "unused" value that we throw away internal to
config.c is cheap.
A "NULL as optional dest" pattern is also more fragile, as the intent
of the caller might be misinterpreted if he were to accidentally pass
"NULL", e.g. when "dest" is passed in from another function.
Another name for this function could have been
"*_config_key_exists()", as suggested in [1]. That would work for all
of these callers, and would currently be equivalent to this function,
as the git_configset_get_value() API normalizes all non-zero return
values to a "1".
But adding that API would set us up to lose information, as e.g. if
git_config_parse_key() in the underlying configset_find_element()
fails we'd like to return -1, not 1.
Let's change the underlying configset_find_element() function to
support this use-case, we'll make further use of it in a subsequent
commit where the git_configset_get_value_multi() function itself will
expose this new return value.
This still leaves various inconsistencies and clobbering or ignoring
of the return value in place. E.g here we're modifying
configset_add_value(), but ever since it was added in [2] we've been
ignoring its "int" return value, but as we're changing the
configset_find_element() it uses, let's have it faithfully ferry that
"ret" along.
Let's also use the "RESULT_MUST_BE_USED" macro introduced in [3] to
assert that we're checking the return value of
configset_find_element().
We're leaving the same change to configset_add_value() for some future
series. Once we start paying attention to its return value we'd need
to ferry it up as deep as do_config_from(), and would need to make
least read_{,very_}early_config() and git_protected_config() return an
"int" instead of "void". Let's leave that for now, and focus on
the *_get_*() functions.
1. 3c8687a73e (add `config_set` API for caching config-like files, 2014-07-28)
2. https://lore.kernel.org/git/xmqqczadkq9f.fsf@gitster.g/
3. 1e8697b5c4 (submodule--helper: check repo{_submodule,}_init()
return values, 2022-09-01),
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A less well known edge case in the config format is that keys can be
value-less, a shorthand syntax for "true" boolean keys. I.e. these two
are equivalent as far as "--type=bool" is concerned:
[a]key
[a]key = true
But as far as our parser is concerned the values for these two are
NULL, and "true". I.e. for a sequence like:
[a]key=x
[a]key
[a]key=y
We get a "struct string_list" with "string" members with ".string"
values of:
{ "x", NULL, "y" }
This behavior goes back to the initial implementation of
git_config_bool() in 17712991a5 (Add ".git/config" file parser,
2005-10-10).
When parts of the config_set API were tested for in [1] they didn't
add coverage for 3/4 of the "(NULL)" cases handled in
"t/helper/test-config.c". We'd test that case for "get_value", but not
"get_value_multi", "configset_get_value" and
"configset_get_value_multi".
We now cover all of those cases, which in turn expose the details of
how this part of the config API works.
1. 4c715ebb96 (test-config: add tests for the config_set API,
2014-07-28)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There were no tests checking for the output of the git_die_config()
function in the config API, added in 5a80e97c82 (config: add
`git_die_config()` to the config-set API, 2014-08-07). We only tested
"test_must_fail", but didn't assert the output.
We need tests for this because a subsequent commit will alter the
return value of git_config_get_value_multi(), which is used to get the
config values in the git_die_config() function. This test coverage
helps to build confidence in that subsequent change.
These tests cover different interactions with git_die_config():
- The "notes.mergeStrategy" test in
"t/t3309-notes-merge-auto-resolve.sh" is a case where a function
outside of config.c (git_config_get_notes_strategy()) calls
git_die_config().
- The "gc.pruneExpire" test in "t5304-prune.sh" is a case where
git_config_get_expiry() calls git_die_config(), covering a different
"type" than the "string" test for "notes.mergeStrategy".
- The "fetch.negotiationAlgorithm" test in
"t/t5552-skipping-fetch-negotiator.sh" is a case where
git_config_get_string*() calls git_die_config().
We also cover both the "from command-line config" and "in file..at
line" cases here.
The clobbering of existing ".git/config" files here is so that we're
not implicitly testing the line count of the default config.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As can easily be seen from grepping in our sources, we had these uses
of "the_repository" in various library code in cases where the
function in question was already getting a "struct repository *"
argument. Let's use that argument instead.
Out of these changes only the changes to "cache-tree.c",
"commit-reach.c", "shallow.c" and "upload-pack.c" would have cleanly
applied before the migration away from the "repo_*()" wrapper macros
in the preceding commits.
The rest aren't new, as we'd previously implicitly refer to
"the_repository", but it's now more obvious that we were doing the
wrong thing all along, and should have used the parameter instead.
The change to change "get_index_format_default(the_repository)" in
"read-cache.c" to use the "r" variable instead should arguably have
been part of [1], or in the subsequent cleanup in [2]. Let's do it
here, as can be seen from the initial code in [3] it's not important
that we use "the_repository" there, but would prefer to always use the
current repository.
This change excludes the "the_repository" use in "upload-pack.c"'s
upload_pack_advertise(), as the in-flight [4] makes that change.
1. ee1f0c242e (read-cache: add index.skipHash config option,
2023-01-06)
2. 6269f8eaad (treewide: always have a valid "index_state.repo"
member, 2023-01-17)
3. 7211b9e753 (repo-settings: consolidate some config settings,
2019-08-13)
4. <Y/hbUsGPVNAxTdmS@coredump.intra.peff.net>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In preceding commits we changed many calls to macros that were
providing a "the_repository" argument to invoke corresponding repo_*()
function instead. Let's follow-up and adjust references to those in
comments, which coccinelle didn't (and inherently can't) catch.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the part of "the_repository.pending.cocci" pertaining to
"revision.h".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the part of "the_repository.pending.cocci" pertaining to
"rerere.h".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the part of "the_repository.pending.cocci" pertaining to
"refs.h".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the part of "the_repository.pending.cocci" pertaining to
"promisor-remote.h".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the part of "the_repository.pending.cocci" pertaining to
"packfile.h".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the part of "the_repository.pending.cocci" pertaining to
"pretty.h".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the part of "the_repository.pending.cocci" pertaining to
"object-store.h".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the part of "the_repository.pending.cocci" pertaining to
"diff.h".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the part of "the_repository.pending.cocci" pertaining to
"commit.h".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the part of "the_repository.pending.cocci" pertaining to
"commit-reach.h".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the part of "the_repository.pending.cocci" pertaining to
"cache.h".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the case of diff.h, rerere.h and revision.h the macros were added
in [1], [2] and [3] when "the_repository.pending.cocci" didn't
exist. None of the subsequently added migration rules covered
them. Let's add those missing rules.
In the case of macros in "cache.h", "commit.h", "packfile.h",
"promisor-remote.h" and "refs.h" those aren't guarded by
"NO_THE_REPOSITORY_COMPATIBILITY_MACROS", but they're also macros that
add "the_repository" as the first argument, so we should migrate away
from them.
1. 2abf350385 (revision.c: remove implicit dependency on the_index,
2018-09-21)
2. e675765235 (diff.c: remove implicit dependency on the_index,
2018-09-21)
3. 35843b1123 (rerere.c: remove implicit dependency on the_index,
2018-09-21)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sort the "the_repository.pending.cocci" file by which header the
macros are in, and add a comment to that effect in front of the
rules. This will make subsequent commits easier to follow, as we'll be
applying these rules on a header-by-header basis.
Once we've fully applied "the_repository.pending.cocci" we'll keep
this rules around for a while in "the_repository.cocci", to help any
outstanding topics and out-of-tree code to resolve textual or semantic
conflicts with these changes, but eventually we'll remove the
"the_repository.cocci" as a follow-up.
So even if some of these functions are subsequently moved and/or split
into other or new headers there's no risk of this becoming stale, if
and when that happens the we should be removing these rules anyway.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When these rules started being added in [1] they didn't use a ";"
after the ")", and would thus catch uses of these macros within
expressions. But as of [2] the new additions were broken in that
they'd only match a subset of the users of these macros.
Rather than narrowly fixing that, let's have these use the much less
verbose pattern introduced in my recent [3]: There's no need to
exhaustively enumerate arguments if we use the "..." syntax. This
means that we can fold all of these different rules into one.
1. afd69dcc21 (object-store: prepare read_object_file to deal with
any repo, 2018-11-13)
2. 21a9651ba3 (commit-reach: prepare get_merge_bases to handle any
repo, 2018-11-13)
3. 0e6550a2c6 (cocci: add a index-compatibility.pending.cocci,
2022-11-19)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "parse_commit_gently" macro went away in [1], so we don't need to
carry this for its migration.
1. ea3f7e598c (revision: use repository from rev_info when parsing
commits, 2020-06-23)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
1f2e05f0b7 ("wildmatch: fix exponential behavior", 2023-03-20)
introduced a new test with a background process. Backgrounding
necessarily gives a result of 0, so that a seemingly broken && chain is
not really broken.
Adjust t3070 slightly so that our chain lint test recognizes the
construct for what it is and does not raise a false positive.
Signed-off-by: Michael J Gruber <git@grubix.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 50b54fd72a (config: be strict on core.commentChar, 2014-05-17)
notes that “multi-byte character encoding could also be misinterpreted”,
and indeed a multi-byte codepoint (non-ASCII) is not accepted as a valid
`core.commentChar`.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 80c928d947 (commit-graph: simplify compute_generation_numbers(),
2023-03-20), the code to compute generation numbers was simplified to
use the same infrastructure as is used to compute topological levels.
This refactoring introduced a bug where the generation numbers are
truncated when they exceed UINT32_MAX because we explicitly cast the
computed generation number to `uint32_t`. This is not required though:
both the computed value and the field of `struct commit_graph_data` are
of the same type `timestamp_t` already, so casting to `uint32_t` will
cause truncation.
This cast can cause us to miscompute generation data overflows:
1. Given a commit with no parents and committer date
`UINT32_MAX + 1`.
2. We compute its generation number as `UINT32_MAX + 1`, but
truncate it to `1`.
3. We calculate the generation offset via `$generation - $date`,
which is thus `1 - (UINT32_MAX + 1)`. The computation underflows
and we thus end up with an offset that is bigger than the maximum
allowed offset.
As a result, we'd be writing generation data overflow information into
the commit-graph that is bogus and ultimately not even required.
Fix this bug by removing the needless cast.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There exists no direct way to interrogate git about which paths are
matched by a given set of sparsity rules. It is possible to get this
information from git, but it includes checking out the commit that
contains the paths, applying the sparse checkout patterns and then using
something like 'git ls-files -t' to check if the skip worktree bit is
set. This works in some case, but there are cases where it is awkward or
infeasible to generate a checkout for this purpose.
Exposing the pattern matching of sparse checkout enables more tooling to
be built and avoids a situation where tools that want to reason about
sparse checkouts start containing parallel implementation of the rules.
To accommodate this, add a 'check-rules' subcommand to the
'sparse-checkout' builtin along the lines of the 'git check-ignore' and
'git check-attr' commands. The new command accepts a list of paths on
stdin and outputs just the ones the match the sparse checkout.
To allow for use in a bare repository and to allow for interrogating
about other patterns than the current ones, include a '--rules-file'
option which allows the caller to explicitly pass sparse checkout rules
in the format accepted by 'sparse-checkout set --stdin'.
To allow for reuse of the handling of input patterns for the
'--rules-file' flag, modify 'add_patterns_from_input()' to be able to
read from a 'FILE' instead of just stdin.
To allow for reuse of the logic which decides whether or not rules
should be interpreted as cone-mode patterns, split that part out of
'update_modes()' such that can be called without modifying the config.
An alternative could have been to create a new 'check-sparsity' command.
However, placing it under 'sparse-checkout' allows for a) more easily
re-using the sparse checkout pattern matching and cone/non-code mode
handling, and b) keeps the documentation for the command next to the
experimental warning and the cone-mode discussion.
Signed-off-by: William Sprent <williams@unity3d.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In preparation for adding a sub-command to 'sparse-checkout' that can be
run in a bare repository, remove the 'NEED_WORK_TREE' flag from its
entry in the 'commands' array of 'git.c'.
To avoid that this changes any behaviour, add calls to
'setup_work_tree()' to all of the 'sparse-checkout' sub-commands and add
tests that verify that 'sparse-checkout <cmd>' still fail with a clear
error message telling the user that the command needs a work tree.
Signed-off-by: William Sprent <williams@unity3d.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This should have been removed in `ab/retire-scripted-add-p` but wasn't.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When copying the `split_index` structure from one index structure to
another, we need to propagate the `SPLIT_INDEX_ORDERED` flag, too, if it
is set, otherwise Git might forget to write the shared index when that
is actually needed.
It just so _happens_ that in many instances when `unpack_trees()` is
called, the result causes the shared index to be written anyway, but
there are edge cases when that is not so.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
