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

util/tracing: remove incorrect import enforcing comment #2283

Merged
merged 1 commit into from
Aug 5, 2021

Conversation

thaJeztah
Copy link
Member

relates to docker/docker-ce-packaging#561

This import comment caused compilation of buildx to fail if GO111MODULE was
set to off:

Without GO111MODULE set (but with -mod=vendor:

echo $GO111MODULE

export PKG=github.com/docker/buildx
export LDFLAGS="-X ${PKG}/version.Version=$(git describe --match 'v[0-9]*' --always --tags) -X ${PKG}/version.Revision=$(git rev-parse HEAD) -X ${PKG}/version.Package=${PKG}"
GOFLAGS=-mod=vendor go build -o bin/docker-buildx -ldflags "${LDFLAGS}" ./cmd/buildx
bin/docker-buildx version
github.com/docker/buildx v0.6.0 d9ee3b134cbc2d09513fa7fee4176a3919e05887

When setting GO111MODULE=off, it fails on the incorrect import path in the
vendored file (looks like GO111MODULE=on ignores import-path comments?):

export GO111MODULE=off
root@5a55ec1c1eed:/go/src/github.com/docker/buildx# GOFLAGS=-mod=vendor go build -o bin/docker-buildx -ldflags "${LDFLAGS}" ./cmd/buildx
vendor/github.com/moby/buildkit/client/client.go:20:2: code in directory /go/src/github.com/docker/buildx/vendor/github.com/moby/buildkit/util/tracing/otlptracegrpc expects import "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/connection/connection.go:33:2: found import comments "go.opentelemetry.io/otel/exporters/otlp/internal/otlpconfig" (options.go) and "go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig" (optiontypes.go) in /go/src/github.com/docker/buildx/vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig

This import comment caused compilation of buildx to fail if `GO111MODULE` was
set to `off`:

Without `GO111MODULE` set (but with `-mod=vendor`:

    echo $GO111MODULE

    export PKG=github.com/docker/buildx
    export LDFLAGS="-X ${PKG}/version.Version=$(git describe --match 'v[0-9]*' --always --tags) -X ${PKG}/version.Revision=$(git rev-parse HEAD) -X ${PKG}/version.Package=${PKG}"
    GOFLAGS=-mod=vendor go build -o bin/docker-buildx -ldflags "${LDFLAGS}" ./cmd/buildx
    bin/docker-buildx version
    github.com/docker/buildx v0.6.0 d9ee3b134cbc2d09513fa7fee4176a3919e05887

When setting `GO111MODULE=off`, it fails on the incorrect import path in the
vendored file (looks like GO111MODULE=on ignores import-path comments?):

    export GO111MODULE=off
    root@5a55ec1c1eed:/go/src/github.com/docker/buildx# GOFLAGS=-mod=vendor go build -o bin/docker-buildx -ldflags "${LDFLAGS}" ./cmd/buildx
    vendor/github.com/moby/buildkit/client/client.go:20:2: code in directory /go/src/github.com/docker/buildx/vendor/github.com/moby/buildkit/util/tracing/otlptracegrpc expects import "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
    vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/connection/connection.go:33:2: found import comments "go.opentelemetry.io/otel/exporters/otlp/internal/otlpconfig" (options.go) and "go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig" (optiontypes.go) in /go/src/github.com/docker/buildx/vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

@crazy-max @AkihiroSuda PTAL

Do we / will we have a v0.9 release branch? We should probably include this in a v0.9.x patch release

@thaJeztah
Copy link
Member Author

Let me post for posterity; looks like a possibly flaky test (ran into that on the first run, but looks like it's passing now);

--- FAIL: TestClientGatewayIntegration (0.43s)
    --- PASS: TestClientGatewayIntegration/TestClientGatewaySolve/worker=containerd (4.86s)
    --- PASS: TestClientGatewayIntegration/TestClientGatewayContainerMounts/worker=containerd (5.78s)
    --- FAIL: TestClientGatewayIntegration/TestClientGatewaySlowCacheExecError/worker=containerd (6.50s)

Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

LGTM

Do we / will we have a v0.9 release branch? We should probably include this in a v0.9.x patch release

We can create a v0.9 branch sure. I have created the 0.9.1 milestone.

@crazy-max crazy-max added this to the v0.9.1 milestone Jul 28, 2021
@crazy-max crazy-max merged commit fc3c182 into moby:master Aug 5, 2021
crazy-max added a commit to crazy-max/buildkit that referenced this pull request Aug 5, 2021
…aints

util/tracing: remove incorrect import enforcing comment
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max crazy-max removed this from the v0.9.1 milestone Aug 5, 2021
@thaJeztah thaJeztah deleted the fix_invalid_import_constraints branch August 5, 2021 06:37
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.

None yet

2 participants