Taylor Blau [Fri, 25 Oct 2024 18:02:36 +0000 (14:02 -0400)]
Merge branch 'jc/a-commands-without-the-repo'
Commands that can also work outside Git have learned to take the
repository instance "repo" when we know we are in a repository, and
NULL when we are not, in a parameter. The uses of the_repository
variable in a few of them have been removed using the new calling
convention.
* jc/a-commands-without-the-repo:
archive: remove the_repository global variable
annotate: remove usage of the_repository global
git: pass in repo to builtin based on setup_git_directory_gently
Taylor Blau [Fri, 25 Oct 2024 18:01:21 +0000 (14:01 -0400)]
Merge branch 'ps/ci-gitlab-windows'
Enable Windows-based CI in GitLab.
* ps/ci-gitlab-windows:
gitlab-ci: exercise Git on Windows
gitlab-ci: introduce stages and dependencies
ci: handle Windows-based CI jobs in GitLab CI
ci: create script to set up Git for Windows SDK
t7300: work around platform-specific behaviour with long paths on MinGW
Seyi Kuforiji [Sat, 19 Oct 2024 16:34:38 +0000 (17:34 +0100)]
t7011: ensure no whitespace after redirect
This change updates the script to conform to the coding standards
outlined in the Git project's documentation. According to the guidelines
in Documentation/CodingGuidelines under "Redirection operators", there
should be no whitespace after redirection operators.
Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Taylor Blau [Tue, 22 Oct 2024 18:40:39 +0000 (14:40 -0400)]
Merge branch 'cw/worktree-relative'
An extra worktree attached to a repository points at each other to
allow finding the repository from the worktree and vice versa
possible. Turn this linkage to relative paths.
* cw/worktree-relative:
worktree: add test for path handling in linked worktrees
worktree: link worktrees with relative paths
worktree: refactor infer_backlink() to use *strbuf
worktree: repair copied repository and linked worktrees
Taylor Blau [Tue, 22 Oct 2024 18:40:38 +0000 (14:40 -0400)]
Merge branch 'ps/cache-tree-w-broken-index-entry'
Fail gracefully instead of crashing when attempting to write the
contents of a corrupt in-core index as a tree object.
* ps/cache-tree-w-broken-index-entry:
unpack-trees: detect mismatching number of cache-tree/index entries
cache-tree: detect mismatching number of index entries
cache-tree: refactor verification to return error codes
Chizoba ODINAKA [Tue, 22 Oct 2024 01:27:01 +0000 (02:27 +0100)]
t6050: avoid pipes with upstream Git commands
In pipes, the exit code of a chain of commands is determined by
the final command. In order not to miss the exit code of a failed
Git command, avoid pipes instead write output of Git commands
into a file.
For better debugging experience, instances of "grep" were changed
to "test_grep". "test_grep" provides more context in case of a
failed "grep".
Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
ref-filter: format iteratively with lexicographic refname sorting
In bd98f9774e (ref-filter.c: filter & format refs in the same callback,
2023-11-14), we have introduced logic into the ref-filter subsystem that
determines whether or not we can output references iteratively instead
of first collecting all references, post-processing them and printing
them once done. This has the advantage that we don't have to store all
refs in memory and, when used with e.g. `--count=1`, that we don't have
to read all refs in the first place.
One restriction we have in place for that is that caller must not ask
for sorted refs, because there is no way to sort the refs without first
reading them all into an array. So the benefits can only be reaped when
explicitly asking for output not to be sorted.
But there is one exception here where we _can_ get away with sorting
refs while streaming: ref backends sort references returned by their
iterators in lexicographic order. So if the following conditions are all
true we can do iterative streaming:
- There must be at most a single sorting specification, as otherwise
we're not using plain lexicographic ordering.
- The sorting specification must use the "refname".
- The sorting specification must not be using any flags, like
case-insensitive sorting.
Now the resulting logic does feel quite fragile overall, which makes me
a bit uneasy. But after thinking about this for a while I couldn't find
any obvious gaps in my reasoning. Furthermore, given that lexicographic
sorting order is the default in git-for-each-ref(1), this is likely to
benefit a whole lot of usecases out there.
The following benchmark executes git-for-each-ref(1) in a crafted repo
with 1 million references:
Benchmark 1: git for-each-ref (revision = HEAD~)
Time (mean ± σ): 6.756 s ± 0.014 s [User: 3.004 s, System: 3.541 s]
Range (min … max): 6.738 s … 6.784 s 10 runs
Benchmark 2: git for-each-ref (revision = HEAD)
Time (mean ± σ): 6.479 s ± 0.017 s [User: 2.858 s, System: 3.422 s]
Range (min … max): 6.450 s … 6.519 s 10 runs
Summary
git for-each-ref (revision = HEAD)
1.04 ± 0.00 times faster than git for-each-ref (revision = HEAD~)
The change results in a slight performance improvement, but nothing that
would really stand out. Something that cannot be seen in the benchmark
though is peak memory usage, which went from 404.5MB to 68.96kB.
A more interesting benchmark is printing a single referenence with
`--count=1`:
Benchmark 1: git for-each-ref --count=1 (revision = HEAD~)
Time (mean ± σ): 6.655 s ± 0.018 s [User: 2.865 s, System: 3.576 s]
Range (min … max): 6.630 s … 6.680 s 10 runs
Benchmark 2: git for-each-ref --count=1 (revision = HEAD)
Time (mean ± σ): 8.6 ms ± 1.3 ms [User: 2.3 ms, System: 6.1 ms]
Range (min … max): 6.7 ms … 14.4 ms 266 runs
Summary
git git for-each-ref --count=1 (revision = HEAD)
770.58 ± 116.19 times faster than git for-each-ref --count=1 (revision = HEAD~)
Whereas we scaled with the number of references before, we now print the
first reference and exit immediately, which provides a massive win.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
"git rebase --rebase-merges" now uses branch names as labels when
able.
* ng/rebase-merges-branch-name-as-label:
rebase-merges: try and use branch names as labels
rebase-update-refs: extract load_branch_decorations
load_branch_decorations: fix memory leak with non-static filters
reftable/stack: adapt `stack_filename()` to handle allocation failures
The `stack_filename()` function cannot pass any errors to the caller as it
has a `void` return type. Adapt it and its callers such that we can
handle errors and start handling allocation failures.
There are two interesting edge cases in `reftable_stack_destroy()` and
`reftable_addition_close()`. Both of these are trying to tear down their
respective structures, and while doing so they try to unlink some of the
tables they have been keeping alive. Any earlier attempts to do that may
fail on Windows because it keeps us from deleting such tables while they
are still open, and thus we re-try on close. It's okay and even expected
that this can fail when the tables are still open by another process, so
we handle the allocation failures gracefully and just skip over any file
whose name we couldn't figure out.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
reftable/record: adapt `reftable_record_key()` to handle allocation failures
The `reftable_record_key()` function cannot pass any errors to the
caller as it has a `void` return type. Adapt it and its callers such
that we can handle errors and start handling allocation failures.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
reftable/stack: adapt `format_name()` to handle allocation failures
The `format_name()` function cannot pass any errors to the caller as it
has a `void` return type. Adapt it and its callers such that we can
handle errors and start handling allocation failures.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Convert the reftable library to use the `reftable_buf` interface instead
of the `strbuf` interface. This is mostly a mechanical change via sed(1)
with some manual fixes where functions for `strbuf` and `reftable_buf`
differ. The converted code does not yet handle allocation failures. This
will be handled in subsequent commits.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
reftable/basics: provide new `reftable_buf` interface
Implement a new `reftable_buf` interface that will replace Git's own
`strbuf` interface. This is done due to three reasons:
- The `strbuf` interfaces do not handle memory allocation failures and
instead causes us to die. This is okay in the context of Git, but is
not in the context of the reftable library, which is supposed to be
usable by third-party applications.
- The `strbuf` interface is quite deeply tied into Git, which makes it
hard to use the reftable library as a standalone library. Any
dependent would have to carefully extract the relevant parts of it
to make things work, which is not all that sensible.
- The `strbuf` interface does not use the pluggable allocators that
can be set up via `reftable_set_alloc()`.
So we have good reasons to use our own type, and the implementation is
rather trivial. Implement our own type. Conversion of the reftable
library will be handled in subsequent commits.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
We're about to introduce our own `reftable_buf` type to replace
`strbuf`. One function we'll have to convert is `strbuf_addf()`, which
is used in a handful of places. This function uses `snprintf()`
internally, which makes porting it a bit more involved:
- It is not available on all platforms.
- Some platforms like Windows have broken implementations.
So by using `snprintf()` we'd also push the burden on downstream users
of the reftable library to make available a properly working version of
it.
Most callsites of `strbuf_addf()` are trivial to convert to not using
it. We do end up using `snprintf()` in our unit tests, but that isn't
much of a problem for downstream users of the reftable library.
While at it, remove a useless call to `strbuf_reset()` in
`t_reftable_stack_auto_compaction_with_locked_tables()`. We don't write
to the buffer before this and initialize it with `STRBUF_INIT`, so there
is no need to reset anything.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
We're about to introduce our own `reftable_buf` type to replace
`strbuf`. Get rid of the seldomly-used `strbuf_addbuf()` function such
that we have to reimplement one less function.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Wolfgang Müller [Thu, 17 Oct 2024 09:35:28 +0000 (11:35 +0200)]
builtin/shortlog: explicitly set hash algo when there is no repo
Whilst git-shortlog(1) does not explicitly need any repository
information when run without reference to one, it still parses some of
its arguments with parse_revision_opt() which assumes that the hash
algorithm is set. However, in c8aed5e8da (repository: stop setting SHA1
as the default object hash, 2024-05-07) we stopped setting up a default
hash algorithm and instead require commands to set it up explicitly.
This was done for most other commands like in ab274909d4 (builtin/diff:
explicitly set hash algo when there is no repo, 2024-05-07) but was
missed for builtin/shortlog, making git-shortlog(1) segfault outside of
a repository when given arguments like --author that trigger a call to
parse_revision_opt().
Fix this for now by explicitly setting the hash algorithm to SHA1. Also
add a regression test for the segfault.
Thanks-to: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Wolfgang Müller <wolf@oriole.systems> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Sören Krecker [Thu, 17 Oct 2024 17:18:20 +0000 (19:18 +0200)]
mingw.c: Fix complier warnings for a 64 bit msvc
Remove some complier warnings from msvc in compat/mingw.c for value
truncation from 64 bit to 32 bit integers.
Compiling compat/mingw.c under a 64 bit version of msvc produces
warnings. An "int" is 32 bit, and ssize_t or size_t should be 64 bit
long. Prepare compat/vcbuild/include/unistd.h to have a 64 bit type
_ssize_t, when _WIN64 is defined and 32 bit otherwise.
Further down in this include file, as before, ssize_t is defined as
_ssize_t, if needed.
Use size_t instead of int for all variables that hold the result of
strlen() or wcslen() (which cannot be negative).
Use ssize_t to hold the return value of read().
Signed-off-by: Sören Krecker <soekkle@freenet.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
On macOS, fsmonitor can fall into a race condition that results in
a client waiting forever to be notified for an event that have
already happened. This problem has been corrected.
* jk/fsmonitor-event-listener-race-fix:
fsmonitor: initialize fs event listener before accepting clients
simple-ipc: split async server initialization and running
Taylor Blau [Tue, 15 Oct 2024 20:56:43 +0000 (16:56 -0400)]
Merge branch 'xx/remote-server-option-config'
A new configuration variable remote.<name>.serverOption makes the
transport layer act as if the --serverOption=<value> option is
given from the command line.
* xx/remote-server-option-config:
ls-remote: leakfix for not clearing server_options
fetch: respect --server-option when fetching multiple remotes
transport.c::handshake: make use of server options from remote
remote: introduce remote.<name>.serverOption configuration
transport: introduce parse_transport_option() method
Philippe Blain [Fri, 11 Oct 2024 17:29:47 +0000 (17:29 +0000)]
Makefile: fix dependency for $(UNIT_TEST_DIR)/clar/clar.o
The clar source file '$(UNIT_TEST_DIR)/clar/clar.c' includes the
generated 'clar.suite', but this dependency is not taken into account by
our Makefile, so that it is possible for a parallel build to fail if
Make tries to build 'clar.o' before 'clar.suite' is generated.
Correctly specify the dependency.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
John Cai [Thu, 10 Oct 2024 21:13:48 +0000 (21:13 +0000)]
archive: remove the_repository global variable
As part of the effort to get rid of global state due to the global
the_repository variable, replace the_repository with the repository
argument that gets passed down through the builtin function.
The repo might be NULL, but we should be safe in write_archive() because
it detects if we are outside of a repository and calls
setup_git_directory() which will error.
Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
John Cai [Thu, 10 Oct 2024 21:13:47 +0000 (21:13 +0000)]
annotate: remove usage of the_repository global
As part of the effort to get rid of global state due to the_repository
variable, remove the the_repository with the repository argument that
gets passed down through the builtin function.
Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
John Cai [Thu, 10 Oct 2024 21:13:46 +0000 (21:13 +0000)]
git: pass in repo to builtin based on setup_git_directory_gently
The current code in run_builtin() passes in a repository to the builtin
based on whether cmd_struct's option flag has RUN_SETUP.
This is incorrect, however, since some builtins that only have
RUN_SETUP_GENTLY can potentially take a repository.
setup_git_directory_gently() tells us whether or not a command is being
run inside of a repository.
Use the output of setup_git_directory_gently() to help determine whether
or not there is a repository to pass to the builtin. If not, then we
just pass NULL.
As part of this patch, we need to modify add to check for a NULL repo
before calling repo_git_config(), since add -h can be run outside of a
repository.
Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 10 Oct 2024 21:22:29 +0000 (14:22 -0700)]
Merge branch 'jk/output-prefix-cleanup'
Code clean-up.
* jk/output-prefix-cleanup:
diff: store graph prefix buf in git_graph struct
diff: return line_prefix directly when possible
diff: return const char from output_prefix callback
diff: drop line_prefix_length field
line-log: use diff_line_prefix() instead of custom helper
Junio C Hamano [Thu, 10 Oct 2024 21:22:24 +0000 (14:22 -0700)]
Merge branch 'ja/doc-synopsis-markup'
The way AsciiDoc is used for SYNOPSIS part of the manual pages has
been revamped. The sources, at least for the simple cases, got
vastly pleasant to work with.
* ja/doc-synopsis-markup:
doc: apply synopsis simplification on git-clone and git-init
doc: update the guidelines to reflect the current formatting rules
doc: introduce a synopsis typesetting
We can only check out commits or branches, not refs in general. And the
problem here is if another worktree is using the branch that we want to
check out.
Let’s be more direct and just talk about branches instead of refs.
Also replace “be held” with “in use”. Further, “in use” is not
restricted to a branch being checked out (e.g. the branch could be busy
on a rebase), hence generalize to “or otherwise in use” in the option
description.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/gc: fix crash when running `git maintenance start`
It was reported on the mailing list that running `git maintenance start`
immediately segfaults starting with b6c3f8e12c (builtin/maintenance: fix
leak in `get_schedule_cmd()`, 2024-09-26). And indeed, this segfault is
trivial to reproduce up to a point where one is scratching their head
why we didn't catch this regression in our test suite.
The root cause of this error is `get_schedule_cmd()`, which does not
populate the `out` parameter in all cases anymore starting with the
mentioned commit. Callers do assume it to always be populated though and
will e.g. call `strvec_split()` on the returned value, which will of
course segfault when the variable is uninitialized.
So why didn't we catch this trivial regression? The reason is that our
tests always set up the "GIT_TEST_MAINT_SCHEDULER" environment variable
via "t/test-lib.sh", which allows us to override the scheduler command
with a custom one so that we don't accidentally modify the developer's
system. But the faulty code where we don't set the `out` parameter will
only get hit in case that environment variable is _not_ set, which is
never the case when executing our tests.
Fix the regression by again unconditionally allocating the value in the
`out` parameter, if provided. Add a test that unsets the environment
variable to catch future regressions in this area.
Reported-by: Shubham Kanodia <shubham.kanodia10@gmail.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Tue, 1 Oct 2024 18:36:52 +0000 (11:36 -0700)]
doc: clarify <src> in refspec syntax
We explicitly avoid saying "ref <src>" when introducing the source
side of a refspec, because it can be a fully-spelled hexadecimal
object name, and it also can be a pattern that is not quite a "ref".
But we are loose when we introduce <dst> and say "ref <dst>", even
though it can also be a pattern. Let's omit "ref" also from the
destination side.
Clarify that <src> can be a ref, a (limited glob) pattern, or an
object name.
Even though the very original design of refspec expected that '*'
was used only at the end (e.g., "refs/heads/*" was expected, but not
"refs/heads/*-wip"), the code and its use evolved to handle a single
'*' anywhere in the pattern. Update the text to remove the mention
of "the same prefix". Anything that matches the pattern are named
by such a (limited glob) pattern in <src>.
Also put a bit more stress on the fact that we accept only one '*'
in the pattern by saying "one and only one `*`".
Helped-by: Monika Kairaitytė <monika@kibit.lt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t7300-clean.sh: use test_path_* helper functions for error logging
This test script uses "test - [def]", but when a test fails because
the file passed to it does not exist,
it fails silently without an error message.
Use test_path_* helper functions, which are designed to give better
error messages when their expectations are not met.
I have added a mechanical validation that applies the same transformation
done in this patch, when the test script is passed to a sed script as shown
below.
Reviewers can use the sed script to tranform the original test script and
compare the result in foo.sh with the results of applying the patch.
You will see an instance of "!(test -e 3)" which was manually replaced with
""test_path_is_missing 3", and everything else should match.
Careful and deliberate observation was done to check instances where
"test ! - [df] foo" was used in the test script to make sure that the test
instances were expecting foo to EITHER be a file or a directory, and NOT a
possibility of being both as this would make replacing "test ! -f foo" with
"test_path_is_missing foo" unreasonable.
In the tests control flow, foo has been created as EITHER a
reguar file OR a directory and should NOT exist
after "git clean" or "git clean -d", as the case maybe, has been called.
This made it reasonable to replace
"test ! -[df] foo" with "test_path_is_missing foo".
Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Karthik Nayak [Wed, 9 Oct 2024 09:58:59 +0000 (02:58 -0700)]
loose: don't rely on repository global state
In `loose.c`, we rely on the global variable `the_hash_algo`. As such we
have guarded the file with the 'USE_THE_REPOSITORY_VARIABLE' definition.
Let's derive the hash algorithm from the available repository variable
and remove this guard. This brings us one step closer to removing the
global 'the_repository' variable.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add jobs that exercise Git on Windows. Unfortunately, building and
especially testing Git on Windows is inherently slower compared to other
Unix-like systems, mostly because spawning processes is way slower. We
thus use the same layout as we use in GitHub Actions, where we have one
build job, and then pass on the resulting build artifacts to ten test
jobs that split up the work across each other.
Unfortunately, the GitLab runners for Windows machines are embarassingly
slow by themselves. So while this strategy leads to around 20 minutes of
build time in GitHub Actions, the same pipeline takes around an hour in
GitLab CI. Still, having late coverage is certainly better than having
none at all.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We're about to add a couple of jobs for Windows. As the Windows runners
are quite slow, we will split those up across two stages: one stage to
build the artifacts, and one stage that runs test slices in parallel.
Introduce stages and "needs" dependencies for the preexisting jobs as a
preparatory step. The stages will lead to a more natural representation
of jobs in the UI, whereas the "needs" dependency ensures that jobs do
not have to wait for all jobs in the preceding stage to finish.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We try to abstract away any differences between different CI platforms
in "ci/lib.sh", such that knowledge specific to e.g. GitHub Actions or
GitLab CI is neatly encapsulated in a single place. Next to some generic
variables, we also set up some variables that are specific to the actual
platform that the CI operates on, e.g. Linux or macOS.
We do not yet support Windows runners on GitLab CI. Unfortunately, those
systems do not use the same "CI_JOB_IMAGE" environment variable as both
Linux and macOS do. Instead, we can use the "OS" variable, which should
have a value of "Windows_NT" on Windows platforms.
Handle the combination of "$OS,$CI_JOB_IMAGE" and introduce support for
Windows.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In order to build and test Git, we have to first set up the Git for
Windows SDK, which contains various required tools and libraries. The
SDK is basically a clone of [1], but that repository is quite large due
to all the binaries it contains. We thus use both shallow clones and
sparse checkouts to speed up the setup. To handle this complexity we use
a GitHub action that is hosted externally at [2].
Unfortunately, this makes it rather hard to reuse the logic for CI
platforms other than GitHub Actions. After chatting with Johannes
Schindelin we came to the conclusion that it would be nice if the Git
for Windows SDK would regularly publish releases that one can easily
download and extract, thus moving all of the complexity into that single
step. Like this, all that a CI job needs to do is to fetch and extract
the resulting archive. This published release comes in the form of a new
"ci-artifacts" tag that gets updated regularly [3].
Implement a new script that knows how to fetch and extract that script
and convert GitHub Actions to use it.
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t7300: work around platform-specific behaviour with long paths on MinGW
Windows by default has a restriction in place to only allow paths up to
260 characters. This restriction can nowadays be lifted by setting a
registry key, but is still active by default.
In t7300 we have one test that exercises the behaviour of git-clean(1)
with such long paths. Interestingly enough, this test fails on my system
that uses Windows 10 with mingw-w64 installed via MSYS2: instead of
observing ENAMETOOLONG, we observe ENOENT. This behaviour is consistent
across multiple different environments I have tried.
I cannot say why exactly we observe a different error here, but I would
not be surprised if this was either dependent on the Windows version,
the version of MinGW, the current working directory of Git or any kind
of combination of these.
Work around the issue by handling both errors.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When interactively rebasing merge commits, the commit message is parsed to
extract a probably meaningful label name. For instance if the merge commit
is “Merge branch 'feature0'”, then the rebase script will have thes lines:
```
label feature0
This heuristic fails in the case of octopus merges or when the merge commit
message is actually unrelated to the parent commits.
An example that combines both is:
```
*---. 967bfa4 (HEAD -> integration) Integration
|\ \ \
| | | * 2135be1 (feature2, feat2) Feature 2
| |_|/
|/| |
| | * c88b01a Feature 1
| |/
|/|
| * 75f3139 (feat0) Feature 0
|/
* 25c86d0 (main) Initial commit
```
yields the labels Integration, Integration-2 and Integration-3.
Fix this by using a branch name for each merge commit's parent that is the
tip of at least one branch, and falling back to a label derived from the
merge commit message otherwise.
In the example above, the labels become feat0, Integration and feature2.
Signed-off-by: Nicolas Guichard <nicolas@guichard.eu> Acked-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Extract load_branch_decorations from todo_list_add_update_ref_commands so
it can be re-used in make_script_with_merges.
Since it can now be called multiple times, use non-static lists and place
it next to load_ref_decorations to re-use the decoration_loaded guard.
Signed-off-by: Nicolas Guichard <nicolas@guichard.eu> Acked-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
load_branch_decorations: fix memory leak with non-static filters
load_branch_decorations calls normalize_glob_ref on each string of filter's
string_lists. This effectively replaces the potentially non-owning char* of
those items with an owning char*.
Set the strdup_string flag on those string_lists.
This was not caught until now because:
- when passing string_lists already with the strdup_string already set, the
behaviour was correct
- when passing static string_lists, the new char* remain reachable until
program exit
Signed-off-by: Nicolas Guichard <nicolas@guichard.eu> Acked-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
• Provide a commit message in the example command.
The command will hang since it is waiting for a commit message on
stdin. Which is usable but not straightforward enough since this is
example code.
• Use `||` directly since that is more straightforward than checking the
last exit status.
Also use `echo` and `exit` since `die` is not defined.
• Expose variable declarations.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Josh Heinrichs [Tue, 8 Oct 2024 14:29:20 +0000 (08:29 -0600)]
git-config.1: remove value from positional args in unset usage
The synopsis for `git config unset` mentions two positional arguments:
`<name>` and `<value>`. While the first argument is correct, the second
is not. Users are expected to provide the value via `--value=<value>`.
Remove the positional argument. The `--value=<value>` option is already
documented correctly, so this is all we need to do to fix the
documentation.
Signed-off-by: Josh Heinrichs <joshiheinrichs@gmail.com> Reviewed-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 8 Oct 2024 08:36:13 +0000 (04:36 -0400)]
fsmonitor: initialize fs event listener before accepting clients
There's a racy hang in fsmonitor on macOS that we sometimes see in CI.
When we serve a client, what's supposed to happen is:
1. The client thread calls with_lock__wait_for_cookie() in which we
create a cookie file and then wait for a pthread_cond event
2. The filesystem event listener sees the cookie file creation, does
some internal book-keeping, and then triggers the pthread_cond.
But there's a problem: we start the listener that accepts client threads
before we start the fs event thread. So it's possible for us to accept a
client which creates the cookie file and starts waiting before the fs
event thread is initialized, and we miss those filesystem events
entirely. That leaves the client thread hanging forever.
In CI, the symptom is that t9210 (which is testing scalar, which always
enables fsmonitor under the hood) may hang forever in "scalar clone". It
is waiting on "git fetch" which is waiting on the fsmonitor daemon.
The race happens more frequently under load, but you can trigger it
predictably with a sleep like this, which delays the start of the fs
event thread:
+ sleep(1);
if (!FSEventStreamStart(data->stream)) {
error(_("Failed to start the FSEventStream"));
goto force_error_stop_without_loop;
One solution might be to reverse the order of initialization: start the
fs event thread before we start the thread listening for clients. But
the fsmonitor code explicitly does it in the opposite direction. The fs
event thread wants to refer to the ipc_server_data struct, so we need it
to be initialized first.
A further complication is that we need a signal from the fs event thread
that it is actually ready and listening. And those details happen within
backend-specific fsmonitor code, whereas the initialization is in the
shared code.
So instead, let's use the ipc_server init/start split added in the
previous commit. The generic fsmonitor code will init the ipc_server but
_not_ start it, leaving that to the backend specific code, which now
needs to call ipc_server_start_async() at the right time.
For macOS, that is right after we start the FSEventStream that you can
see in the diff above.
It's not clear to me if Windows suffers from the same problem (and we
simply don't trigger it in CI), or if it is immune. Regardless, the
obvious place to start accepting clients there is right after we've
established the ReadDirectoryChanges watch.
This makes the hangs go away in our macOS CI environment, even when
compiled with the sleep() above.
Helped-by: Koji Nakamaru <koji.nakamaru@gree.net> Signed-off-by: Jeff King <peff@peff.net> Acked-by: Koji Nakamaru <koji.nakamaru@gree.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 8 Oct 2024 08:33:47 +0000 (04:33 -0400)]
simple-ipc: split async server initialization and running
To start an async ipc server, you call ipc_server_run_async(). That
initializes the ipc_server_data object, and starts all of the threads
running, which may immediately start serving clients.
This can create some awkward timing problems, though. In the fsmonitor
daemon (the sole user of the simple-ipc system), we want to create the
ipc server early in the process, which means we may start serving
clients before the rest of the daemon is fully initialized.
To solve this, let's break run_async() into two parts: an initialization
which allocates all data and spawns the threads (without letting them
run), and a start function which actually lets them begin work. Since we
have two simple-ipc implementations, we have to handle this twice:
- in ipc-unix-socket.c, we have a central listener thread which hands
connections off to worker threads using a work_available mutex. We
can hold that mutex after init, and release it when we're ready to
start.
We do need an extra "started" flag so that we know whether the main
thread is holding the mutex or not (e.g., if we prematurely stop the
server, we want to make sure all of the worker threads are released
to hear about the shutdown).
- in ipc-win32.c, we don't have a central mutex. So we'll introduce a
new startup_barrier mutex, which we'll similarly hold until we're
ready to let the threads proceed.
We again need a "started" flag here to make sure that we release the
barrier mutex when shutting down, so that the sub-threads can
proceed to the finish.
I've renamed the run_async() function to init_async() to make sure we
catch all callers, since they'll now need to call the matching
start_async().
We could leave run_async() as a wrapper that does both, but there's not
much point. There are only two callers, one of which is fsmonitor, which
will want to actually do work between the two calls. And the other is
just a test-tool wrapper.
For now I've added the start_async() calls in fsmonitor where they would
otherwise have happened, so there should be no behavior change with this
patch.
Signed-off-by: Jeff King <peff@peff.net> Acked-by: Koji Nakamaru <koji.nakamaru@gree.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Caleb White [Tue, 8 Oct 2024 03:12:32 +0000 (22:12 -0500)]
worktree: add test for path handling in linked worktrees
A failure scenario reported in an earlier patch series[1] that several
`git worktree` subcommands failed or misbehaved when invoked from within
linked worktrees that used relative paths.
This adds a test that executes a `worktree prune` command inside both an
internally and an externally linked worktree and asserts that the other
worktree was not pruned.
Caleb White [Tue, 8 Oct 2024 03:12:31 +0000 (22:12 -0500)]
worktree: link worktrees with relative paths
Git currently stores absolute paths to both the main repository and
linked worktrees. However, this causes problems when moving repositories
or working in containerized environments where absolute paths differ
between systems. The worktree links break, and users are required to
manually execute `worktree repair` to repair them, leading to workflow
disruptions. Additionally, mapping repositories inside of containerized
environments renders the repository unusable inside the containers, and
this is not repairable as repairing the worktrees inside the containers
will result in them being broken outside the containers.
To address this, this patch makes Git always write relative paths when
linking worktrees. Relative paths increase the resilience of the
worktree links across various systems and environments, particularly
when the worktrees are self-contained inside the main repository (such
as when using a bare repository with worktrees). This improves
portability, workflow efficiency, and reduces overall breakages.
Although Git now writes relative paths, existing repositories with
absolute paths are still supported. There are no breaking changes
to workflows based on absolute paths, ensuring backward compatibility.
At a low level, the changes involve modifying functions in `worktree.c`
and `builtin/worktree.c` to use `relative_path()` when writing the
worktree’s `.git` file and the main repository’s `gitdir` reference.
Instead of hardcoding absolute paths, Git now computes the relative path
between the worktree and the repository, ensuring that these links are
portable. Locations where these respective file are read have also been
updated to properly handle both absolute and relative paths. Generally,
relative paths are always resolved into absolute paths before any
operations or comparisons are performed.
Additionally, `repair_worktrees_after_gitdir_move()` has been introduced
to address the case where both the `<worktree>/.git` and
`<repo>/worktrees/<id>/gitdir` links are broken after the gitdir is
moved (such as during a re-initialization). This function repairs both
sides of the worktree link using the old gitdir path to reestablish the
correct paths after a move.
The `worktree.path` struct member has also been updated to always store
the absolute path of a worktree. This ensures that worktree consumers
never have to worry about trying to resolve the absolute path themselves.
Signed-off-by: Caleb White <cdwhite3@pm.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Caleb White [Tue, 8 Oct 2024 03:12:30 +0000 (22:12 -0500)]
worktree: refactor infer_backlink() to use *strbuf
This lays the groundwork for the next patch, which needs the backlink
returned from infer_backlink() as a `strbuf`. It seemed inefficient to
convert from `strbuf` to `char*` and back to `strbuf` again.
This refactors infer_backlink() to return an integer result and use a
pre-allocated `strbuf` for the inferred backlink path, replacing the
previous `char*` return type and improving efficiency.
Signed-off-by: Caleb White <cdwhite3@pm.me> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Xing Xin [Tue, 8 Oct 2024 03:38:19 +0000 (03:38 +0000)]
ls-remote: leakfix for not clearing server_options
Ensure `server_options` is properly cleared using `string_list_clear()`
in `builtin/ls-remote.c:cmd_ls_remote`.
Although we cannot yet enable `TEST_PASSES_SANITIZE_LEAK=true` for
`t/t5702-protocol-v2.sh` due to other existing leaks, this fix ensures
that "git-ls-remote" related server options tests pass the sanitize leak
check:
...
ok 12 - server-options are sent when using ls-remote
ok 13 - server-options from configuration are used by ls-remote
...
Signed-off-by: Xing Xin <xingxin.xx@bytedance.com> Reviewed-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Xing Xin [Tue, 8 Oct 2024 03:38:18 +0000 (03:38 +0000)]
fetch: respect --server-option when fetching multiple remotes
Fix an issue where server options specified via the command line
(`--server-option` or `-o`) were not sent when fetching from multiple
remotes using Git protocol v2.
To reproduce the issue with a repository containing multiple remotes:
Observe that no server options are sent to any remote.
The root cause was identified in `builtin/fetch.c:fetch_multiple`, which
is invoked when fetching from more than one remote. This function forks
a `git-fetch` subprocess for each remote but did not include the
specified server options in the subprocess arguments.
This commit ensures that command-line specified server options are
properly passed to each subprocess. Relevant tests have been added.
Signed-off-by: Xing Xin <xingxin.xx@bytedance.com> Reviewed-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Xing Xin [Tue, 8 Oct 2024 03:38:17 +0000 (03:38 +0000)]
transport.c::handshake: make use of server options from remote
Utilize the `server_options` from the corresponding remote during the
handshake in `transport.c` when Git protocol v2 is detected. This helps
initialize the `server_options` in `transport.h:transport` if no server
options are set for the transport (typically via `--server-option` or
`-o`).
While another potential place to incorporate server options from the
remote is in `transport.c:transport_get`, setting server options for a
transport using a protocol other than v2 could lead to unexpected errors
(see `transport.c:die_if_server_options`).
Relevant tests and documentation have been updated accordingly.
Signed-off-by: Xing Xin <xingxin.xx@bytedance.com> Reviewed-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
