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

Using label as id for py targets #1023

Merged
merged 3 commits into from
Jan 27, 2023
Merged

Using label as id for py targets #1023

merged 3 commits into from
Jan 27, 2023

Conversation

linzhp
Copy link
Contributor

@linzhp linzhp commented Jan 27, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Gazelle plugin generates a UUID for each py_library target for py_binary and py_test in the same package to refer to. However, creating another identifier such as UUID is not necessary because Bazel already has id for each target, which is its label.

Furthermore, in some large monorepos where it's too slow for Gazelle to index the whole repo, we turn off the indexing with bazel run //:gazelle -- -index=false and rely on CrossResolver to resolve dependencies based on paths conventions. The UUID makes it impossible to resolve dependencies without indexing the whole repository.

What is the new behavior?

Reuse Bazel label as the id for the library and avoid depending on the UUID library. Now CrossResolvers can easily resolve same-package dependencies

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

If this approach is acceptable, I can create a follow up PR to rename variables to reflect the fact that we are no longer using UUID

@linzhp linzhp requested a review from f0rmiga as a code owner January 27, 2023 00:55
gazelle/python/generate.go Outdated Show resolved Hide resolved
@f0rmiga
Copy link
Collaborator

f0rmiga commented Jan 27, 2023

Does go mod tidy prune the uuid dependency?

Copy link
Collaborator

@f0rmiga f0rmiga left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comments.

@f0rmiga f0rmiga merged commit 58c7958 into bazelbuild:main Jan 27, 2023
@linzhp linzhp deleted the uuid branch January 27, 2023 21:50
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.

2 participants