Replace maintenance with 'git maintenance run' #398
Merged
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
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
derrickstolee
force-pushed
the
maintenance
branch
3 times, most recently
from
74c1650 to
89674f2
Compare
derrickstolee
force-pushed
the
maintenance
branch
from
89674f2 to
32faa21
Compare
|
/azp run microsoft.scalar |
|
Azure Pipelines successfully started running 1 pipeline(s). |
derrickstolee
force-pushed
the
maintenance
branch
3 times, most recently
from
0eb91c1 to
e1709a9
Compare
Merged
derrickstolee
force-pushed
the
maintenance
branch
from
e1709a9 to
1d12c5e
Compare
5 tasks
derrickstolee
added a commit
that referenced
this pull request
Aug 12, 2020
These cleanups were discovered while working on #398: 1. Update Watchman download to get newer version. (Link was broken by recent change in release process.) 2. Use a better error when downloading the tip commit fails. 3. Use multi-valued config for `log.excludeDecoration` since it will need multiple values. 4. Do a better job interacting with `GIT_OBJECT_DIRECTORY` in `GitProcess`. 5. Stop exiting on an unknown exception during `GitMaintenanceStep`.
derrickstolee
force-pushed
the
maintenance
branch
from
1d12c5e to
ef62046
Compare
|
Note: this PR needs some decision points: Do we only run maintenance if their Git version supports it? Or, do we keep the C# code around for a version? I think we should say "use our Git if you want maintenance." |
derrickstolee
commented
Oct 1, 2020
derrickstolee
force-pushed
the
maintenance
branch
5 times, most recently
from
d54887b to
1222e49
Compare
derrickstolee
changed the title
The maintenance builtin is available in Git 2.28.0.vfs.1.0 or higher (that is, the 'vfs' will be required until at least 2.30.0). Signed-off-by: Derrick Stolee <[email protected]>
* Disable the 'gc' task, enable the other tasks. * Disable all auto-checks for the other tasks. * Set proper schedules for these tasks. Signed-off-by: Derrick Stolee <[email protected]>
Add unit tests to verify this works with the expected compatibility versions. Signed-off-by: Derrick Stolee <[email protected]>
Signed-off-by: Derrick Stolee <[email protected]>
Signed-off-by: Derrick Stolee <[email protected]>
Signed-off-by: Derrick Stolee <[email protected]>
Signed-off-by: Derrick Stolee <[email protected]>
This adds extra dependencies on gvfs-helper. Signed-off-by: Derrick Stolee <[email protected]>
Signed-off-by: Derrick Stolee <[email protected]>
On Linux, when using Git v2.28.0.vfs.1.0 or later (and this will be adjusted with the v2.29.0.vfs.0.0 release), we have access to the cron-enabled background maintenance in Git. This enables background maintenance for Scalar on Linux. While we _could_ use this integration on macOS, we will still ship with Scalar.Service on that platform for now to minimize possible issues. Signed-off-by: Derrick Stolee <[email protected]>
We removed all of the complicated logic from the C# FetchStep and instead rely on gvfs-helper to do all of this work for us. Note that these extra-careful steps are mostly due to previously-bad implementations of these steps in an earlier version of VFS for Git. Signed-off-by: Derrick Stolee <[email protected]>
The 'git maintenance run' version of these steps behave slightly differently and are tested in the Git codebase. Signed-off-by: Derrick Stolee <[email protected]>
derrickstolee
force-pushed
the
maintenance
branch
from
1222e49 to
f404a14
Compare
mjcheetham
approved these changes
Oct 8, 2020
Signed-off-by: Derrick Stolee <[email protected]>
derrickstolee
added a commit
that referenced
this pull request
Oct 16, 2020
The ConfigStep makes a decision about calling 'git maintenance start' depending on the platform (Linux only for now) and the Git version. However, the code for computing the feature flags is in ScalarVerb and needs to be injected by passing it through the ConfigStep constructor. This was missed in the earlier work that inserted this into other maintenance steps. This fixes a feature that should have been introduced in #398.
derrickstolee
added a commit
that referenced
this pull request
Mar 31, 2021
…1.0+ Resolves #501. The feature flag was originally introduced in cd0e476 (#398) but at that time we didn't know if the core Git client was ready, so we didn't enable it for versions of Git that did not include the `.vfs.` platform. Fix this since background maintenance is now available on all platforms on v2.31.0 and later.
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.
Scalar inherited its background maintenance model from VFS for Git, which does incremental maintenance by running some Git commands. Some of these steps have been implemented in Git proper, minus a few steps (such as verifying the file after writing it). We are working to move towards using the
git maintenancebuiltin for all maintenance.This PR is a step in that direction, but is in fact a half-measure.
CommitGraphStepandPackfileMaintenanceStepstill verify the written files in the C# layer since Git doesn't do that (yet).git maintenance startto actually integrate withcroninstead of using theScalar.Serviceproject.git maintenance startinstead ofScalar.Service(as in [WIP] [DO NOT MERGE] Drop Scalar.Service from macOS #416), but we keep it around to minimize the risk in this release.There was a late change from earlier WIP versions: we need to support Git versions that are not the latest
.vfs.release! This adds aGitFeatureFlags.MaintenanceBuiltinflag that is checked only if we are in versionv2.28.0.vfs.1.0orv2.29.0.vfs.0.0or later. When this builtin is fully integrated into later versions of Git (v2.30.0at minimum) then we can expand the feature flag. At that point, we will drop the parallel implementation in the C# layer that calls thecommit-graphandmulti-pack-indexbuiltins directly.This update to the Git version also takes a lot of dependencies in at once, but all that are available in v2.29.0. The most troublesome is git/git@1eb22c7 which changes the behavior of
git multi-pack-index repack --batch-size=<size>to repack frequently. Combining this with the fact that we no longer specify a batch size throughgit maintenance run --task=incremental-repack, some functional tests needed to change to accommodate this.Finally, we remove a lot of tests that check that the
PackfileMaintenanceStepclears up bad prefetch pack data. This was necessary in VFS for Git to clean up bad data from a previous version, but is less necessary now. We will want to be aware of anyone accumulating too many temp files (from early-exiting Git processes) to see ifgit gvfs-helperneeds to take some of these features.The commits are laid out in this sequence.
Update the version of Git:
Update some config schedules (only matters on Linux and future Scalar versions) and update
GitFeatureFlags:Replace existing Git calls with
git maintenance run --task=<task>when possible.On Linux, launch background maintenance with
git maintenance start:Fix the functional tests as necessary, due to new behavior: