Recent reftable updates mistook a NULL return from a request for
0-byte allocation as OOM and died unnecessarily, which has been
corrected.
* ps/reftable-alloc-failures-zalloc-fix:
reftable/basics: return NULL on zero-sized allocations
reftable/stack: fix zero-sized allocation when there are no readers
reftable/merged: fix zero-sized allocation when there are no readers
reftable/stack: don't perform auto-compaction with less than two tables
In the preceding commits we have fixed a couple of issues when
allocating zero-sized objects. These issues were masked by
implementation-defined behaviour. Quoting malloc(3p):
If size is 0, either:
* A null pointer shall be returned and errno may be set to an
implementation-defined value, or
* A pointer to the allocated space shall be returned. The
application shall ensure that the pointer is not used to access an
object.
So it is perfectly valid that implementations of this function may or
may not return a NULL pointer in such a case.
Adapt both `reftable_malloc()` and `reftable_realloc()` so that they
return NULL pointers on zero-sized allocations. This should remove any
implementation-defined behaviour in our allocators and thus allows us to
detect such platform-specific issues more easily going forward.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar as the preceding commit, we may try to do a zero-sized
allocation when reloading a reftable stack that ain't got any tables.
It is implementation-defined whether malloc(3p) returns a NULL pointer
in that case or a zero-sized object. In case it does return a NULL
pointer though it causes us to think we have run into an out-of-memory
situation, and thus we return an error.
Fix this by only allocating arrays when they have at least one entry.
Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It was reported [1] that Git started to fail with an out-of-memory error
when initializing repositories with the reftable backend on NonStop
platforms. A bisect led to 802c0646ac (reftable/merged: handle
allocation failures in `merged_table_init_iter()`, 2024-10-02), which
changed how we allocate memory when initializing a merged table.
The root cause of this seems to be that NonStop returns a `NULL` pointer
when doing a zero-sized allocation. This would've already happened
before the above change, but we never noticed because we did not check
the result. Now we do notice and thus return an out-of-memory error to
the caller.
Fix the issue by skipping the allocation altogether in case there are no
readers.
[1]: <00ad01db5017$aa9ce340$ffd6a9c0$@nexbridge.com>
Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In order to compact tables we need at least two tables. Bail out early
from `reftable_stack_auto_compact()` in case we have less than two
tables.
In the original, `stack_table_sizes_for_compaction()` yields an array
that has the same length as the number of tables. This array is then
passed on to `suggest_compaction_segment()`, which returns an empty
segment in case we have less than two tables. The segment is then passed
to `segment_size()`, which will return `0` because both start and end of
the segment are `0`. And because we only call `stack_compact_range()` in
case we have a positive segment size we don't perform auto-compaction at
all. Consequently, this change does not result in a user-visible change
in behaviour when called with a single table.
But when called with no tables this protects us against a potential
out-of-memory error: `stack_table_sizes_for_compaction()` would try to
allocate a zero-byte object when there aren't any tables, and that may
lead to a `NULL` pointer on some platforms like NonStop which causes us
to bail out with an out-of-memory error.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git bundle create" with an annotated tag on the positive end of
the revision range had a workaround code for older limitation in
the revision walker, which has become unnecessary.
* tc/bundle-with-tag-remove-workaround:
bundle: remove unneeded code
"git fetch" honors "remote.<remote>.followRemoteHEAD" settings to
tweak the remote-tracking HEAD in "refs/remotes/<remote>/HEAD".
* bf/fetch-set-head-config:
remote set-head: set followRemoteHEAD to "warn" if "always"
fetch set_head: add warn-if-not-$branch option
fetch set_head: move warn advice into advise_if_enabled
fetch: add configuration for set_head behaviour
"git fetch" from a configured remote learned to update a missing
remote-tracking HEAD but it asked the remote about their HEAD even
when it did not need to, which has been corrected. Incidentally,
this also corrects "git fetch --tags $URL" which was broken by the
new feature in an unspecified way.
* jc/set-head-symref-fix:
fetch: do not ask for HEAD unnecessarily
When "git fetch $remote" notices that refs/remotes/$remote/HEAD is
missing and discovers what branch the other side points with its
HEAD, refs/remotes/$remote/HEAD is updated to point to it.
* bf/set-head-symref:
fetch set_head: handle mirrored bare repositories
fetch: set remote/HEAD if it does not exist
refs: add create_only option to refs_update_symref_extended
refs: add TRANSACTION_CREATE_EXISTS error
remote set-head: better output for --auto
remote set-head: refactor for readability
refs: atomically record overwritten ref in update_symref
refs: standardize output of refs_read_symbolic_ref
t/t5505-remote: test failure of set-head
t/t5505-remote: set default branch to main
* 'master' of https://github.com/j6t/gitk:
gitk: offer "Copy commit ID to X11 selection" only on X11
gitk: support auto-copy comit ID to primary clipboard
gitk: prefs dialog: refine Auto-select UI
gitk: UI text: change "SHA1 ID" to "Commit ID"
gitk: add text wrapping preferences
gitk: make headings of preferences bold
gitk: check main window visibility before waiting for it to show
gitk: sv.po: Update Swedish translation (323t)
* ah/commit-id-to-clipboard:
gitk: offer "Copy commit ID to X11 selection" only on X11
gitk: support auto-copy comit ID to primary clipboard
gitk: prefs dialog: refine Auto-select UI
gitk: UI text: change "SHA1 ID" to "Commit ID"
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Build procedure update plus introduction of Meson based builds.
* ps/build: (24 commits)
Introduce support for the Meson build system
Documentation: add comparison of build systems
t: allow overriding build dir
t: better support for out-of-tree builds
Documentation: extract script to generate a list of mergetools
Documentation: teach "cmd-list.perl" about out-of-tree builds
Documentation: allow sourcing generated includes from separate dir
Makefile: simplify building of templates
Makefile: write absolute program path into bin-wrappers
Makefile: allow "bin-wrappers/" directory to exist
Makefile: refactor generators to be PWD-independent
Makefile: extract script to generate gitweb.js
Makefile: extract script to generate gitweb.cgi
Makefile: extract script to massage Python scripts
Makefile: extract script to massage Shell scripts
Makefile: use "generate-perl.sh" to massage Perl library
Makefile: extract script to massage Perl scripts
Makefile: consistently use PERL_PATH
Makefile: generate doc versions via GIT-VERSION-GEN
Makefile: generate "git.rc" via GIT-VERSION-GEN
...
Fix performance regression of a recent "fatten promisor pack with
local objects" protection against an unwanted gc.
* jt/fix-fattening-promisor-fetch:
index-pack --promisor: also check commits' trees
index-pack --promisor: don't check blobs
index-pack --promisor: dedup before checking links
The syntax ":/<text>" to name the latest commit with the matching
text was broken with a recent change, which has been corrected.
* ps/commit-with-message-syntax-fix:
object-name: fix reversed ordering with ":/<text>" revisions
The advice messages now tell the newer 'git config set' command to
set the advice.token configuration variable to squelch a message.
* bf/explicit-config-set-in-advice-messages:
advice: suggest using subcommand "git config set"
"git tag" has been taught to refuse to create refs/tags/HEAD
as such a tag will be confusing in the context of UI provided by
the Git Porcelain commands.
* jc/forbid-head-as-tagname:
tag: "git tag" refuses to use HEAD as a tagname
t5604: do not expect that HEAD can be a valid tagname
refs: drop strbuf_ prefix from helpers
refs: move ref name helpers around
"git describe" optimization.
* jk/describe-perf:
describe: split "found all tags" and max_candidates logic
describe: stop traversing when we run out of names
describe: stop digging for max_candidates+1
t/perf: add tests for git-describe
t6120: demonstrate weakness in disjoint-root handling
This option is only useful where a selection clipboard is available, which
is only the case on X11. Do not clutter the UI in other environments.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
The --ancestry-path option is designed to be given a commit that is
on the path, which was not documented, which has been corrected.
* kk/doc-ancestry-path:
doc: mention rev-list --ancestry-path restrictions
Yet another "pass the repository through the callchain" topic.
* kn/midx-wo-the-repository:
midx: inline the `MIDX_MIN_SIZE` definition
midx: pass down `hash_algo` to functions using global variables
midx: pass `repository` to `load_multi_pack_index`
midx: cleanup internal usage of `the_repository` and `the_hash_algo`
midx-write: pass down repository to `write_midx_file[_only]`
write-midx: add repository field to `write_midx_context`
midx-write: use `revs->repo` inside `read_refs_snapshot`
midx-write: pass down repository to static functions
packfile.c: remove unnecessary prepare_packed_git() call
midx: add repository to `multi_pack_index` struct
config: make `packed_git_(limit|window_size)` non-global variables
config: make `delta_base_cache_limit` a non-global variable
packfile: pass down repository to `for_each_packed_object`
packfile: pass down repository to `has_object[_kept]_pack`
packfile: pass down repository to `odb_pack_name`
packfile: pass `repository` to static function in the file
packfile: use `repository` from `packed_git` directly
packfile: add repository to struct `packed_git`
Backport oss-fuzz tests for us to our codebase.
* es/oss-fuzz:
fuzz: port fuzz-url-decode-mem from OSS-Fuzz
fuzz: port fuzz-parse-attr-line from OSS-Fuzz
fuzz: port fuzz-credential-from-url-gently from OSS-Fuzz
"git fast-import" learned to reject paths with ".." and "." as
their components to avoid creating invalid tree objects.
* en/fast-import-verify-path:
t9300: test verification of renamed paths
fast-import: disallow more path components
fast-import: disallow "." and ".." path components
"git bundle --unbundle" and "git clone" running on a bundle file
both learned to trigger fsck over the new objects with configurable
fck check levels.
* jt/bundle-fsck:
transport: propagate fsck configuration during bundle fetch
fetch-pack: split out fsck config parsing
bundle: support fsck message configuration
bundle: add bundle verification options type
To show a remerge diff, the merge needs to be recreated. For that to
work, the merge base(s) need to be found, which means that the commits'
parents have to be traversed until common ancestors are found (if any).
However, one optimization that hails all the way back to cb115748ec
(Some more memory leak avoidance, 2006-06-17) is to release the commit's
list of parents immediately after showing it _and to set that parent
list to `NULL`_. This can break the merge base computation.
This problem is most obvious when traversing the commits in reverse: In
that instance, if a parent of a merge commit has been shown as part of
the `git log` command, by the time the merge commit's diff needs to be
computed, that parent commit's list of parent commits will have been set
to `NULL` and as a result no merge base will be found (even if one
should be found).
Traversing commits in reverse is far from the only circumstance in which
this problem occurs, though. There are many avenues to traversing at
least one commit in the revision walk that will later be part of a merge
base computation, for example when not even walking any revisions in
`git show <merge1> <merge2>` where `<merge1>` is part of the commit
graph between the parents of `<merge2>`.
Another way to force a scenario where a commit is traversed before it
has to be traversed again as part of a merge base computation is to
start with two revisions (where the first one is reachable from the
second but not in a first-parent ancestry) and show the commit log with
`--topo-order` and `--first-parent`.
Let's fix this by special-casing the `remerge_diff` mode, similar to
what we did with reflogs in f35650dff6 (log: do not free parents when
walking reflog, 2017-07-07).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Auto-select ("Copy commit ID to X11 selection") is useful when a
selection cliboard exists, but otherwise generally meaningless, for
instance on Windows.
Add a similar pref and behavior which copies the commit ID to the
primary clipboard - for platforms without a selection clipboard, but
which can also be useful additionally on platforms with selection.
Note that while autoselect is enabled by default, autocopy isn't.
That's because the selection clipboard is typically dispensable, while
the primary clipboard can be considered a more precious resource,
which we don't want to (clear and) overwrite by default.
Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
Tl;DR: change Auto-select text, move the length input to a new line.
The Auto-select preference auto-selects [part of] the commit ID text
at the respective widget on startup, and when the current commit at
the graph changes.
Its real premise, however, is to populate the selection clipboard
with the commit ID. Consider, for instance, how meaningless it is on
platforms without a selection clipboard - like Windows or macOS (on
Windows the selection is not even visible with the default Tk theme,
because it's only visible in focused widgets - which the commit ID
widget is not during normal application of this selection).
So rename the Auto-select label to "Copy commit ID to X11 selection",
to reflect better the ultimate outcome of its application
Note that there exists other, non-X11 platforms with a selection
clipboard, like Wayland, and if a native Tk client exists on such
platforms, then the description will not be accurate, but hopefully
it's not too misleading either.
Additionally, move the length input widget to a new line, because:
- This length applies to both Auto-select and "Copy commit reference"
context menu item, so it's not exclusive to the selection length.
- The next commit will add support for primary clipboard as well,
where this length will also be used.
Also, move the "Hide remotes" item above these selection prefs, to
keep the selection prefs semi-grouped before the spacing of the
following title "Diff display options".
Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
SHA1 might not stay forever, and plans to use SHA256 already exist,
so use the official name for it - "Commit ID".
Only visible UI texts are modified to reduce the noise when using
git-blame, while comments and variable names still contain SHA1/sha1.
Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com>
The changes in commit c06793a4ed (allow git-bundle to create bottomless
bundle, 2007-08-08) ensure annotated tags are properly preserved when
creating a bundle using a revision range operation.
At the time the range notation would peel the ends to their
corresponding commit, meaning ref v2.0 would point to the v2.0^0 commit.
So the above workaround was introduced. This code looks up the ref
before it's written to the bundle, and if the ref doesn't point to the
object we expect (for tags this would be a tag object), we skip the ref
from the bundle. Instead, when the ref is a tag that's the positive end
of the range (e.g. v2.0 from the range "v1.0..v2.0"), then that ref is
written to the bundle instead.
Later, in 895c5ba3c1 (revision: do not peel tags used in range notation,
2013-09-19), the behavior of parsing ranges was changed and the problem
was fixed at the cause. But the workaround in bundle.c was not reverted.
Now it seems this workaround can cause a race condition. git-bundle(1)
uses setup_revisions() to parse the input into `struct rev_info`. Later,
in write_bundle_refs(), it uses this info to write refs to the bundle.
As mentioned at this point each ref is looked up again and checked
whether it points to the object we expect. If not, the ref is not
written to the bundle. But, when creating a bundle in a heavy traffic
repository (a repo with many references, and frequent ref updates) it's
possible a branch ref was updated between setup_revisions() and
write_bundle_refs() and thus the extra check causes the ref to be
skipped.
The workaround was originally added to deal with tags, but the code path
also gets hit by non-tag refs, causing this race condition. Because it's
no longer needed, remove it and fix the possible race condition.
Signed-off-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Optimize reading random references out of the reftable backend by
allowing reuse of iterator objects.
* ps/reftable-iterator-reuse:
refs/reftable: reuse iterators when reading refs
reftable/merged: drain priority queue on reseek
reftable/stack: add mechanism to notify callers on reload
refs/reftable: refactor reflog expiry to use reftable backend
refs/reftable: refactor reading symbolic refs to use reftable backend
refs/reftable: read references via `struct reftable_backend`
refs/reftable: figure out hash via `reftable_stack`
reftable/stack: add accessor for the hash ID
refs/reftable: handle reloading stacks in the reftable backend
refs/reftable: encapsulate reftable stack
Isolates the reftable subsystem from the rest of Git's codebase by
using fewer pieces of Git's infrastructure.
* ps/reftable-detach:
reftable/system: provide thin wrapper for lockfile subsystem
reftable/stack: drop only use of `get_locked_file_path()`
reftable/system: provide thin wrapper for tempfile subsystem
reftable/stack: stop using `fsync_component()` directly
reftable/system: stop depending on "hash.h"
reftable: explicitly handle hash format IDs
reftable/system: move "dir.h" to its only user
Loosen overly strict ownership check introduced in the recent past,
to keep the promise "cloning a suspicious repository is a safe
first step to inspect it".
* bc/allow-upload-pack-from-other-people:
Allow cloning from repositories owned by another user
End-user experience of "git mergetool" when the command errors out
has been improved.
* pb/mergetool-errors:
git-difftool--helper.sh: exit upon initialize_merge_tool errors
git-mergetool--lib.sh: add error message for unknown tool variant
git-mergetool--lib.sh: add error message if 'setup_user_tool' fails
git-mergetool--lib.sh: use TOOL_MODE when erroring about unknown tool
completion: complete '--tool-help' in 'git mergetool'
Describe a case where an option value needs to be spelled as a
separate argument, i.e. "--opt val", not "--opt=val".
* jc/doc-opt-tilde-expand:
doc: option value may be separate for valid reasons
Drop support for ancient environments in various CI jobs.
* bc/ancient-ci:
Add additional CI jobs to avoid accidental breakage
ci: remove clause for Ubuntu 16.04
gitlab-ci: switch from Ubuntu 16.04 to 20.04
We use a singleton empty array to initialize a `struct strvec`;
similar to the empty string singleton we use to initialize a `struct
strbuf`.
Note that an empty strvec instance (with zero elements) does not
necessarily need to be an instance initialized with the singleton.
Let's refer to strvec instances initialized with the singleton as
"empty-singleton" instances.
As a side note, this is the current `strvec_pop()`:
void strvec_pop(struct strvec *array)
{
if (!array->nr)
return;
free((char *)array->v[array->nr - 1]);
array->v[array->nr - 1] = NULL;
array->nr--;
}
So, with `strvec_pop()` an instance can become empty but it does
not going to be the an "empty-singleton".
This "empty-singleton" circumstance requires us to be careful when
adding elements to instances. Specifically, when adding the first
element: when we detach the strvec instance from the singleton and
set the internal pointer in the instance to NULL. After this point we
apply `realloc()` on the pointer. We do this in
`strvec_push_nodup()`, for example.
The recently introduced `strvec_splice()` API is expected to be
normally used with non-empty strvec's. However, it can also end up
being used with "empty-singleton" strvec's:
struct strvec arr = STRVEC_INIT;
int a = 0, b = 0;
... no modification to arr, a or b ...
const char *rep[] = { "foo" };
strvec_splice(&arr, a, b, rep, ARRAY_SIZE(rep));
So, we'll try to add elements to an "empty-singleton" strvec instance.
Avoid misapplying `realloc()` to the singleton in `strvec_splice()` by
adding a special case for strvec's initialized with the singleton.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit c08589efdc (index-pack: repack local links into promisor packs,
2024-11-01) seems to contain an oversight in that the tree of a commit
is not checked. Teach git to check these trees.
The fix slows down a fetch from a certain repo at $DAYJOB from 2m2.127s
to 2m45.052s, but in order to make the fetch correct, it seems worth it.
In order to test this, we could create server and client repos as
follows...
C S
\ /
O
(O and C are commits both on the client and server. S is a commit
only on the server. C and S have the same tree but different commit
messages. The diff between O and C is non-zero.)
...and then, from the client, fetch S from the server.
In theory, the client declares "have C" and the server can use this
information to exclude S's tree (since it knows that the client has C's
tree, which is the same as S's tree). However, it is also possible for
the server to compute that it needs to send S and not O, and proceed
from there; therefore the objects of C are not considered at all when
determining what to send in the packfile. In order to prevent a test of
client functionality from having such a dependence on server behavior, I
have not included such a test.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As a follow-up to the parent of this commit, it was found that not
checking for the existence of blobs linked from trees sped up the fetch
from 24m47.815s to 2m2.127s. Teach Git to do that.
The tradeoff of not checking blobs is documented in a code comment.
(Blobs may also be linked from tag objects, but it is impossible to know
the type of an object linked from a tag object without looking it up in
the object database, so the code for that is untouched.)
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit c08589efdc (index-pack: repack local links into promisor packs,
2024-11-01) fixed a bug with what was believed to be a negligible
decrease in performance [1] [2]. But at $DAYJOB, with at least one repo,
it was found that the decrease in performance was very significant.
Looking at the patch, whenever we parse an object in the packfile to
be indexed, we check the targets of all its outgoing links for its
existence. However, this could be optimized by first collecting all such
targets into an oidset (thus deduplicating them) before checking. Teach
Git to do that.
On a certain fetch from the aforementioned repo, this improved
performance from approximately 7 hours to 24m47.815s. This number will
be further reduced in a subsequent patch.
[1] https://lore.kernel.org/git/CAG1j3zGiNMbri8rZNaF0w+yP+6OdMz0T8+8_Wgd1R_p1HzVasg@mail.gmail.com/
[2] https://lore.kernel.org/git/20241105212849.3759572-1-jonathantanmy@google.com/
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Git documentation refers to $HOME and $XDG_CONFIG_HOME often, but does
not specify how or where these values come from on Windows where neither
is set by default. The new documentation reflects the behavior of
setup_windows_environment() in compat/mingw.c.
Signed-off-by: Alejandro Barreto <alejandro.barreto@ni.com>
Signed-off-by: M Hickford <mirth.hickford@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano