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 v2.20.0.vfs.1.1 #600

Merged
merged 2 commits into from
Dec 13, 2018

Conversation

derrickstolee
Copy link
Contributor

@derrickstolee derrickstolee commented Dec 10, 2018

Includes v2.20.0.windows.1.

It also disables reset.quiet (correctness issue) and rebase.useBuiltin (performance issue).

@derrickstolee
Copy link
Contributor Author

@kewillford The functional tests are breaking on things like 'reset' and 'status', and the biggest change in gvfs-2.19.1 since the last time we had functional tests succeeding with a 2.20 release candidate is the post-index-changed hook. Could you take a look at these build failures?

@kewillford
Copy link
Member

@derrickstolee @benpeart was there some change to the output of reset --mixed to not display the files that are updated/deleted. Maybe we aren't setting a flag in the control repo that we are using in VFSForGit? I will dig in and see what I can find.

@derrickstolee derrickstolee force-pushed the vfs-2.20.0 branch 5 times, most recently from 9cc6534 to 9b17c8d Compare December 11, 2018 12:29
@derrickstolee
Copy link
Contributor Author

@kewillford @benpeart @jrbriggs It appears that the "reset.quiet" setting doesn't work well in our setting. I tried setting it to true in our control repo, but that wasn't enough. Disabling it in our processes made the functional tests pass.

My guess is that the index-changed hook merged cleanly with the reset quiet code in 2.20.0, but that merge introduced a different behavior than the hook expects. For now, I'll clean up this PR to only update the Git version and remove the "reset.quiet" setting and we can work to fix the integration in a follow-up PR.

@derrickstolee
Copy link
Contributor Author

@benpeart The latest functional test failures all have a message like this:

Extra: fatal: 'git gc' is not supported on a GVFS repo

This is because I forgot to include your command-canceling PR in the previous builds.

Turns out: we are calling git gc --auto from a lot of commands, and that is being halted by that patch.

I'm tempted to pull the change out of the v2.20.0 PR and work to reapply the change directly to that branch. We can then find the best way to halt auto-GC in a VFS for Git repo.

@benpeart
Copy link
Contributor

I'm tempted to pull the change out of the v2.20.0 PR and work to reapply the change directly to that branch. We can then find the best way to halt auto-GC in a VFS for Git repo.

Yes, please pull this patch out of 2.20.0 - I'll go back and figure out best to handle the automatic 'git gc --auto' commands (which will fail silently later as we turn off auto GC in VFSForGIT repos).

My bad - I hadn't merged the PR to switch VFSForGIT to use the new version of git yet and didn't expect it to get pulled into 2.20.0 before I'd had a chance to run the functional tests.

@kewillford
Copy link
Member

Do you know what is running the gc --auto or why it is running? We set { "gc.auto", "0" }, in the config for the repo which I though should be disabling it.

@benpeart
Copy link
Contributor

Do you know what is running the gc --auto or why it is running? We set { "gc.auto", "0" }, in the config for the repo which I though should be disabling it.

Several git commands spawn "gc --auto" themselves (am, commit, fetch, merge, rebase, etc). I've updated the patch in git to check for --auto and will only die if the config setting 'gc.auto' isn't "0" (ie turned off).

@derrickstolee derrickstolee changed the title Update Git to [PR BUILD] vfs-2.20.0 Update Git to v2.20.0.vfs.1.1 Dec 11, 2018
@derrickstolee
Copy link
Contributor Author

The latest push disables the builtin rebase. This is to check the performance results. It may cause a failure in the functional tests due to different warning/error messages.

@derrickstolee derrickstolee merged commit 5df68ee into microsoft:master Dec 13, 2018
@derrickstolee derrickstolee mentioned this pull request Dec 13, 2018
@jrbriggs jrbriggs added this to the S147 milestone Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants