Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Comments

gps: return error if 'git fsck' checks are enabled and repo is corrupt#1981

Merged
kevinburke merged 1 commit intogolang:masterfrom
kevinburke:kill-child-process
Jan 24, 2019
Merged

gps: return error if 'git fsck' checks are enabled and repo is corrupt#1981
kevinburke merged 1 commit intogolang:masterfrom
kevinburke:kill-child-process

Conversation

@kevinburke
Copy link
Collaborator

Previously, if dep tried to clone a repository over ssh that contained
zero padded file modes, dep would fail to clone and then hang. You can
reproduce this failure with the following .gitconfig:

[url "git@github.com:"]
    insteadOf = https://github.com/

[transfer]
    fsckobjects = true

[fetch]
    fsckobjects = true

[receive]
    fsckobjects = true

and the following Gopkg.toml. (It is not my intention to single out
this project - I searched Github for Go projects with zero padded file
mode errors, and found this one.)

[[constraint]]
  name = "github.com/remogatto/gospeccy"
  branch = "master"

dep ensure hangs because the git clone operation spins up an ssh
process as a child process. cmd.Process.Kill kills the parent git
operation, but not the child ssh operation, so it's orphaned and we
don't return properly from the function. (It's unclear to me at this
point why the ssh operation did not return when it completed.)

By sending the negative Pgrp value to the Kill() function we can kill
the entire child process group, not just the "parent child." See
https://medium.com/@felixge/killing-a-child-process-and-all-of-its-children-in-go-54079af94773
for more information.

Updates #1257.

@kevinburke
Copy link
Collaborator Author

Just fixed the Travis CI build, whoops.

@kevinburke kevinburke changed the title gps: kill entire child process group on failure gps: return error if 'git fsck' checks are enabled and repo is corrupt Sep 18, 2018
@kevinburke kevinburke requested a review from theckman January 23, 2019 19:00
Previously, if dep tried to clone a repository over ssh that contained
zero padded file modes, dep would fail to clone and then hang. You can
reproduce this failure with the following .gitconfig:

    [url "git@github.com:"]
        insteadOf = https://github.com/

    [transfer]
        fsckobjects = true

    [fetch]
        fsckobjects = true

    [receive]
        fsckobjects = true

and the following Gopkg.toml. (It is not my intention to single out
this project - I searched Github for Go projects with zero padded file
mode errors, and found this one.)

    [[constraint]]
      name = "github.com/remogatto/gospeccy"
      branch = "master"

`dep ensure` hangs because the git clone operation spins up an `ssh`
process as a child process. `cmd.Process.Kill` kills the parent `git`
operation, but not the child `ssh` operation, so it's orphaned and we
don't return properly from the function. (It's unclear to me at this
point why the ssh operation did not return when it completed.)

By sending the negative Pgrp value to the Kill() function we can kill
the entire child process group, not just the "parent child." See
https://medium.com/@felixge/killing-a-child-process-and-all-of-its-children-in-go-54079af94773
for more information.

Updates golang#1257.
@kevinburke kevinburke merged commit 5494240 into golang:master Jan 24, 2019
@kevinburke kevinburke deleted the kill-child-process branch January 24, 2019 18:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants