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

all: deprecate the module #1306

Merged
merged 1 commit into from
Mar 29, 2021
Merged

all: deprecate the module #1306

merged 1 commit into from
Mar 29, 2021

Conversation

dsnet
Copy link
Member

@dsnet dsnet commented Mar 25, 2021

Use the new deprecation feature to mark this module as deprecated.
See https://golang.org/issue/40357.

Considerations:

  • google.golang.org/protobuf/cmd/[email protected] and below
    used to generate a hard dependency on github.com/golang/protobuf,
    which would be frustrating since it would force an explicit dependency
    on a deprecated module. However, that is no longer the case in v1.26.0.
  • google.golang.org/protobuf and github.com/golang/protobuf have
    a cyclic dependency on each other. However, this should not be a problem
    since proposal 40357 only marks direct dependencies in the go.mod file,
    rather than all transitive dependencies in the go.sum file.

@dsnet
all: deprecate the module
5a07a66 
Use the new deprecation feature to mark this module as deprecated.
See https://golang.org/issue/40357.

Considerations:
* google.golang.org/protobuf/cmd/[email protected] and below
used to generate a hard dependency on github.com/golang/protobuf,
which would be frustrating since it would force an explicit dependency
on a deprecated module. However, that is no longer the case in v1.26.0.
* google.golang.org/protobuf and github.com/golang/protobuf have
a cyclic dependency on each other. However, this should not be a problem
since proposal 40357 only marks direct dependencies in the go.mod file,
rather than all transitive dependencies in the go.sum file.
@dsnet dsnet requested a review from neild March 25, 2021 03:29
@dsnet
Copy link
Member Author

dsnet commented Mar 25, 2021 •
edited
Loading

\cc @jayconrod, can you verify that the deprecating github.com/golang/protobuf won't be problematic given that github.com/golang/protobuf and google.golang.org/protobuf have a cyclic dependency on each other.

@jayconrod
Copy link

@dsnet I'm still implementing module deprecation, and this sounds like an interesting test case to add :)

Currently, the plan is to show deprecation messages in two scenarios:

  • When go list -m -u is run to check for updates on a deprecated module; go list -m -u all would show this.
  • When go get is run on a deprecated module or a package in a deprecated module needed to build one of the command line arguments.

Will users that depend on google.golang.org/protobuf end up transitively importing packages in github.com/golang/protobuf? If not, they'll probably only see a (deprecated) marker from go list -m -u all, which is probably fine.

If this ends up being too much of a nuisance, we could restrict deprecation warnings, e.g., to direct requirements only.

@dsnet
Copy link
Member Author

dsnet commented Mar 25, 2021

Will users that depend on google.golang.org/protobuf end up transitively importing packages in github.com/golang/protobuf?

