Commit Graph

163452 Commits

Author SHA1 Message Date
Philip Oakley
75dc17dd67 CMake: show Win32 and Generator_platform build-option values
Ensure key CMake option values are part of the CMake output to
facilitate user support when tool updates impact the wider CMake
actions, particularly ongoing 'improvements' in Visual Studio.

These CMake displays perform the same function as the build-options.txt
provided in the main Git for Windows. CMake is already chatty.
The setting of CMAKE_EXPORT_COMPILE_COMMANDS is also reported.

Include the environment's CMAKE_EXPORT_COMPILE_COMMANDS value which
may have been propogated to CMake's internal value.

Testing the CMAKE_EXPORT_COMPILE_COMMANDS processing can be difficult
in the Visual Studio environment, as it may be cached in many places.
The 'environment' may include the OS, the user shell, CMake's
own environment, along with the Visual Studio presets and caches.

See previous commit for arefacts that need removing for a clean test.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
2025-02-06 19:32:53 +01:00
Philip Oakley
cded29a530 CMakeLists: add default "x64-windows" arch for Visual Studio
In Git-for-Windows, work on using ARM64 has progressed. The
commit 2d94b77b27 (cmake: allow building for Windows/ARM64, 2020-12-04)
failed to notice that /compat/vcbuild/vcpkg_install.bat will default to
using the "x64-windows" architecture for the vcpkg installation if not set,
but CMake is not told of this default. Commit 635b6d99b3 (vcbuild: install
ARM64 dependencies when building ARM64 binaries, 2020-01-31) later updated
vcpkg_install.bat to accept an arch (%1) parameter, but retained the default.

This default is neccessary for the use case where the project directory is
opened directly in Visual Studio, which will find and build a CMakeLists.txt
file without any parameters, thus expecting use of the default setting.

Also Visual studio will generate internal .sln solution and .vcxproj project
files needed for some extension tools. Inform users of the additional
.sln/.vcxproj generation.

** How to test:
 rm -rf '.vs' # remove old visual studio settings
 rm -rf 'compat/vcbuild/vcpkg' # remove any vcpkg downloads
 rm -rf 'contrib/buildsystems/out' # remove builds & CMake artifacts
 with a fresh Visual Studio Community Edition, File>>Open>>(git *folder*)
   to load the project (which will take some time!).
 check for successful compilation.
The implicit .sln (etc.) are in the hidden .vs directory created by
Visual Studio.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
2025-02-06 19:32:53 +01:00
Philip Oakley
ee51d5ff43 .gitignore: add Visual Studio CMakeSetting.json file
The CMakeSettings.json file is tool generated. Developers may track it
should they provide additional settings.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
2025-02-06 19:32:53 +01:00
Philip Oakley
af3af556ad CMake: default Visual Studio generator has changed
Correct some wording and inform users regarding the Visual Studio
changes (from V16.6) to the default generator.

Subsequent commits ensure that Git for Windows can be directly
opened in modern Visual Studio without needing special configuration
of the CMakeLists settings.

It appeares that internally Visual Studio creates it's own version of the
.sln file (etc.) for extension tools that expect them.

The large number of references below document the shifting of Visual Studio
default and CMake setting options.

refs: https://docs.microsoft.com/en-us/search/?scope=C%2B%2B&view=msvc-150&terms=Ninja

1. https://docs.microsoft.com/en-us/cpp/linux/cmake-linux-configure?view=msvc-160
(note the linux bit)
 "In Visual Studio 2019 version 16.6 or later ***, Ninja is the default
