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

fix(gazelle): _actually_ don't write narrow dependencies. #1683

Closed
wants to merge 5 commits into from
Closed

fix(gazelle): _actually_ don't write narrow dependencies. #1683

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0a1955d
Cease write narrow visibilities
aryamccarthy Jan 11, 2024
7d740aa
Update generate.go
aryamccarthy Jan 11, 2024
11640e3
Fix unused var issue
aryamccarthy Jan 11, 2024
4f31891
Update tests to match new visibility behavior
aryamccarthy Jan 12, 2024
7b31e91
Delete accidentally included MODULE files
aryamccarthy Jan 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions gazelle/python/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
}

parser := newPython3Parser(args.Config.RepoRoot, args.Rel, cfg.IgnoresDependency)
visibility := fmt.Sprintf("//%s:__subpackages__", pythonProjectRoot)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, this is desirable in usecases where you have a monorepo, where you have many requirements.txt files that you don't want to mix. This change would be a regression in that front.

The go plugin for gazelle has directives to control the visibility as is documented in https://github.com/bazelbuild/bazel-gazelle, maybe introducing a directive would be useful here as well?

Copy link

Choose a reason for hiding this comment

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

With this PR you can still get the old behavior by adding this to your Python root BUILD file:

package(default_visibility = ["//" + package_name() + ":__subpackages__"])

There's some more context in this previous PR that accidentally included the wrong change but the right PR description.


var result language.GenerateResult
result.Gen = make([]*rule.Rule, 0)
Expand Down Expand Up @@ -247,7 +246,6 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
continue
}
pyBinary := newTargetBuilder(pyBinaryKind, pyBinaryTargetName, pythonProjectRoot, args.Rel, pyFileNames).
addVisibility(visibility).
addSrc(filename).
addModuleDependencies(mainModules[filename]).
generateImportsAttribute().build()
Expand All @@ -274,7 +272,6 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
}

pyLibrary := newTargetBuilder(pyLibraryKind, pyLibraryTargetName, pythonProjectRoot, args.Rel, pyFileNames).
addVisibility(visibility).
addSrcs(srcs).
addModuleDependencies(allDeps).
generateImportsAttribute().
Expand Down Expand Up @@ -322,7 +319,6 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes

pyBinaryTarget := newTargetBuilder(pyBinaryKind, pyBinaryTargetName, pythonProjectRoot, args.Rel, pyFileNames).
setMain(pyBinaryEntrypointFilename).
addVisibility(visibility).
addSrc(pyBinaryEntrypointFilename).
addModuleDependencies(deps).
generateImportsAttribute()
Expand Down Expand Up @@ -354,7 +350,6 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
conftestTarget := newTargetBuilder(pyLibraryKind, conftestTargetname, pythonProjectRoot, args.Rel, pyFileNames).
addSrc(conftestFilename).
addModuleDependencies(deps).
addVisibility(visibility).
setTestonly().
generateImportsAttribute()

Expand Down