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

nogo: detect deprecated go standard library packages #92783

Merged
merged 1 commit into from
Dec 2, 2022

Conversation

healthy-pod
Copy link
Contributor

In #87327, we patched staticcheck to detect deprecated "objects" from the standard library. This patch ensures that we also detect deprecated "packages".

Closes #84877

Release note: None

@healthy-pod healthy-pod requested a review from a team as a code owner December 1, 2022 01:30
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rickystewart
Copy link
Collaborator

[01:41:54 ](https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_UnitTests_BazelBuildLinuxArmCross/7781604?buildTab=log&focusLine=5194&linesState=5194)    pkg/security/securityassets/security_assets.go:14:2: "io/ioutil" has been deprecated since Go 1.16 (SA1019)

@healthy-pod
Copy link
Contributor Author

healthy-pod commented Dec 1, 2022

So it looks like we still have work to do here. We either want to remove all uses of io/ioutil or find a way that let's us ignore excluded uses like what we do in testutil. [context]

@rickystewart
Copy link
Collaborator

@healthy-pod

In pkg/security/securitytest/embedded.go and pkg/roachprod/vm/aws/embedded.go you have a call to ioutil.WriteFile. This can just be replaced by os.WriteFile. For the other uses of ioutil.ReadDir, one thing we can do is enumerate the existing uses, file bugs, and assign them to appropriate owners so they can be fixed by someone who knows what they're doing.

Uses of ReadDir:

pkg/roachprod/install/cluster_synced.go
pkg/security/securityassets/security_assets.go
pkg/security/securitytest/securitytest.go
pkg/server/dumpstore/dumpstore.go
pkg/server/dumpstore/dumpstore_test.go
pkg/server/heapprofiler/profilestore_test.go
pkg/util/log/file_api.go

Possible some of these can be easily fixed too. You can exclude these files in exclude_files for the analyzer in build/bazelutil/nogo_config.json.

In cockroachdb#87327, we patched `staticcheck` to detect deprecated
"objects" from the standard library. This patch ensures that
we also detect deprecated "packages".

Closes cockroachdb#84877

Release note: None
@healthy-pod
Copy link
Contributor Author

Alright it's looking good now TFTR!

I also opened the following issues to alert teams: #92864 #92862 #92861

bors r=rickystewart

@craig
Copy link
Contributor

craig bot commented Dec 2, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Dec 2, 2022

Build succeeded:

@craig craig bot merged commit e7b15eb into cockroachdb:master Dec 2, 2022
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.

bazel: nogo does not seem to catch all deprecation errors
3 participants