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

use carved out module for x/tools/go/vcs #3671

Merged
merged 1 commit into from
Aug 28, 2023
Merged

use carved out module for x/tools/go/vcs #3671

merged 1 commit into from
Aug 28, 2023

Conversation

malt3
Copy link
Contributor

@malt3 malt3 commented Aug 23, 2023

This is a part of #3654 that was carved out to make it smaller and easier to understand.
The vcs part of golang.org/x/tools was removed from tools. The vcs code is still available in a separate module with a version marked as "deprecated". Let's use it for now.

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

  • Use deprecated module golang.org/x/tools/go/vcs (the package was removed from golang.org/x/tools)

Which issues(s) does this PR fix?

You can now upgrade golang.org/x/tools and rules_go will no longer try to use the vcs package from that module.
Note however, that updating golang.org/x/tools still triggers #3640.
To fix this, we need to update golang.org/x/tools in rules_go and use the new function signature (as shown in d09bc19).

Other notes for review

We should probably vendor this code in the long term.

@malt3 malt3 marked this pull request as ready for review August 23, 2023 15:30
@hawkingrei
Copy link
Contributor

@fmeum @sluongng PTAL

Copy link
Contributor

@sluongng sluongng left a comment

Choose a reason for hiding this comment

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

LGTM. I am a bit concerned about the potential conflict of importpath for downstream users, but I think the usage of this package is rare enough and we could always advise users to upgrade/patch instead.

@hawkingrei
Copy link
Contributor

LGTM. I am a bit concerned about the potential conflict of importpath for downstream users, but I think the usage of this package is rare enough and we could always advise users to upgrade/patch instead.

Yes, I agree with you. TiDB use this package in some linters. we will upgrade rules_go with upgrading all linters to the latest one.

go/private/repositories.bzl Outdated Show resolved Hide resolved
This part of golang.org/x/tools was removed from tools.
The vcs code is still available in a separate module with a version marked as "deprecated".
Let's use it for now.
@fmeum fmeum merged commit 5206498 into bazel-contrib:master Aug 28, 2023
1 check passed
@sluongng
Copy link
Contributor

sluongng commented Sep 5, 2023

There are a few references in tests/integration/popular_repos/... that were missed by this PR.

But I will attempt to fix it when upgrade x/tools

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.

4 participants