Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Git to include race condition fix in gvfs-helper #157

Merged
merged 1 commit into from
Oct 5, 2019
Merged

Update Git to include race condition fix in gvfs-helper #157

merged 1 commit into from
Oct 5, 2019

Conversation

derrickstolee
Copy link
Contributor

Resolves #156.

See microsoft/git#205 for the Git code change. One race condition still existed: who creates the loose object directory first? The simple change is to check if the directory exists after the mkdir fails.

@derrickstolee derrickstolee added the bug correctness issues label Oct 4, 2019
@derrickstolee derrickstolee added this to the MVP milestone Oct 4, 2019
@@ -218,7 +218,7 @@ private void LoadBlobsViaGit(ScalarFunctionalTestEnlistment enlistment)
{
// 'git rev-list --objects' will check for all objects' existence, which
// triggers an object download on every missing blob.
ProcessResult result = GitHelpers.InvokeGitAgainstScalarRepo(enlistment.RepoRoot, "rev-list --objects HEAD^{tree}");
ProcessResult result = GitHelpers.InvokeGitAgainstScalarRepo(enlistment.RepoRoot, "rev-list --all --objects");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing the tests that rely on this method pass makes the issue in #147 all the more strange.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I would not be surprised if most of these tests are not validating where git decides to download the objects. If the parallel read tests end up download the same objects for each repo (in .git/objects) I'm not sure that they'd fail.

That could explain why #147 is still an issue, because that test specifically validates the shared cache folder was recreated.

@derrickstolee
Copy link
Contributor Author

(I pushed the update for the Git version before the installer build is complete.)

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
@derrickstolee derrickstolee merged commit 7d437dc into microsoft:master Oct 5, 2019
@derrickstolee derrickstolee deleted the gvfs-helper-race-fix branch November 18, 2019 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug correctness issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GVFS Helper] Fix parallel load test
2 participants