Add GIT_TEST_MULTI_PACK_INDEX environment variable#27
Closed
derrickstolee wants to merge 3 commits intogitgitgadget:mastergitgitgadget/git:masterfrom
derrickstolee:midx-test/upstreamderrickstolee/git:midx-test/upstreamCopy head branch name to clipboard
Closed
Add GIT_TEST_MULTI_PACK_INDEX environment variable#27derrickstolee wants to merge 3 commits intogitgitgadget:mastergitgitgadget/git:masterfrom derrickstolee:midx-test/upstreamderrickstolee/git:midx-test/upstreamCopy head branch name to clipboard
derrickstolee wants to merge 3 commits intogitgitgadget:mastergitgitgadget/git:masterfrom
derrickstolee:midx-test/upstreamderrickstolee/git:midx-test/upstreamCopy head branch name to clipboard
Conversation
Author
|
Waiting for Junio to pick up #21 so I can target that branch. |
2465f6d to
04e3e91
Compare
Author
|
/submit |
|
Submitted as pull.27.git.gitgitgadget@gmail.com |
d444cc6 to
6ffc06e
Compare
When closing a multi-pack-index, we intend to close each pack-file and free the struct packed_git that represents it. However, this line was previously freeing the array of pointers, not the pointer itself. This leads to a double-free issue. Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
When repacking, we may remove pack-files. This invalidates the multi-pack-index (if it exists). Previously, we removed the multi-pack-index file before removing any pack-file. In some cases, the repack command may load the multi-pack-index into memory. This may lead to later in-memory references to the non-existent pack- files. Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
The multi-pack-index feature is tested in isolation by t5319-multi-pack-index.sh, but there are many more interesting scenarios in the test suite surrounding pack-file data shapes and interactions. Since the multi-pack-index is an optional data structure, it does not make sense to include it by default in those tests. Instead, add a new GIT_TEST_MULTI_PACK_INDEX environment variable that enables core.multiPackIndex and writes a multi-pack-index after each 'git repack' command. This adds extra test coverage when needed. There are a few spots in the test suite that need to react to this change: * t5319-multi-pack-index.sh: there is a test that checks that 'git repack' deletes the multi-pack-index. Disable the environment variable to ensure this still happens. * t5310-pack-bitmaps.sh: One test moves a pack-file from the object directory to an alternate. This breaks the multi-pack-index, so delete the multi-pack-index at this point, if it exists. * t9300-fast-import.sh: One test verifies the number of files in the .git/objects/pack directory is exactly 8. Exclude the multi-pack-index from this count so it is still 8 in all cases. Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
6ffc06e to
57c64e8
Compare
Author
|
/submit |
|
Submitted as pull.27.v2.git.gitgitgadget@gmail.com |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
To increase coverage of the multi-pack-index feature, add a GIT_TEST_MULTI_PACK_INDEX environment variable similar to other GIT_TEST_* variables.
After creating the environment variable and running the test suite with it enabled, I found a few bugs in the multi-pack-index implementation. These are handled by the first two patches.
I have set up a CI build on Azure Pipelines [1] that runs the test suite with a few optional features enabled, including GIT_TEST_MULTI_PACK_INDEX and GIT_TEST_COMMIT_GRAPH. I'll use this to watch the features and ensure they work well with the rest of the ongoing work. Eventually, we can add these variables to the Travis CI scripts.
[1] https://git.visualstudio.com/git/_build?definitionId=4