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.

Sign up for GitHub

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

Jump to bottom

Migrate use of github.com/golang/protobuf to google.golang.org/protobuf #11869

Open
4 tasks
abhinav opened this issue Jan 13, 2023 · 0 comments
Open
4 tasks

Migrate use of github.com/golang/protobuf to google.golang.org/protobuf #11869

abhinav opened this issue Jan 13, 2023 · 0 comments
Labels
area/engine Pulumi engine kind/engineering Work that is not visible to an external user size/M Estimated effort to complete (up to 5 days).

Comments

@abhinav
Copy link
Contributor

abhinav commented Jan 13, 2023 •
edited by RobbieMcKinstry

github.com/golang/protobuf has been superseded by google.golang.org/protobuf.
Our codegen already uses the new variant, and the prior version is mostly a shim to the new version.

We have a handful of references to the deprecated modules across the codebase:

% rg --no-filename -g '*.go' 'github.com/golang/protobuf[\w/]+' -o | sort | uniq -c
   1 github.com/golang/protobuf/jsonpb
   2 github.com/golang/protobuf/proto
  26 github.com/golang/protobuf/ptypes/empty
   7 github.com/golang/protobuf/ptypes/struct

There's a near drop-in replacement for each package:

In fact, ptypes/empty and struct are just type aliases to the known/*pb variants now.

This should be safe to do, but we should still do this separately to reduce risk.

Creating this issue to track the upgrade.
@abhinav abhinav added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Jan 13, 2023
abhinav added a commit that referenced this issue Jan 13, 2023
@abhinav
golangci-lint: Enable staticcheck
09c9ea9 
Remove staticcheck from the list of disabled linters.
It's enabled by default in golangci-lint.

This also fixes minor remaining staticcheck issues
that don't merit their own pull requests,
or opts out of those that cannot be fixed yet.

Notably, we're opting out of:

- Resource.Name is deprecated (#9469)
- github.com/golang/protobuf is deprecated (#11869)
- strings.Title has been deprecated (#11870)

Besides that, other issues addressed in this change are:

```
// all issues are in pkg
codegen/schema/docs_parser.go:103:4: SA4006: this value of `text` is never used (staticcheck)
codegen/schema/loader.go:253:3: SA9003: empty branch (staticcheck)
resource/deploy/step_executor.go:328:12: SA9003: empty branch (staticcheck)
resource/deploy/step_generator.go:141:10: SA9003: empty branch (staticcheck)
codegen/pcl/invoke.go:97:10: SA9003: empty branch (staticcheck)
codegen/hcl2/model/type_const.go:57:2: SA9003: empty branch (staticcheck)
codegen/hcl2/model/type_enum.go:99:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck)
codegen/go/gen_test.go:399:19: SA4017: HasPrefix is a pure function but its return value is ignored (staticcheck)
```

Depends on #11857, #11858, #11859, #11860, #11862, #11865, #11866, 11867, #11868

Resolves #11808
@abhinav abhinav mentioned this issue Jan 13, 2023
Merged
abhinav added a commit that referenced this issue Jan 13, 2023
@abhinav
golangci-lint: Enable staticcheck
c517523 
Remove staticcheck from the list of disabled linters.
It's enabled by default in golangci-lint.

This also fixes minor remaining staticcheck issues
that don't merit their own pull requests,
or opts out of those that cannot be fixed yet.

Notably, we're opting out of:

- Resource.Name is deprecated (#9469)
- github.com/golang/protobuf is deprecated (#11869)
- strings.Title has been deprecated (#11870)

Besides that, other issues addressed in this change are:

```
// all issues are in pkg
codegen/schema/docs_parser.go:103:4: SA4006: this value of `text` is never used (staticcheck)
codegen/schema/loader.go:253:3: SA9003: empty branch (staticcheck)
resource/deploy/step_executor.go:328:12: SA9003: empty branch (staticcheck)
resource/deploy/step_generator.go:141:10: SA9003: empty branch (staticcheck)
codegen/pcl/invoke.go:97:10: SA9003: empty branch (staticcheck)
codegen/hcl2/model/type_const.go:57:2: SA9003: empty branch (staticcheck)
codegen/hcl2/model/type_enum.go:99:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck)
codegen/go/gen_test.go:399:19: SA4017: HasPrefix is a pure function but its return value is ignored (staticcheck)
```

Depends on #11857, #11858, #11859, #11860, #11862, #11865, #11866, #11867, #11868

Resolves #11808
abhinav added a commit that referenced this issue Jan 14, 2023
@abhinav
golangci-lint: Enable staticcheck
3d8dc3a 
Remove staticcheck from the list of disabled linters.
It's enabled by default in golangci-lint.

This also fixes minor remaining staticcheck issues
that don't merit their own pull requests,
or opts out of those that cannot be fixed yet.

Notably, we're opting out of:

- Resource.Name is deprecated (#9469)
- github.com/golang/protobuf is deprecated (#11869)
- strings.Title has been deprecated (#11870)

Besides that, other issues addressed in this change are:

```
// all issues are in pkg
codegen/schema/docs_parser.go:103:4: SA4006: this value of `text` is never used (staticcheck)
codegen/schema/loader.go:253:3: SA9003: empty branch (staticcheck)
resource/deploy/step_executor.go:328:12: SA9003: empty branch (staticcheck)
resource/deploy/step_generator.go:141:10: SA9003: empty branch (staticcheck)
codegen/pcl/invoke.go:97:10: SA9003: empty branch (staticcheck)
codegen/hcl2/model/type_const.go:57:2: SA9003: empty branch (staticcheck)
codegen/hcl2/model/type_enum.go:99:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck)
codegen/go/gen_test.go:399:19: SA4017: HasPrefix is a pure function but its return value is ignored (staticcheck)
```

Depends on #11857, #11858, #11859, #11860, #11862, #11865, #11866, #11867, #11868

Resolves #11808
@RobbieMcKinstry RobbieMcKinstry added kind/engineering Work that is not visible to an external user size/M Estimated effort to complete (up to 5 days). area/engine Pulumi engine and removed kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Jan 14, 2023
abhinav added a commit that referenced this issue Jan 15, 2023
@abhinav
golangci-lint: Enable staticcheck
d8c8c74 
Remove staticcheck from the list of disabled linters.
It's enabled by default in golangci-lint.

This also fixes minor remaining staticcheck issues
that don't merit their own pull requests,
or opts out of those that cannot be fixed yet.

Notably, we're opting out of:

- Resource.Name is deprecated (#9469)
- github.com/golang/protobuf is deprecated (#11869)
- strings.Title has been deprecated (#11870)

Besides that, other issues addressed in this change are:

```
// all issues are in pkg
codegen/schema/docs_parser.go:103:4: SA4006: this value of `text` is never used (staticcheck)
codegen/schema/loader.go:253:3: SA9003: empty branch (staticcheck)
resource/deploy/step_executor.go:328:12: SA9003: empty branch (staticcheck)
resource/deploy/step_generator.go:141:10: SA9003: empty branch (staticcheck)
codegen/pcl/invoke.go:97:10: SA9003: empty branch (staticcheck)
codegen/hcl2/model/type_const.go:57:2: SA9003: empty branch (staticcheck)
codegen/hcl2/model/type_enum.go:99:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck)
codegen/go/gen_test.go:399:19: SA4017: HasPrefix is a pure function but its return value is ignored (staticcheck)
```

Depends on #11857, #11858, #11859, #11860, #11862, #11865, #11866, #11867, #11868

Resolves #11808
bors bot added a commit that referenced this issue Jan 15, 2023
@bors @abhinav
Merge #11871
8b7e084 
11871: golangci-lint: Enable staticcheck r=RobbieMcKinstry a=abhinav

Remove staticcheck from the list of disabled linters.
It's enabled by default in golangci-lint.

This also fixes minor remaining staticcheck issues
that don't merit their own pull requests,
or opts out of those that cannot be fixed yet.

Notably, we're opting out of:

- Resource.Name is deprecated (#9469)
- github.com/golang/protobuf is deprecated (#11869)
- strings.Title has been deprecated (#11870)

Besides that, other issues addressed in this change are:

```
// all issues are in pkg
codegen/schema/docs_parser.go:103:4: SA4006: this value of `text` is never used (staticcheck)
codegen/schema/loader.go:253:3: SA9003: empty branch (staticcheck)
resource/deploy/step_executor.go:328:12: SA9003: empty branch (staticcheck)
resource/deploy/step_generator.go:141:10: SA9003: empty branch (staticcheck)
codegen/pcl/invoke.go:97:10: SA9003: empty branch (staticcheck)
codegen/hcl2/model/type_const.go:57:2: SA9003: empty branch (staticcheck)
codegen/hcl2/model/type_enum.go:99:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck)
codegen/go/gen_test.go:399:19: SA4017: HasPrefix is a pure function but its return value is ignored (staticcheck)
```

Depends on #11857, #11858, #11859, #11860, #11862, #11865, #11866, #11867, #11868

Resolves #11808

---

**NOTE**: This PR's base branch is currently #11868.
The base branch will be updated when that lands.

Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/engine Pulumi engine kind/engineering Work that is not visible to an external user size/M Estimated effort to complete (up to 5 days).
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@abhinav @RobbieMcKinstry