Two last-minute changes before 2.46.0 #678
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 reverts commit b7d6f23. This commit caused a regression in the incremental-repack maintenance step where not all objects in a pack-file are repacked, thus 'git multi-pack-index expire' cannot delete the pack-files. This will require some careful explanation upstream and some careful untangling of how server-side repacks may be depending on the new logic. Since microsoft/git only serves client users, it is safe to revert this in isolation for now. Signed-off-by: Derrick Stolee <[email protected]>
Typically, forcing a sparse index to expand to a full index means that
Git could not determine the status of a file outside of the
sparse-checkout and needed to expand sparse trees into the full list of
sparse blobs. This operation can be very slow when the sparse-checkout
is much smaller than the full tree at HEAD.
When users are in this state, there is usually a modified or untracked
file outside of the sparse-checkout mentioned by the output of 'git
status'. There are a number of reasons why this is insufficient:
1. Users may not have a full understanding of which files are inside or
outside of their sparse-checkout. This is more common in monorepos
that manage the sparse-checkout using custom tools that map build
dependencies into sparse-checkout definitions.
2. In some cases, an empty directory could exist outside the
sparse-checkout and these empty directories are not reported by 'git
status' and friends.
3. If the user has '.gitignore' or 'exclude' files, then 'git status'
will squelch the warnings and not demonstrate any problems.
In order to help users who are in this state, add a new advice message
to indicate that a sparse index is expanded to a full index. This
message should be written at most once per process, so add a static
global 'give_advice_on_expansion' to sparse-index.c. Further, there is a
case in 'git sparse-checkout set' that uses the sparse index as an
in-memory data structure (even when writing a full index) so we need to
disable the message in that kind of case.
The t1092-sparse-checkout-compatibility.sh test script compares the
behavior of several Git commands across full and sparse repositories,
including sparse repositories with and without a sparse index. We need
to disable the advice in the sparse-index repo to avoid differences in
stderr. By leaving the advice on in the sparse-checkout repo (without
the sparse index), we can test the behavior of disabling the advice in
convert_to_sparse(). (Indeed, these tests are how that necessity was
discovered.) Add a test that reenables the advice and demonstrates that
the message is output.
The advice message is defined outside of expand_index() to avoid super-
wide lines. It is also defined as a macro to avoid compile issues with
-Werror=format-security.
Signed-off-by: Derrick Stolee <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
derrickstolee
changed the title
dscho
reviewed
Jul 16, 2024
There was a problem hiding this comment.
Sounds very good, thank you so much for working on this @derrickstolee !
I left a couple of comments. Before merging, I would to address at least the Scalar Functional Tests failure.
|
@dscho I will update my branch with your suggestions after microsoft/scalar#533 merges. |
These seem to be custom tests to microsoft/git as they break without these changes, but these changes are not needed upstream. Signed-off-by: Derrick Stolee <[email protected]>
derrickstolee
force-pushed
the
last-minute
branch
from
d5abfda
to
7976fdc
Compare
dscho
approved these changes
Jul 16, 2024
Closed
2 tasks
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.
These two changes would be nice to have before rebasing onto 2.46.0.
Revert b7d6f23 due to the regression it introduced to Git 2.45.0 and the
incremental-repackmaintenance step. Sincegit pack-objects --stdin-packsuses a notion of "included" and "excluded" packs, it separates the choice to repack an object from whether or not the multi-pack-index points to the copy in the selected pack-files. Thus,.scalarCaches are filling up with pack-files with fewer than a dozen reference objects, but they still can't be expired withgit multi-pack-index expire. This revert fixes the problem but will need more justification, testing, and consideration of the server-side experience when moving upstream.Add an advice message around the sparse index expanding. This commit is already in
nextin upstream, but just missed thev2.46.0release window. This advice will be valuable formicrosoft/gitcustomers, especially those using the Office monorepo. The commit was cherry-picked fromnext, but requires an additional change on top to make somemicrosoft/git-specific tests pass due to comparingstderr.