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

Fix infinite forking on Bash on Windows #429

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

davidtwco
Copy link

Fixes #411, see that issue for a clearer idea of what this does.

Lyncredible and others added 3 commits July 31, 2017 21:57
Detect conflicts by checking for existence of .git/index.lock file.

The old way of varaible locking would not work because parallel checkout
commands run in isolated sub processes, hence variables could not be shared.

This fixes issue zplug#411. It repros when there are multiple plugins from the same
repo, e.g. oh-my-zsh. OSX and Ubuntu seem to be fine by ignoring the errors,
but Bash on Windows just hangs there when such conflict happens.
@davidtwco davidtwco mentioned this pull request Aug 1, 2017
4 tasks
@@ -66,6 +66,7 @@ do
"$fg[green]$repo$reset_color"
;;
esac
exit $?

Choose a reason for hiding this comment

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

Is exit required here? I doubt that, but if so I think it should be added to other commands as well, including install, update and status

Copy link
Author

Choose a reason for hiding this comment

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

I wasn't certain - I wasn't sure if there were any issues with the jobs dying in Bash on Windows and so I added that to have a greater degree of confidence that it would. I think it can be removed though - I'll do that as soon as I can.

Copy link
Author

Choose a reason for hiding this comment

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

Removed the last commit that had this.


__zplug::utils::git::is_git_locked()
{
[[ -e "$tags[dir]/.git/index.lock" || -e "$tags[dir]/.git/HEAD.lock" ]]

Choose a reason for hiding this comment

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

I think $tags[dir] should be passed in as arguments

Copy link
Author

Choose a reason for hiding this comment

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

I'll change this as soon as I can.

Copy link
Author

Choose a reason for hiding this comment

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

Changed.

Copy link

@Lyncredible Lyncredible left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!!

I made a few comments inline and would like to get them addressed.

@babarot
Copy link
Member

babarot commented Oct 13, 2017

Nice work. LGTM

@davidtwco
Copy link
Author

@b4b4r07 thanks. Would you be able to merge this then?

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.

3 participants