cmd/go/internal/get, x/tools/go/vcs: sync vcs package
adg opened this issue · 17 comments
Now that we have internal packages we should make the go tool use an internal copy of the vcs package from the tools repo, so that we can keep the two in sync.
Totally.
I will leave this open but please don't. Code duplication is not the end of the world. In particular the minimal duplication here seems preferable to more complexity in the build. I don't even think the public package vcs API is enough for the go command, but that's a separate issue.
At the moment they drift apart already. E.g. the version distributed with the go binary has quite sophisticated git support, which is missing (and thus a subtle broken) in every tool using tools/go/vcs.
I think there are two separate issues:
- The behaviour of go's internal
vcsand thevcsintoolsdiffer, especially with git. This makes it difficult for the authours of 3rd-party tools to match the behaviour of the go command. - The go tool could use an internal copy of
vcsfromtoolsto avoid duplication and future drift issues.
I think the first issue has a simple solution: update tools by applying the relevant changes made to go's internal vcs.
The second issue may be worth debating. @rsc already indicated that he'd rather not add more complexity to the build. Looking briefly, it doesn't appear too difficult to have the go command use the public API, though some breaking changes would likely have to be made. I am not sure what the policy is for x/tools. Then x/tools/go/vcs could be bundle'd into internal/. This approach definitely feels pretty kludgey over all. I am also unsure of the long-term vision for breaking out useful functionality from go tools.
Can we split this issue? I would at least like to see/make headway on the first part, and cannot think of any reasons to object.
CL https://golang.org/cl/21342 mentions this issue.
CL https://golang.org/cl/21345 mentions this issue.
CL https://golang.org/cl/21795 mentions this issue.
The latest decision here is from Nov 4, 2015:
I will leave this open but please don't. Code duplication is not the end of the world. In particular the minimal duplication here seems preferable to more complexity in the build.
Now that #18653 is happening, perhaps this issue should be revisited.
I'm still in favor of it, especially if we put the standard "This is not a stable API" disclaimer at the top of the x/tools/vcs package so we can change it at will.
CL https://golang.org/cl/42017 mentions this issue.
Change https://golang.org/cl/68352 mentions this issue: go/vcs: reject update of VCS inside VCS
Not sure there's much point to keeping this open.
Does that mean this proposal is rejected?
I was in favor of this (indicated via 👍 reaction), primarily because I wanted to there to be a single canonical source of truth regarding the import path resolution algorithm. Secondarily, as someone who sent many CLs to sync x/tools/go/vcs with cmd/go, I wish I didn't have to.
I admit I don't have a good understanding of the increased complexity in the build that @rsc mentioned that would be required to resolve the issue. Perhaps I'm significantly underestimating it, causing me to be in favor while otherwise I would not be?
Given this is closed now, I want to ask, should it be the responsibility of people sending CLs to either of the two places to also send a second CL to keep them in sync, or should that responsibility fall on the people who care about using x/tools/go/vcs (and thus need it to be up to date and in sync with cmd/go)?
Change https://golang.org/cl/93081 mentions this issue: go/vcs: add package comment
Change https://golang.org/cl/94899 mentions this issue: go/vcs: fix command injection in VCS path
Change https://golang.org/cl/159817 mentions this issue: go/vcs: remove go.googlesource.com vcsPath entry
Change https://golang.org/cl/159818 mentions this issue: go/vcs: deprecate package in favor of go list -json