Nope. The dependency on on github.com/golang/protobuf from google.golang.org/protobuf is only a weak module dependency. There are no packages in `github.com/golang/protobuf`` that are imported under normal build tags.

@dsnet dsnet merged commit ae97035 into master Mar 29, 2021
@dsnet dsnet deleted the deprecate branch March 29, 2021 18:21
@codefromthecrypt codefromthecrypt mentioned this pull request Mar 31, 2021
Closed
@amenzhinsky amenzhinsky mentioned this pull request Apr 5, 2021
Closed
@ilovesusu ilovesusu mentioned this pull request Jun 28, 2021
Closed
zoncoen added a commit to zoncoen/scenarigo that referenced this pull request Dec 28, 2021
@zoncoen
chore: use google.golang.org/protobuf only
c6967bd 
github.com/golang/protobuf is deprecated.
ref. golang/protobuf#1306
@zoncoen zoncoen mentioned this pull request Dec 28, 2021
Merged
bflad added a commit to hashicorp/terraform-plugin-go that referenced this pull request Jan 14, 2022
@bflad
Regenerate Protocol Buffers Go code to remove dependency on deprecate…
2178599 
…d Go module

Reference: golang/protobuf#1306

This does not appear to be generating any meaningful differences.

Updated via:

```console
pushd tfprotov5/internal/tfplugin5
./generate.sh
popd
pushd tfprotov6/internal/tfplugin6
./generate.sh
popd
go mod tidy
```
@bflad bflad mentioned this pull request Jan 14, 2022
Merged
bflad added a commit to hashicorp/terraform-plugin-go that referenced this pull request Jan 18, 2022
@bflad
Regenerate Protocol Buffers Go code to remove dependency on deprecate…
0e9a819 
…d Go module (#140)

Reference: golang/protobuf#1306

This does not appear to be generating any meaningful differences.

Updated via:

```console
pushd tfprotov5/internal/tfplugin5
./generate.sh
popd
pushd tfprotov6/internal/tfplugin6
./generate.sh
popd
go mod tidy
```
benma added a commit to benma/bitbox02-api-go that referenced this pull request Aug 15, 2022
@benma
deps: update protobuf
bd718df 
The module we used previouly, github.com/golang/protobuf/proto, is
deprecated. See https://github.com/golang/protobuf/releases/tag/v1.5.2
and golang/protobuf#1306.

We use the official succssor:

```
(fix imports)
go get google.golang.org/protobuf/[email protected]
go mod tidy
```

The .pb.go generated files also needd to be updated with an updated
protoc-gen-go:

```
go install google.golang.org/protobuf/cmd/[email protected]
```

A new script was added to generate the files, as the
`import_path=messages` flag seems to be removed in favour of the `M`
flag, which unfortunately has to be specified for each .proto file :(
@benma benma mentioned this pull request Aug 15, 2022
Merged
benma added a commit to benma/bitbox02-api-go that referenced this pull request Aug 15, 2022
@benma
deps: update protobuf
c42bf9e 
The module we used previouly, github.com/golang/protobuf/proto, is
deprecated. See https://github.com/golang/protobuf/releases/tag/v1.5.2
and golang/protobuf#1306.

We use the official succssor:

```
(fix imports)
go get google.golang.org/protobuf/[email protected]
go mod tidy
```

The .pb.go generated files also needed to be updated with an updated
protoc-gen-go:

```
go install google.golang.org/protobuf/cmd/[email protected]
```

A new script was added to generate the files, as the
`import_path=messages` flag seems to be removed in favour of the `M`
flag, which unfortunately has to be specified for each .proto file :(
benma added a commit to benma/bitbox02-api-go that referenced this pull request Aug 15, 2022
@benma
deps: update protobuf
ef8efcd 
The module we used previouly, github.com/golang/protobuf/proto, is
deprecated. See https://github.com/golang/protobuf/releases/tag/v1.5.2
and golang/protobuf#1306.

We use the official succssor:

```
(fix imports)
go get google.golang.org/protobuf/[email protected]
go mod tidy
```

The .pb.go generated files also needed to be updated with an updated
protoc-gen-go:

```
go install google.golang.org/protobuf/cmd/[email protected]
```

A new script was added to generate the files, as the
`import_path=messages` flag seems to be removed in favour of the `M`
flag, which unfortunately has to be specified for each .proto file :(

-----------

In `BTCSign()`, the inputs are not copied anymore due to:

```
api/firmware/btc.go:300:20: cannot use &input (value of type **messages.BTCSignInputRequest) as *messages.BTCSignInputRequest value in struct literal (typecheck)
					BtcSignInput: &input,
```
benma added a commit to benma/bitbox02-api-go that referenced this pull request Aug 16, 2022
@benma
deps: update protobuf
cdeb9ef 
The module we used previouly, github.com/golang/protobuf/proto, is
deprecated. See https://github.com/golang/protobuf/releases/tag/v1.5.2
and golang/protobuf#1306.

We use the official succssor:

```
(fix imports)
go get google.golang.org/protobuf/[email protected]
go mod tidy
```

The .pb.go generated files also needed to be updated with an updated
protoc-gen-go:

```
go install google.golang.org/protobuf/cmd/[email protected]
```

A new script was added to generate the files, as the
`import_path=messages` flag seems to be removed in favour of the `M`
flag, which unfortunately has to be specified for each .proto file :(

-----------

In `BTCSign()`, the inputs are not copied anymore due to:

```
api/firmware/btc.go:300:20: cannot use &input (value of type **messages.BTCSignInputRequest) as *messages.BTCSignInputRequest value in struct literal (typecheck)
					BtcSignInput: &input,
```
@matteo-pergolesi matteo-pergolesi mentioned this pull request Mar 29, 2023
Closed
@ohDaddyPlease ohDaddyPlease mentioned this pull request May 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neild neild neild approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dsnet @jayconrod @neild