generator for configurations targeting a remote system or WSL. For more
information, see this post on the C++ Team Blog
[https://devblogs.microsoft.com/cppblog/linux-development-with-visual-studio-first-class-support-for-gdbserver-improved-build-times-with-ninja-and-updates-to-the-connection-manager/].

For more information about these settings, see CMakeSettings.json reference
[https://docs.microsoft.com/en-us/cpp/build/cmakesettings-reference?view=msvc-160]."

2. https://docs.microsoft.com/en-us/cpp/build/cmake-presets-vs?view=msvc-160
"CMake supports two files that allow users to specify common configure,
build, and test options and share them with others: CMakePresets.json
and CMakeUserPresets.json."

" Both files are supported in Visual Studio 2019 version 16.10 or later.
***"
3. https://devblogs.microsoft.com/cppblog/linux-development-with-visual-studio-first-class-support-for-gdbserver-improved-build-times-with-ninja-and-updates-to-the-connection-manager/
" Ninja has been the default generator (underlying build system) for
CMake configurations targeting Windows for some time***, but in Visual
Studio 2019 version 16.6 Preview 3*** we added support for Ninja on Linux."

4. https://docs.microsoft.com/en-us/cpp/build/cmakesettings-reference?view=msvc-160
" `generator`: specifies CMake generator to use for this configuration.
May be one of:

    Visual Studio 2019 only:
        Visual Studio 16 2019
        Visual Studio 16 2019 Win64
        Visual Studio 16 2019 ARM

    Visual Studio 2017 and later:
        Visual Studio 15 2017
        Visual Studio 15 2017 Win64
        Visual Studio 15 2017 ARM
        Visual Studio 14 2015
        Visual Studio 14 2015 Win64
        Visual Studio 14 2015 ARM
        Unix Makefiles
        Ninja

Because Ninja is designed for fast build speeds instead of flexibility
and function, it is set as the default. However, some CMake projects may
be unable to correctly build using Ninja. If this occurs, you can
instruct CMake to generate Visual Studio projects instead.

To specify a Visual Studio generator in Visual Studio 2017, open the
settings editor from the main menu by choosing CMake | Change CMake
Settings. Delete "Ninja" and type "V". This activates IntelliSense,
which enables you to choose the generator you want."

"To specify a Visual Studio generator in Visual Studio 2019, right-click
on the CMakeLists.txt file in Solution Explorer and choose CMake
Settings for project > Show Advanced Settings > CMake Generator.

When the active configuration specifies a Visual Studio generator, by
default MSBuild.exe is invoked with` -m -v:minimal` arguments."

5. https://docs.microsoft.com/en-us/cpp/build/cmake-presets-vs?view=msvc-160#enable-cmakepresetsjson-integration-in-visual-studio-2019
"Enable CMakePresets.json integration in Visual Studio 2019

CMakePresets.json integration isn't enabled by default in Visual Studio
2019. You can enable it for all CMake projects in Tools > Options >
CMake > General: (tick a box)" ... see more.

6. https://docs.microsoft.com/en-us/cpp/build/cmakesettings-reference?view=msvc-140
(whichever v140 is..)
"CMake projects are supported in Visual Studio 2017 and later."

7. https://docs.microsoft.com/en-us/cpp/overview/what-s-new-for-cpp-2017?view=msvc-150
"Support added for the CMake Ninja generator."

8. https://docs.microsoft.com/en-us/cpp/overview/what-s-new-for-cpp-2017?view=msvc-150#cmake-support-via-open-folder
"CMake support via Open Folder
Visual Studio 2017 introduces support for using CMake projects without
converting to MSBuild project files (.vcxproj). For more information,
see CMake projects in Visual
Studio[https://docs.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=msvc-150].
Opening CMake projects with Open Folder automatically configures the
environment for C++ editing, building, and debugging." ... +more!

9. https://docs.microsoft.com/en-us/cpp/build/cmake-presets-vs?view=msvc-160#supported-cmake-and-cmakepresetsjson-versions
"Visual Studio reads and evaluates CMakePresets.json and
CMakeUserPresets.json itself and doesn't invoke CMake directly with the
--preset option. So, CMake version 3.20 or later isn't strictly required
when you're building with CMakePresets.json inside Visual Studio. We
recommend using CMake version 3.14 or later."

10. https://docs.microsoft.com/en-us/cpp/build/cmake-presets-vs?view=msvc-160#enable-cmakepresetsjson-integration-in-visual-studio-2019
"If you don't want to enable CMakePresets.json integration for all CMake
projects, you can enable CMakePresets.json integration for a single
CMake project by adding a CMakePresets.json file to the root of the open
folder. You must close and reopen the folder in Visual Studio to
activate the integration.

11. https://docs.microsoft.com/en-us/cpp/build/cmake-presets-vs?view=msvc-160#default-configure-presets
***(doesn't actually say which version..)
"Default Configure Presets
If no CMakePresets.json or CMakeUserPresets.json file exists, or if
CMakePresets.json or CMakeUserPresets.json is invalid, Visual Studio
will fall back*** on the following default Configure Presets:

Windows example
JSON
{
  "name": "windows-default",
  "displayName": "Windows x64 Debug",
  "description": "Sets Ninja generator, compilers, x64 architecture,
build and install directory, debug build type",
  "generator": "Ninja",
  "binaryDir": "${sourceDir}/out/build/${presetName}",
  "architecture": {
    "value": "x64",
    "strategy": "external"
  },
  "cacheVariables": {
    "CMAKE_BUILD_TYPE": "Debug",
    "CMAKE_INSTALL_PREFIX": "${sourceDir}/out/install/${presetName}"
  },
  "vendor": {
    "microsoft.com/VisualStudioSettings/CMake/1.0": {
      "hostOS": [ "Windows" ]
    }
  }
},
"

Signed-off-by: Philip Oakley <philipoakley@iee.email>
2025-02-06 19:32:53 +01:00
Dennis Ameling
ccaf1757be cmake(): allow setting HOST_CPU for cross-compilation
Git's regular Makefile mentions that HOST_CPU should be defined when cross-compiling Git: 37796bca76/Makefile (L438-L439)

This is then used to set the GIT_HOST_CPU variable when compiling Git: 37796bca76/Makefile (L1337-L1341)

Then, when the user runs `git version --build-options`, it returns that value: 37796bca76/help.c (L658)

This commit adds the same functionality to the CMake configuration. Users can now set -DHOST_CPU= to set the target architecture.

Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
2025-02-06 19:32:52 +01:00
Dennis Ameling
d95a8eab11 Add schannel to curl installation
Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
2025-02-06 19:32:52 +01:00
Dennis Ameling
1f64d1f31b ci(vs-build) also build Windows/ARM64 artifacts
There are no Windows/ARM64 agents in GitHub Actions yet, therefore we
just skip adjusting the `vs-test` job for now.

Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:52 +01:00
Dennis Ameling
2dfcd30bed cmake: allow building for Windows/ARM64
Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:51 +01:00
Ian Bearman
3a442eba39 vcbuild: add an option to install individual 'features'
In this context, a "feature" is a dependency combined with its own
dependencies.

Signed-off-by: Ian Bearman <ianb@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:51 +01:00
Ian Bearman
460c3f432b vcbuild: install ARM64 dependencies when building ARM64 binaries
Co-authored-by: Dennis Ameling <dennis@dennisameling.com>
Signed-off-by: Ian Bearman <ianb@microsoft.com>
Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:51 +01:00
Johannes Schindelin
d1380747eb Merge branch 'ci-fixes'
Backport a couple fixes to make the CI build run again (so much for
reproducible builds...).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:51 +01:00
Philip Oakley
8809b24783 vcpkg_install: add comment regarding slow network connections
The vcpkg downloads may not succeed. Warn careful readers of the time out.

A simple retry will usually resolve the issue.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:51 +01:00
Philip Oakley
62137780ac vcpkg_install: detect lack of Git
The vcpkg_install batch file depends on the availability of a
working Git on the CMD path. This may not be present if the user
has selected the 'bash only' option during Git-for-Windows install.

Detect and tell the user about their lack of a working Git in the CMD
window.

Fixes #2348.
A separate PR https://github.com/git-for-windows/build-extra/pull/258
now highlights the recommended path setting during install.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
2025-02-06 19:32:51 +01:00
Johannes Schindelin
f9b4cc28c5 ci: adjust Azure Pipeline for runs_on_pool
These refactorings are really gifts that keep on giving.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:43 +01:00
Johannes Schindelin
c6ca5c1064 Merge branch 'fixes-from-the-git-mailing-list'
These fixes have been sent to the Git mailing list but have not been
picked up by the Git project yet.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:43 +01:00
Johannes Schindelin
e74c9487b1 config.mak.uname: drop the vcxproj target
Now that we dropped `contrib/buildsystems/generate` to generate Visual
Studio Solution files, it is time to also drop the `vcxproj` Makefile
target that depended on that script.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:43 +01:00
Junio C Hamano
278028ef96 Merge branch 'mh/credential-cache-authtype-request-fix'
The "cache" credential back-end did not handle authtype correctly,
which has been corrected.

* mh/credential-cache-authtype-request-fix:
  credential-cache: respect authtype capability

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:43 +01:00
Johannes Schindelin
a8339b0f2e contrib/buildsystems: drop support for building .vcproj/.vcxproj files
Before we had CMake support, the only way to build Git in Visual Studio
was via this hacky `generate` script.

For a while I tried to fix whenever things got broken, in particular to
allow building confidence in embargoed releases by running the CI builds
in Azure Pipelines in a private Azure DevOps project. I even carried the
patches in Git for Windows with the intention of upstreaming them,
eventually.

However, it is a lot of work with too little benefit. CMake is much
better supported by Visual Studio. So let's drop this hacky script (plus
support code).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:43 +01:00
Johannes Schindelin
df6fb8e128 ci: stop linking the prove cache
It is not useful because we do not have any persisted directory anymore,
not since dropping our Travis CI support.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:43 +01:00
Junio C Hamano
a225dbd554 Merge branch 'jc/show-index-h-update'
Doc and short-help text for "show-index" has been clarified to
stress that the command reads its data from the standard input.

* jc/show-index-h-update:
  show-index: the short help should say the command reads from its input

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:42 +01:00
Junio C Hamano
803d56f5d4 Merge branch 'bf/fetch-set-head-fix' into jch
Fetching into a bare repository incorrectly assumed it always used
a mirror layout when deciding to update remote-tracking HEAD, which
has been corrected.

* bf/fetch-set-head-fix:
  fetch set_head: fix non-mirror remotes in bare repositories

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:42 +01:00
Junio C Hamano
e97324a463 Merge branch 'rs/ref-filter-used-atoms-value-fix'
"git branch --sort=..." and "git for-each-ref --format=... --sort=..."
did not work as expected with some atoms, which has been corrected.

* rs/ref-fitler-used-atoms-value-fix:
  ref-filter: remove ref_format_clear()
  ref-filter: move is-base tip to used_atom
  ref-filter: move ahead-behind bases into used_atom

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:42 +01:00
Junio C Hamano
2dace82faa Merge branch 'kn/reflog-migration-fix-followup'
Code clean-up.

* kn/reflog-migration-fix-followup:
  reftable: prevent 'update_index' changes after adding records
  refs: use 'uint64_t' for 'ref_update.index'
  refs: mark `ref_transaction_update_reflog()` as static

These patches have been actually rebased onto a better base (the
`kn/reflog-migration` tip instead of the merge commit that merged this
tip).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:41 +01:00
Junio C Hamano
9607fee092 Merge branch 'kn/reflog-migration-fix'
"git refs migrate" for migrating reflog data was broken.

* kn/reflog-migration-fix:
  reftable: write correct max_update_index to header

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:41 +01:00
Junio C Hamano
02e4e7b521 Merge branch 'en/object-name-with-funny-refname-fix'
Extended SHA-1 expression parser did not work well when a branch
with an unusual name (e.g. "foo{bar") is involved.

* en/object-name-with-funny-refname-fix:
  object-name: be more strict in parsing describe-like output
  object-name: fix resolution of object names containing curly braces

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:41 +01:00
Junio C Hamano
24f5d72492 Merge branch 'jk/pack-header-parse-alignment-fix'
It was possible for "git unpack-objects" and "git index-pack" to
make an unaligned access, which has been corrected.

* jk/pack-header-parse-alignment-fix:
  index-pack, unpack-objects: use skip_prefix to avoid magic number
  index-pack, unpack-objects: use get_be32() for reading pack header
  parse_pack_header_option(): avoid unaligned memory writes
  packfile: factor out --pack_header argument parsing
  bswap.h: squelch potential sparse -Wcast-truncate warnings

These patches have actually been rebased onto v2.46.2 for easier
merging.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:41 +01:00
Jeff King
51354d1af5 update-ref: do set reflog's old_oid
In git 2.48.1, the `git update-ref` subcommand no longer correctly
updates the reflog in some cases. Specifically, it appears that the
`old_oid` field will not be updated when modifying a branch referenced
by another symbolic ref (e.g. HEAD). This doesn't break the `git
reflog` subcommand, but does break references like `HEAD@{1}`, which
appear to read the `old_oid` field:

  git init -b main
  git commit --allow-empty -m "A"
  git commit --allow-empty -m "B"
  git update-ref -m "reason" refs/heads/main HEAD~ HEAD

The `old_oid` field is now empty (all zeroes). This is only the case in
derived reflogs (in this case .git/logs/HEAD). The reflog for
`refs/heads/main` appears to be updated correctly.

This was broken in 297c09eabb (refs: allow multiple reflog entries for
the same refname, 2024-12-16).

The reason for that was that there was assumed the flow of
`lock_ref_for_update()` for reflog only updates was to capture the lock
only. But this is wrong since this misses the `old_oid` population. As
such this patch is the correct fix.

Reported-by: Nika Layzell <nika@thelayzells.com>
Acked-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:40 +01:00
Junio C Hamano
9b7b978476 Merge branch 'ps/object-collision-check'
CI jobs gave sporadic failures, which turns out that that the
object finalization code was giving an error when it did not have
to.

* ps/object-collision-check:
  object-file: retry linking file into place when occluding file vanishes
  object-file: don't special-case missing source file in collision check
  object-file: rename variables in `check_collision()`
  object-file: fix race in object collision check

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:40 +01:00
Jeff King
a2e7060a1f grep: prevent ^$ false match at end of file
In some implementations, `regexec_buf()` assumes that it is fed lines;
Without `REG_NOTEOL` it thinks the end of the buffer is the end of a
line. Which makes sense, but trips up this case because we are not
feeding lines, but rather a whole buffer. So the final newline is not
the start of an empty line, but the true end of the buffer.

This causes an interesting bug:

  $ echo content >file.txt
  $ git grep --no-index -n '^$' file.txt
  file.txt:2:

This bug is fixed by making the end of the buffer consistently the end
of the final line.

The patch was applied from
https://lore.kernel.org/git/20250113062601.GD767856@coredump.intra.peff.net/

Reported-by: Olly Betts <olly@survex.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:40 +01:00
Junio C Hamano
cbb262adf2 Merge branch 'jk/lsan-race-ignore-false-positive'
The code to check LSan results has been simplified and made more
robust.

* jk/lsan-race-ignore-false-positive:
  test-lib: add a few comments to LSan log checking
  test-lib: simplify lsan results check
  test-lib: invert return value of check_test_results_san_file_empty

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:40 +01:00
Adam Murray
299435c3e5 trace2: prevent segfault on config collection where no value specified
When TRACE2 analytics is enabled, a git config option that has no value
causes a segfault.

Steps to Reproduce
GIT_TRACE2=true GIT_TRACE2_CONFIG_PARAMS=status.*
git -c status.relativePaths version
Expected Result
git version 2.46.0
Actual Result
zsh: segmentation fault GIT_TRACE2=true

This adds checks to prevent the segfault and instead return
an empty value.

Signed-off-by: Adam Murray <ad@canva.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:40 +01:00
M Hickford
f40199fe18 credential-cache: respect authtype capability
Previously, credential-cache populated authtype regardless whether
"get" request had authtype capability. As documented in
git-credential.txt, authtype "should not be sent unless the appropriate
capability ... is provided".

Add test. Without this change, the test failed because "credential fill"
printed an incomplete credential with only protocol and host attributes
(the unexpected authtype attribute was discarded by credential.c).

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:39 +01:00
Junio C Hamano
3e20693c1d show-index: the short help should say the command reads from its input
The short help text given by "git show-index -h" says

    $ git show-index -h
    usage: git show-index [--object-format=<hash-algorithm>]

        --[no-]object-format <hash-algorithm>
                              specify the hash algorithm to use

The command takes a pack .idx file from its standard input.  The
user has to _know_ this, as there is no indication from this output.

Give a hint that the data to work on is fed from its standard input.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:39 +01:00
Bence Ferdinandy
c4c6f21a2e fetch set_head: fix non-mirror remotes in bare repositories
In b1b713f722 (fetch set_head: handle mirrored bare repositories,
2024-11-22) it was implicitly assumed that all remotes will be mirrors
in a bare repository, thus fetching a non-mirrored remote could lead to
HEAD pointing to a non-existent reference. Make sure we only overwrite
HEAD if we are in a bare repository and fetching from a mirror.
Otherwise, proceed as normally, and create
refs/remotes/<nonmirrorremote>/HEAD instead.

Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com>
Reported-by: Christian Hesse <list@eworm.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:39 +01:00
René Scharfe
135a432f4c ref-filter: remove ref_format_clear()
Now that ref_format_clear() no longer releases any memory we don't need
it anymore.  Remove it and its counterpart, ref_format_init().

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:39 +01:00
René Scharfe
2fbabb941b ref-filter: move is-base tip to used_atom
The string_list "is_base_tips" in struct ref_format stores the
committish part of "is-base:<committish>".  It has the same problems
that its sibling string_list "bases" had.  Fix them the same way as the
previous commit did for the latter, by replacing the string_list with
fields in "used_atom".

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:39 +01:00
René Scharfe
8849ac4117 ref-filter: move ahead-behind bases into used_atom
verify_ref_format() parses a ref-filter format string and stores
recognized items in the static array "used_atom".  For
"ahead-behind:<committish>" it stores the committish part in a
string_list member "bases" of struct ref_format.

ref_sorting_options() also parses bare ref-filter format items and
stores stores recognized ones in "used_atom" as well.  The committish
parts go to a dummy struct ref_format in parse_sorting_atom(), though,
and are leaked and forgotten.

If verify_ref_format() is called before ref_sorting_options(), like in
git for-each-ref, then all works well if the sort key is included in the
format string.  If it isn't then sorting cannot work as the committishes
are missing.

If ref_sorting_options() is called first, like in git branch, then we
have the additional issue that if the sort key is included in the format
string then filter_ahead_behind() can't see its committish, will not
generate any results for it and thus it will be expanded to an empty
string.

Fix those issues by replacing the string_list with a field in used_atom
for storing the committish.  This way it can be shared for handling both
ref-filter format strings and sorting options in the same command.

Reported-by: Ross Goldberg <ross.goldberg@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:39 +01:00
Karthik Nayak
2b0861883e reftable: prevent 'update_index' changes after adding records
The function `reftable_writer_set_limits()` allows updating the
'min_update_index' and 'max_update_index' of a reftable writer. These
values are written to both the writer's header and footer.

Since the header is written during the first block write, any subsequent
changes to the update index would create a mismatch between the header
and footer values. The footer would contain the newer values while the
header retained the original ones.

To fix this bug, prevent callers from updating these values after any
record is written. To do this, modify the function to return an error
whenever the limits are modified after any record adds. Check for record
adds within `reftable_writer_set_limits()` by checking the `last_key`
variable, which is set whenever a new record is added.

Modify all callers of the function to anticipate a return type and
handle it accordingly.

Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:38 +01:00
Karthik Nayak
8ba42f1987 reftable: write correct max_update_index to header
In 297c09eabb (refs: allow multiple reflog entries for the same refname,
2024-12-16), the reftable backend learned to handle multiple reflog
entries within the same transaction. This was done modifying the
`update_index` for reflogs with multiple indices. During writing the
logs, the `max_update_index` of the writer was modified to ensure the
limits were raised to the modified `update_index`s.

However, since ref entries are written before the modification to the
`max_update_index`, if there are multiple blocks to be written, the
reftable backend writes the header with the old `max_update_index`. When
all logs are finally written, the footer will be written with the new
`min_update_index`. This causes a mismatch between the header and the
footer and causes the reftable file to be corrupted. The existing tests
only spawn a single block and since headers are lazily written with the
first block, the tests didn't capture this bug.

To fix the issue, the appropriate `max_update_index` limit must be set
even before the first block is written. Add a `max_index` field to the
transaction which holds the `max_index` within all its updates, then
propagate this value to the reftable backend, wherein this is used to
the set the `max_update_index` correctly.

Add a test which creates a few thousand reference updates with multiple
reflog entries, which should trigger the bug.

Reported-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:38 +01:00
Elijah Newren
5aeb1e471a object-name: be more strict in parsing describe-like output
From Documentation/revisions.txt:
    '<describeOutput>', e.g. 'v1.7.4.2-679-g3bee7fb'::
      Output from `git describe`; i.e. a closest tag, optionally
      followed by a dash and a number of commits, followed by a dash, a
      'g', and an abbreviated object name.
which means that output of the format
    ${REFNAME}-${INTEGER}-g${HASH}
should parse to fully expanded ${HASH}.  This is fine.  However, we
currently don't validate any of ${REFNAME}-${INTEGER}, we only parse
-g${HASH} and assume the rest is valid.  That is problematic, since it
breaks things like

    git cat-file -p branchname:path/to/file/named/i-gaffed

which, when commit (or tree or blob) affed exists, will not return us
information about the file we are looking for but will instead
erroneously tell us about object affed.

A few additional notes:
  - This is a slight backward incompatibility break, because we used
    to allow ${GARBAGE}-g${HASH} as a way to spell ${HASH}.  However,
    a backward incompatible break is necessary, because there is no
    other way for someone to be more specific and disambiguate that they
    want the blob master:path/to/who-gabbed instead of the object abbed.
  - There is a possibility that check_refname_format() rules change in
    the future.  However, we can only realistically loosen the rules
    for what that function accepts rather than tighten.  If we were to
    tighten the rules, some real world repositories may already have
    refnames that suddenly become unacceptable and we break those
    repositories.  As such, any describe-like syntax of the form
    ${VALID_FOR_A_REFNAME}-${INTEGER}-g${HASH} that is valid with the
    changes in this commit will remain valid in the future.
  - The fact that check_refname_format() rules could loosen in the
    future is probably also an important reason to make this change.  If
    the rules loosen, there might be additional cases within
    ${GARBAGE}-g${HASH} that become ambiguous in the future.  While
    abbreviated hashes can be disambiguated by abbreviating less, it may
    well be that these alternative object names have no way of being
    disambiguated (much like pathnames cannot be).  Accepting all random
    ${GARBAGE} thus makes it difficult for us to allow future
    extensions to object naming.

So, tighten up the parsing to make sure ${REFNAME} and ${INTEGER} are
present in the string, and would be considered a valid ref and
non-negative integer.

Also, add a few tests for git describe using object names of the form
    ${REVISION_NAME}${MODIFIERS}
since an early version of this patch failed on constructs like
    git describe v2.48.0-rc2-161-g6c2274cdbc^0

Reported-by: Gabriel Amaral <gabriel-amaral@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:38 +01:00
Karthik Nayak
80d74ed7c2 refs: use 'uint64_t' for 'ref_update.index'
The 'ref_update.index' variable is used to store an index for a given
reference update. This index is used to order the updates in a
predetermined order, while the default ordering is alphabetical as per
the refname.

For large repositories with millions of references, it should be safer
to use 'uint64_t'. Let's do that. This also is applied for all other
code sections where we store 'index' and pass it around.

Reported-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:38 +01:00
Elijah Newren
5a88867729 object-name: fix resolution of object names containing curly braces
Given a branch name of 'foo{bar', commands like

    git cat-file -p foo{bar:README.md

should succeed (assuming that branch had a README.md file, of course).
However, the change in cce91a2cae (Change 'master@noon' syntax to
'master@{noon}'., 2006-05-19) presumed that curly braces would always
come after an '@' or '^' and be paired, causing e.g. 'foo{bar:README.md'
to entirely miss the ':' and assume there's no object being referenced.
In short, git would report:

    fatal: Not a valid object name foo{bar:README.md

Change the parsing to only make the assumption of paired curly braces
immediately after either a '@' or '^' character appears.

Add tests for this, as well as for a few other test cases that initial
versions of this patch broke:
  * 'foo@@{...}'
  * 'foo^{/${SEARCH_TEXT_WITH_COLON}}:${PATH}'

Note that we'd prefer not duplicating the special logic for "@^" characters
here, because if get_oid_basic() or interpret_nth_prior_checkout() or
get_oid_basic() or similar gain extra methods of using curly braces,
then the logic in get_oid_with_context_1() would need to be updated as
well.  But it's not clear how to refactor all of these to have a simple
common callpoint with the specialized logic.

Reported-by: Gabriel Amaral <gabriel-amaral@github.com>
Helped-by: Michael Haggerty <mhagger@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:38 +01:00
Karthik Nayak
62a5707f7c refs: mark ref_transaction_update_reflog() as static
The `ref_transaction_update_reflog()` function is only used within
'refs.c', so mark it as static.

Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:38 +01:00
Jeff King
dae2c5e5c7 index-pack, unpack-objects: use skip_prefix to avoid magic number
When parsing --pack_header=, we manually skip 14 bytes to the data.
Let's use skip_prefix() to do this automatically.

Note that we overwrite our pointer to the front of the string, so we
have to add more context to the error message. We could avoid this by
declaring an extra pointer to hold the value, but I think the modified
message is actually preferable; it should give translators a bit more
context.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:37 +01:00
Patrick Steinhardt
a6aca325db object-file: retry linking file into place when occluding file vanishes
Prior to 0ad3d65652 (object-file: fix race in object collision check,
2024-12-30), callers could expect that a successful return from
`finalize_object_file()` means that either the file was moved into
place, or the identical bytes were already present. If neither of those
happens, we'd return an error.

Since that commit, if the destination file disappears between our
link(3p) call and the collision check, we'd return success without
actually checking the contents, and without retrying the link. This
solves the common case that the files were indeed the same, but it means
that we may corrupt the repository if they weren't (this implies a hash
collision, but the whole point of this function is protecting against
hash collisions).

We can't be pessimistic and assume they're different; that hurts the
common case that the mentioned commit was trying to fix. But after
seeing that the destination file went away, we can retry linking again.
Adapt the code to do so when we see that the destination file has racily
vanished. This should generally succeed as we have just observed that
the destination file does not exist anymore, except in the very unlikely
event that it gets recreated by another concurrent process again.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:37 +01:00
Jeff King
16be54a929 index-pack, unpack-objects: use get_be32() for reading pack header
Both of these commands read the incoming pack into a static unsigned
char buffer in BSS, and then parse it by casting the start of the buffer
to a struct pack_header. This can result in SIGBUS on some platforms if
the compiler doesn't place the buffer in a position that is properly
aligned for 4-byte integers.

This reportedly happens with unpack-objects (but not index-pack) on
sparc64 when compiled with clang (but not gcc). But we are definitely in
the wrong in both spots; since the buffer's type is unsigned char, we
can't depend on larger alignment. When it works it is only because we
are lucky.

We'll fix this by switching to get_be32() to read the headers (just like
the last few commits similarly switched us to put_be32() for writing
into the same buffer).

It would be nice to factor this out into a common helper function, but
the interface ends up quite awkward. Either the caller needs to hardcode
how many bytes we'll need, or it needs to pass us its fill()/use()
functions as pointers. So I've just fixed both spots in the same way;
this is not code that is likely to be repeated a third time (most of the
pack reading code uses an mmap'd buffer, which should be properly
aligned).

I did make one tweak to the shared code: our pack_version_ok() macro
expects us to pass the big-endian value we'd get by casting. We can
introduce a "native" variant which uses the host integer ordering.

Reported-by: Koakuma <koachan@protonmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:37 +01:00
Patrick Steinhardt
cec3ebbdd8 object-file: don't special-case missing source file in collision check
In 0ad3d65652 (object-file: fix race in object collision check,
2024-12-30) we have started to ignore ENOENT when opening either the
source or destination file of the collision check. This was done to
handle races more gracefully in case either of the potentially-colliding
disappears.

The fix is overly broad though: while the destination file may indeed
vanish racily, this shouldn't ever happen for the source file, which is
a temporary object file (either loose or in packfile format) that we
have just created. So if any concurrent process would have removed that
temporary file it would indicate an actual issue.

Stop treating ENOENT specially for the source file so that we always
bubble up this error.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:37 +01:00
Jeff King
5285cdc7f9 parse_pack_header_option(): avoid unaligned memory writes
In order to recreate a pack header in our in-memory buffer, we cast the
buffer to a "struct pack_header" and assign the individual fields. This
is reported to cause SIGBUS on sparc64 due to alignment issues.

We can work around this by using put_be32() which will write individual
bytes into the buffer.

Reported-by: Koakuma <koachan@protonmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:37 +01:00
Patrick Steinhardt
5e1d9aa05e object-file: rename variables in check_collision()
Rename variables used in `check_collision()` to clearly identify which
file is the source and which is the destination. This will make the next
step easier to reason about when we start to treat those files different
from one another.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-06 19:32:37 +01:00
Jeff King
2dbdf2c476 packfile: factor out --pack_header argument parsing
Both index-pack and unpack-objects accept a --pack_header argument. This
is an undocumented internal argument used by receive-pack and fetch to
pass along information about the header of the pack, which they've
already read from the incoming stream.

In preparation for a bugfix, let's factor the duplicated code into a
common helper.

The callers are still responsible for identifying the option. While this
could likewise be factored out, it is more flexible this way (e.g., if
they ever started using parse-options and wanted to handle both the
stuck and unstuck forms).

Likewise, the callers are responsible for reporting errors, though they
both just call die(). I've tweaked unpack-objects to match index-pack in
marking the error for translation.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2025-02-06 19:32:37 +01:00