This should fix#490, at least in the case that I've been able to
verify.
The issue is that reset was setting the skip-worktree bit more
frequently than it should have. The previous fix in #494 was focused on
the case where a file is added or removed across the diff. However, when
the file exists on both sides but still needs to be staged in a mixed
reset then we should avoid the skip-worktree bit.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Add a test verifying that sparse-checkout (with and without sparse index
enabled) treat untracked files & directories correctly when changing sparse
patterns. Specifically, it ensures that 'git sparse-checkout set'
* deletes empty directories outside the sparse cone
* does _not_ delete untracked files outside the sparse cone
Signed-off-by: Victoria Dye <vdye@github.com>
In ac8acb4f2c (sparse-index: complete partial expansion, 2022-05-23),
'expand_index()' was updated to expand the index to a given pathspec.
However, the 'path_matches_pattern_list()' method used to facilitate this
has the side effect of initializing or updating the index hash variables
('name_hash', 'dir_hash', and 'name_hash_initialized'). This operation is
performed on 'istate', though, not 'full'; as a result, the initialized
hashes are later overwritten when copied from 'full'. To ensure the correct
hashes are in 'istate' after the index expansion, change the arg used in
'path_matches_pattern_list()' from 'istate' to 'full'.
Note that this does not fully solve the problem. If 'istate' does not have
an initialized 'name_hash' when its contents are copied to 'full',
initialized hashes will be copied back into 'istate' but
'name_hash_initialized' will be 0. Therefore, we also need to copy
'full->name_hash_initialized' back to 'istate' after the index expansion is
complete.
Signed-off-by: Victoria Dye <vdye@github.com>
In the Office monorepo, we've recently had an uptick in issues with
`scalar clone`. These issues didn't make sense at first and seemed like
the users weren't using `microsoft/git` but instead the upstream
version's `scalar clone`. Instead of using GVFS cache servers, they were
attempting to use the Git protocol's partial clone (which times out).
It turns out that what's actually happening is that some network issue
is causing the connection with Azure DevOps to error out during the
`/gvfs/config` request. In the Git traces, we see the following error
during this request:
(curl:56) Failure when receiving data from the peer [transient]
This isn't 100% of the time, but has increased enough to cause problems
for a variety of users.
The solution being proposed in this pull request is to remove the
fall-back mechanism and instead have an explicit choice to use the GVFS
protocol. To avoid significant disruption to Azure DevOps customers (the
vast majority of `microsoft/git` users who use `scalar clone` based on
my understanding), I added some inferring of a default value from the
clone URL.
This fallback mechanism was first implemented in the C# version of
Scalar in microsoft/scalar#339. This was an attempt to make the Scalar
client interesting to non-Azure DevOps customers, especially as GitHub
was about to launch the availability of partial clones. Now that the
`scalar` client is available upstream, users don't need the GVFS-enabled
version to get these benefits.
In addition, this will resolve#384 since those requests won't happen
against non-ADO URLs unless requested.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
Teach index-pack to silently omit the reverse index if the
index file does not have the standard ".idx" suffix.
In e37d0b8730 (builtin/index-pack.c: write reverse indexes, 2021-01-25)
we learned to create `.rev` reverse indexes in addition to `.idx` index
files. The `.rev` file pathname is constructed by replacing the suffix
on the `.idx` file. The code assumes a hard-coded "idx" suffix.
In a8dd7e05b1 (config: enable `pack.writeReverseIndex` by default, 2023-04-12)
reverse indexes were enabled by default.
If the `-o <idx-path>` argument is used, the index file may have a
different suffix. This causes an error when it tries to create the
reverse index pathname.
Since we do not know why the user requested a non-standard suffix for
the index, we cannot guess what the proper corresponding suffix should
be for the reverse index. So we disable it.
The t5300 test has been updated to verify that we no longer error
out and that the .rev file is not created.
TODO We could warn the user that we skipped it (perhaps only if they
TODO explicitly requested `--rev-index` on the command line).
TODO
TODO Ideally, we should add an `--rev-index-path=<path>` argument
TODO or change `--rev-index` to take a pathname.
TODO
TODO I'll leave these questions for a future series.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
During the latest v2.45.0 update, 'scalar reconfigure --all' started to
segfault on my machine. Breaking it down via the debugger, it was
faulting on a NULL reference to the_hash_algo, which is a macro pointing
to the_repository->hash_algo.
In my case, this is due to one of my repositories having a detached HEAD,
which requires get_oid_hex() to parse that the HEAD reference is valid.
Another way to cause a failure is to use the "includeIf.onbranch" config
key, which will lead to a BUG() statement.
My first inclination was to try to refactor cmd_reconfigure() to execute
'git for-each-repo' instead of this loop. In addition to the difficulty
of executing 'scalar reconfigure' within 'git for-each-repo', it would
be difficult to perform the clean-up logic for non-existent repos if we
relied on that child process.
Instead, I chose to move the temporary repo to be within the loop and
reinstate the_repository to its old value after we are done performing
logic on the current array item.
Add tests to t9210-scalar.sh to test 'scalar reconfigure --all' with
multiple registered repos. There are two different ways that the old
use of the_repository could trigger bugs. These issues are being solved
independently to be more careful about the_repository being
uninitialized, but the change in this patch around the use of
the_repository is still a good safety precaution.
Co-authored-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add test case to demonstrate that `git index-pack -o <idx-path> pack-path`
fails if <idx-path> does not end in ".idx" when `--rev-index` is
enabled.
In e37d0b8730 (builtin/index-pack.c: write reverse indexes, 2021-01-25)
we learned to create `.rev` reverse indexes in addition to `.idx` index
files. The `.rev` file pathname is constructed by replacing the suffix
on the `.idx` file. The code assumes a hard-coded "idx" suffix.
In a8dd7e05b1 (config: enable `pack.writeReverseIndex` by default, 2023-04-12)
reverse indexes were enabled by default.
If the `-o <idx-path>` argument is used, the index file may have a
different suffix. This causes an error when it tries to create the
reverse index pathname.
The test here demonstrates the failure. (The test forces `--rev-index`
to avoid interaction with `GIT_TEST_NO_WRITE_REV_INDEX` during CI runs.)
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
The 'scalar reconfigure' command is intended to update registered repos
with the latest settings available. However, up to now we were not
reregistering the repos with background maintenance.
In particular, this meant that the background maintenance schedule would
not be updated if there are improvements between versions.
Be sure to register repos for maintenance during the reconfigure step.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
At the moment, some background jobs are getting blocked on credentials
during the 'prefetch' task. This leads to other tasks, such as
incremental repacks, getting blocked. Further, if a user manages to fix
their credentials, then they still need to cancel the background process
before their background maintenance can continue working.
Update the background schedules for our four scheduler integrations to
include these config options via '-c' options:
* 'credential.interactive=false' will stop Git and some credential
helpers from prompting in the UI (assuming the '-c' parameters are
carried through and respected by GCM).
* 'core.askPass=true' will replace the text fallback for a username
and password into the 'true' command, which will return a success in
its exit code, but Git will treat the empty string returned as an
invalid password and move on.
We can do some testing that the credentials are passed, at least in the
systemd case due to writing the service files.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
When scripts or background maintenance wish to perform HTTP(S) requests,
there is a risk that our stored credentials might be invalid. At the
moment, this causes the credential helper to ping the user and block the
process. Even if the credential helper does not ping the user, Git falls
back to the 'askpass' method, which includes a direct ping to the user
via the terminal.
Even setting the 'core.askPass' config as something like 'echo' will
causes Git to fallback to a terminal prompt. It uses
git_terminal_prompt(), which finds the terminal from the environment and
ignores whether stdin has been redirected. This can also block the
process awaiting input.
Create a new config option to prevent user interaction, favoring a
failure to a blocked process.
The chosen name, 'credential.interactive', is taken from the config
option used by Git Credential Manager to already avoid user
interactivity, so there is already one credential helper that integrates
with this option. However, older versions of Git Credential Manager also
accepted other string values, including 'auto', 'never', and 'always'.
The modern use is to use a boolean value, but we should still be
careful that some users could have these non-booleans. Further, we
should respect 'never' the same as 'false'. This is respected by the
implementation and test, but not mentioned in the documentation.
The implementation for the Git interactions takes place within
credential_getpass(). The method prototype is modified to return an
'int' instead of 'void'. This allows us to detect that no attempt was
made to fill the given credential, changing the single caller slightly.
Also, a new trace2 region is added around the interactive portion of the
credential request. This provides a way to measure the amount of time
spent in that region for commands that _are_ interactive. It also makes
a conventient way to test that the config option works with
'test_region'.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
These are two highly-requested items from an internal team considering a
move to Scalar using Azure Repos.
1. Remove the requirement that we create a `src` directory at clone time.
2. Allow `git worktree` even when using the GVFS protocol.
These are not difficult to implement. The `--no-src` option could even
be submitted upstream (though the commit will need to drop one bit about
an interaction with the local cache path).
This allows fixing settings after a Scalar upgrade, or after botching
the enlistments configuration.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Prepare `scalar` to use the GVFS protocol instead of partial clone
(required to support Azure Repos).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We should not be putting the .scalarCache inside the enlistment as a
sibling to the 'src' directory. This only happens in "unattended" mode,
but it also negates any benefit of a shared object cache because each
enlistment absolutely does not share any objects with others.
Move the shared object cache in this case to a level above the
enlistment, so at least there is some hope that it can be reused. This
is also critical to the upcoming --no-src option, since the shared
object cache cannot be located within the Git repository.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
When FSMonitor was upstreamed, the 'core.useBuiltinFSMonitor' config was
deprecated and replaced with an overload of the 'core.fsmonitor' config
(i.e., if a boolean value was specified in 'core.fsmonitor', it is treated
the way 'core.useBuiltinFSMonitor' originally was). Because 'scalar
register' actively sets that config, use it to upgrade the deprecated config
setting.
Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Victoria Dye <vdye@github.com>
Scalar's Functional Test suite is pretty comprehensive, and caught more
than just one bug in the built-in FSMonitor that was missed by Git's own
test suite.
To benefit from this test suite, automatically run it on the `vfs-*`
and `features/*` branches.
Note: for simplicity, we're building Git from scratch in all matrix
jobs.
Also note: for speed, we are using `git-sdk-64-minimal`, even if it
lacks the `/bin/install` that we need to install Git's files; We're
providing a minimal shell script shim instead. Also, we do not need to
bother with the Tcl/Tk parts, therefore we're skipping them, too.
Finally, we use GIT_FORCE_UNTRACKED_CACHE in the functional tests, to
give the untracked cache a thorough work-out.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The strip_last_component() method is helpful for finding the parent
directory of a path stored in a strbuf. Extract it to a global method
advertised in abspath.h. With that additional visibility, it is helpful to
rename it to be more specific to paths.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Those tests specifically verify that the `.zip` file path is shown on
`stdout`. Let's do that again, under the assumption that there are
scripts out there that rely on this behavior.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Scalar in Microsoft's Git fork can do a little more than Scalar in
upstream Git: in Microsoft's Git, it supports the GVFS protocol so that
Scalar can clone from Azure DevOps.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In Scalar's functional tests, we do not do anything with authentication.
Therefore, we do want to avoid accessing the `vsts/info` endpoint
because it requires authentication even on otherwise public
repositories.
Let's introduce the environment variable `SCALAR_TEST_SKIP_VSTS_INFO`
which can be set to `true` to simply skip that step (and force the
`url_*` style repository IDs instead of `id_*` whenever possible).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This allows setting the GVFS-enabled cache server, or listing the one(s)
associated with the remote repository.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This adds a new builtin, `git update-microsoft-git`, that executes the platform-specific upgrade steps to get the latest version of `microsoft-git`.
On Windows, this means running `git update-git-for-windows` which was updated to use the `microsoft/git` releases page, when appropriate. See #321 for details.
On macOS, this means running a sequence of `brew` commands. These are adapted from the `UpgradeVerb` in `microsoft/scalar`, with an important simplification: we don't need to differentiate between the `scalar` and `scalar-azrepos` cask.
When the virtualfilesystem is enabled the previous implementation of
clear_ce_flags would iterate all of the cache entries and query whether
each one is in the virtual filesystem to determine whether to clear one
of the SKIP_WORKTREE bits. For each cache entry, we would do a hash
lookup for each parent directory in the is_included_in_virtualfilesystem
function.
The former approach is slow for a typical Windows OS enlistment with
3 million files where only a small percentage is in the virtual
filesystem. The cost is
O(n_index_entries * n_chars_per_path * n_parent_directories_per_path).
In this change, we use the same approach as apply_virtualfilesystem,
which iterates the set of entries in the virtualfilesystem and searches
in the cache for the corresponding entries in order to clear their
flags. This approach has a cost of
O(n_virtual_filesystem_entries * n_chars_per_path * log(n_index_entries)).
The apply_virtualfilesystem code was refactored a bit and modified to
clear flags for all names that 'alias' a given virtual filesystem name
when ignore_case is set.
n_virtual_filesystem_entries is typically much less than
n_index_entries, in which case the new approach is much faster. We wind
up building the name hash for the index, but this occurs quickly thanks
to the multi-threading.
This PR updates our `vfs-2.29.0` branch's version of `git maintenance` to match the latest in upstream. Unfortunately, not all of these commits made it to the `2.30` release candidate, but there are more commits from the series making it in. They will cause a conflict in the `vfs-2.30.0` rebase, so merge them in here. This also includes the `fixup!` reverts of the earlier versions.
Finally, I also noticed that we started depending on `git maintenance start` in Scalar for macOS, but we never checked that this worked with the shared object cache. It doesn't! 😨 The very tip commit of this PR includes logic to make `git maintenance run` care about `gvfs.sharedCache`. Functional test updates in Scalar will follow.
Teach `gvfs-helper` to ignore the optional `.idx` files that may be
included in a `prefetch` response and always use `git index-pack` to
create them from the `.pack` files received in the data stream.
This is a little wasteful in terms of client-side compute and of the
network bandwidth, but allows us to use the full packfile verification
code contained within `git index-pack` to ensure that the received
packfiles are valid.
Add virtual file system settings and hook proc. On index load,
clear/set the skip worktree bits based on the virtual file system data.
Use virtual file system data to update skip-worktree bit in
unpack-trees. Use virtual file system data to exclude files and folders
not explicitly requested.
The hook was first contributed in private, but was extended via the
following pull requests:
#15#27#33#70
Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Most of these were done in private before microsoft/git. However,
the following pull requests modified the core feature:
#85#89#91#98#243#263
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
This replaces #493 (can't reopen a PR after a force-push...).
I updated this commit with a more firm version of the fix. This hopefully answers Victoria's excellent concerns with the previous approach.
I did not manage to get an automated test for this, but I did carefully verify this manually with a few commits in a VFS for Git enlistment (with different files every time). I updated the commit message with more details about why this works.
---
This fork contains changes specific to monorepo scenarios. If you are an
external contributor, then please detail your reason for submitting to
this fork:
* [X] This change only applies to the virtualization hook and VFS for Git.
Resolves#490.
* t1092: remove the 'git update-index' test that currently fails
because the command ignores the bad path, but doesn't return a
failure.
* dir.c: prevent matching against sparse-checkout patterns when the
virtual filesystem is enabled. Should prevent some corner case
issues.
* t1092: add quiet mode for some rebase tests because the stderr
output can change in some of the modes.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
During the 2.35.0 rebase, we ejected 570f64b (Fix reset when using the
sparse-checkout feature., 2017-03-15) because of a similar change
upstream that actually works with the expected behavior of
sparse-checkout.
That commit only ever existed in microsoft/git, but when it was
considered for upstream we realized that it behaved strangely for a
sparse-checkout scenario.
The root problem is that during a mixed reset, 'git reset <commit>'
updates the index to aggree with <commit> but leaves the worktree the
same as it was before. The issue with sparse-checkout is that some files
might not be in the worktree and thus the information from those files
would be "lost".
The upstream decision was to leave these files as ignored, because
that's what the SKIP_WORKTREE bit means: don't put these files in the
worktree and ignore their contents. If there already were files in the
worktree, then Git does not change them. The case for "losing" data is
if a committed change outside of the sparse-checkout was in the previous
HEAD position. However, this information could be recovered from the
reflog.
The case where this is different is in a virtualized filesystem. The
virtualization is projecting the index contents onto the filesystem, so
we need to do something different here. In a virtual environment, every
file is considered "important" and we abuse the SKIP_WORKTREE bit to
indicate that Git does not need to process a projected file. When a file
is populated, the virtual filesystem hook provides the information for
removing the SKIP_WORKTREE bit.
In the case of these mixed resets, we have the issue where we change the
projection of the worktree for these cache entries that change. If a
file is populated in the worktree, then the populated file will persist
and appear in a follow-up 'git status'. However, if the file is not
populated and only projected, we change the projection from the current
value to the new value, leaving a clean 'git status'.
The previous version of this commit includes a call to checkout_entry(),
which populates the file. This causes the file to be actually in the
working tree and no longer projected.
To make this work with the upstream changes, stop setting the
skip-worktree bit for the new cache entry. This seemed to work fine
without this change, but it's likely due to some indirection with the
virtual filesystem. Better to do the best-possible thing here so we
don't hide a corner-case bug by accident.
Helped-by: Victoria Dye <vdye@github.com>
Signed-off-by: Kevin Willford <kewillf@microsoft.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
During a run of the Scalar functional tests, we hit a case where the
inexact rename detection of a 'git cherry-pick' command slowed to the
point of writing its delayed progress, failing the test because stderr
differed from the control case. Showing progress like this when stderr
is not a terminal is non-standard for Git, so inject an isatty(2) when
initializing the progress option in sequencer.c.
Unfortunately, there is no '--quiet' option in 'git cherry-pick'
currently wired up. This could be considered in the future, and the
isatty(2) could be moved to that position. This would also be needed for
commands like 'git rebase', so we leave that for another time.
Derrick Stolee
Johannes Schindelin
Victoria Dye
Jeff Hostetler
Derrick Stolee
Jeff Hostetler
Derrick Stolee
Johannes Schindelin
Victoria Dye
Johannes Schindelin
Jeff Hostetler
Kevin Willford