Skip to content

x/proto: create repo and setup Gerrit #26723

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

Closed
dsnet opened this issue Jul 31, 2018 · 17 comments
Closed

x/proto: create repo and setup Gerrit #26723

dsnet opened this issue Jul 31, 2018 · 17 comments

Comments

@dsnet
Copy link
Member

dsnet commented Jul 31, 2018

We are starting development of the next generation of Go protocol buffers (with a heavy focus around proto reflection; see golang/protobuf#364). Those of us primarily involved with the development prefer to use a code review tool that is better able to track review comments, patch updates, and what not. We believe the choice of review tool is especially important in the starting phases where there is likely to be a large quantity of CLs of relatively large size, where Gerrit shows its strengths. As such, we prefer to develop using Gerrit as opposed to GitHub pull requests. The current workflow is contrary to most other golang/x projects in that protobuf still uses GitHub PRs, which is fine when each change is not that large.

As we explore using Gerrit, there are several thoughts/questions:

  1. Can we create a new repo at go.googlesource.com/proto?
  2. Can we mirror a branch named api2 on go.googlesource.com/proto to a branch named api2 on github.com/golang/protobuf?
  3. Does the above imply that the master branch must also be managed by Gerrit on go.googlesource.com/proto? If so, can we set up the GitHubPR to GerrittCL bot so that existing contributors can continue working with GitHub?
  4. What are our options of pre-submit testing? Are we going to be using try-bots? If so, will we be able to test across multiple Go versions (e.g., Go1.6, Go1.7, etc?). Will the try-bot have network access such that we can download external dependencies (e.g., "github.com/google/go-cmp") or dependents (e.g., "google.golang.org/grpc")? Can we get try-bot to do more complex operations like download protoc and run it over proto files? Can we build and run a C++ program (e.g., the conformance test runner; it may be possible to just download the test runner as a binary)?

\cc @paranoiacblack @neild @cybrcodr @bradfitz @andybons

@gopherbot gopherbot added this to the Unreleased milestone Jul 31, 2018
@dsnet
Copy link
Member Author

dsnet commented Jul 31, 2018

\cc @jhump @tamird, who are other frequent contributors
\cc @puellanivis @awalterschulze, who are active in the issue tracker (the issue tracker isn't being changed or moved, so if you only care about keeping on top of issues, then you can ignore all this).
\cc @randall77 @LMMilewski @bufflig, who are active with protobuf optimizations
\cc @xfxyjwf from the protobuf team

@andybons
Copy link
Member

Can we create a new repo at go.googlesource.com/proto?

The repo has been created: go.googlesource.com/proto

Can we mirror a branch named api2 on go.googlesource.com/proto to a branch named api2 on github.com/golang/protobuf?

Would you also have github.com/golang/proto and github.com/golang/protobuf? gitmirror, I believe, is all or nothing when it comes to mirroring at this time.

Does the above imply that the master branch must also be managed by Gerrit on go.googlesource.com/proto? If so, can we set up the GitHubPR to GerrittCL bot so that existing contributors can continue working with GitHub?

Yes it does. You should add proto to the Gerrit project whitelist at https://go.googlesource.com/build/+/master/cmd/gerritbot/gerritbot.go#190

Is the idea to have the GitHub repo be golang/protobuf and the Gerrit repo be go.googlesource.com/proto?

What are our options of pre-submit testing? Are we going to be using try-bots? If so, will we be able to test across multiple Go versions (e.g., Go1.6, Go1.7, etc?). Will the try-bot have network access such that we can download external dependencies (e.g., "github.com/google/go-cmp") or dependents (e.g., "google.golang.org/grpc")? Can we get try-bot to do more complex operations like download protoc and run it over proto files? Can we build and run a C++ program (e.g., the conformance test runner; it may be possible to just download the test runner as a binary)?

I will let @bradfitz answer this one.

@bradfitz
Copy link
Contributor

Maybe we shouldn't have created the Gerrit repo until we figured out the whole plan here. There's also the https://code.googlesource.com/ GoB repo as used by e.g. https://code.googlesource.com/gocloud and https://code.googlesource.com/google-api-go-client/

Is protobuf more of a Go thing or a Google thing? shrug

Trybots testing repos against older Go releases is #17626 ... not hard, but not done.

Downloading external dependencies only works for go.googlesource.com/* deps at the moment, not GitHub. I was waiting on Go Modules until I did that, because with hitting GitHub we need to be a bit more careful with caching, and I thought the versions from go.mod files would help with the caching and preventing unnecessary origin freshness checks.

Can we get try-bot to do more complex operations like download protoc and run it over proto files?

It runs any code that "go test" runs. So it could. But without help it would do lots of uncached downloads. Maybe that's okay, if the origin doesn't block it.

Can we build and run a C++ program (e.g., the conformance test runner; it may be possible to just download the test runner as a binary)?

There's a C toolchain available in the builder image.

@andybons
Copy link
Member

Removed the proto repo for now until we understand better what you need.

@dsnet
Copy link
Member Author

dsnet commented Jul 31, 2018

Per @bradfitz and @andybons answers:

  1. It seems that Try-Bot in its current state is insufficient to provide the level of testing that we currently have today with Travis-CI. As a result, I don't think we want the master branch in github.com/golang/protobuf to be managed by Gerrit (and Try-Bot) yet.
  2. It seems Gerrit's management of a repo is an all or none ideal (e.g., you can't mirror to a single branch, but leave the master branch unmanaged). Combined with point 1, this implies that we can't mirror to github.com/golang/protobuf.
  3. Since we are developing a new API from scratch, it is okay if the development doesn't have as rigorous testing as the master branch. This API is definitely not production ready. The existing level of Try-Bot support (i.e., simply running "go test") should be sufficient, and given the low number of initial developers, we can rely on individual precommit hooks for more advanced testing.
  4. Thus, can we request that go.googlesource.com/protobuf be created?
    • Regarding name: per emails with the protobuf team, they mentioned that "proto" is too ambiguous. Also, "protobuf" better matches the existing GitHub repo.
    • Regarding mirroring: at this time, we will not be mirror the repo to GitHub (that is an eventual goal). This also means that we won't need the GitHubPR <-> GerritCL bot.
    • We will continue to use github.com/golang/protobuf repo for issue tracking of the new API development.
    • Contributors can only use Gerrit (unfortunately no GitHub PRs), but since the primary developers of the new API prefer Gerrit, this is fine.

@tamird
Copy link
Contributor

tamird commented Aug 1, 2018

This is pretty unfortunate in that it will alienate ~all non-Google contributors. I understand that the primary developers like Gerrit, but if the desire is to have a review tool that understands patch updates, I suggest giving reviewable a look. CockroachDB has been using it for a few years now, and that is a much larger project than this will be.

@ianlancetaylor
Copy link
Contributor

@tamird Note that our Gerrit instances support GitHub pull requests, and I assume that would also be true for this repo.

We aren't going to be switching to something like reviewable. This has been discussed at length in the past. I can't see any good reason for x/proto to use a third review system, different from all the other Go repos.

@tamird
Copy link
Contributor

tamird commented Aug 1, 2018

@dsnet wrote:

It seems Gerrit's management of a repo is an all or none ideal (e.g., you can't mirror to a single branch, but leave the master branch unmanaged). Combined with point 1, this implies that we can't mirror to github.com/golang/protobuf.
...
Regarding mirroring: at this time, we will not be mirror the repo to GitHub (that is an eventual goal). This also means that we won't need the GitHubPR <-> GerritCL bot.

I don't own stock in reviewable, but the other solutions are lacking.

@dsnet
Copy link
Member Author

dsnet commented Aug 1, 2018

To give some background. There are some competing goals:

  • We want to develop the new reflection APIs in an open environment. Projects like gogo/protobuf, golang/protobuf, and other advanced protobuf usages are likely going to be consumers of the API and it is helpful to have feedback on sharp-edges and/or use-cases that it can't fulfill.
  • Myself and several others I CC'd in the first comment are going to be primary contributors initially on the API. There is going to be thousands of lines of new code, and the thought of using GitHub PRs to review that was something we believe is going to impede progress.
  • We do appreciate community contributions to the development and believe that the community can also help speed progress.
  • We also understand that much of the community prefers GitHub PRs.
  • We would like to have GitHub <-> Gerrit syncing from the start, but there are technical reasons why we can't set that up at this time.

Taking in all of the above considerations together, it seems that using Gerrit is still the best alternative:

  • We considered doing all development internally using code review tools that we are familiar with and then releasing all our work much later. However, this would be contrary to the first point about wanting to be open in the first place.
  • We could just use GitHub PRs, but we personally find it more difficult to do code review and also believe that Gerrit does a better job preserving history. In my attempt to understand protobufs, I often had to go back in time to the original Go protobuf implementation by Rob Pike and/or look at ancient commits in the original C++ code to understand why things are the way they are. I hope someone 10 years from now can easily trace through the decisions that we make today.
  • The use of Gerrit does not prevent external contributions. When I first contributed to the Go project (before I was even a Google employee), I can confess that setting up Gerrit seems daunting at first, but I have come to see when its strengths greatly outshine GitHub PRs (especially with large reviews). It seems that the main barrier is the time spent setting it up. That is a detriment, but not an insurmountable one.
  • Most realistically, myself (@neild, @paranoiacblack, and @cybrcodr) are going to be submitting a bulk majority of the initial code and it makes sense for use to use the tool that works best for us. If the majority of the contributors prefer a different tool, then it makes sense to switch.

@awalterschulze
Copy link

Really great to see the start of this project :) I hope nullable=false and customtype or some version of it and time.* gets proper support, but that is a separate discussion for a later time in a separate issue. Sorry for qualifying my excitement, but don't get me wrong I am hopeful for a great fix to the go/gogo protobuf situation and more to happen in this repo. And this to me is more important than the review tool.

But maybe I can ask, have you thought about making smaller commits?

@awalterschulze
Copy link

\cc @johanbrandhorst @donaldgraham @jmarais gogo members

@puellanivis
Copy link

Can we create a new repo at go.googlesource.com/proto?

I believe that googlesource was moribund as of three~four or so years ago, right? One likely does not want to setup the repo on a service that at any time is known to be likely to receive an “An Update On…” email.

I think trying to find a way to stick somehow to using github would increase the most amount of external eyes at least lurking and watching development. (People like me.)

On the other hand, if I am wrong, and googlesource is long-term viable, and one can easily sign up to watch a repo… then none of my text here has any point.

Can we mirror a branch named api2 on go.googlesource.com/proto to a branch named api2 on github.com/golang/protobuf?

I believe ideally to be down with the new go modules style would be to start a v2/ sub-directory under github.com/golang/protobuf or something like that. It would likely of course still need a whole different branch still though (to keep the v2.x.y tags attached to), and dealing with coördinating various checkins from two different versions into the same master branch would of course be somewhat alleviated by having the two separate filespaces (one in ./ the other in in v2/) there is still the wonderful issues around rapid development of the api2 stuff over the regular, and the headaches of the pace of checkins being merged to master from api2 to the mainline work.

Short answer: Ugh, that sounds like a bad idea, but then everything kind of sounds like it would be a bad idea…

@jhump
Copy link

jhump commented Aug 1, 2018

@puellanivis: I think you are thinking of googlecode.com. All Go stuff is (and has been?) hosted on go.googlesource.com.

Regarding creating a "v2" folder in the existing repo, the new Go module system only requires that the new version have a different import path; it does not require that it have the same import prefix but a version number suffix. So this new location for v2, IIUC, would have an import path of "golang.org/x/protobuf", which I think satisfies the module system requirement. (I could be wrong; I've been trying to keep up with the module system stuff but am not using vgo yet.)

@puellanivis
Copy link

I think the subdir of v2 is in part an expectation of allowing the “breaking change but continuity of module” to be explicit in the import path.

But if we’re moving toward a golang.org/x/protobuf import string, then continuity of import path and module is clearly unnecessary.

@dsnet
Copy link
Member Author

dsnet commented Aug 1, 2018

Let's avoid all discussion about import path. It's an important topic, but it is also orthogonal from the original post's purpose, which is to setup a repo for development. While import path is often identical to repo location, it also doesn't have to be.

@andybons
Copy link
Member

andybons commented Aug 1, 2018

https://go.googlesource.com/protobuf has been created. Have fun!

@andybons andybons closed this as completed Aug 1, 2018
@dsnet
Copy link
Member Author

dsnet commented Aug 1, 2018

I am manually mirror the Gerrit repo to the "api-v2" branch for anyone who wants to follow our progress on GitHub.

@golang golang locked and limited conversation to collaborators Aug 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants