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
Usman Akinyemi [Thu, 24 Oct 2024 00:24:58 +0000 (00:24 +0000)]
imap: replace atoi() with strtol_i() for UIDVALIDITY and UIDNEXT parsing
Replace unsafe uses of atoi() with strtol_i() to improve error handling
when parsing UIDVALIDITY, UIDNEXT, and APPENDUID in IMAP commands.
Invalid values, such as those with letters, now trigger error messages and
prevent malformed status responses.
I did not add any test for this commit as we do not have any test
for git-imap-send(1) at this point.
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Usman Akinyemi [Thu, 24 Oct 2024 00:24:57 +0000 (00:24 +0000)]
merge: replace atoi() with strtol_i() for marker size validation
Replace atoi() with strtol_i() for parsing conflict-marker-size to
improve error handling. Invalid values, such as those containing letters
now trigger a clear error message.
Update the test to verify invalid input handling.
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Usman Akinyemi [Thu, 24 Oct 2024 00:24:56 +0000 (00:24 +0000)]
daemon: replace atoi() with strtoul_ui() and strtol_i()
Replace atoi() with strtoul_ui() for --timeout and --init-timeout
(non-negative integers) and with strtol_i() for --max-connections
(signed integers). This improves error handling and input validation
by detecting invalid values and providing clear error messages.
Update tests to ensure these arguments are properly validated.
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Karthik Nayak [Thu, 24 Oct 2024 10:53:57 +0000 (12:53 +0200)]
CodingGuidelines: discourage arbitrary suffixes in function names
We often name functions with arbitrary suffixes like `_1` as an
extension of another existing function. This creates confusion and
doesn't provide good clarity into the functions purpose. Let's document
good function naming etiquette in our CodingGuidelines.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
`git mv a/a.txt a b/` is a nonsense instruction. Instead of failing
gracefully the command trips over itself,[1] leaving behind unfinished
work:
1. first it moves `a/a.txt` to `b/a.txt`; then
2. tries to move `a/`, including `a/a.txt`; then
3. figures out that it’s in a bad state (assertion); and finally
4. aborts.
Now you’re left with a partially-updated index.
The command should instead fail gracefully and make no changes to the
index until it knows that it can complete a sensible action.
For now just add a failing test since this has been known about for
a while.[2]
† 1: Caused by a `pos >= 0` assertion
[2]: https://lore.kernel.org/git/d1f739fe-b28e-451f-9e01-3d2e24a0fe0d@app.fastmail.com/
Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Taylor Blau <me@ttaylorr.com>
brian m. carlson [Wed, 23 Oct 2024 00:46:00 +0000 (00:46 +0000)]
gitweb: make use of s///r
In Perl 5.14, released in May 2011, the r modifier was added to the s///
operator to allow it to return the modified string instead of modifying
the string in place. This allows to write nicer, more succinct code in
several cases, so let's do that here.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
brian m. carlson [Wed, 23 Oct 2024 00:45:59 +0000 (00:45 +0000)]
Require Perl 5.26.0
Our platform support policy states that we require "versions of
dependencies which are generally accepted as stable and supportable,
e.g., in line with the version used by other long-term-support
distributions". Of Debian, Ubuntu, RHEL, and SLES, the four most common
distributions that provide LTS versions, the version with mainstream
long-term security support with the oldest Perl is 5.26.0 in SLES 15.6.
This is a major upgrade, since Perl 5.8.1, according to the Perl
documentation, was released in September of 2003. It brings a lot of
new features that we can choose to use, such as s///r to return the
modified string, the postderef functionality, and subroutine signatures,
although the latter was still considered experimental until 5.36.
This change was made with the following one-liner, which intentionally
excludes modifying the vendored modules we include to avoid conflicts:
brian m. carlson [Wed, 23 Oct 2024 00:45:58 +0000 (00:45 +0000)]
INSTALL: document requirement for libcurl 7.61.0
Our platform support policy states that we require "versions of
dependencies which are generally accepted as stable and supportable,
e.g., in line with the version used by other long-term-support
distributions". Of Debian, Ubuntu, and RHEL, the three most common
distributions that provide LTS versions, the version with mainstream
long-term security support with the oldest libcurl is 7.61.0 in RHEL 8.
Update the documentation to state that this is the new base version for
libcurl. Remove text that is no longer applicable to older versions.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
brian m. carlson [Wed, 23 Oct 2024 00:45:57 +0000 (00:45 +0000)]
git-curl-compat: remove check for curl 7.56.0
libcurl 7.56.0 was released in September 2017, which is over seven years
ago, and no major operating system vendor is still providing security
support for it. Debian 10, which is out of mainstream security support,
has supported a newer version, and Ubuntu 20.04 and RHEL 8, which are
still in support, also have a newer version.
Remove the check for this version and use this functionality
unconditionally.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
brian m. carlson [Wed, 23 Oct 2024 00:45:56 +0000 (00:45 +0000)]
git-curl-compat: remove check for curl 7.53.0
libcurl 7.53.0 was released in February 2017, which is over seven years
ago, and no major operating system vendor is still providing security
support for it. Debian 10 and Ubuntu 18.04, both of which are out of
mainstream security support, have supported a newer version, and RHEL 8,
which is still in support, also has a newer version.
Remove the check for this version and use this functionality
unconditionally.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
brian m. carlson [Wed, 23 Oct 2024 00:45:55 +0000 (00:45 +0000)]
git-curl-compat: remove check for curl 7.52.0
libcurl 7.52.0 was released in August 2017, which is over seven years
ago, and no major operating system vendor is still providing security
support for it. Debian 9 and Ubuntu 18.04, both of which are out of
mainstream security support, have supported a newer version, and RHEL 8,
which is still in support, also has a newer version.
Remove the check for this version and use this functionality
unconditionally.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
brian m. carlson [Wed, 23 Oct 2024 00:45:54 +0000 (00:45 +0000)]
git-curl-compat: remove check for curl 7.44.0
libcurl 7.44.0 was released in August 2015, which is over nine years
ago, and no major operating system vendor is still providing security
support for it. Debian 9 and Ubuntu 16.04, both of which are out of
mainstream security support, have supported a newer version, and RHEL 8,
which is still in support, also has a newer version.
Remove the check for this version and use this functionality
unconditionally.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
brian m. carlson [Wed, 23 Oct 2024 00:45:53 +0000 (00:45 +0000)]
git-curl-compat: remove check for curl 7.43.0
libcurl 7.43.0 was released in June 2015, which is over nine years
ago, and no major operating system vendor is still providing security
support for it. Debian 9 and Ubuntu 16.04, both of which are out of
mainstream security support, have supported a newer version, and RHEL 8,
which is still in support, also has a newer version.
Remove the check for this version and use this functionality
unconditionally.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
brian m. carlson [Wed, 23 Oct 2024 00:45:52 +0000 (00:45 +0000)]
git-curl-compat: remove check for curl 7.39.0
libcurl 7.39.0 was released in November 2014, which is almost ten years
ago, and no major operating system vendor is still providing security
support for it. Debian 9 and Ubuntu 16.04, both of which are out of
mainstream security support, have supported a newer version, and RHEL 8,
which is still in support, also has a newer version.
Remove the check for this version and use this functionality
unconditionally.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
brian m. carlson [Wed, 23 Oct 2024 00:45:51 +0000 (00:45 +0000)]
git-curl-compat: remove check for curl 7.34.0
libcurl 7.34.0 was released in December 2013, which is well over ten
years ago, and no major operating system vendor is still providing
security support for it. Debian 8 and Ubuntu 14.04, both of which are
out of mainstream security support, have supported a newer version, and
RHEL 8, which is still in support, also has a newer version.
Remove the check for this version and use this functionality
unconditionally.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
brian m. carlson [Wed, 23 Oct 2024 00:45:50 +0000 (00:45 +0000)]
git-curl-compat: remove check for curl 7.25.0
libcurl 7.25.0 was released in March 2012, which is well over ten years
ago, and no major operating system vendor is still providing security
support for it. Debian 8, RHEL 7, and Ubuntu 12.10, all of which are
out of mainstream security support, have all supported a newer version.
Remove the check for this version and use this functionality
unconditionally.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
brian m. carlson [Wed, 23 Oct 2024 00:45:49 +0000 (00:45 +0000)]
git-curl-compat: remove check for curl 7.21.5
libcurl 7.21.5 was released in April 2011, which is well over ten years
ago, and no major operating system vendor is still providing security
support for it. Debian 7, RHEL 7, and Ubuntu 12.04, all of which are
out of mainstream security support, have all supported a newer version.
Remove the check for this version and use this functionality
unconditionally.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Seyi Kuforiji [Wed, 23 Oct 2024 12:11:12 +0000 (13:11 +0100)]
t9101: 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>
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
Caleb White [Tue, 22 Oct 2024 00:08:49 +0000 (00:08 +0000)]
doc: consolidate extensions in git-config documentation
The `technical/repository-version.txt` document originally served as the
master list for extensions, requiring that any new extensions be defined
there. However, the `config/extensions.txt` file was introduced later
and has since become the de facto location for describing extensions,
with several extensions listed there but missing from
`repository-version.txt`.
This consolidates all extension definitions into `config/extensions.txt`,
making it the authoritative source for extensions. The references in
`repository-version.txt` are updated to point to `config/extensions.txt`,
and cross-references to related documentation such as
`gitrepository-layout[5]` and `git-config[1]` are added.
Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Caleb White <cdwhite3@pm.me> Signed-off-by: Taylor Blau <me@ttaylorr.com>
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>
René Scharfe [Sun, 20 Oct 2024 11:02:32 +0000 (13:02 +0200)]
grep: disable lookahead on error
regexec(3) can fail. E.g. on macOS it fails if it is used with an UTF-8
locale to match a valid regex against a buffer containing invalid UTF-8
characters.
git grep has two ways to search for matches in a file: Either it splits
its contents into lines and matches them separately, or it matches the
whole content and figures out line boundaries later. The latter is done
by look_ahead() and it's quicker in the common case where most files
don't contain a match.
Fall back to line-by-line matching if look_ahead() encounters an
regexec(3) error by propagating errors out of patmatch() and bailing out
of look_ahead() if there is one. This way we at least can find matches
in lines that contain only valid characters. That matches the behavior
of grep(1) on macOS.
pcre2match() dies if pcre2_jit_match() or pcre2_match() fail, but since
we use the flag PCRE2_MATCH_INVALID_UTF it handles invalid UTF-8
characters gracefully. So implement the fall-back only for regexec(3)
and leave the PCRE2 matching unchanged.
Reported-by: David Gstir <david@sigma-star.at> Signed-off-by: René Scharfe <l.s.r@web.de> Tested-by: David Gstir <david@sigma-star.at> Signed-off-by: Taylor Blau <me@ttaylorr.com>
cmake: set up proper dependencies for generated clar headers
The auto-generated headers used by clar are written at configure time
and thus do not get regenerated automatically. Refactor the build
recipes such that we use custom commands instead, which also has the
benefit that we can reuse the same infrastructure as our Makefile.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
The compilation of clar-based unit tests is broken because we do not
add the binary directory into which we generate the "clar-decls.h" and
"clar.suite" files as include directories. Instead, we accidentally set
up the source directory as include directory.
Fix this by including the binary directory instead of the source
directory. Furthermore, set up the include directories as PUBLIC instead
of PRIVATE such that they propagate from "unit-tests.lib" to the
"unit-tests" executable, which needs to include the same directory.
Reported-by: Ed Reel <edreel@gmail.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Makefile: extract script to generate clar declarations
Extract the script to generate function declarations for the clar unit
testing framework into a standalone script. This is done such that we
can reuse it in other build systems.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Makefile: adjust sed command for generating "clar-decls.h"
This moves the end-of-line marker out of the captured group, matching
the start-of-line marker and for some reason fixing generation of
"clar-decls.h" on some older, more esoteric platforms.
Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
- 1516124 (Merge pull request #97 from pks-t/pks-whitespace-fixes, 2024-08-15).
To:
- 206accb (Merge pull request #108 from pks-t/pks-uclibc-without-wchar, 2024-10-21)
This update includes a bunch of fixes and improvements that we have
discussed in Git when initial support for clar was merged:
- There is a ".editorconfig" file now.
- Compatibility with Windows has been improved so that the clar
compiles on this platform without an issue. This has been tested
with Cygwin, MinGW and Microsoft Visual Studio.
- clar now uses CMake. This does not impact us at all as we wire up
the clar into our own build infrastructure anyway. This conversion
was done such that we can easily run CI jobs against Windows.
- Allocation failures are now checked for consistently.
- We now define feature test macros in "clar.c", which fixes
compilation on some platforms that didn't previously pull in
non-standard functions like lstat(3p) or strdup(3p). This was
reported by a user of OpenSUSE Leap.
- We stop using `struct timezone`, which is undefined behaviour
nowadays and results in a compilation error on some platforms.
- We now use the combination of mktemp(3) and mkdir(3) on SunOS, same
as we do on NonStop.
- We now support uClibc without support for <wchar.h>.
The most important bits here are the improved platform compatibility
with Windows, OpenSUSE, SunOS and uClibc.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Add a paragraph which just emphasizes that the command without any
options does not support refs in the final arguments. This is clear
already from the names `<new-oid>` and `<old-oid>` but the right balance
of redundancy makes documentation robust against stray interpretation.
This is also a good place to mention why `--stdin` has those `symref-*`
commands.
This paragraph interrupts the flow of the section by going into detail
about what a symbolic ref file is and how it is implemented. It is not
clear what the purpose is since symbolic refs were already mentioned
prior (“possibly dereferencing the symbolic refs”). Worse, it can
confuse the reader about what argument can be a symbolic ref since it
just says “it” and not which of the parameters; in turn the reader can
be lead to try `<new-oid>` and then get a confusing error since
update-ref will just say that it is not a valid SHA1.
gitglossary(7) already documents what a symref is, concretely, and quite
well at that.
Documentation/git-update-ref.txt: demote symlink to last section
Move the discussion of file system symbolic links to a new “Notes”
section (inspired by the one in git-symbolic-ref(1)) since this is
mostly of historical note at this point, not something that is needed in
the main section of the documentation.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Remove paragraphs which explain that using this command is safer than
echoing the branch name into `HEAD`.
Evoking the echo strategy is wrong now under the reftable backend since
this file does not exist. And the ref file backend majority user base
use porcelain commands to manage `HEAD` unless they are intentionally
poking at the implementation.
Maybe this warning was relevant for the usage patterns when it was
added[1] but now it just takes up space.
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>
notes: teach the -e option to edit messages in editor
Notes can be added to a commit using:
- "-m" to provide a message on the command line.
- -C to copy a note from a blob object.
- -F to read the note from a file.
When these options are used, Git does not open an editor,
it simply takes the content provided via these options and
attaches it to the commit as a note.
Improve flexibility to fine-tune the note before finalizing it
by allowing the messages to be prefilled in the editor and edited
after the messages have been provided through -[mF].
Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Karthik Nayak [Fri, 18 Oct 2024 08:46:45 +0000 (10:46 +0200)]
clang-format: re-adjust line break penalties
In 42efde4c29 (clang-format: adjust line break penalties, 2017-09-29) we
adjusted the line break penalties to really fine tune what we care about
while doing line breaks. Modify some of those to be more inline with
what we care about in the Git project now.
We need to understand that the values set to penalties in
'.clang-format' are relative to each other and do not hold any absolute
value. The penalty arguments take an 'Unsigned' value, so we have some
liberty over the values we can set.
First, in that commit, we decided, that under no circumstances do we
want to exceed 80 characters. This seems a bit too strict. We do
overshoot this limit from time to time to prioritize readability. So
let's reduce the value for 'PenaltyExcessCharacter' to 10. This means we
that we add a penalty of 10 for each character that exceeds the column
limit. By itself this is enough to restrict to column limit. Tuning
other penalties in relation to this is what is important.
The penalty `PenaltyBreakAssignment` talks about the penalty for
breaking an assignment operator on to the next line. In our project, we
are okay with this, so giving a value of 5, which is below the value for
'PenaltyExcessCharacter' ensures that in the end, even 1 character over
the column limit is not worth keeping an assignment on the same line.
Similarly set the penalty for breaking before the first call parameter
'PenaltyBreakBeforeFirstCallParameter' and the penalty for breaking
comments 'PenaltyBreakComment' and the penalty for breaking string
literals 'PenaltyBreakString' also to 5.
Finally, we really care about not breaking the return type into its own
line and we really care about not breaking before an open parenthesis.
This avoids weird formatting like:
(note: the tabs here have been replaced by spaces for easier reading)
This is done by bumping the values of 'PenaltyReturnTypeOnItsOwnLine'
and 'PenaltyBreakOpenParenthesis' to 300. This is so that we can allow a
few characters above the 80 column limit to make code more readable.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Ramsay Jones [Fri, 18 Oct 2024 05:29:52 +0000 (01:29 -0400)]
credential-cache: treat ECONNABORTED like ECONNRESET
On Cygwin, t0301 fails because "git credential-cache exit" returns a
non-zero exit code. What's supposed to happen here is:
1. The client (the "credential-cache" invocation above) connects to a
previously-spawned credential-cache--daemon.
2. The client sends an "exit" command to the daemon.
3. The daemon unlinks the socket and then exits, closing the
descriptor back to the client.
4. The client sees EOF on the descriptor and exits successfully.
That works on most platforms, and even _used_ to work on Cygwin. But
that changed in Cygwin's ef95c03522 (Cygwin: select: Fix FD_CLOSE
handling, 2021-04-06). After that commit, the client sees a read error
with errno set to ECONNABORTED, and it reports the error and dies.
It's not entirely clear if this is a Cygwin bug. It seems that calling
fclose() on the filehandles pointing to the sockets is sufficient to
avoid this error return, even though exiting should in general look the
same from the client's perspective.
However, we can't just call fclose() here. It's important in step 3
above to unlink the socket before closing the descriptor to avoid the
race mentioned by 7d5e9c9849 (credential-cache--daemon: clarify "exit"
action semantics, 2016-03-18). The client will exit as soon as it sees
the descriptor close, and the daemon may or may not have actually
unlinked the socket by then. That makes test code like this:
before we exit(). Or by replacing the exit() with a return up the stack,
in which case the fclose() happens as we unwind. But in that case we'd
still need to call delete_tempfile() here to avoid the race.
But simpler still is that we can notice that we already special-case
ECONNRESET on the client side, courtesy of 1f180e5eb9 (credential-cache:
interpret an ECONNRESET as an EOF, 2017-07-27). We can just do the same
thing here (I suspect that prior to the Cygwin commit that introduced
this problem, we were really just seeing ECONNRESET instead of
ECONNABORTED, so the "new" problem is just the switch of the errno
values).
"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>
Eric Sesterhenn [Mon, 14 Oct 2024 21:04:10 +0000 (14:04 -0700)]
fuzz: port fuzz-url-decode-mem from OSS-Fuzz
Git's fuzz tests are run continuously as part of OSS-Fuzz [1]. Several
additional fuzz tests have been contributed directly to OSS-Fuzz;
however, these tests are vulnerable to bitrot because they are not built
during Git's CI runs, and thus breaking changes are much less likely to
be noticed by Git contributors.
Port one of these tests back to the Git project:
fuzz-url-decode-mem
This test was originally written by Eric Sesterhenn as part of a
security audit of Git [2]. It was then contributed to the OSS-Fuzz repo
in commit c58ac4492 (Git fuzzing: uncomment the existing and add new
targets. (#11486), 2024-02-21) by Jaroslav Lobačevski. I (Josh Steadmon)
have verified with both Eric and Jaroslav that they're OK with moving
this test to the Git project.
Eric Sesterhenn [Mon, 14 Oct 2024 21:04:09 +0000 (14:04 -0700)]
fuzz: port fuzz-parse-attr-line from OSS-Fuzz
Git's fuzz tests are run continuously as part of OSS-Fuzz [1]. Several
additional fuzz tests have been contributed directly to OSS-Fuzz;
however, these tests are vulnerable to bitrot because they are not built
during Git's CI runs, and thus breaking changes are much less likely to
be noticed by Git contributors.
Port one of these tests back to the Git project:
fuzz-parse-attr-line
This test was originally written by Eric Sesterhenn as part of a
security audit of Git [2]. It was then contributed to the OSS-Fuzz repo
in commit c58ac4492 (Git fuzzing: uncomment the existing and add new
targets. (#11486), 2024-02-21) by Jaroslav Lobačevski. I (Josh Steadmon)
have verified with both Eric and Jaroslav that they're OK with moving
this test to the Git project.
Eric Sesterhenn [Mon, 14 Oct 2024 21:04:08 +0000 (14:04 -0700)]
fuzz: port fuzz-credential-from-url-gently from OSS-Fuzz
Git's fuzz tests are run continuously as part of OSS-Fuzz [1]. Several
additional fuzz tests have been contributed directly to OSS-Fuzz;
however, these tests are vulnerable to bitrot because they are not built
during Git's CI runs, and thus breaking changes are much less likely to
be noticed by Git contributors.
Port one of these tests back to the Git project:
fuzz-credential-from-url-gently
This test was originally written by Eric Sesterhenn as part of a
security audit of Git [2]. It was then contributed to the OSS-Fuzz repo
in commit c58ac4492 (Git fuzzing: uncomment the existing and add new
targets. (#11486), 2024-02-21) by Jaroslav Lobačevski. I (Josh Steadmon)
have verified with both Eric and Jaroslav that they're OK with moving
this test to the Git project.
The `result` parameter passed to `http_request_reauth()` may either
point to a `struct strbuf` or a `FILE *`, where the `target` parameter
tells us which of either it actually is. To accommodate for both types
the pointer is a `void *`, which we then pass directly to functions
without doing a cast.
This is fine on most platforms, but it breaks on FreeBSD because
`fileno()` is implemented as a macro that tries to directly access the
`FILE *` structure.
Fix this issue by storing the `FILE *` in a local variable before we
pass it on to other functions.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
builtin/credential-cache: fix missing parameter for stub function
When not compiling the credential cache we may use a stub function for
`cmd_credential_cache()`. With commit 9b1cb5070f (builtin: add a
repository parameter for builtin functions, 2024-09-13), we have added a
new parameter to all of those top-level `cmd_*()` functions, and did
indeed adapt the non-stubbed-out `cmd_credential_cache()`. But we didn't
adapt the stubbed-out variant, so the code does not compile.
Fix this by adding the missing parameter.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.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: Taylor Blau <me@ttaylorr.com>
t5500, t5601: skip tests which exercise paths with '[::1]' on Cygwin
Parsing repositories which contain '[::1]' is broken on Cygwin. It seems
as if Cygwin is confusing those as drive letter prefixes or something
like this, but I couldn't deduce the actual root cause.
Mark those tests as broken for now.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
t3404: work around platform-specific behaviour on macOS 10.15
Two of our tests in t3404 use indented HERE docs where leading tabs on
some of the lines are actually relevant. The tabs do get removed though,
and we try to fix this up by using sed(1) to replace leading tabs in the
actual output, as well. But macOS 10.15 uses an oldish version of sed(1)
that has BSD lineage, which does not understand "\t", and thus we fail
to strip those leading tabs and fail the test.
Address this issue by using `q_to_tab` such that we do not have to strip
leading tabs from the actual output.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
t1401: make invocation of tar(1) work with Win32-provided one
Windows nowadays provides a tar(1) binary in "C:\Windows\system32". This
version of tar(1) doesn't seem to handle the case where directory paths
end with a trailing forward slash. And as we do that in t1401 the result
is that the test fails.
Drop the trailing slash. Other tests that use tar(1) work alright, this
is the only instance where it has been failing.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
In "t/lib-gpg.sh" we set up the "GNUPGHOME" environment variable to
point to a test-specific directory. This is done by using "$PWD/gpghome"
as value, where "$PWD" is the current test's trash directory.
This is broken for MinGW though because "$PWD" will use Windows-style
paths that contain drive letters. What we really want in this context is
a Unix-style path, which we can get by using `$(pwd)` instead. It is
somewhat puzzling that nobody ever hit this issue, but it may easily be
that nobody ever tests on Windows with GnuPG installed, which would make
us skip those tests.
Adapt the code accordingly to fix tests using this library.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
t/lib-gitweb: test against the build version of gitweb
When testing gitweb we set up the CGI script as "gitweb.perl", which is
the source file of the build target "gitweb.cgi". This file doesn't have
a patched shebang and still contains `++REPLACEMENT++` markers, but
things generally work because we replace the configuration with our own
test configuration.
But this only works as long as "$GIT_BUILD_DIR" actually points to the
source tree, because "gitweb.cgi" and "gitweb.perl" happen to sit next
to each other. This is not the case though once you have out-of-tree
builds like with CMake, where the source and built versions live in
different directories. Consequently, "$GIT_BUILD_DIR/gitweb/gitweb.perl"
won't exist there.
While we could ask build systems with out-of-tree builds to instead set
up GITWEB_TEST_INSTALLED, which allows us to override the location of
the script, it goes against the spirit of this environment variable. We
_don't_ want to test against an installed version, we want to use the
version we have just built.
Fix this by using "gitweb.cgi" instead. This means that you cannot run
test scripts without building that file, but in general we do expect
developers to build stuff before they test it anyway.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
The iconv library is used by Git to reencode files, commit messages and
other things. As such it is a rather integral part, but given that many
platforms nowadays use UTF-8 everywhere you can live without support for
reencoding in many situations. It is thus optional to build Git with
iconv, and some of our platforms wired up in "config.mak.uname" disable
it. But while we support building without it, running our test suite
with "NO_ICONV=Yes" causes many test failures.
Wire up a new test prerequisite ICONV that gets populated via our
GIT-BUILD-OPTIONS. Annotate failing tests accordingly.
Note that this commit does not do a deep dive into every single test to
assess whether the failure is expected or not. Most of the tests do
smell like the expected kind of failure though.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
When assembling our LSAN_OPTIONS that configure the leak sanitizer we
end up prepending the string with various different colon-separated
options via calls to `prepend_var`. One of the settings we add is the
path where the sanitizer should store logs, which can be an arbitrary
filesystem path.
Naturally, filesystem paths may contain whitespace characters. And while
it does seem as if we were quoting the value, we use escaped quotes and
consequently split up the value if it does contain spaces. This leads to
the following error in t0000 when having a value with whitespaces:
.../t/test-lib.sh: eval: line 64: unexpected EOF while looking for matching `"'
++ return 1
error: last command exited with $?=1
not ok 5 - subtest: 3 passing tests
The error itself is a bit puzzling at first. The basic problem is that
the code sees the leading escaped quote during eval, but because we
truncate everything after the space character it doesn't see the
trailing escaped quote and thus fails to parse the string.
Properly quote the value to fix the issue while using single-quotes to
quote the inner value passed to eval. The issue can be reproduced by
t0000 with such a path that contains spaces.
Signed-off-by: Patrick Steinhardt <ps@pks.im> 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
Linus Arver [Sun, 13 Oct 2024 11:58:42 +0000 (11:58 +0000)]
trailer: spread usage of "trailer_block" language
Deprecate the "trailer_info" struct name and replace it with
"trailer_block". This is more readable, for two reasons:
1. "trailer_info" on the surface sounds like it's about a single
trailer when in reality it is a collection of one or more trailers,
and
2. the "*_block" suffix is more informative than "*_info", because it
describes a block (or region) of contiguous text which has trailers
in it, which has been parsed into the trailer_block structure.
Rename the
size_t trailer_block_start, trailer_block_end;
members of trailer_info to just "start" and "end". Rename the "info"
pointer to "trailer_block" because it is more descriptive. Update
comments accordingly.
Signed-off-by: Linus Arver <linus@ucla.edu> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Usman Akinyemi [Sat, 12 Oct 2024 23:09:33 +0000 (23:09 +0000)]
t3404: replace test with test_line_count()
Refactor t3404 to replace instances of `test` with `test_line_count()`
for checking line counts. This improves readability and aligns with Git's
current test practices.
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Usman Akinyemi [Sat, 12 Oct 2024 23:09:32 +0000 (23:09 +0000)]
t3404: avoid losing exit status with focus on `git show` and `git cat-file`
The exit code of the preceding command in a pipe is disregarded. So
if that preceding command is a Git command that fails, the test would
not fail. Instead, by saving the output of that Git command to a file,
and removing the pipe, we make sure the test will fail if that Git
command fails. This particular patch focuses on all `git show` and
some instances of `git cat-file`.
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
projects / thirdparty / git.git / log
