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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Never lock the index #1238

Merged
merged 9 commits into from Oct 31, 2017

Conversation

Projects
None yet
2 participants
@BinaryMuse
Member

BinaryMuse commented Oct 30, 2017

git/git@27344d6 has finally landed in Git 2.15.0 and in Dugite 1.49.0! This updates dependencies as appropriate, utilizes the new option via the GIT_OPTIONAL_LOCKS environment variable, and removes DeferredCallbackQueue (as it was introduced to help alleviate index lock contention in #726).

I verified this on my own computer using this test plan:

  1. Open a project in Atom with atom -d .
  2. Set Atom as my commit editor with export EDITOR="atom -d --wait"
  3. Begin an interactive rebase with git rebase -i head~5
  4. In the Atom window that opens, change the first and third item to r to rewrite the commit message.
  5. Save and close the window.
  6. In the window that next opens (to allow editing the commit), change the commit text, save, and close the window.

Prior to this PR, the rebase would immediately stop, because the window opened in step 1 would receive focus and run status, locking the index. After this PR, I was able to edit multiple commit messages with no problems. 馃帀

Fixes #961
Fixes akonwi/git-plus#693
Ref platformio/platformio-atom-ide-terminal#297 (may fix some reported issues)

BinaryMuse added some commits Oct 30, 2017

@BinaryMuse BinaryMuse requested a review from kuychaco Oct 30, 2017

@kuychaco

Looks great! So nice seeing this complexity ripped out! 馃挜

I think it'd be worth trying to add a test case for this if it's not too much of a hassle.

BinaryMuse added some commits Oct 31, 2017

@BinaryMuse

This comment has been minimized.

Show comment
Hide comment
@BinaryMuse

BinaryMuse Oct 31, 2017

Member

Well, I was able to design a test that failed on my machine with the env var unset, but it caused a lot of issues on CI so I've decided to remove it 鈥 this is not likely to regress.

Member

BinaryMuse commented Oct 31, 2017

Well, I was able to design a test that failed on my machine with the env var unset, but it caused a lot of issues on CI so I've decided to remove it 鈥 this is not likely to regress.

@BinaryMuse BinaryMuse merged commit f11cb4c into master Oct 31, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@BinaryMuse BinaryMuse deleted the mkt-if-the-repos-rockin-dont-come-a-lockin branch Oct 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment