Elijah Newren [Tue, 16 May 2023 06:33:48 +0000 (06:33 +0000)]
statinfo: move stat_{data,validity} functions from cache/read-cache
These functions do not depend upon struct cache_entry or struct
index_state in any way, and it seems more logical to break them out into
this file, especially since statinfo.h already has the struct stat_data
declaration.
Diff best viewed with `--color-moved`.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:47 +0000 (06:33 +0000)]
read-cache: move shared add/checkout/commit code
The function add_files_to_cache(), plus associated helper functions,
were defined in builtin/add.c, but also shared with builtin/checkout.c
and builtin/commit.c. Move these shared functions to read-cache.c.
Diff best viewed with `--color-moved`.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:46 +0000 (06:33 +0000)]
add: modify add_files_to_cache() to avoid globals
The function add_files_to_cache() is used by all three of builtin/{add,
checkout, commit}.c. That suggests this is common library code, and
should be moved somewhere else, like read-cache.c. However, the
function and its helpers made use of two global variables that made
straight code movement difficult:
* the_index
* include_sparse
The latter was perhaps more problematic since it was only accessible in
builtin/add.c but was still affecting builtin/checkout.c and
builtin/commit.c without this fact being very clear from the code. I'm
not sure if the other two callers would want to add a `--sparse` flag
similar to add.c to get non-default behavior, but exposing this
dependence will help if we ever decide we do want to add such a flag.
Modify add_files_to_cache() and its helpers to accept the necessary
arguments instead of relying on globals.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:45 +0000 (06:33 +0000)]
read-cache: move shared commit and ls-files code
The function overlay_tree_on_index(), plus associated helper functions,
were defined in builtin/ls-files.c, but also shared with
builtin/commit.c. Move these shared functions to read-cache.c.
Diff best viewed with `--color-moved`.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:43 +0000 (06:33 +0000)]
init-db, clone: change unnecessary global into passed parameter
Much like the parent commit, this commit was prompted by a desire to
move the functions which builtin/init-db.c and builtin/clone.c share out
of the former file and into setup.c. A secondary issue that made it
difficult was the init_shared_repository global variable; replace it
with a simple parameter that is passed to the relevant functions.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:42 +0000 (06:33 +0000)]
init-db: remove unnecessary global variable
This commit was prompted by a desire to move the functions which
builtin/init-db.c and builtin/clone.c share out of the former file and
into setup.c. One issue that made it difficult was the
init_is_bare_repository global variable.
init_is_bare_repository's sole use in life it to cache a value in
init_db(), and then be used in create_default_files(). This is a bit
odd since init_db() directly calls create_default_files(), and is the
only caller of that function. Convert the global to a simple function
parameter instead.
(Of course, this doesn't fix the fact that this value is then ignored by
create_default_files(), as noted in a big TODO comment in that function,
but it at least includes no behavioral change other than getting rid of
a very questionable global variable.)
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:41 +0000 (06:33 +0000)]
init-db: document existing bug with core.bare in template config
The comments in create_default_files() talks about reading config from
the config file in the specified `--templates` directory, which leads to
the question of whether core.bare could be set in such a config file and
thus whether the code is doing the right thing. It turns out, that it
doesn't; it unconditionally ignores core.bare in the config file in any
--templates directory. It is not clear to me that fixing it can be done
within this function; it seems to occur too late:
* create_default_files() is called by init_db()
* init_db() is called by both builtin/{clone.c,init-db.c}
* both callers of init_db() call set_git_work_tree() before init_db()
and in order to actual affect whether a repository is bear, we'd need to
somewhere reset these values, not just the is_bare_repository_cfg
setting.
I do not want to open this can of worms at this time; I'm trying to
clean up some headers, for which I need to move some functions, for
which I need to clean up some globals, and that's far enough down the
rabbit hole. So, simply document the issue with a careful TODO comment
and a few testcases.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git [-c log.follow=true] log [--follow] ':(glob)f**'" used to barf.
* jk/log-follow-with-non-literal-pathspec:
diff: detect pathspec magic not supported by --follow
diff: factor out --follow pathspec check
pathspec: factor out magic-to-name function
The value of config.worktree is per-repository, but has been kept
in a singleton global variable per process. This has been OK as
most Git operations interacted with a single repository at a time,
but not right for operations like recursive "grep" that want to
access multiple repositories from a single process without forking.
The global variable has been eliminated and made into a member in
the per-repository data structure.
* vd/worktree-config-is-per-repository:
repository: move 'repository_format_worktree_config' to repo scope
config: pass 'repo' directly to 'config_with_options()'
config: use gitdir to get worktree config
Junio C Hamano [Tue, 20 Jun 2023 22:53:12 +0000 (15:53 -0700)]
Merge branch 'tb/submodule-null-deref-fix'
"git submodule" code trusted the data coming from the config (and
the in-tree .gitmodules file) too much without validating, leading
to NULL dereference if the user mucks with a repository (e.g.
submodule.<name>.url is removed). This has been corrected.
Junio C Hamano [Tue, 20 Jun 2023 22:53:12 +0000 (15:53 -0700)]
Merge branch 'jc/test-modernization-2'
Test style updates.
* jc/test-modernization-2:
t9400-git-cvsserver-server: modernize test format
t9200-git-cvsexportcommit: modernize test format
t9104-git-svn-follow-parent: modernize test format
t9100-git-svn-basic: modernize test format
t7700-repack: modernize test format
t7600-merge: modernize test format
t7508-status: modernize test format
t7201-co: modernize test format
t7111-reset-table: modernize test format
t7110-reset-merge: modernize test format
Junio C Hamano [Tue, 20 Jun 2023 22:53:12 +0000 (15:53 -0700)]
Merge branch 'jc/test-modernization'
* jc/test-modernization:
t7101-reset-empty-subdirs: modernize test format
t6050-replace: modernize test format
t5306-pack-nobase: modernize test format
t5303-pack-corruption-resilience: modernize test format
t5301-sliding-window: modernize test format
t5300-pack-object: modernize test format
t4206-log-follow-harder-copies: modernize test format
t4202-log: modernize test format
t4004-diff-rename-symlink: modernize test format
t4003-diff-rename-1: modernize test format
t4002-diff-basic: modernize test format
t3903-stash: modernize test format
t3700-add: modernize test format
t3500-cherry: modernize test format
t1006-cat-file: modernize test format
t1002-read-tree-m-u-2way: modernize test format
t1001-read-tree-m-2way: modernize test format
t3210-pack-refs: modernize test format
t0030-stripspace: modernize test format
t0000-basic: modernize test format
Document more pseudo-refs and teach the command line completion
machinery to complete AUTO_MERGE.
* pb/complete-and-document-auto-merge-and-friends:
completion: complete AUTO_MERGE
Documentation: document AUTO_MERGE
git-merge.txt: modernize word choice in "True merge" section
completion: complete REVERT_HEAD and BISECT_HEAD
revisions.txt: document more special refs
revisions.txt: use description list for special refs
Junio C Hamano [Tue, 20 Jun 2023 22:53:10 +0000 (15:53 -0700)]
Merge branch 'ps/fetch-cleanups'
Code clean-up.
* ps/fetch-cleanups:
fetch: use `fetch_config` to store "submodule.fetchJobs" value
fetch: use `fetch_config` to store "fetch.parallel" value
fetch: use `fetch_config` to store "fetch.recurseSubmodules" value
fetch: use `fetch_config` to store "fetch.showForcedUpdates" value
fetch: use `fetch_config` to store "fetch.pruneTags" value
fetch: use `fetch_config` to store "fetch.prune" value
fetch: pass through `fetch_config` directly
fetch: drop unneeded NULL-check for `remote_ref`
fetch: drop unused DISPLAY_FORMAT_UNKNOWN enum value
Linus Arver [Thu, 15 Jun 2023 02:53:50 +0000 (02:53 +0000)]
doc: trailer: add more examples in DESCRIPTION
Be more up-front about what trailers are in practice with examples, to
give the reader a visual cue while they go on to read the rest of the
description.
Also add an example for multiline values.
Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus Arver [Thu, 15 Jun 2023 02:53:45 +0000 (02:53 +0000)]
doc: trailer: examples: avoid the word "message" by itself
Previously, "message" could mean the input, output, commit message, or
"internal body text inside the commit message" (in the EXAMPLES
section). Avoid overloading this term by using the appropriate meanings
explicitly.
Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus Arver [Thu, 15 Jun 2023 02:53:44 +0000 (02:53 +0000)]
doc: trailer: drop "commit message part" phrasing
The command can take inputs that are either just a commit message, or
an email-like output such as git-format-patch which includes a commit
message, "---" divider, and patch part. The existing explanation blends
these two inputs together in the first sentence
This command reads some patches or commit messages
which then necessitates using the "commit message part" phrasing (as
opposed to just "commit message") because the input is ambiguous per the
above definition.
This change separates the two input types and explains them separately,
and so there is no longer a need to use the "commit message part"
phrase.
Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Tue, 13 Jun 2023 19:29:46 +0000 (12:29 -0700)]
Merge branch 'zh/ls-files-format-atoms'
Some atoms that can be used in "--format=<format>" for "git ls-tree"
were not supported by "git ls-files", even though they were relevant
in the context of the latter.
* zh/ls-files-format-atoms:
ls-files: align format atoms with ls-tree
Junio C Hamano [Tue, 13 Jun 2023 19:29:45 +0000 (12:29 -0700)]
Merge branch 'jc/pack-ref-exclude-include'
"git pack-refs" learns "--include" and "--exclude" to tweak the ref
hierarchy to be packed using pattern matching.
* jc/pack-ref-exclude-include:
pack-refs: teach pack-refs --include option
pack-refs: teach --exclude option to exclude refs from being packed
docs: clarify git-pack-refs --all will pack all refs
Junio C Hamano [Tue, 13 Jun 2023 19:29:45 +0000 (12:29 -0700)]
Merge branch 'sa/doc-ls-remote'
Doc update.
* sa/doc-ls-remote:
ls-remote doc: document the output format
ls-remote doc: explain what each example does
ls-remote doc: show peeled tags in examples
ls-remote doc: remove redundant --tags example
show-branch doc: say <ref>, not <reference>
show-ref doc: update for internal consistency
Junio C Hamano [Tue, 13 Jun 2023 19:29:44 +0000 (12:29 -0700)]
Merge branch 'jc/diff-s-with-other-options'
The "-s" (silent, squelch) option of the "diff" family of commands
did not interact with other options that specify the output format
well. This has been cleaned up so that it will clear all the
formatting options given before.
* jc/diff-s-with-other-options:
diff: fix interaction between the "-s" option and other options
Junio C Hamano [Tue, 13 Jun 2023 19:29:44 +0000 (12:29 -0700)]
Merge branch 'kh/keep-tag-editmsg-upon-failure'
"git tag" learned to leave the "$GIT_DIR/TAG_EDITMSG" file when the
command failed, so that the user can salvage what they typed.
* kh/keep-tag-editmsg-upon-failure:
tag: keep the message file in case ref transaction fails
t/t7004-tag: add regression test for successful tag creation
doc: tag: document `TAG_EDITMSG`
Its better to document the struct members directly instead of on a
function that takes a pointer to the struct. This will also make it
easier to update the documentation in the future.
Make adjustments for this new context. Also drop “may contain” since we
don’t need to emphasize that a list could be empty.
Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
notes: update documentation for `use_default_notes`
`suppress_default_notes` was renamed to `use_default_notes` in 3a03cf6b1d (notes: refactor display notes default handling,
2011-03-29).
The commit message says that “values less than one [indicates] “not
set” ”, but what was meant was probably “less than zero” (the author of 3a03cf6b1d agrees on this point).
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 1 Jun 2023 18:10:40 +0000 (14:10 -0400)]
ci: drop linux-clang job
Since the linux-asan-ubsan job runs using clang under Linux, there is
not much point in running a separate clang job. Any errors that a normal
clang compile-and-test cycle would find are likely to be a subset of
what the sanitizer job will find. Since this job takes ~14 minutes to
run in CI, this shaves off some of our CPU load (though it does not
affect end-to-end runtime, since it's typically run in parallel and is
not the longest job).
Technically this provides us with slightly less signal for a given run,
since you won't immediately know if a failure in the sanitizer job is
from using clang or from the sanitizers themselves. But it's generally
obvious from the logs, and anyway your next step would be to fix the
probvlem and re-run CI, since we expect all of these jobs to pass
normally.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 1 Jun 2023 18:09:47 +0000 (14:09 -0400)]
ci: run ASan/UBSan in a single job
When we started running sanitizers in CI via 1c0962c0c4 (ci: add address
and undefined sanitizer tasks, 2022-10-20), we ran them as two separate
CI jobs, since as that commit notes, the combination "seems to take
forever".
And indeed, it does with gcc. However, since the previous commit
switched to using clang, the situation is different, and we can save
some CPU by using a single job for both. Comparing before/after CI runs,
this saved about 14 minutes (the single combined job took 54m, versus
44m plus 24m for ASan and UBSan jobs, respectively). That's wall-clock
and not CPU, but since our jobs are mostly CPU-bound, the two should be
closely proportional.
This does increase the end-to-end time of a CI run, though, since before
this patch the two jobs could run in parallel, and the sanitizer job is
our longest single job. It also means that we won't get a separate
result for "this passed with UBSan but not with ASan" or vice versa).
But as 1c0962c0c4 noted, that is not a very useful signal in practice.
Below are some more detailed timings of gcc vs clang that I measured by
running the test suite on my local workstation. Each measurement counts
only the time to run the test suite with each compiler (not the compile
time itself). We'll focus on the wall-clock times for simplicity, though
the CPU times follow roughly similar trends.
Here's a run with CC=gcc as a baseline:
real 1m12.931s
user 9m30.566s
sys 8m9.538s
Running with SANITIZE=address increases the time by a factor of ~4.7x:
real 5m40.352s
user 49m37.044s
sys 36m42.950s
Running with SANITIZE=undefined increases the time by a factor of ~1.7x:
real 2m5.956s
user 12m42.847s
sys 19m27.067s
So let's call that 6.4 time units to run them separately (where a unit
is the time it takes to run the test suite with no sanitizers). As a
simplistic model, we might imagine that running them together would take
5.4 units (we save 1 unit because we are no longer running the test
suite twice, but just paying the sanitizer overhead on top of a single
run).
But that's not what happens. Running with SANITIZE=address,undefined
results in a factor of 9.3x:
real 11m9.817s
user 77m31.284s
sys 96m40.454s
So not only did we not get faster when doing them together, we actually
spent 1.5x as much CPU as doing them separately! And while those
wall-clock numbers might not look too terrible, keep in mind that this
is on an unloaded 8-core machine. In the CI environment, wall-clock
times will be much closer to CPU times. So not only are we wasting CPU,
but we risk hitting timeouts.
Now let's try the same thing with clang. Here's our no-sanitizer
baseline run, which is almost identical to the gcc one (which is quite
convenient, because we can keep using the same "time units" to get an
apples-to-apples comparison):
real 1m11.844s
user 9m28.313s
sys 8m8.240s
And now again with SANITIZE=address, we get a 5x factor (so slightly
worse than gcc's 4.7x, though I wouldn't read too much into it; there is
a fair bit of run-to-run noise):
real 6m7.662s
user 49m24.330s
sys 44m13.846s
And with SANITIZE=undefined, we are at 1.5x, slightly outperforming gcc
(though again, that's probably mostly noise):
real 1m50.028s
user 11m0.973s
sys 16m42.731s
So running them separately, our total cost is 6.5x. But if we combine
them in a single run (SANITIZE=address,undefined), we get:
real 6m51.804s
user 52m32.049s
sys 51m46.711s
which is a factor of 5.7x. That's along the lines we'd hoped for!
Running them together saves us almost a whole time unit. And that's not
counting any time spent outside the test suite itself (starting the job,
setting up the environment, compiling) that we're no longer duplicating
by having two jobs.
So clang behaves like we'd hope: the overhead to run the sanitizers is
additive as you add more sanitizers. Whereas gcc's numbers seem very
close to multiplicative, almost as if the sanitizers were enforcing
their overheads on each other (though that is purely a guess on what is
going on; ultimately what matters to us is the amount of time it takes).
And that roughly matches the CI improvement I saw. A "time unit" there
is more like 12 minutes, and the observed time savings was 14 minutes
(with the extra presumably coming from avoiding duplicated setup, etc).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 1 Jun 2023 18:03:51 +0000 (14:03 -0400)]
ci: use clang for ASan/UBSan checks
Both gcc and clang support the "address" and "undefined" sanitizers.
However, they may produce different results. We've seen at least two
real world cases where gcc missed a UBSan problem but clang found it:
1. Clang's UBSan (using clang 14.0.6) found a string index that was
subtracted to "-1", causing an out-of-bounds read (curiously this
didn't trigger ASan, but that may be because the string was in the
argv memory, not stack or heap). Using gcc (version 12.2.0) didn't
find the same problem.
Original thread:
https://lore.kernel.org/git/20230519005447.GA2955320@coredump.intra.peff.net/
2. Clang's UBSan (using clang 4.0.1) complained about pointer
arithmetic with NULL, but gcc at the time did not. This was in
2017, and modern gcc does seem to find the issue, though.
Original thread:
https://lore.kernel.org/git/32a8b949-638a-1784-7fba-948ae32206fc@web.de/
Since we don't otherwise have a particular preference for one compiler
over the other for this test, let's switch to the one that we think may
be more thorough.
Note that it's entirely possible that the two are simply _different_,
and we are trading off problems that gcc would find that clang wouldn't.
However, my subjective and anecdotal experience has been that clang's
sanitizer support is a bit more mature (e.g., I recall other oddities
around leak-checking where clang performed more sensibly).
Obviously running both and cross-checking the results would give us the
best coverage, but that's very expensive to run (and these are already
some of our most expensive CI jobs). So let's use clang as our best
guess, and we can re-evaluate if we get more data points.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 1 Jun 2023 17:43:51 +0000 (13:43 -0400)]
diff: detect pathspec magic not supported by --follow
The --follow code doesn't handle most forms of pathspec magic. We check
that no unexpected ones have made it to try_to_follow_renames() with a
runtime GUARD_PATHSPEC() check, which gives behavior like this:
The same is true of ":(glob)", ":(attr)", and so on. It's good that we
notice the problem rather than continuing and producing a wrong answer.
But there are two non-ideal things:
1. The idea of GUARD_PATHSPEC() is to catch programming errors where
low-level code gets unexpected pathspecs. We'd usually try to catch
unsupported pathspecs by passing a magic_mask to parse_pathspec(),
which would give the user a much better message like:
pathspec magic not supported by this command: 'icase'
That doesn't happen here because git-log usually _does_ support
all types of pathspec magic, and so it passes "0" for the mask
(this call actually happens in setup_revisions()). It needs to
distinguish the normal case from the "--follow" one but currently
doesn't.
2. In addition to --follow, we have the log.follow config option. When
that is set, we try to turn on --follow mode only when there is a
single pathspec (since --follow doesn't handle anything else). But
really, that ought to be expanded to "use --follow when the
pathspec supports it". Otherwise, we'd complain any time you use an
exotic pathspec:
We should instead just avoid enabling follow mode if it's not
supported by this particular invocation.
This patch expands our diff_check_follow_pathspec() function to cover
pathspec magic, solving both problems.
A few final notes:
- we could also solve (1) by passing the appropriate mask to
parse_pathspec(). But that's not great for two reasons. One is that
the error message is less precise. It says "magic not supported by
this command", but really it is not the command, but rather the
--follow option which is the problem. The second is that it always
calls die(). But for our log.follow code, we want to speculatively
ask "is this pathspec OK?" and just get a boolean result.
- This is obviously the right thing to do for ':(icase)' and most
other magic options. But ':(glob)' is a bit odd here. The --follow
code doesn't support wildcards, but we allow them anyway. From
try_to_follow_renames():
#if 0
/*
* We should reject wildcards as well. Unfortunately we
* haven't got a reliable way to detect that 'foo\*bar' in
* fact has no wildcards. nowildcard_len is merely a hint for
* optimization. Let it slip for now until wildmatch is taught
* about dry-run mode and returns wildcard info.
*/
if (opt->pathspec.has_wildcard)
BUG("wildcards are not supported");
#endif
So something like "git log --follow 'Make*'" is already doing the
wrong thing, since ":(glob)" behavior is already the default (it is
used only to countermand an earlier --noglob-pathspecs).
So we _could_ loosen the guard to allow :(glob), since it just
behaves the same as pathspecs do by default. But it seems like a
backwards step to do so. It already doesn't work (it hits the BUG()
case currently), and given that the user took an explicit step to
say "this pathspec should glob", it is reasonable for us to say "no,
--follow does not support globbing" (or in the case of log.follow,
avoid turning on follow mode). Which is what happens after this
patch.
- The set of allowed pathspec magic is obviously the same as in
GUARD_PATHSPEC(). We could perhaps factor these out to avoid
repetition. The point of having separate masks and GUARD calls is
that we don't necessarily know which parsed pathspecs will be used
where. But in this case, the two are heavily correlated. Still,
there may be some value in keeping them separate; it would make
anyone think twice about adding new magic to the list in
diff_check_follow_pathspec(). They'd need to touch
try_to_follow_renames() as well, which is the code that would
actually need to be updated to handle more exotic pathspecs.
- The documentation for log.follow says that it enables --follow
"...when a single <path> is given". We could possibly expand that to
say "with no unsupported pathspec magic", but that raises the
question of documenting which magic is supported. I think the
existing wording of "single <path>" sufficiently encompasses the
idea (the forbidden magic is stuff that might match multiple
entries), and the spirit remains the same.
Reported-by: Jim Pryor <dubiousjim@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 1 Jun 2023 17:41:06 +0000 (13:41 -0400)]
diff: factor out --follow pathspec check
In --follow mode, we require exactly one pathspec. We check this
condition in two places:
- in diff_setup_done(), we complain if --follow is used with an
inapropriate pathspec
- in git-log's revision "tweak" function, we enable log.follow only if
the pathspec allows it
The duplication isn't a big deal right now, since the logic is so
simple. But in preparation for it becoming more complex, let's pull it
into a shared function.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 1 Jun 2023 17:38:14 +0000 (13:38 -0400)]
pathspec: factor out magic-to-name function
When we have unsupported magic in a pathspec (because a command or code
path does not support particular items), we list the unsupported ones in
an error message.
Let's factor out the code here that converts the bits back into their
human-readable names, so that it can be used from other callers, which
may want to provide more flexible error messages.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Fri, 26 May 2023 01:33:00 +0000 (01:33 +0000)]
repository: move 'repository_format_worktree_config' to repo scope
Move 'repository_format_worktree_config' out of the global scope and into
the 'repository' struct. This change is similar to how
'repository_format_partial_clone' was moved in ebaf3bcf1ae (repository: move
global r_f_p_c to repo struct, 2021-06-17), adding it to the 'repository'
struct and updating 'setup.c' & 'repository.c' functions to assign the value
appropriately.
The primary goal of this change is to be able to load the worktree config of
a submodule depending on whether that submodule - not its superproject - has
'extensions.worktreeConfig' enabled. To ensure 'do_git_config_sequence()'
has access to the newly repo-scoped configuration, add a 'struct repository'
argument to 'do_git_config_sequence()' and pass it the 'repo' value from
'config_with_options()'.
Finally, add/update tests in 't3007-ls-files-recurse-submodules.sh' to
verify 'extensions.worktreeConfig' is read an used independently by
superprojects and submodules.
Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Fri, 26 May 2023 01:32:59 +0000 (01:32 +0000)]
config: pass 'repo' directly to 'config_with_options()'
Add a 'struct repository' argument to 'config_with_options()' and remove the
'repo' field from 'struct git_config_source'.
A 'struct repository' instance was originally added to the config source in e3e8bf046e9 (submodule-config: pass repo upon blob config read, 2021-08-16)
to improve how submodule blob config content was accessed. At the time, this
was the only use for a 'repository' instance, so it was naturally added only
where it was needed: to 'struct git_config_source'. However, in upcoming
patches, 'config_with_options()' will need the repository instance to access
extension information (regardless of whether a 'config_source' exists). To
make the 'struct repository' instance more easily accessible, move it into
the function's arguments.
Update all callers of 'config_with_options()' to pass the appropriate 'repo'
value:
* in 'builtin/config.c', use 'the_repository'
* in 'submodule--config.c', use the 'repo' arg in 'config_from_gitmodules()'
* in 'read_[very_]early_config()' & 'read_protected_config()', set 'repo' to
NULL (repository instances aren't available there)
* in 'populate_remote_urls()', use the repo instance that has been added to
the 'struct config_include_data'
* in 'repo_read_config()', use the given 'repo' arg
Finally, note that this patch eliminates the fallback to 'the_repository'
that previously existed for the 'config_source' repo instance if it was
NULL. The fallback is no longer necessary, as the 'repo' is set explicitly
in all cases where it is needed.
Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Fri, 26 May 2023 01:32:58 +0000 (01:32 +0000)]
config: use gitdir to get worktree config
Update 'do_git_config_sequence()' to read the worktree config from
'config.worktree' in 'opts->git_dir' rather than the gitdir of
'the_repository'.
The worktree config is loaded from the path returned by
'git_pathdup("config.worktree")', the 'config.worktree' relative to the
gitdir of 'the_repository'. If loading the config for a submodule, this path
is incorrect, since 'the_repository' is the superproject. 'opts->git_dir' is
the gitdir of the submodule being configured, so the config file in that
location should be read instead.
To ensure the use of 'opts->git_dir' is safe, require that 'opts->git_dir'
is set if-and-only-if 'opts->commondir' is set (rather than "only-if" as it
is now). In all current usage of 'config_options', these values are set
together, so the stricter check does not change any behavior.
Finally, add tests to 't3007-ls-files-recurse-submodules.sh' to verify the
corrected config is loaded. Use 'ls-files' to test this because, unlike some
other '--recurse-submodules' commands, 'ls-files' parses the config of the
submodule in the same process as the superproject (via 'show_submodule()' ->
'repo_read_index()' -> 'prepare_repo_settings()'). As a result,
'the_repository' points to the config of the superproject but the
commondir/gitdir in the config sequence will be that of the submodule,
providing the exact scenario needed to verify this patch.
The first test ('--recurse-submodules parses submodule repo config') checks
that the submodule's *repo* config is read when running 'ls-files' on the
superproject; this confirms already-working behavior, serving as a reference
for how worktree config parsing should behave. The second test
('--recurse-submodules parses submodule worktree config') tests the same
scenario as the previous but instead using the *worktree* config,
demonstrating the corrected behavior. The 'test_config' helper is extended
for this case so that it properly applies the '--worktree' option to the
configure/unconfigure operations it performs.
Note that, although the submodule worktree config is now parsed instead of
the superproject's, 'extensions.worktreeConfig' in the superproject still
controls whether or not the worktree config is enabled at all in the
submodule. This will be fixed in a later patch.
Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Todd Zullinger [Thu, 25 May 2023 03:10:24 +0000 (23:10 -0400)]
t/lib-gpg: fix ssh-keygen -Y check-novalidate with openssh-9.0
OpenSSH-9.0 requires a namespace option with `-Y check-novalidate`.
This was added in openssh-portable commit a0b5816f8 (upstream:
ssh-keygen -Y check-novalidate requires namespace or SEGV, 2022-03-18).
The -n option was documented as a required option since check-novalidate
was added in openssh-portable 8aa2aa3cd (upstream: Allow testing
signature syntax and validity without verifying, 2019-09-16).
Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Todd Zullinger [Thu, 25 May 2023 03:12:07 +0000 (23:12 -0400)]
trace2 tests: fix PTHREADS prereq
The prereq guard added in 14903c8e92 (trace2 tests: guard pthread test
with "PTHREAD", 2022-11-24) lacks the S in PTHREADS, causing it to never
be satisfied. Fix the spelling of the prereq.
Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In e0a862fdaf (submodule helper: convert relative URL to absolute URL if
needed, 2018-10-16), `prepare_to_clone_next_submodule()` lost the
ability to handle URL-less submodules, due to a change from:
if (repo_get_config_string_const(the_repostiory, sb.buf, &url))
url = sub->url;
to
if (repo_get_config_string_const(the_repostiory, sb.buf, &url)) {
if (starts_with_dot_slash(sub->url) ||
starts_with_dot_dot_slash(sub->url)) {
/* ... */
}
}
, which will segfault when `sub->url` is NULL, since both
`starts_with_dot_slash()` does not guard its arguments as non-NULL.
Guard the checks to both of the above functions by first checking
whether `sub->url` is non-NULL. There is no need to check whether `sub`
itself is NULL, since we already perform this check earlier in
`prepare_to_clone_next_submodule()`.
By adding a NULL-ness check on `sub->url`, we'll fall into the 'else'
branch, setting `url` to `sub->url` (which is NULL). Before attempting
to invoke `git submodule--helper clone`, check whether `url` is NULL,
and die() if it is.
Reported-by: Tribo Dar <3bodar@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ZheNing Hu [Tue, 23 May 2023 09:00:18 +0000 (09:00 +0000)]
ls-files: align format atoms with ls-tree
"git ls-files --format" can be used to format the output of
multiple file entries in the index, while "git ls-tree --format"
can be used to format the contents of a tree object. However,
the current set of %(objecttype), "(objectsize)", and
"%(objectsize:padded)" atoms supported by "git ls-files --format"
is a subset of what is available in "git ls-tree --format".
Users sometimes need to establish a unified view between the index
and tree, which can help with comparison or conversion between the two.
Therefore, this patch adds the missing atoms to "git ls-files --format".
"%(objecttype)" can be used to retrieve the object type corresponding
to a file in the index, "(objectsize)" can be used to retrieve the
object size corresponding to a file in the index, and "%(objectsize:padded)"
is the same as "(objectsize)", except with padded format.
Signed-off-by: ZheNing Hu <adlternative@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Philippe Blain [Mon, 22 May 2023 19:29:00 +0000 (19:29 +0000)]
Documentation: document AUTO_MERGE
Since 5291828df8 (merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a
conflict, 2021-03-20), when using the 'ort' merge strategy, the special
ref AUTO_MERGE is written when a merge operation results in conflicts.
This ref points to a tree recording the conflicted state of the working
tree and is very useful during conflict resolution. However, this ref is
not documented.
Add some documentation for AUTO_MERGE in git-diff(1), git-merge(1),
gitrevisions(7) and in the user manual.
In git-diff(1), mention it at the end of the description section, when
we mention that the command also accepts trees instead of commits, and
also add an invocation to the "Various ways to check your working tree"
example.
In git-merge(1), add a step to the list of things that happen "when it
is not obvious how to reconcile the changes", under the "True merge"
section. Also mention AUTO_MERGE in the "How to resolve conflicts"
section, when mentioning 'git diff'.
In gitrevisions(7), add a mention of AUTO_MERGE along with the other
special refs.
In the user manual, add a paragraph describing AUTO_MERGE to the
"Getting conflict-resolution help during a merge" section, and include
an example of a 'git diff AUTO_MERGE' invocation for the example
conflict used in that section. Note that for uniformity we do not use
backticks around AUTO_MERGE here since the rest of the document does not
typeset special refs differently.
Closes: https://github.com/gitgitgadget/git/issues/1471 Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Philippe Blain [Mon, 22 May 2023 19:28:59 +0000 (19:28 +0000)]
git-merge.txt: modernize word choice in "True merge" section
The "True merge" section of the 'git merge' documentation mentions that
in case of conflicts, the conflicted working tree files contain "the
result of the "merge" program". This probably refers to RCS's 'merge'
program, which is mentioned further down under "How conflicts are
presented".
Since it is not clear at that point of the document which program is
referred to, and since most modern readers probably do not relate to RCS
anyway, let's just write "the merge operation" instead.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Philippe Blain [Mon, 22 May 2023 19:28:57 +0000 (19:28 +0000)]
revisions.txt: document more special refs
Some special refs, namely HEAD, FETCH_HEAD, ORIG_HEAD, MERGE_HEAD and
CHERRY_PICK_HEAD, are mentioned and described in 'gitrevisions', but some
others, namely REBASE_HEAD, REVERT_HEAD, and BISECT_HEAD, are not.
Add a small description of these special refs.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Philippe Blain [Mon, 22 May 2023 19:28:56 +0000 (19:28 +0000)]
revisions.txt: use description list for special refs
The special refs listed in 'gitrevisions' (under the '<refname>' entry)
are on separate lines in the Asciidoc source, but end up as a single
continuous paragraph in the rendered documentation (see e.g. [1]). In
following commits we will mention additional special refs, so to improve
legibility, use a description list such that every entry appears on its
own line. Since we are already in a description list, use ':::' as the
term delimiter.
In order for the new description list to be aligned with the description
under the '<refname>' entry, instead of being aligned with the last
entry of the "in the following rules" nested list, use the "ancestor
list continuation" syntax [2], i.e., leave an empty line before the
continuation '+'. Do the same for the paragraph following the new
description list ("Note that any...").
While at it, also use a continuation '+' before the "in the following
rules" list, for correctness. The parser seems not to care here, but
it's best to keep the sources correct.
Suggested-by: Victoria Dye <vdye@github.com> Based-on-patch-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jiang Xin [Sat, 20 May 2023 00:44:08 +0000 (08:44 +0800)]
Merge branch 'master' of github.com:git/git
* 'master' of github.com:git/git:
A few more topics after 2.41-rc1
Git 2.41-rc1
t/lib-httpd: make CGIPassAuth support conditional
t9001: mark the script as no longer leak checker clean
send-email: clear the $message_id after validation
upload-pack: advertise capabilities when cloning empty repos
A bit more before -rc1
imap-send: include strbuf.h
run-command.c: fix missing include under `NO_PTHREADS`
test: do not negate test_path_is_* to assert absense
t2021: do not negate test_path_is_dir
tests: do not negate test_path_exists
doc/git-config: add unit for http.lowSpeedLimit
rebase -r: fix the total number shown in the progress
rebase --update-refs: fix loops
attr: teach "--attr-source=<tree>" global option to "git"
projects / thirdparty / git.git / log
