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

the walking dead (pt 3) #1012

Merged
merged 3 commits into from Feb 18, 2016
Merged

the walking dead (pt 3) #1012

merged 3 commits into from Feb 18, 2016

Conversation

technoweenie
Copy link
Contributor

Building on #1008 (/cc @rlaakkol). Just moved the Wait() call before closing the channels, as described in #1008 (comment).

Riku Lääkkölä and others added 3 commits February 12, 2016 12:01
Waiting for the child process right aftetr closing the stdin seemed to cause truncated output at random. This way seems to work better.
@technoweenie
Copy link
Contributor Author

/cc @rubyist, @sinbad: would love some 👀 here if you have any time.

@technoweenie
Copy link
Contributor Author

FWIW, I audited the app's other uses of the os/exec package. Looks like we're calling Wait() everywhere else. I'm not sure what that extra defunct git process is from. I added trace messages around startCommand() in lfs/scanner.go (diff) and saw every command it started was finishing properly:

$ git push ghe --all
CMD START /Library/Developer/CommandLineTools/usr/libexec/git-core/git [git rev-list --objects 16f09066000e3328fb2d5c54beb55fc25c1af15c --not --remotes=ghe]
CMD START /Library/Developer/CommandLineTools/usr/libexec/git-core/git [git cat-file --batch-check]
CMD START /Library/Developer/CommandLineTools/usr/libexec/git-core/git [git cat-file --batch]
CMD STOP /Library/Developer/CommandLineTools/usr/libexec/git-core/git [git rev-list --objects 16f09066000e3328fb2d5c54beb55fc25c1af15c --not --remotes=ghe]
CMD STOP /Library/Developer/CommandLineTools/usr/libexec/git-core/git [git cat-file --batch-check]
CMD STOP /Library/Developer/CommandLineTools/usr/libexec/git-core/git [git cat-file --batch]

@rubyist
Copy link
Contributor

rubyist commented Feb 17, 2016

The waits look right to me 👍

@sinbad
Copy link
Contributor

sinbad commented Feb 18, 2016

Yeah, looks good - I bet Windows was especially susceptible to this, I've had issues before of git leaving files locked sometimes because of things like this. Nice one.

@technoweenie
Copy link
Contributor Author

Thanks for the reviews!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants