Skip to content
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

generated files: update google/protobuf v1.3.5, protoc v3.14.0 and fix install for go modules #2146

Merged
merged 2 commits into from
Jun 1, 2021

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jun 1, 2021

generated files: update google/protobuf v1.3.5, and fix install for go modules

This patch updates the script and Dockerfile to prevent issues when using go
modules.

The first change is to use a plain git clone instead of go get -d to download
the source. While (like "pre go-modules") go get -d won't build the binaries
when go modules are used, it will use go modules to download the package
(and dependencies); as a result:

  • no git repository will be cloned in gopath
  • go modules will be downloaded for "master" (not the version that we specify
    to build)

Note: Once we update to Go 1.16, this script could be updated to use go install <repo>@version

The second change is an update to the update-generated-files script to detect
the correct version of google/protobuf to use from the go.mod's replace rule,
to make sure we generate (and verify) using the correct version.

The Dockerfile was also updated to update the default versions specified in the
PROTOBUF_VERSION and GOGO_VERSION build-args (although not strictly necessary).

Regenerating the files with this version resulted in a minor formatting change.

generated-files: update protoc to v3.11.4 to match google/protobuf

see https://github.com/golang/protobuf/blob/v1.3.5/.travis.yml#L15

Note that gogo/protobuf v1.3.2 updated protoc to v3.14.0; https://github.com/gogo/protobuf/releases/tag/v1.3.2
however, regenerating protobufs with this switched an import to use
google.golang.org/protobuf/types/known/timestamppb instead of
github.com/golang/protobuf/ptypes/timestamp, so using the older
version for now

Comment on lines 12 to 19
_ "github.com/golang/protobuf/ptypes/timestamp"
types "github.com/moby/buildkit/api/types"
pb "github.com/moby/buildkit/solver/pb"
github_com_moby_buildkit_util_entitlements "github.com/moby/buildkit/util/entitlements"
github_com_opencontainers_go_digest "github.com/opencontainers/go-digest"
grpc "google.golang.org/grpc"
codes "google.golang.org/grpc/codes"
status "google.golang.org/grpc/status"
_ "google.golang.org/protobuf/types/known/timestamppb"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated protoc in a separate commit for visibility, and in case this change is problematic

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's problematic indeed;

#28 [buildctl 1/1] RUN --mount=target=. --mount=target=/root/.cache,type=cache   --mount=target=/go/pkg/mod,type=cache   --mount=source=/tmp/.ldflags,target=/tmp/.ldflags,from=buildkit-version   xx-go build -ldflags "$(cat /tmp/.ldflags)" -o /usr/bin/buildctl ./cmd/buildctl &&   xx-verify --static /usr/bin/buildctl
#28 sha256:43c1d64f11a2dca8edd62a6ac448d7355a481e1faa15d331e267849e86aba17e
#28 0.434 build google.golang.org/protobuf/types/known/timestamppb: cannot load google.golang.org/protobuf/types/known/timestamppb: open /src/vendor/google.golang.org/protobuf/types/known/timestamppb: no such file or directory
#28 ERROR: executor failed running [/bin/sh -c xx-go build -ldflags "$(cat /tmp/.ldflags)" -o /usr/bin/buildctl ./cmd/buildctl &&   xx-verify --static /usr/bin/buildctl]: exit code: 1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me update to 3.11.4 instead (version used by google/protobuf); https://github.com/golang/protobuf/blob/v1.3.5/.travis.yml#L15

@thaJeztah thaJeztah force-pushed the fix_protobuf_install branch from c9296bd to 89bdbe0 Compare June 1, 2021 21:21
thaJeztah added 2 commits June 1, 2021 23:23
…o modules

This patch updates the script and Dockerfile to prevent issues when using go
modules.

The first change is to use a plain `git clone` instead of `go get -d` to download
the source. While (like "pre go-modules") `go get -d` won't *build* the binaries
when go modules are used, it *will* use go modules to download the package
(and dependencies); as a result:

- no git repository will be cloned in gopath
- go modules will be downloaded for "master" (not the version that we specify
  to build)

Note: Once we update to Go 1.16, this script could be updated to use `go install <repo>@version`

The second change is an update to the `update-generated-files` script to detect
the correct version of google/protobuf to use from the `go.mod`'s  `replace` rule,
to make sure we generate (and verify) using the correct version.

The Dockerfile was also updated to update the default versions specified in the
PROTOBUF_VERSION and GOGO_VERSION build-args (although not strictly necessary).

Regenerating the files with this version resulted in a minor formatting change.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
see https://github.com/golang/protobuf/blob/v1.3.5/.travis.yml#L15

Note that gogo/protobuf v1.3.2 updated protoc to v3.14.0; https://github.com/gogo/protobuf/releases/tag/v1.3.2
however, regenerating protobufs with this switched an import to use
google.golang.org/protobuf/types/known/timestamppb instead of
github.com/golang/protobuf/ptypes/timestamp, so using the older
version for now

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the fix_protobuf_install branch from 89bdbe0 to acbe79f Compare June 1, 2021 21:24
@thaJeztah thaJeztah mentioned this pull request Jun 1, 2021
@thaJeztah
Copy link
Member Author

@tonistiigi ptal 🤗

@tonistiigi tonistiigi merged commit 12cfc87 into moby:master Jun 1, 2021
@thaJeztah thaJeztah deleted the fix_protobuf_install branch June 1, 2021 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants