Now that we have a function which can verify a single layer of a
commit-graph chain, implement `verify_commit_graph()` in terms of
iterating over commit-graphs along their `->base_graph` pointers.
This further prepares us to consolidate the progress output of `git
commit-graph verify`.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the `verify_commit_graph()` function was extended to support
commit-graph chains via 3da4b609bb (commit-graph: verify chains with
--shallow mode, 2019-06-18), it did so by recursively calling itself on
each layer of the commit-graph chain.
In practice this poses no issues, since commit-graph chains do not loop,
and there are few enough of them that adding additional frames to the
stack is not a problem.
A future commit will consolidate the progress output from `git
commit-graph verify` when verifying chained commit-graphs to print a
single line instead of one progress meter per commit-graph layer.
Prepare for this by extracting a routine to verify a single layer of a
commit-graph.
Note that `verify_commit_graph()` is still recursive after this patch,
but this will change in the subsequent patch.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a similar spirit as the previous commit, address a bug where `git
fsck` produces output when calling `git multi-pack-index verify` even
when invoked with `--no-progress`.
$ git.compile fsck --connectivity-only --no-progress --no-dangling
Verifying OID order in multi-pack-index: 100% (605677/605677), done.
Sorting objects by packfile: 100% (605678/605678), done.
Verifying object offsets: 100% (605678/605678), done.
The three lines produced by `git fsck` come from `git multi-pack-index
verify`, but should be squelched due to `--no-progress`.
The MIDX machinery learned to generate these progress messages as early
as 430efb8a74 (midx: add progress indicators in multi-pack-index
verify, 2019-03-21), but did not respect `--progress` or `--no-progress`
until ad60096d1c (midx: honor the MIDX_PROGRESS flag in
verify_midx_file, 2019-10-21).
But the `git multi-pack-index verify` step was added to fsck in
66ec0390e7 (fsck: verify multi-pack-index, 2018-09-13), pre-dating any
of the above patches.
Pass `--[no-]progress` as appropriate to ensure that we don't produce
output when told not to.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since e0fd51e1d7 (fsck: verify commit-graph, 2018-06-27), `fsck` runs
`git commit-graph verify` to check the integrity of any commit-graph(s).
Originally, the `git commit-graph verify` step would always print to
stdout/stderr, regardless of whether or not `fsck` was invoked with
`--[no-]progress` or not. But in 7371612255 (commit-graph: add
--[no-]progress to write and verify, 2019-08-26), the commit-graph
machinery learned the `--[no-]progress` option, though `fsck` was not
updated to pass this new flag (or not).
This led to seeing output from running `git fsck`, even with
`--no-progress` on repositories that have a commit-graph:
$ git.compile fsck --connectivity-only --no-progress --no-dangling
Verifying commits in commit graph: 100% (4356/4356), done.
Verifying commits in commit graph: 100% (131912/131912), done.
Ensure that `fsck` passes `--[no-]progress` as appropriate when calling
`git commit-graph verify`.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The match_pathspec_item() function takes "prefix" value, allowing a
caller to chop off the common leading prefix of pathspec pattern
strings from the path and only use the remainder of the path to
match the pathspec patterns (after chopping the same leading prefix
of them, of course).
This "common leading prefix" optimization has two main features:
* discard the entries in the in-core index that are outside of the
common leading prefix; if you are doing "ls-files one/a one/b",
we know all matches must be from "one/", so first the code
discards all entries outside the "one/" directory from the
in-core index. This allows us to work on a smaller dataset.
* allow skipping the comparison of the leading bytes when matching
pathspec with path. When "ls-files" finds the path "one/a/1" in
the in-core index given "one/a" and "one/b" as the pathspec,
knowing that common leading prefix "one/" was found lets the
pathspec matchinery not to bother comparing "one/" part, and
allows it to feed "a/1" down, as long as the pathspec element
"one/a" gets corresponding adjustment to "a".
When the "attr" pathspec magic is in effect, however, the current
code breaks down.
The attributes, other than the ones that are built-in and the ones
that come from the $GIT_DIR/info/attributes file and the top-level
.gitattributes file, are lazily read from the filesystem on-demand,
as we encounter each path and ask if it matches the pathspec. For
example, if you say "git ls-files "(attr:label)sub/" in a repository
with a file "sub/file" that is given the 'label' attribute in
"sub/.gitattributes":
* The common prefix optimization finds that "sub/" is the common
prefix and prunes the in-core index so that it has only entries
inside that directory. This is desirable.
* The code then walks the in-core index, finds "sub/file", and
eventually asks do_match_pathspec() if it matches the given
pathspec.
* do_match_pathspec() calls match_pathspec_item() _after_ stripping
the common prefix "sub/" from the path, giving it "file", plus
the length of the common prefix (4-bytes), so that the pathspec
element "(attr:label)sub/" can be treated as if it were "(attr:label)".
The last one is what breaks the match in the current code, as the
pathspec subsystem ends up asking the attribute subsystem to find
the attribute attached to the path "file". We need to ask about the
attributes on "sub/file" when calling match_pathspec_attrs(); this
can be done by looking at "prefix" bytes before the beginning of
"name", which is the same trick already used by another piece of the
code in the same match_pathspec_item() function.
Unfortunately this was not discovered so far because the code works
with slightly different arguments, e.g.
$ git ls-files "(attr:label)sub"
$ git ls-files "(attr:label)sub/" "no/such/dir/"
would have reported "sub/file" as a path with the 'label' attribute
just fine, because neither would trigger the common prefix
optimization.
Reported-by: Matthew Hughes <mhughes@uw.co.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A few places failed to differenciate the case where the index is
truly empty (nothing added) and we haven't yet read from the
on-disk index file, which have been corrected.
* js/empty-index-fixes:
commit -a -m: allow the top-level tree to become empty again
split-index: accept that a base index can be empty
do_read_index(): always mark index as initialized unless erroring out
The strbuf_expand_step() loop in userformat_find_requirements() iterates
through the percent signs in the string "fmt", but we're not interested
in its effect on the strbuf "dummy". Use strchr(3) instead and get rid
of the strbuf that we no longer need.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
hex2chr() takes care not to run over the end of a NUL-terminated string.
It's used in packet_length(), but both callers of that function pass a
four-byte buffer, making NUL-checks unnecessary. packet_length() could
accidentally be used with a pointer to a buffer of unknown size at new
call-sites, though, and the compiler wouldn't complain.
Add a size parameter plus check, and remove the NUL-checks by calling
hexval() directly. This trades three NUL checks against one size check
and the ability to report the use of a short buffer at runtime.
If any of the four bytes is NUL or -- more generally -- not a
hexadecimal digit, then packet_length() still returns a negative value.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The tree-walk.c:do_match() function takes base_offset but just like
tree_entry_interesting() we dealt with earlier, nobody passes a
value other than 0 in it. Get rid of the parameter to avoid having
to worry about potential bugs lurking unexercised.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The tree_entry_interesting() function takes base_offset, allowing
its callers to potentially pass a non-zero number to skip the early
part of the path string.
The feature is never exercised and we do not even know what bugs are
lurking there, as all callers pass 0 to the parameter.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The test coverage on attribute magic combined with path pattern
was a bit thin. Let's add a few and make sure "(attr:X)sub" and
"(attr:X)sub/" behave the same.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test for equality with no_flush, which has enough negation already. Get
rid of the unnecessary parentheses while at it.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git ls-tree has two prefixes: The one handed to cmd_ls_tree(), i.e. the
current subdirectory in the repository (if any) and the "display" prefix
used by the show_tree_*() functions. The option --full-name clears the
last one, i.e. it shows full paths, and --full-tree clears both, i.e. it
acts as if the command was started in the root of the repository.
The show_tree_*() functions use the ls_tree_options members chomp_prefix
and ls_tree_prefix to determine their prefix values. Calculate it once
in cmd_ls_tree() instead, once the main prefix value is finalized.
This allows chomp_prefix to become a local variable. Stop using
strlen(3) to determine its initial value -- we only care whether we got
a non-empty string, not precisely how long it is.
Rename ls_tree_prefix to prefix to demonstrate that we converted all
users and because the ls_tree_ part is no longer necessary since
030a3d5d9e (ls-tree: use a "struct options", 2023-01-12) turned it from
a global variable to a struct member.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reduce reliance on a global state in the config reading API.
* gc/config-context:
config: pass source to config_parser_event_fn_t
config: add kvi.path, use it to evaluate includes
config.c: remove config_reader from configsets
config: pass kvi to die_bad_number()
trace2: plumb config kvi
config.c: pass ctx with CLI config
config: pass ctx with config files
config.c: pass ctx in configsets
config: add ctx arg to config_fn_t
urlmatch.h: use config_fn_t type
config: inline git_color_default_config
During a cherry-pick or revert session that works on multiple
commits, "git status" did not give correct information, which has
been corrected.
* jk/cherry-pick-revert-status:
fix cherry-pick/revert status when doing multiple commits
"git apply" punts when it is fed too large a patch input; the error
message it gives when it happens has been clarified.
* pw/apply-too-large:
apply: improve error messages when reading patch
'git notes append' was taught '--separator' to specify string to insert
between paragraphs.
* tl/notes-separator:
notes: introduce "--no-separator" option
notes.c: introduce "--[no-]stripspace" option
notes.c: append separator instead of insert by pos
notes.c: introduce '--separator=<paragraph-break>' option
t3321: add test cases about the notes stripspace behavior
notes.c: use designated initializers for clarity
notes.c: cleanup 'strbuf_grow' call in 'append_edit'
Partially revert a sanity check that the rest of the config code
was not ready, to avoid triggering it in a corner case.
* gc/config-partial-submodule-kvi-fix:
config: don't BUG when both kvi and source are set
Move functions that are not about pure string manipulation out of
strbuf.[ch]
* cw/strbuf-cleanup:
strbuf: remove global variable
path: move related function to path
object-name: move related functions to object-name
credential-store: move related functions to credential-store file
abspath: move related functions to abspath
strbuf: clarify dependency
strbuf: clarify API boundary
In some shells, such as bash and zsh, it's possible to use a command
substitution to provide the output of a command as a file argument to
another process, like so:
diff -u <(printf "a\nb\n") <(printf "a\nc\n")
However, this syntax does not produce useful results with "git diff
--no-index". On macOS, the arguments to the command are named pipes
under /dev/fd, and git diff doesn't know how to handle a named pipe. On
Linux, the arguments are symlinks to pipes, so git diff "helpfully"
diffs these symlinks, comparing their targets like "pipe:[1234]" and
"pipe:[5678]".
To address this "diff --no-index" is changed so that if a path given on
the commandline is a named pipe or a symbolic link that resolves to a
named pipe then we read the data to diff from that pipe. This is
implemented by generalizing the code that already exists to handle
reading from stdin when the user passes the path "-".
If the user tries to compare a named pipe to a directory then we die as
we do when trying to compare stdin to a directory.
As process substitution is not support by POSIX this change is tested by
using a pipe and a symbolic link to a pipe.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git diff --no-index" supports reading from stdin with the path "-".
There is no test coverage for this so add a regression test before
changing the code in the next commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If there is an error when reading from stdin then "diff --no-index"
prints an error message but continues to try and diff a file named "-"
resulting in an error message that looks like
error: error while reading from stdin: Invalid argument
fatal: stat '-': No such file or directory
assuming that no file named "-" exists. If such a file exists it prints
the first error message and generates the diff from that file which is
not what the user wanted. Instead just die() straight away if we cannot
read from stdin.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the user runs
git diff --no-index file directory
we follow the behavior of POSIX diff and rewrite the arguments as
git diff --no-index file directory/file
Doing that when "file" is "-" (which means "read from stdin") does not
make sense so we should error out if the user asks us to compare "-" to
a directory. This matches the behavior of GNU diff and diff on *BSD.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the first test in this script, 'creates a report with content in the
right places', we generate a report and pipe it into our helper
`check_all_headers_populated()`. The idea of the helper is to find all
lines that look like headers ("[Some Header Here]") and to check that
the next line is non-empty. This is supposed to catch erroneous outputs
such as the following:
[A Header]
something
more here
[Another Header]
[Too Early Header]
contents
However, we provide the lines of the bug report as filenames to grep,
meaning we mostly end up spewing errors:
grep: : No such file or directory
grep: [System Info]: No such file or directory
grep: git version:: No such file or directory
grep: git version 2.41.0.2.gfb7d80edca: No such file or directory
This doesn't disturb the test, which tugs along and reports success, not
really having verified the contents of the report at all.
Note that after 788a776069 ("bugreport: collect list of populated
hooks", 2020-05-07), the bug report, which is created in our hook-less
test repo, contains an empty section with the enabled hooks. Thus, even
the intention of our helper is a bit misguided: there is nothing
inherently wrong with having an empty section in the bug report.
Let's instead split this test into three: first verify that we generate
a report at all, then check that the introductory blurb looks the way it
should, then verify that the "[System Info]" seems to contain the right
things. (The "[Enabled Hooks]" section is tested later in the script.)
Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
alloc_nr, ALLOC_GROW, and ALLOC_GROW_BY are commonly used macros for
dynamic array allocation. Moving these macros to git-compat-util.h with
the other alloc macros focuses alloc.[ch] to allocation for Git objects
and additionally allows us to remove inclusions to alloc.h from files
that solely used the above macros.
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This table was originally introduced to solely be used with kwset
machinery (0f871cf56e), so it would make sense for it to belong in
kwset.[ch] rather than ctype.c and git-compat-util.h. It is only used in
diffcore-pickaxe.c, which already includes kwset.h so no other headers
have to be modified.
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Splitting these macros from git-compat-util.h cleans up the file and
allows future third-party sources to not use these overrides if they do
not wish to.
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since the functions in wrapper.c are widely used across the codebase,
include it by default in git-compat-util.h. A future patch will remove
now unnecessary inclusions of wrapper.h from other files.
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
While functions like starts_with() probably should not belong in the
boundaries of the strbuf library, this commit focuses on first splitting
out headers from git-compat-util.h.
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The imap_cmd_cb struct has several fields which are totally unused.
Presumably they did useful things in the upstream isync code from which
this is derived, but they don't in our more limited program. This is
particularly confusing for the "done" callback, which (as of the
previous patch) no longer matches the signature of the adjacent "cont"
callback.
Since we're unlikely to share code with isync going forward, we should
feel free to simplify the code here. Note that "done" is examined but
never set, so we can also drop a little bit of code outside of the
struct definition.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There's a generic callback mechanism for handling plus-continuation of
IMAP commands. It takes the imap_cmd struct itself as an argument. That
seems reasonable, and in a larger imap-using program it might be used.
But in imap-send, we have only one such callback (auth_cram_md5) and it
doesn't use this value, triggering -Wunused-parameter warnings.
We could just mark the parameter as UNUSED. But since this is the only
such function, and because we are not likely to share code with the
upstream isync anymore, we can just simplify the interface to remove
this parameter.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our caller passes in an imap_server_conf struct, but we ignore it
totally, and instead read the config directly from the global "server"
variable. This works OK, since our sole caller will pass in that same
global variable. But the intent seems to have been to use the passed-in
variable, as otherwise it has no purpose (and many other functions use
the same pattern).
Let's use the passed-in value, which also silences a -Wunused-parameter
warning.
It would be nice if "server" was not a global here, as we could avoid
making similar mistakes. But changing that would be a larger refactor,
as it must be accessed as a global in a few spots (e.g., filling it in
with the config callback).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The tutorial in Documentation/MyFirstObjectWalk.txt
contains the functions trace_printf(), oid_to_hex(),
and pp_commit_easy(), and struct oidset, which are used
without any hint of where they are defined. When the provided
code is compiled, the compiler returns an error, stating that
the functions and the struct are used before declaration. Therefore,include
necessary header files (the ones which have no mentions in the tutorial).
Signed-off-by: Vinayak Dev <vinayakdev.sci@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add more "git var" for toolsmiths to learn various locations Git is
configured with either via the configuration or hardcoded defaults.
* bc/more-git-var:
var: add config file locations
var: add attributes files locations
attr: expose and rename accessor functions
var: adjust memory allocation for strings
var: format variable structure with C99 initializers
var: add support for listing the shell
t: add a function to check executable bit
var: mark unused parameters in git_var callbacks
The set-up code for the get_revision() API now allows feeding
options like --all and --not in the --stdin mode.
* ps/revision-stdin-with-options:
revision: handle pseudo-opts in `--stdin` mode
revision: small readability improvement for reading from stdin
revision: reorder `read_revisions_from_stdin()`
When the external merge driver is killed by a signal, its output
should not be trusted as a resolution with conflicts that is
proposed by the driver, but the code did.
* jc/abort-ll-merge-with-a-signal:
t6406: skip "external merge driver getting killed by a signal" test on Windows
ll-merge: killing the external merge driver aborts the merge
Header files cleanup.
* en/header-split-cache-h-part-3: (28 commits)
fsmonitor-ll.h: split this header out of fsmonitor.h
hash-ll, hashmap: move oidhash() to hash-ll
object-store-ll.h: split this header out of object-store.h
khash: name the structs that khash declares
merge-ll: rename from ll-merge
git-compat-util.h: remove unneccessary include of wildmatch.h
builtin.h: remove unneccessary includes
list-objects-filter-options.h: remove unneccessary include
diff.h: remove unnecessary include of oidset.h
repository: remove unnecessary include of path.h
log-tree: replace include of revision.h with simple forward declaration
cache.h: remove this no-longer-used header
read-cache*.h: move declarations for read-cache.c functions from cache.h
repository.h: move declaration of the_index from cache.h
merge.h: move declarations for merge.c from cache.h
diff.h: move declaration for global in diff.c from cache.h
preload-index.h: move declarations for preload-index.c from elsewhere
sparse-index.h: move declarations for sparse-index.c from cache.h
name-hash.h: move declarations for name-hash.c from cache.h
run-command.h: move declarations for run-command.c from cache.h
...
We create .pack and then .idx, we consider only packfiles that have
.idx usable (those with only .pack are not ready yet), so we should
remove .idx before removing .pack for consistency.
* ds/remove-idx-before-pack:
packfile: delete .idx files before .pack files