Rebase to v2.36.0-rc2 #3785
Merged
Rebase to v2.36.0-rc2 #3785
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
derrickstolee
approved these changes
Apr 13, 2022
The intention of this change is to align with how the top-level git `Makefile` defines its own test target (which also internally calls `$(MAKE) -C t/ all`). This change also ensures the consistency of `make -C contrib/subtree test` with other testing in CI executions (which rely on `$DEFAULT_TEST_TARGET` being defined as `prove`). Signed-off-by: Victoria Dye <vdye@github.com>
In Git-for-Windows, work on using ARM64 has progressed. The commit 2d94b77 (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 635b6d9 (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>
Refactor daemon thread startup to make it easier to start a third thread class to monitor the health of the daemon. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
As mentioned in the Makefile and CMakeLists.txt: "When cross-compiling, define HOST_CPU as the canonical name of the CPU on which the built Git will run (for instance "x86_64")" This commit sets the HOST_CPU variable since Git for Windows arm64 is cross-compiled from an amd64 host. Signed-off-by: Dennis Ameling <dennis@dennisameling.com>
We now have a GitHub Action to download and cache Azure Pipelines artifacts (such as the `vcpkg` artifacts), hiding gnarly internals, and also providing some robustness against network glitches. Let's use it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
As reported in newren/git-filter-repo#225, it looks like 99 bytes is not really sufficient to represent e.g. the full path to Python when installed via Windows Store (and this path is used in the hasb bang line when installing scripts via `pip`). Let's increase it to what is probably the maximum sensible path size: MAX_PATH. This makes `parse_interpreter()` in line with what `lookup_prog()` handles. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Vilius Šumskas <vilius@sumskas.eu>
We used to have that `make vcxproj` hack, but a hack it is. In the meantime, we have a much cleaner solution: using CMake, either explicitly, or even more conveniently via Visual Studio's built-in CMake support (simply open Git's top-level directory via File>Open>Folder...). Let's let the `README` reflect this. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This adds support for a new http.sslAutoClientCert config value. In cURL 7.77 or later the schannel backend does not automatically send client certificates from the Windows Certificate Store anymore. This config value is only used if http.sslBackend is set to "schannel", and can be used to opt in to the old behavior and force cURL to send client certificates. This fixes #3292 Signed-off-by: Pascal Muller <pascalmuller@gmail.com>
Because `git subtree` (unlike most other `contrib` modules) is included as part of the standard release of Git for Windows, its stability should be verified as consistently as it is for the rest of git. By including the `git subtree` tests in the CI workflow, these tests are as much of a gate to merging and indicator of stability as the standard test suite. Signed-off-by: Victoria Dye <vdye@github.com>
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>
In Git for Windows, `has_symlinks` is set to 0 by default. Therefore, we need to parse the config setting `core.symlinks` to know if it has been set to `true`. In `git init`, we must do that before copying the templates because they might contain symbolic links. Even if the support for symbolic links on Windows has not made it to upstream Git yet, we really should make sure that all the `core.*` settings are parsed before proceeding, as they might very well change the behavior of `git init` in a way the user intended. This fixes #3414 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Rename platform-specific listener thread related variables and data types as we prepare to add another backend thread type. [] `struct fsmonitor_daemon_backend_data` becomes `struct fsm_listen_data` [] `state->backend_data` becomes `state->listen_data` [] `state->error_code` becomes `state->listen_error_code` Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Create another thread to watch over the daemon process and automatically shut it down if necessary. This commit creates the basic framework for a "health" thread to monitor the daemon and/or the file system. Later commits will add platform-specific code to do the actual work. The "health" thread is intended to monitor conditions that would be difficult to track inside the IPC thread pool and/or the file system listener threads. For example, when there are file system events outside of the watched worktree root or if we want to have an idle-timeout auto-shutdown feature. This commit creates the health thread itself, defines the thread-proc and sets up the thread's event loop. It integrates this new thread into the existing IPC and Listener thread models. This commit defines the API to the platform-specific code where all of the monitoring will actually happen. The platform-specific code for MacOS is just stubs. Meaning that the health thread will immediately exit on MacOS, but that is OK and expected. Future work can define MacOS-specific monitoring. The platform-specific code for Windows sets up enough of the WaitForMultipleObjects() machinery to watch for system and/or custom events. Currently, the set of wait handles only includes our custom shutdown event (sent from our other theads). Later commits in this series will extend the set of wait handles to monitor other conditions. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
This commit prepares for adding batch-fsync to the bulk-checkin infrastructure. The bulk-checkin infrastructure is currently used to batch up addition of large blobs to a packfile. When a blob is larger than big_file_threshold, we unconditionally add it to a pack. If bulk checkins are 'plugged', we allow multiple large blobs to be added to a single pack until we reach the packfile size limit; otherwise, we simply make a new packfile for each large blob. The 'unplug' call tells us when the series of blob additions is done so that we can finish the packfiles and make their objects available to subsequent operations. Stated another way, bulk-checkin allows callers to define a transaction that adds multiple objects to the object database, where the object database can optimize its internal operations within the transaction boundary. Batched fsync will fit into bulk-checkin by taking advantage of the plug/unplug functionality to determine the appropriate time to fsync and make newly-added objects available in the primary object database. * Rename 'state' variable to 'bulk_checkin_state', since we will later be adding 'bulk_fsync_objdir'. This also makes the variable easier to find in the debugger, since the name is more unique. * Move the 'plugged' data member of 'bulk_checkin_state' into a separate static variable. Doing this avoids resetting the variable in finish_bulk_checkin when zeroing the 'bulk_checkin_state'. As-is, we seem to unintentionally disable the plugging functionality the first time a new packfile must be created due to packfile size limits. While disabling the plugging state only results in suboptimal behavior for the current code, it would be fatal for the bulk-fsync functionality later in this patch series. Signed-off-by: Neeraj Singh <neerajsi@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Extend the Windows version of the "health" thread to periodically inspect the system and shutdown if warranted. This commit updates the thread's wait loop to use a timeout and defines a (currently empty) table of functions to poll the system. A later commit will add functions to the table to actually inspect the system. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Make it clearer in the naming and documentation of the plug_bulk_checkin and unplug_bulk_checkin APIs that they can be thought of as a "transaction" to optimize operations on the object database. These transactions may be nested so that subsystems like the cache-tree writing code can optimize their operations without caring whether the top-level code has a transaction active. Signed-off-by: Neeraj Singh <neerajsi@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Force shutdown fsmonitor daemon if the worktree root directory is moved, renamed, or deleted. Use Windows low-level GetFileInformationByHandle() to get and compare the Windows system unique ID for the directory with a cached version when we started up. This lets us detect the case where someone renames the directory that we are watching and then creates a new directory with the original pathname. This is important because we are listening to a named pipe for requests and they are stored in the Named Pipe File System (NPFS) which a kernel-resident pseudo filesystem not associated with the actual NTFS directory. For example, if the daemon was watching "~/foo/", it would have a directory-watch handle on that directory and a named-pipe handle for "//./pipe/...foo". Moving the directory to "~/bar/" does not invalidate the directory handle. (So the daemon would actually be watching "~/bar" but listening on "//./pipe/...foo". If the user then does "git init ~/foo" and causes another daemon to start, the first daemon will still have ownership of the pipe and the second daemon instance will fail to start. "git status" clients in "~/foo" will ask "//./pipe/...foo" about changes and the first daemon instance will tell them about "~/bar". This commit causes the first daemon to shutdown if the system unique ID for "~/foo" changes (changes from what it was when the daemon started). Shutdown occurs after a periodic poll. After the first daemon exits and releases the lock on the named pipe, subsequent Git commands may cause another daemon to be started on "~/foo". Similarly, a subsequent Git command may cause another daemon to be started on "~/bar". Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
When adding many objects to a repo with `core.fsync=loose-object`,
the cost of fsync'ing each object file can become prohibitive.
One major source of the cost of fsync is the implied flush of the
hardware writeback cache within the disk drive. This commit introduces
a new `core.fsyncMethod=batch` option that batches up hardware flushes.
It hooks into the bulk-checkin odb-transaction functionality, takes
advantage of tmp-objdir, and uses the writeout-only support code.
When the new mode is enabled, we do the following for each new object:
1a. Create the object in a tmp-objdir.
2a. Issue a pagecache writeback request and wait for it to complete.
At the end of the entire transaction when unplugging bulk checkin:
1b. Issue an fsync against a dummy file to flush the log and hardware
writeback cache, which should by now have seen the tmp-objdir writes.
2b. Rename all of the tmp-objdir files to their final names.
3b. When updating the index and/or refs, we assume that Git will issue
another fsync internal to that operation. This is not the default
today, but the user now has the option of syncing the index and there
is a separate patch series to implement syncing of refs.
On a filesystem with a singular journal that is updated during name
operations (e.g. create, link, rename, etc), such as NTFS, HFS+, or XFS
we would expect the fsync to trigger a journal writeout so that this
sequence is enough to ensure that the user's data is durable by the time
the git command returns. This sequence also ensures that no object files
appear in the main object store unless they are fsync-durable.
Batch mode is only enabled if core.fsync includes loose-objects. If
the legacy core.fsyncObjectFiles setting is enabled, but core.fsync does
not include loose-objects, we will use file-by-file fsyncing.
In step (1a) of the sequence, the tmp-objdir is created lazily to avoid
work if no loose objects are ever added to the ODB. We use a tmp-objdir
to maintain the invariant that no loose-objects are visible in the main
ODB unless they are properly fsync-durable. This is important since
future ODB operations that try to create an object with specific
contents will silently drop the new data if an object with the target
hash exists without checking that the loose-object contents match the
hash. Only a full git-fsck would restore the ODB to a functional state
where dataloss doesn't occur.
In step (1b) of the sequence, we issue a fsync against a dummy file
created specifically for the purpose. This method has a little higher
cost than using one of the input object files, but makes adding new
callers of this mechanism easier, since we don't need to figure out
which object file is "last" or risk sharing violations by caching the fd
of the last object file.
_Performance numbers_:
Linux - Hyper-V VM running Kernel 5.11 (Ubuntu 20.04) on a fast SSD.
Mac - macOS 11.5.1 running on a Mac mini on a 1TB Apple SSD.
Windows - Same host as Linux, a preview version of Windows 11.
Adding 500 files to the repo with 'git add' Times reported in seconds.
object file syncing | Linux | Mac | Windows
--------------------|-------|-------|--------
disabled | 0.06 | 0.35 | 0.61
fsync | 1.88 | 11.18 | 2.47
batch | 0.15 | 0.41 | 1.53
Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach the listener thread to shutdown the daemon if the spelling of the worktree root directory changes. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Take advantage of the odb transaction infrastructure around writing the cached tree to the object database. Signed-off-by: Neeraj Singh <neerajsi@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach Git to perform binary search over the cache-entries for a directory notification and then linearly scan forward to find the immediate children. Previously, when the FSMonitor reported a modified directory Git would perform a linear search on the entire cache-entry array for all entries matching that directory prefix and invalidate them. Since the cache-entry array is already sorted, we can use a binary search to find the first matching entry and then only linearly walk forward and invalidate entries until the prefix changes. Also, the original code would invalidate anything having the same directory prefix. Since a directory event should only be received for items that are immediately within the directory (and not within sub-directories of it), only invalidate those entries and not the whole subtree. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
The add_files_to_cache function is invoked internally by builtin/commit.c and builtin/checkout.c for their flags that stage modified files before doing the larger operation. These commands can benefit from batched fsyncing. Signed-off-by: Neeraj Singh <neerajsi@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Create unit tests to move a directory. Verify that `git status` gives the same result with and without FSMonitor enabled. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
On LLP64 systems, such as Windows, the size of `long`, `int`, etc. is only 32 bits (for backward compatibility). Git's use of `unsigned long` for file memory sizes in many places, rather than size_t, limits the handling of large files on LLP64 systems (commonly given as `>4GB`). Provide a minimum test for handling a >4GB file. The `hash-object` command, with the `--literally` and without `-w` option avoids writing the object, either loose or packed. This avoids the code paths hitting the `bigFileThreshold` config test code, the zlib code, and the pack code. Subsequent patches will walk the test's call chain, converting types to `size_t` (which is larger in LLP64 data models) where appropriate. Signed-off-by: Philip Oakley <philipoakley@iee.email> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The update-index functionality is used internally by 'git stash push' to setup the internal stashed commit. This change enables odb-transactions for update-index infrastructure to speed up adding new objects to the object database by leveraging the batch fsync functionality. There is some risk with this change, since under batch fsync, the object files will be in a tmp-objdir until update-index is complete, so callers using the --stdin option will not see them until update-index is done. This risk is mitigated by not keeping an ODB transaction open around --stdin processing if in --verbose mode. Without --verbose mode, a caller feeding update-index via --stdin wouldn't know when update-index adds an object, event without an ODB transaction. Signed-off-by: Neeraj Singh <neerajsi@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes since approval
shears.shto get changes from Include trailing slash in prefix migration instructions #3790 and Various fixes aroundsafe.directory#3791 (updated range-diff, diff stayed the same)Changes
This was a bit of a weird one, since I needed to deal with the merge of v2.35.2.windows.1. That merge introduced a bunch of duplicate commits, so for this rebase I:
I'm happy to move either or both to "ready-for-upstream" as needed (although, at least one of them will ideally make it into upstream v2.36.0, so it may not be worth the effort?).
Otherwise, the fixup from #3783 was squashed into the appropriate commit.
Diff vs
mainBecause of how the v2.35.2 merge was rebased, the range diff shows a bunch of duplicate "deleted" commits. The simple diff, on the other hand, gives us a clear picture of what actually changed upstream:
Range-diff
Included for reference, but it's cluttered with "fake" removed commits.