| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The help text for the '-l' option of 'git diff' has been updated.
* en/diff-l-opt-help:
diff: avoid misleading statement about -l option
|
|
The 'pack-objects' and delta-encoding code paths have been updated to
use 'size_t' instead of 'unsigned long' for object sizes and offset
limits, avoiding potential truncation issues on 64-bit Windows.
* js/pack-objects-delta-size-t:
packfile: widen `unpack_object_header_buffer()` to `size_t`
git-zlib: widen `git_deflate_bound()` to `size_t`
t/helper/test-pack-deltas: widen `do_compress()`'s maxsize local to `size_t`
http-push: widen `start_put()`'s size local from `ssize_t` to `size_t`
diff: widen `deflate_it()`'s bound local from int to `size_t`
archive-zip: widen `zlib_deflate_raw()`'s maxsize local to `size_t`
packfile, git-zlib: widen `use_pack()` and zstream avail fields to `size_t`
delta: widen `create_delta()` and `diff_delta()` to `size_t`
pack-objects: widen `mem_usage` and `try_delta()`'s out-param to `size_t`
pack-objects: widen `free_unpacked()` return to `size_t`
pack-objects: widen delta-cache accounting to `size_t`
delta: widen `create_delta_index()` parameter to `size_t`
diff-delta: widen `struct delta_index`' size fields to `size_t`
|
|
In commit 6623a528e00b (doc: clarify documentation for rename/copy
limits, 2021-07-15), the wording around rename limit options and config
variables were updated to point out that only the quadratic portion of
rename detection (or "exhaustive portion of rename/copy detection" as
used in that commit) was limited by these options, because exact rename
detection and basename-guided rename detection (which both run in time
linear in the number of files) still run before this limit is checked.
However, the short help message wasn't updated at the time; update it
too.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Fixes a pre-existing silent narrowing from `git_deflate_bound()`'s
`unsigned long` return into an `int` local: anything past 2 GiB has
always wrapped negative here and then been re-extended to `size_t`
inside `xmalloc()`. Also prep for the upcoming `git_deflate_bound()`
widening to `size_t`, which would extend the narrowing further if
`bound` stayed `int`.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Last stop in the delta-encoding API widening for >4 GiB blobs on
Windows: with `create_delta_index()` done in the prior commit and
`create_delta()`/`diff_delta()` finished here, every byte count that
crosses delta.h is now `size_t`. The struct fields they store into have
been `size_t` since the diff-delta struct widening.
The API change must move with all callers in the same commit (the build
only passes when every `&delta_size` matches the new `size_t*`). Caller
updates are kept minimal:
* builtin/pack-objects.c `get_delta()` and `try_delta()`: widen only
the local `delta_size` variable; the surrounding unsigned-long
locals and their `cast_size_t_to_ulong()` shims are out of scope
here and will be cleaned up in their own commits.
* builtin/fast-import.c, diff.c, t/helper/test-pack-deltas.c:
keep the local unsigned-long delta size (each feeds a still-
unsigned-long downstream consumer: zlib's `avail_in`,
`deflate_it()`, the test helper's own `do_compress()`), and bridge
via a temporary `size_t` plus `cast_size_t_to_ulong()`. The new
casts are paid back in later topics that widen those consumers.
* t/helper/test-delta.c: widen the local outright (no downstream
consumer beyond the test's own `out_size`, which is already
`size_t`).
Note that GCC struggles a bit to figure out that `deltalen` is always
initialized before it is used; To help it along, we initialize it to 0.
This work-around will go away in a later patch series when `deltalen`
can be widened to `size_t`.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The diff.c:is_conflict_marker() and rerere.c:is_cmarker() functions
implement duplicate logic for identifying conflict marker lines
(lines that begin with a run of '<', '=', '>', and '|' characters).
diff.c's original version from 049540435f (diff --check: detect
leftover conflict markers, 2008-06-26) accepts any whitespace (such
as a newline) immediately following '<<<<<<<' and '>>>>>>>', whereas
rerere.c's version from 191f241717 (rerere: prepare for customizable
conflict marker length, 2010-01-16) strictly requires a space
character (' ') after them.
Implement is_conflict_marker_line() in merge-ll.c to serve as a
replacement for both, and update diff.c and rerere.c to use the new
helper. The unified helper intentionally adopts rerere's stricter
rule, as the conflicts generated by Git always show the "ours" and
"theirs" labels after these markers separated by a space.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We'd like to add more logic to git_hash_init(), but many callers skip it
and call algop->init_fn() directly. Let's make sure we're consistently
using the wrapper by adding a coccinelle rule.
Besides the coccinelle file itself, this is a purely mechanical
conversion based on the patch it generates. There should be no bare
init_fn() calls left (except for the one in the wrapper).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
* jk/hash-algo-leak-fixes:
hash: add platform-specific discard functions
hash: fix memory leak copying sha256 gcrypt handles
http: discard hash in dumb-http http_object_request
check_stream_oid(): discard hash on read error
patch-id: discard hash when done
csum-file: provide a function to release checkpoints
csum-file: always finalize or discard hash
hash: add discard primitive
csum-file: drop discard_hashfile()
|
|
When computing a patch-id, we have a flush_one_hunk() helper that calls
git_hash_final() on our running hunk git_hash_ctx, and then
reinitializes that context for the next hunk.
When we run out of hunks to look at, we return, discarding the
git_hash_ctx. This can cause a leak if the hash implementation we are
using allocates any memory during its initialization. This includes
OpenSSL >= 3.0, for both SHA-1 and SHA-256. Normally we would not use
SHA-1 here at all, as we only recommend using non-DC implementations for
the "unsafe" variant (and patch-id, though they probably _could_ use the
unsafe variant, were never taught to do so).
But it is certainly a problem for SHA-256, which you can see with:
make SANITIZE=leak \
OPENSSL_SHA256=1 \
GIT_TEST_DEFAULT_HASH=sha256 \
test
That results in leak failures of 60 scripts, 57 of which are fixed by
this patch (basically anything which runs rebase will hit this case).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
* js/objects-larger-than-4gb-on-windows-more:
odb: use size_t for object_info.sizep and the size APIs
packfile,delta: drop the `cast_size_t_to_ulong()` wrappers
pack-objects: use size_t for in-core object sizes
packfile: widen unpack_entry()'s size out-parameter to size_t
pack-objects(check_pack_inflate()): use size_t instead of unsigned long
patch-delta: use size_t for sizes
compat/msvc: use _chsize_s for ftruncate
|
|
When `js/objects-larger-than-4gb-on-windows` widened the streaming,
index-pack and unpack-objects code paths, in the interest of keeping the
patches somewhat reasonably-sized, it left the public ODB API still
typed in `unsigned long`. In particular `struct object_info::sizep` and
the four wrappers built on top of it (`odb_read_object`,
`odb_read_object_peeled`, `odb_read_object_info`, `odb_pretend_object`)
still return the unpacked size through `unsigned long *`, so on Windows
`cat-file -s` and the `git add` / `git status` paths for a >4 GiB blob
silently cap at 4 GiB.
Widen the field and the four wrappers. The previous commits already
widened the `unpack_entry()` cascade and pack-objects' in-core size
accessors, so most of the cascade arrives here with no further work: the
temporary shims in `packed_object_info_with_index_pos()` and in
`unpack_entry()`'s delta-base recovery path go away, the two
`SET_SIZE(entry, cast_size_t_to_ulong(canonical_size))` calls in
`check_object()` and the matching one in `drop_reused_delta()` collapse
to plain `SET_SIZE`, and `oe_get_size_slow()`'s tail
`cast_size_t_to_ulong()` is gone too.
What remains narrow are the boundaries this series does not
intend to touch: the diff, blame, textconv and fast-import machinery.
Even so, this patch is unfortunately quite large.
Assisted-by: Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Many core configuration variables have been migrated from global
variables into 'repo_config_values' to tie them to a specific
repository instance, avoiding cross-repository state leakage.
* ob/more-repo-config-values:
environment: move "warn_on_object_refname_ambiguity" into `struct repo_config_values`
environment: move "sparse_expect_files_outside_of_patterns" into `struct repo_config_values`
environment: move "core_sparse_checkout_cone" into `struct repo_config_values`
environment: move "precomposed_unicode" into `struct repo_config_values`
environment: move "pack_compression_level" into `struct repo_config_values`
environment: move `zlib_compression_level` into `struct repo_config_values`
environment: move "check_stat" into `struct repo_config_values`
environment: move "trust_ctime" into `struct repo_config_values`
|
|
The `zlib_compression_level` configuration is currently stored in the
global variable `zlib_compression_level`, which makes it shared across
repository instances within a single process.
Store it instead in `repo_config_values`, where eagerly‑parsed
repository configuration lives. `zlib_compression_level` is parsed
eagerly because it determines compression behaviour for objects and
packs – core operations where a lazy parse could lead to unpredictable
results and hinder libification. This preserves the existing
eager‑parsing behavior while tying the value to the repository it
was read from, avoiding cross‑repository state leakage and continuing
the effort to reduce reliance on global configuration state.
Update all references to use `repo_config_values()`.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The command line parser for "git diff" learned a few options take
only non-negative integers.
* mm/diff-U-takes-no-negative-values:
parse-options: clarify what "negated" means for PARSE_OPT_NONEG
xdiff: guard against negative context lengths
diff: reject negative values for -U/--unified
diff: reject negative values for --inter-hunk-context
|
|
Passing a negative value to -U is silently accepted and produces
corrupt unified diff output with malformed hunk headers:
$ git log -1 -p -U-500 -- GIT-VERSION-GEN | grep '^@@'
@@ -503,999- +503,999- @@
Line 503 of a 106-line file, count "999-" is not a valid integer.
The config variable diff.context already rejects negative values, but
the command line callback diff_opt_unified() uses strtol() with no
range check.
Change the type of diff_options.context and its static default from
int to unsigned int, matching the change to interhunkcontext in the
previous commit. The type change requires reworking the callback and
config parsing to validate in a local variable before assigning to
the now-unsigned field.
Unlike --inter-hunk-context which could be converted to OPT_UNSIGNED,
-U needs OPT_CALLBACK_F for PARSE_OPT_OPTARG (bare -U with no value
enables patch output). Add a range check in the callback instead.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Negative values for --inter-hunk-context produce structurally invalid
diff output with overlapping hunks:
$ git log -1 -p -U3 --inter-hunk-context=-100 791aeddfa2 \
-- git-compat-util.h | grep '^@@'
@@ -110,6 +110,9 @@
@@ -115,6 +118,9 @@
@@ -116,6 +122,7 @@
Hunk 1 covers lines 110-115, hunk 2 starts at 115 (overlap), hunk 3
starts at 116 (overlaps both). The resulting patch cannot be applied.
The config variable diff.interHunkContext already rejects negative
values, but the command line option does not.
Change the type of diff_options.interhunkcontext and its static
default from int to unsigned int, and switch the option parser from
OPT_INTEGER_F to OPT_UNSIGNED. This rejects negative values at parse
time via git_parse_unsigned() and enforces the correct type at compile
time via BARF_UNLESS_UNSIGNED.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
f85b49f3d4a (diff: improve scaling of filenames in diffstat to handle
UTF-8 chars, 2026-01-16) introduced a loop in show_stats() that calls
utf8_width() repeatedly to skip leading characters until the displayed
width fits. However, utf8_width() can return problematic values:
- For invalid UTF-8 sequences, pick_one_utf8_char() sets the name
pointer to NULL and utf8_width() returns 0. Since name_len does
not change, the loop iterates once more and pick_one_utf8_char()
dereferences the NULL pointer, crashing.
- For control characters, utf8_width() returns -1, so name_len
grows when it is expected to shrink. This can cause the loop to
consume more characters than the string contains, reading past
the trailing NUL.
By default, fill_print_name() will C-quote filenames which escapes
control characters and invalid bytes to printable text. That avoids
this bug from being triggered; however, with core.quotePath=false,
most characters are no longer escaped (though some control characters
still are) and raw bytes can reach this code.
Add tests exercising both failure modes with core.quotePath=false and
a narrow --stat-name-width to force truncation: one with a bare 0xC0
byte (invalid UTF-8 lead byte, triggers NULL deref) and one with
several C1 control characters (repeats of 0xC2 0x9F, causing
the loop to read past the end of the string). The second test
reliably catches the out-of-bounds read when run under ASan, though
it may pass silently without sanitizers.
Fix both issues by introducing utf8_ish_width(), a thin wrapper
around utf8_width() that guarantees the pointer always advances and
the returned width is never negative:
- On invalid UTF-8 it restores the pointer, advances by one byte,
and returns width 1 (matching the strlen()-based fallback used
by utf8_strwidth()).
- On a control character it returns 0 (matching utf8_strnwidth()
which skips them).
Also add a "&& *name" guard to the while-loop condition so it
terminates at end-of-string even when utf8_strwidth()'s strlen()
fallback causes name_len to exceed the sum of per-character widths.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The way the "git log -L<range>:<file>" feature is bolted onto the
log/diff machinery is being reworked a bit to make the feature
compatible with more diff options, like -S/G.
* mm/line-log-use-standard-diff-output:
doc: note that -L supports patch formatting and pickaxe options
t4211: add tests for -L with standard diff options
line-log: route -L output through the standard diff pipeline
line-log: fix crash when combined with pickaxe options
|
|
`git log -L` has always bypassed the standard diff pipeline.
`dump_diff_hacky()` in line-log.c hand-rolls its own diff headers and
hunk output, which means most diff formatting options are silently
ignored. A NEEDSWORK comment has acknowledged this since the feature
was introduced:
/*
* NEEDSWORK: manually building a diff here is not the Right
* Thing(tm). log -L should be built into the diff pipeline.
*/
Remove `dump_diff_hacky()` and its helpers and route -L output through
`builtin_diff()` / `fn_out_consume()`, the same path used by `git diff`
and `git log -p`. The mechanism is a pair of callback wrappers that sit
between `xdi_diff_outf()` and `fn_out_consume()`, filtering xdiff's
output to only the tracked line ranges. To ensure xdiff emits all lines
within each range as context, the context length is inflated to span the
largest range.
Wire up the `-L` implies `--patch` default in revision setup rather
than forcing it at output time, so `line_log_print()` is just
`diffcore_std()` + `diff_flush()` with no format save/restore.
Rename detection is a no-op since pairs are already resolved during
the history walk in `queue_diffs()`, but running `diffcore_std()`
means `-S`/`-G` (pickaxe), `--orderfile`, and `--diff-filter` now
work with `-L`, and `diff_resolve_rename_copy()` sets pair statuses
correctly without manual assignment.
Switch `diff_filepair_dup()` from `xmalloc` to `xcalloc` so that new
fields (including `line_ranges`) are zero-initialized by default.
As a result, diff formatting options that were previously silently
ignored (e.g. --word-diff, --no-prefix, -w, --color-moved) now work
with -L, and output gains `index` lines, `new file mode` headers, and
funcname context in `@@` headers. This is a user-visible output change:
tools that parse -L output may need to handle the additional header
lines.
The context-length inflation means xdiff may process more output than
needed for very wide line ranges, but benchmarks on files up to 7800
lines show no measurable regression.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
"git diff --no-index --find-object=<object-name>" outside a
repository of course wouldn't be able to find the object and died
while parsing the command line, which is made to die in a bit more
user-friendly way.
* mm/diff-no-index-find-object:
diff: fix crash with --find-object outside repository
|
|
"git log --graph --stat" did not count the display width of colored
graph part of its own output correctly, which has been corrected.
* lp/diff-stat-utf8-display-width-fix:
t4052: test for diffstat width when prefix contains ANSI escape codes
diff: handle ANSI escape codes in prefix when calculating diffstat width
|
|
Mark the marge-ort codebase to prevent more uses of the_repository
from getting added.
* en/merge-ort-almost-wo-the-repository:
replay: prevent the_repository from coming back
merge-ort: prevent the_repository from coming back
merge-ort: replace the_hash_algo with opt->repo->hash_algo
merge-ort: replace the_repository with opt->repo
merge-ort: pass repository to write_tree()
merge,diff: remove the_repository check before prefetching blobs
|
|
When "git diff --find-object=<oid>" is run outside a git repository,
the option parsing callback eagerly resolves the OID via
repo_get_oid(), which reaches get_main_ref_store() and hits a BUG()
assertion because no repository has been set up.
Check startup_info->have_repository before attempting to resolve the
OID, and return a user-friendly error instead.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
It does not make much sense to apply the "incomplete-line"
whitespace rule to symbolic links, whose contents almost always
lack the final newline. "git apply" and "git diff" are now taught
to exclude them for a change to symbolic links.
* jc/whitespace-incomplete-line:
whitespace: symbolic links usually lack LF at the end
|
|
The diffstat width is calculated by taking the terminal width and
incorrectly subtracting the `strlen()` of `line_prefix`, instead of the
actual display width of `line_prefix`, which may contain ANSI escape
codes (e.g., ANSI-colored strings in `log --graph --stat`).
Utilize the display width instead, obtained via `utf8_strnwidth()` with
the flag `skip_ansi`.
Signed-off-by: LorenzoPegorari <lorenzo.pegorari2002@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Prefetching of blobs from promisor remotes was added to diff in
7fbbcb21b162 (diff: batch fetching of missing blobs, 2019-04-05). In
that commit,
https://lore.kernel.org/git/20190405170934.20441-1-jonathantanmy@google.com/
was squashed into
https://lore.kernel.org/git/44de02e584f449481e6fb00cf35d74adf0192e9d.1553895166.git.jonathantanmy@google.com/
without the extra explanation about the squashed changes being added to
the commit message; in particular, this explanation from that first link
is absent:
> Also, prefetch only if the repository being diffed is the_repository
> (because we do not support lazy fetching for any other repository
> anyway).
Then, later, this checking was spread from diff.c to diffcore-rename.c
and diffcore-break.c by 95acf11a3dc3 (diff: restrict when prefetching
occurs, 2020-04-07) and then further split in d331dd3b0c82
(diffcore-rename: allow different missing_object_cb functions,
2021-06-22). I also copied the logic from prefetching blobs from
diff.c to merge-ort.c in 2bff554b23e8 (merge-ort: add prefetching for
content merges, 2021-06-22).
The reason for all these checks was noted above -- we only supported
lazy fetching for the_repository. However, that changed with
ef830cc43412 (promisor-remote: teach lazy-fetch in any repo,
2021-06-17), so these checks are now unnecessary. Remove them.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
ISO C23 redefines strchr and friends that tradiotionally took
a const pointer and returned a non-const pointer derived from it to
preserve constness (i.e., if you ask for a substring in a const
string, you get a const pointer to the substring). Update code
paths that used non-const pointer to receive their results that did
not have to be non-const to adjust.
* cf/c23-const-preserving-strchr-updates-0:
gpg-interface: remove an unnecessary NULL initialization
global: constify some pointers that are not written to
|
|
For a patch that touches a symbolic link, it is perfectly normal
that the contents ends with "\ No newline at end of file". The
checks introduced recently to detect incomplete lines (i.e., a text
file that lack the newline on its final line) should not trigger.
Disable the check early for symbolic links, both in "git apply" and
"git diff" and test them. For "git apply", we check only when the
postimage is a symbolic link regardless of the preimage, and we only
care about preimage when applying in reverse. Similarly, "git diff"
would warn only when the postimage is a symbolic link, or the
preimage when running "git diff -R".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The recent glibc 2.43 release had the following change listed in its
NEWS file:
For ISO C23, the functions bsearch, memchr, strchr, strpbrk, strrchr,
strstr, wcschr, wcspbrk, wcsrchr, wcsstr and wmemchr that return
pointers into their input arrays now have definitions as macros that
return a pointer to a const-qualified type when the input argument is
a pointer to a const-qualified type.
When compiling with GCC 15, which defaults to -std=gnu23, this causes
many warnings like this:
merge-ort.c: In function ‘apply_directory_rename_modifications’:
merge-ort.c:2734:36: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
2734 | char *last_slash = strrchr(cur_path, '/');
| ^~~~~~~
This patch fixes the more obvious ones by making them const when we do
not write to the returned pointer.
Signed-off-by: Collin Funk <collin.funk1@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The computation of column width made by "git diff --stat" was
confused when pathnames contain non-ASCII characters.
* lp/diff-stat-utf8-display-width-fix:
t4073: add test for diffstat paths length when containing UTF-8 chars
diff: improve scaling of filenames in diffstat to handle UTF-8 chars
|
|
The `show_stats()` function tries to scale the filenames in the diffstat to
ensure they don't exceed the given `name-width`. It does so by calculating
the "display width" of the characters to be dropped, but then advances the
filename pointer by that number of bytes.
However, the "display width" of a character is not always equal to its byte
count. The result is that sometimes, when displaying UTF-8 characters,
filenames exceed the given `name-width`, and frequently the bytes of the
UTF-8 characters are truncated.
The following is an example of the issue, where the 2 files are "HelloHi" and
"Hello你好", and `name-width=6`:
...oHi | 0
...<BD><A0>好 | 0
Make the filename pointer move by the actual number of bytes of the
characters to drop from the filename, rather than their display width, using
the `utf8_width()` function.
Force `len` to not be less than 0 (this happens if the given `name-width` is
2 or less), otherwise an infinite loop is entered.
Signed-off-by: LorenzoPegorari <lorenzo.pegorari2002@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When computing a diff in a partial clone, there is a chance that we
could trigger a prefetch of missing objects at the same time as we are
freeing entries from the global diff queue. This is difficult to
reproduce, as we need to have some objects be freed from the queue
before triggering the prefetch of missing objects. There is a new test
in t4067 that does trigger the segmentation fault that results in this
case.
The fix is to set the queue pointer to NULL after it is freed, and then
to be careful about NULL values in the prefetch.
The more elaborate explanation is that within diffcore_std(), we may
skip the initial prefetch due to the output format (--name-only in the
test) and go straight to diffcore_skip_stat_unmatch(). In that method,
the index entries that have been invalidated by path changes show up as
entries but may be deleted because they are not actually content diffs
and only newer timestamps than expected. As those entries are deleted,
later entries are checked with diff_filespec_check_stat_unmatch(), which
uses diff_queued_diff_prefetch() as the missing_object_cb in its diff
options. That can trigger downloading missing objects if the appropriate
scenario occurs to trigger a call to diff_popoulate_filespec(). It's
finally within that callback to diff_queued_diff_prefetch() that the
segfault occurs.
The test was hard to find because it required some real differences,
some not-different files that had a newer modified time, and the order
of those files alphabetically was important to trigger the deletion
before the prefetch was triggered.
I briefly considered a "lock" member for the diff queue, but it was a
much larger diff and introduced many more possible error scenarios.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Halve the memory consumed by artificial filepairs created during
"git diff --find-copioes-harder", also making the operation run
faster.
* rs/diff-index-find-copies-harder-optim:
diff-index: don't queue unchanged filepairs with diff_change()
|
|
Both "git apply" and "git diff" learn a new whitespace error class,
"incomplete-line".
* jc/whitespace-incomplete-line:
attr: enable incomplete-line whitespace error for this project
diff: highlight and error out on incomplete lines
apply: check and fix incomplete lines
whitespace: allocate a few more bits and define WS_INCOMPLETE_LINE
apply: revamp the parsing of incomplete lines
diff: update the way rewrite diff handles incomplete lines
diff: call emit_callback ecbdata everywhere
diff: refactor output of incomplete line
diff: keep track of the type of the last line seen
diff: correct suppress_blank_empty hack
diff: emit_line_ws_markup() if/else style fix
whitespace: correct bit assignment comments
|
|
diff_cache() queues unchanged filepairs if the flag find_copies_harder
is set, and uses diff_change() for that. This function allocates a
filespec for each side, does a few other things that are unnecessary for
unchanged filepairs and always sets the diff_flag has_changes, which is
simply misleading in this case.
Add a new streamlined function for queuing unchanged filepairs and
use it in show_modified(), which is called by diff_cache() via
oneway_diff() and do_oneway_diff(). It allocates only a single filespec
for each filepair and uses it twice with reference counting. This has a
measurable effect if there are a lot of them, like in the Linux repo:
Benchmark 1: ./git_v2.52.0 -C ../linux diff --cached --find-copies-harder
Time (mean ± σ): 31.8 ms ± 0.2 ms [User: 24.2 ms, System: 6.3 ms]
Range (min … max): 31.5 ms … 32.3 ms 85 runs
Benchmark 2: ./git -C ../linux diff --cached --find-copies-harder
Time (mean ± σ): 23.9 ms ± 0.2 ms [User: 18.1 ms, System: 4.6 ms]
Range (min … max): 23.5 ms … 24.4 ms 111 runs
Summary
./git -C ../linux diff --cached --find-copies-harder ran
1.33 ± 0.01 times faster than ./git_v2.52.0 -C ../linux diff --cached --find-copies-harder
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
"git blame" learns "--diff-algorithm=<algo>" option.
* ad/blame-diff-algorithm:
blame: make diff algorithm configurable
xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK
|
|
As "git diff --quiet" only cares about the existence of any
changes, disable rename/copy detection to skip more expensive
processing whose result will be discarded anyway.
* rs/diff-quiet-no-rename:
diff: disable rename detection with --quiet
|
|
The XDF_DIFF_ALGORITHM_MASK bit mask only includes bits for the patience
and histogram diffs, not for the minimal one. This means that when
reseting the diff algorithm to the default one, one needs to separately
clear the bit for the minimal diff. There are places in the code that fail
to do that: merge-ort.c and builtin/merge-file.c.
Add the XDF_NEED_MINIMAL bit to the bit mask, and remove the separate
clearing of this bit in the places where it hasn't been forgotten.
Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Teach "git diff" to highlight "\ No newline at end of file" message
as a whitespace error when incomplete-line whitespace error class is
in effect. Thanks to the previous refactoring of complete rewrite
code path, we can do this at a single place.
Unlike whitespace errors in the payload where we need to annotate in
line, possibly using colors, the line that has whitespace problems,
we have a dedicated line already that can serve as the error
message, so paint it as a whitespace error message.
Also teach "git diff --check" to notice incomplete lines as
whitespace errors and report when incomplete-line whitespace error
class is in effect.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Reserve a few more bits in the diff flags word to be used for future
whitespace rules. Add WS_INCOMPLETE_LINE without implementing the
behaviour (yet).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The diff_symbol based output framework uses one DIFF_SYMBOL_* enum
value per the kind of output lines of "git diff", which corresponds
to one output line from the xdiff machinery used internally. Most
notably, DIFF_SYMBOL_PLUS and DIFF_SYMBOL_MINUS that correspond to
"+" and "-" lines are designed to always take a complete line, even
if the output from xdiff machinery may produce "\ No newline at the
end of file" immediately after them.
But this is not true in the rewrite-diff codepath, which completely
bypasses the xdiff machinery. Since the code path feeds the bytes
directly from the payload to the output routines, the output layer
has to deal with an incomplete line with DIFF_SYMBOL_PLUS and
DIFF_SYMBOL_MINUS, which never would see an incomplete line in the
normal code paths. This lack of final newline is compensated by an
ugly hack for a fabricated DIFF_SYMBOL_NO_LF_EOF token to inject an
extra newline to the output to simulate output coming from the xdiff
machinery.
Revamp the way the complete-rewrite code path feeds the lines to the
output layer by treating the last line of the pre/post image when it
is an incomplete line specially.
This lets us remove the DIFF_SYMBOL_NO_LF_EOF hack and use the usual
DIFF_SYMBOL_CONTEXT_INCOMPLETE code path, which will later learn how
to handle whitespace errors.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Everybody else, except for emit_rewrite_lines(), calls the
emit_callback data ecbdata. Make sure we call the same thing by
the same name for consistency.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Create a helper function that reacts to "\ No newline at the end of
file" in preparation for unifying the incomplete line handling in
the code path that handles xdiff output and the code path that
bypasses xdiff and produces a complete-rewrite patch.
Currently the output from the DIFF_SYMBOL_CONTEXT_INCOMPLETE case
still (ab)uses the same code as what is used for context lines, but
that would change in a later step where we introduce support to treat
an incomplete line as a whitespace error.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The "\ No newline at the end of the file" can come after any of the
"-" (deleted preimage line), " " (unchanged line), or "+" (added
postimage line). In later steps in this series, we will start
treating a change that makes a file to end in an incomplete line
as a whitespace error, and we would need to know what the previous
line was when we react to "\ No newline" in the diff output. If
the previous line was a context (i.e., unchanged) line, the file
lacked the final newline before the change, and the change did not
touch that line and left it still incomplete, so we do not want to
warn in such a case.
Teach fn_out_consume() function to keep track of what the previous
line was, and prepare an otherwise empty switch statement to let us
react differently to "\ No newline" based on that.
Note that there is an existing curiosity (read: likely to be a bug)
in the code that increments line number in the preimage file every
time it sees a line with "\ No newline" on it, regardless of what
the previous line was. I left it as-is, because it does not affect
the main theme of this series, and more importantly, I do not think
it matters, as these numbers are used only to compare them with
blank_at_eof_in_{pre,post}image to issue a warning when we see more
empty line was added at the end, but by definition, after we see
"\ No newline at the end of the file" for an added line, we will not
see an added line for the file.
An independent audit to ensure that this curious increment can be
safely removed would make a good #leftoverbits clean-up (we may even
find some code that decrements this counter or over-increments the
other quantity this counter is compared with that compensates the
effect of this curious increment that hides a bug, in which case we
may also need to remove them).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The suppress-blank-empty feature abused the CONTEXT_INCOMPLETE
symbol that was meant to be used only for "\ No newline at the end
of file" code path.
The intent of the feature was to turn a context line we receive from
xdiff machinery (which always uses ' ' for context lines, even an
empty one) and spit it out as a truly empty line.
Perform such a conversion very locally at where a line from xdiff
that begins with ' ' is handled for output; there are many checks
before the control reaches such place that checks the first letter
of the diff output line to see if it is a context line, and having
to check for '\n' and treat it as a special case is error prone.
In order to catch similar hacks in the future, make sure the code
path that is meant for "\ No newline" case checks the first byte is
indeed a backslash.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Apply the simple rule: if you need {} in one arm of the if/else
if/else... cascade, have {} in all of them.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
A comment in diff.c claimed that bits up to 12th (counting from 0th)
are whitespace rules, and 13th thru 15th are for new/old/context,
but it turns out it was miscounting. Correct them, and clarify
where the whitespace rule bits come from in the comment. Extend bit
assignment comments to cover bits used for color-moved, which
weren't described.
Also update the way these bit constants are defined to use (1 << N)
notation, instead of octal constants, as it tends to make it easier
to notice a breakage like this.
Sprinkle a few blank lines between logically distinct groups of CPP
macro definitions to make them easier to read.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Detecting renames and copies improves diff's output. This effort is
wasted if we don't show any. Disable detection in that case.
This actually fixes the error code when using the options --cached,
--find-copies-harder, --no-ext-diff and --quiet together:
run_diff_index() indirectly calls diff-lib.c::show_modified(), which
queues even non-modified entries using diff_change() because we need
them for copy detection. diff_change() sets flags.has_changes, though,
which causes diff_can_quit_early() to declare we're done after seeing
only the very first entry -- way too soon.
Using --cached, --find-copies-harder and --quiet together without
--no-ext-diff was not affected even before, as it causes the flag
flags.diff_from_contents to be set, which disables the optimization
in a different way.
Reported-by: D. Ben Knoble <ben.knoble@gmail.com>
Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
We'd sometimes end up in run_external_diff() to do a dry-run diff (e.g.,
to find content-level changes for --quiet). We recognize this quiet mode
by seeing the lack of DIFF_FORMAT_PATCH in the output format.
But since introducing an explicit dry-run check via 3ed5d8bd73 (diff:
stop output garbled message in dry run mode, 2025-10-20), this logic can
never trigger. We can only get to this function by calling
diff_flush_patch(), and that comes from only two places:
1. A dry-run flush comes from diff_flush_patch_quietly(), which is
always in dry-run mode (so the other half of our "||" is true
anyway).
2. A regular flush comes from diff_flush_patch_all_file_pairs(),
which is only called when output_format has DIFF_FORMAT_PATCH in
it.
So we can simplify our "quiet" condition to just checking dry-run mode
(which used to be a specific flag, but recently became just a NULL
"file" pointer). And since it's so simple, we can just do that inline.
This makes the logic about o->file more obvious, since we handle the
NULL and non-stdout cases next to each other.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
As an added protection against dry-run diffs accidentally producing
output, we redirect diff_options.file to /dev/null. But as of the
previous patch, this now does nothing, since dry-run diffs are
implemented by setting "file" to NULL.
So we can drop this extra code with no change in behavior. This is
effectively a revert of 623f7af284 (diff: restore redirection to
/dev/null for diff_from_contents, 2025-10-17) and 3da4413dbc (diff: make
sure the other caller of diff_flush_patch_quietly() is silent,
2025-10-22), but:
1. We get a conflict because we already dropped the color_moved
handling in an earlier patch. But we just resolve the conflicts to
"theirs" (removing all of the code).
2. We retain the test from 623f7af284.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
