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

panic at gps/vcs_source:301 #1379

Closed
Civil opened this issue Nov 16, 2017 · 8 comments · Fixed by #1380
Closed

panic at gps/vcs_source:301 #1379

Civil opened this issue Nov 16, 2017 · 8 comments · Fixed by #1380
Labels

Comments

@Civil
Copy link
Contributor

Civil commented Nov 16, 2017

What version of dep are you using (dep version)?

devel@2b7a08040ebc4699bfbd38128e9887eeb3092168

What dep command did you run?

dep init

What did you expect to see?

It should run fine

What did you see instead?

vsmirnov@C02PK283FVH6 ~/go/gopath_third_party/src/gitlab.[somecompany]/[some]/[repo] $ ~/go/gopath_third_party/src/github.com/golang/dep/cmd/dep/dep init
panic: runtime error: slice bounds out of range

goroutine 91 [running]:
github.com/golang/dep/gps.(*gitSource).listVersions(0xc420422110, 0x1771200, 0xc4203981e0, 0xc4204b4900, 0x10127b8, 0x10, 0x14aaee0, 0xc4204b4901)
	/Users/vsmirnov/go/gopath_third_party/src/github.com/golang/dep/gps/vcs_source.go:301 +0x1392
github.com/golang/dep/gps.maybeGitSource.try.func1(0x1771200, 0xc4203981e0, 0x1770f00, 0xc420063340)
	/Users/vsmirnov/go/gopath_third_party/src/github.com/golang/dep/gps/maybe_source.go:97 +0x71
github.com/golang/dep/gps.(*supervisor).do(0xc420067140, 0x1770f40, 0xc42001c040, 0x151aeca, 0xc, 0x1, 0xc4204b4a68, 0xc42050c038, 0x0)
	/Users/vsmirnov/go/gopath_third_party/src/github.com/golang/dep/gps/source_manager.go:657 +0xd5
github.com/golang/dep/gps.maybeGitSource.try(0xc42009ac80, 0x1770f40, 0xc42001c040, 0xc4200195f0, 0x2d, 0x1773d60, 0xc420398190, 0xc420067140, 0x30, 0x30, ...)
	/Users/vsmirnov/go/gopath_third_party/src/github.com/golang/dep/gps/maybe_source.go:95 +0x20f
github.com/golang/dep/gps.(*sourceGateway).require(0xc420254480, 0x1770f40, 0xc42001c040, 0x1, 0x50, 0x0, 0x0)
	/Users/vsmirnov/go/gopath_third_party/src/github.com/golang/dep/gps/source.go:571 +0x210
github.com/golang/dep/gps.(*sourceGateway).sourceURL(0xc420254480, 0x1770f40, 0xc42001c040, 0x0, 0x0, 0x0, 0x0)
	/Users/vsmirnov/go/gopath_third_party/src/github.com/golang/dep/gps/source.go:539 +0xb5
github.com/golang/dep/gps.(*sourceCoordinator).getSourceGatewayFor(0xc420181ab0, 0x1770f40, 0xc42001c040, 0xc42001a410, 0x1f, 0x0, 0x0, 0x0, 0x0, 0x0)
	/Users/vsmirnov/go/gopath_third_party/src/github.com/golang/dep/gps/source.go:205 +0x9f7
github.com/golang/dep/gps.(*SourceMgr).SyncSourceFor(0xc4200671a0, 0xc42001a410, 0x1f, 0x0, 0x0, 0x0, 0x0)
	/Users/vsmirnov/go/gopath_third_party/src/github.com/golang/dep/gps/source_manager.go:501 +0x82
github.com/golang/dep/gps.(*bridge).SyncSourceFor(0xc420256210, 0xc42001a410, 0x1f, 0x0, 0x0, 0xc42016b180, 0x1a)
	/Users/vsmirnov/go/gopath_third_party/src/github.com/golang/dep/gps/bridge.go:222 +0x5b
created by github.com/golang/dep/gps.(*solver).selectRoot
	/Users/vsmirnov/go/gopath_third_party/src/github.com/golang/dep/gps/solver.go:644 +0x614

Some extra information:
I've got custom ~/.gitconfig, it contains:

[url "git@gitlab.[somecompany]:"]
    insteadOf = https://gitlab.[somecompany]/

As a recommended workaround for corporate gitlab which requires ssh access to clone repos.

I've added a debug print before that line: fmt.Printf("SIZE: %v, LINE: %+v\n", len(pair), string(pair))

What I see just before panic:

SIZE: 59, LINE: 6cae4212beea4eeb8576347acac6418910cfda0f	refs/pull/99/merge
SIZE: 20, LINE: Killed by signal 1.

(Killed by signal 1 comes from ssh-proxy on a jump host).
If I add following code before 301 it works as expected:

                if len(pair) < 41 {
                        fmt.Printf("BUGBUG: strange pair version: '%v', len=%v\n", string(pair), len(pair))
                        continue
                }

It prints out:
', len=20trange pair version: 'Killed by signal 1.

@Civil
Copy link
Contributor Author

Civil commented Nov 16, 2017

I'm not sure that skipping this malformed line is the best way to workaround that panic. I also have no idea where it comes from.

@Civil
Copy link
Contributor Author

Civil commented Nov 16, 2017

If that matters, host os:
MacOS X 10.12
git 2.11.0 (gentoo-prefix)

@Civil
Copy link
Contributor Author

Civil commented Nov 16, 2017

Yup, it tried to parse 'Killed by signal 1' that comes from ssh-proxy and gets panic at that point

@Civil
Copy link
Contributor Author

Civil commented Nov 16, 2017

So what the actual problem is:

  1. https://github.com/golang/dep/blob/master/gps/vcs_source.go#L244-L257 - you do git ls-remote.
  2. I have an environment that uses ssh-proxy through jump host and because how it's configured I always see "Killed by signal 1" in stderr from ssh. But error code is 0.
  3. You are getting both stderr and stdout output
  4. There is no sanity check on https://github.com/golang/dep/blob/master/gps/vcs_source.go#L301 that what you got from ls-remote is actually commit hashes.

So either there should be a check before line 301 that output can be parsed or stderr should be ignored.

@sdboyer
Copy link
Member

sdboyer commented Nov 16, 2017

ugh, sorry about that. we've had a few issues with that slapdash parser; it definitely needs improvement. PR definitely welcome for this 😄

@sdboyer sdboyer added the bug label Nov 16, 2017
@Civil
Copy link
Contributor Author

Civil commented Nov 16, 2017

What is better - ignore malformed string or discard stderr of git ls-remote?

@jmank88
Copy link
Collaborator

jmank88 commented Nov 16, 2017

Linking to #1209

@sdboyer
Copy link
Member

sdboyer commented Nov 16, 2017

@Civil i suspect the ideal case here is only looking at stdout. i think that may be a bit tricky in our current design, though.

@jmank88 i wonder if maybe we should use io.MultiWriter to record stdout and stderr both independently and together in three distinct buffers, so that we have the flexibility to use whatever we think we may need when it comes to error reporting but can also have the option of returning only stdout for cases like this.

that wouldn't be the only solution to this, but it's the only one i can think of that doesn't involve more methods and indirection. i think i'd be fine trading memory space for logical complexity at this point.

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

Successfully merging a pull request may close this issue.

3 participants