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

chore: upgrade to Go 1.22 #1249

Merged
merged 10 commits into from
Feb 29, 2024
Merged

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Feb 29, 2024

Motivation

Part of #1248
We want to cut a release of celestia-core with Go 1.22 support to enable celestia-node to bump to Go 1.22.

Description

  1. Upgrade to Go 1.22
  2. Bump pyroscope deps
  3. Copy over golangci-lint config from CometBFT v0.34.x branch
  4. Resolve a few lint issues by copying code from CometBFT v0.34.x branch

@rootulp rootulp self-assigned this Feb 29, 2024
@rootulp
Copy link
Collaborator Author

rootulp commented Feb 29, 2024

Still observing build errors:

Error: ../../../go/pkg/mod/github.com/pyroscope-io/godeltaprof@v0.1.2/internal/pprof/delta_mutex.go:25:20: undefined: runtime_cyclesPerSecond
Error: ../../../go/pkg/mod/github.com/pyroscope-io/godeltaprof@v0.1.2/internal/pprof/proto.go:400:8: undefined: runtime_expandFinalInlineFrame

perhaps I have to bump godeltaprof? cc: @walldiss

Update: I think I need godeltaprof v0.1.5+

Ref: grafana/pyroscope-go#90 and grafana/pyroscope-go#86

@rootulp
Copy link
Collaborator Author

rootulp commented Feb 29, 2024

I can reproduce the proto-check-breaking error locally:

$ make proto-check-breaking
Checking for breaking changes in Protobuf files against local branch
Note: This is only useful if your changes have not yet been committed.
      Otherwise read up on buf's "breaking" command usage:
      https://docs.buf.build/breaking/usage
# github.com/bufbuild/buf/private/pkg/observabilityzap
../../../../go/pkg/mod/github.com/bufbuild/buf@v1.15.1/private/pkg/observabilityzap/tracer_provider_closer.go:25:30: cannot use &tracerProviderCloser{} (value of type *tracerProviderCloser) as "go.opentelemetry.io/otel/trace".TracerProvider value in variable declaration: *tracerProviderCloser does not implement "go.opentelemetry.io/otel/trace".TracerProvider (missing method tracerProvider)
../../../../go/pkg/mod/github.com/bufbuild/buf@v1.15.1/private/pkg/observabilityzap/observabilityzap.go:33:25: cannot use tracerProvider (variable of type *tracerProviderCloser) as "go.opentelemetry.io/otel/trace".TracerProvider value in argument to otel.SetTracerProvider: *tracerProviderCloser does not implement "go.opentelemetry.io/otel/trace".TracerProvider (missing method tracerProvider)

It seems odd that v1.15.1 is used because the latest is much higher: 1.29.0. See https://github.com/bufbuild/buf/releases

@rootulp
Copy link
Collaborator Author

rootulp commented Feb 29, 2024

A bunch of linters are now failing but a few of them are disabled upstream. See https://github.com/cometbft/cometbft/blob/v0.34.x/.golangci.yml so we should disable revive and nakedret and likely fix the rest.

@rootulp
Copy link
Collaborator Author

rootulp commented Feb 29, 2024

make lint on the cometbft v0.34.x branch doesn't report any issues so I think we actually need to fix these in this repo.

Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

thanks!!

@rootulp rootulp enabled auto-merge (squash) February 29, 2024 22:27
Copy link
Contributor

@staheri14 staheri14 left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -154,7 +154,7 @@ endif

proto-gen: check-proto-deps
@echo "Generating Protobuf files"
@go run github.com/bufbuild/buf/cmd/buf generate
@go run github.com/bufbuild/buf/cmd/buf@v1.29.0 generate
Copy link
Contributor

Choose a reason for hiding this comment

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

[Suggestion] To facilitate future refactoring and considering that a specific version of this repo has been designated for use, I recommend introducing a variable to hold this version number and employing it throughout the Makefile in order to simplify version updates in the future.

- libs/pubsub/query/query.peg.go
goconst:
ignore-tests: true
depguard:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice one 👍

@rootulp rootulp merged commit f95a434 into celestiaorg:v0.34.x-celestia Feb 29, 2024
17 checks passed
@TropicalDog17 TropicalDog17 mentioned this pull request Apr 28, 2024
3 tasks
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

3 participants