Initial ecosystem implementation for Go.#3709
Conversation
9274b0b to
6650efa
Compare
Naatan
left a comment
There was a problem hiding this comment.
Much simpler than I was imagining this would be! Hopefully this holds true as we start testing it more thoroughly.
Just some minor churn.
pkg/runtime/ecosystem/golang.go
Outdated
| if err != nil { | ||
| return nil, errs.Wrap(err, "Unable to unpack downloaded module") | ||
| } | ||
| err = fileutils.CopyFile(filepath.Join(unpackDir, artifact.NameAndVersion(), "go.mod"), filepath.Join(vDir, artifact.Version()+".mod")) |
There was a problem hiding this comment.
Jeremy pointed out not every package will have a go.mod, is that a concern here? I vaguely recall something about the go proxy injecting one, which DI is using so maybe it's fine..
There was a problem hiding this comment.
Also is artifact.NameAndVersion() robust enough? Feels a bit risky to assume a 1:1 mapping there. Perhaps we could locate the go.mod file instead? Assuming the answer to my previous question is that it always exists.
There was a problem hiding this comment.
Fair points. I've changed this to a list-dir-and-search approach. If go.mod wasn't found, we create one, according to the goproxy protocol.
pkg/runtime/ecosystem/golang.go
Outdated
| err = os.RemoveAll(unpackDir) | ||
| if err != nil { | ||
| return nil, errs.Wrap(err, "Unable to remove unpacked module") | ||
| } |
There was a problem hiding this comment.
Should defer this after its creation so we're also cleaning up on errors.
|
@Naatan ping |
Despite the "go: downloading" line, it's downloading from the local filesystem proxy, not the internet.
Note: I patched my local state tool to allow ':' as a namespace separator so I could run this demo. We should prioritize CP-972.