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

Find and use Gazelle's repo config in bb fix #8270

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented Jan 30, 2025

In a mixed MODULE.bazel/WORKSPACE setup, go_deps has almost certainly been migrated, so prefer MODULE.bazel over WORKSPACE in that case to go through the Bzlmod path in bb fix.

In a mixed MODULE.bazel/WORKSPACE setup, `go_deps` has almost certainly been migrated, so prefer MODULE.bazel over WORKSPACE in that case to go through the Bzlmod path in `bb fix`.
@fmeum fmeum requested a review from sluongng January 30, 2025 19:45
@fmeum
Copy link
Contributor Author

fmeum commented Jan 31, 2025

@sluongng I wonder whether running a prebuilt gazelle is the best approach with Bzlmod. Setting up gazelle requires a single line in MODULE.bazel (which we could add automatically) and at that point we could delegate to bazel run @gazelle without worrying about Gazelle-internals. It will be slower on the first run, but equally fast with caching from there on. What do you think?

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 think the goal with bb fix is to provide a more seamless ootb experience for newer users. I don't mind delegating to Starlark Gazelle when it's found, but we should still have a fallback option of using the standalone embedded Gazelle for new users.

return "", fmt.Errorf("failed to find repo config file: %w", err)
}
if len(matches) == 0 {
return "", errors.New("failed to find repo config file, please run a build first")
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems like we are failing at this step in CI.

Perhaps we can materialize the repo by running a quick bazel query?

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