Run Protobuild on GitHub Actions#1302
Conversation
|
|
| // A compilation error at this line likely means your copy of the | ||
| // proto package needs to be updated. | ||
| const _ = proto.GoGoProtoPackageIsVersion2 // please upgrade the proto package | ||
| const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package |
There was a problem hiding this comment.
Note that this is aligned with containerd.
e60508f to
c59138c
Compare
|
|
68fbf50 to
6dac14b
Compare
|
The tests are all green in my fork. https://github.com/kzys/hcsshim/actions/runs/1883043415 |
.github/workflows/ci.yml
Outdated
|
|
||
| jobs: | ||
| proto: | ||
| # TODO: use Windows after merginig https://github.com/containerd/protobuild/pull/51. |
There was a problem hiding this comment.
@TBBle Should I use Windows for consistency?
There was a problem hiding this comment.
I honestly don't know if it makes a difference. In fact, I hope it doesn't matter, as that would suggest something else is wrong, if we get different results from this based on platform.
When doing this by hand I did this on Windows myself, but the relevant procedure should be roughly the same, so maybe doing this validation on Linux is an extra level of prevention if a platform-specific difference does sneak in.
There was a problem hiding this comment.
#51 got merged! Let me update this PR. I agree that generating Go files from proto files is platform-independent. Running Protobuild on different platforms is something Protobuild has to do. So hcsshim doesn't have to do that.
.github/workflows/ci.yml
Outdated
| git clone --depth 1 https://github.com/containerd/containerd.git -b v1.6.0 | ||
| cd containerd/cmd/protoc-gen-gogoctrd | ||
| go build | ||
| mkdir $GOPATH/bin | ||
| mv protoc-gen-gogoctrd $GOPATH/bin |
There was a problem hiding this comment.
Is the inability to use
go install github.com/containerd/containerd/cmd/protoc-gen-gogoctrd@v1.6.0
a containerd bug?
I tried this out of curiosity, and it failed with
> go install github.com/containerd/containerd/cmd/protoc-gen-gogoctrd@v1.6.0
go install: github.com/containerd/containerd/cmd/protoc-gen-gogoctrd@v1.6.0 (in github.com/containerd/containerd@v1.6.0):
The go.mod file for the module providing named packages contains one or
more replace directives. It must not contain directives that would cause
it to be interpreted differently than if it were the main module.
but that might be me doing something wrong.
There was a problem hiding this comment.
It is sadly "working as expected". golang/go#44840
The good news is that I will remove protoc-gen-gogoctrd soon from the build process, once we remove gogo/protobuf.
c4a53a2 to
b576476
Compare
430008a to
603dd84
Compare
|
Needs a quick rebase |
|
@kzys Ping on the rebase (we added a new shim option here so the proto needs to be regenerated). Thanks! |
cf7c2cc to
fa1205f
Compare
containerd is planning to migrate off from github.com/gogo/protobuf which will affect hcsshim. See containerd/containerd#6564 for the overall progress. Before that, this commit runs Protobuild in GitHub Actions to make sure all generated files are reproducible from .proto files. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
|
@dcantah Thanks. Rebased the PR. |
containerd is planning to migrate off from github.com/gogo/protobuf which will affect hcsshim. See containerd/containerd#6564 for the overall progress. Before that, this commit runs Protobuild in GitHub Actions to make sure all generated files are reproducible from .proto files. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
containerd is planning to migrate off from github.com/gogo/protobuf which will affect hcsshim. See containerd/containerd#6564 for the overall progress.
Before that, this commit runs Protobuild in GitHub Actions to make sure all generated files are reproducible from .proto files.
Signed-off-by: Kazuyoshi Kato katokazu@amazon.com