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

fix(gazelle): __init__.py in per-file targets #1582

Merged
merged 9 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ A brief description of the categories of changes:
### Added

* (docs) bzlmod extensions are now documented on rules-python.readthedocs.io
* (gazelle) `file` generation mode can now also add `__init__.py` to the srcs
attribute for every target in the package. This is enabled through a separate
directive `python_generation_mode_per_file_include_init`.

[0.XX.0]: https://github.com/bazelbuild/rules_python/releases/tag/0.XX.0

Expand Down
2 changes: 2 additions & 0 deletions gazelle/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ Python-specific directives are as follows:
| Controls whether the Python import statements should be validated. Can be "true" or "false" | |
| `# gazelle:python_generation_mode`| `package` |
| Controls the target generation mode. Can be "file", "package", or "project" | |
| `# gazelle:python_generation_mode_per_file_include_init`| `package` |
| Controls whether `__init__.py` files are included as srcs in each generated target when target generation mode is "file". Can be "true", or "false" | |
| `# gazelle:python_library_naming_convention`| `$package_name$` |
| Controls the `py_library` naming convention. It interpolates \$package_name\$ with the Bazel package name. E.g. if the Bazel package name is `foo`, setting this to `$package_name$_my_lib` would result in a generated target named `foo_my_lib`. | |
| `# gazelle:python_binary_naming_convention` | `$package_name$_bin` |
Expand Down
7 changes: 7 additions & 0 deletions gazelle/python/configure.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ func (py *Configurer) KnownDirectives() []string {
pythonconfig.IgnoreDependenciesDirective,
pythonconfig.ValidateImportStatementsDirective,
pythonconfig.GenerationMode,
pythonconfig.GenerationModePerFileIncludeInit,
pythonconfig.LibraryNamingConvention,
pythonconfig.BinaryNamingConvention,
pythonconfig.TestNamingConvention,
Expand Down Expand Up @@ -149,6 +150,12 @@ func (py *Configurer) Configure(c *config.Config, rel string, f *rule.File) {
pythonconfig.GenerationMode, d.Value)
log.Fatal(err)
}
case pythonconfig.GenerationModePerFileIncludeInit:
v, err := strconv.ParseBool(strings.TrimSpace(d.Value))
if err != nil {
log.Fatal(err)
}
config.SetPerFileGenerationIncludeInit(v)
case pythonconfig.LibraryNamingConvention:
config.SetLibraryNamingConvention(strings.TrimSpace(d.Value))
case pythonconfig.BinaryNamingConvention:
Expand Down
29 changes: 20 additions & 9 deletions gazelle/python/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,18 +272,16 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
result.Imports = append(result.Imports, pyLibrary.PrivateAttr(config.GazelleImportsKey))
}
if cfg.PerFileGeneration() {
hasInit, nonEmptyInit := hasLibraryEntrypointFile(args.Dir)
pyLibraryFilenames.Each(func(index int, filename interface{}) {
if filename == pyLibraryEntrypointFilename {
stat, err := os.Stat(filepath.Join(args.Dir, filename.(string)))
if err != nil {
log.Fatalf("ERROR: %v\n", err)
}
if stat.Size() == 0 {
return // ignore empty __init__.py
}
pyLibraryTargetName := strings.TrimSuffix(filepath.Base(filename.(string)), ".py")
if filename == pyLibraryEntrypointFilename && !nonEmptyInit {
return // ignore empty __init__.py.
}
srcs := treeset.NewWith(godsutils.StringComparator, filename)
pyLibraryTargetName := strings.TrimSuffix(filepath.Base(filename.(string)), ".py")
if cfg.PerFileGenerationIncludeInit() && hasInit && nonEmptyInit {
srcs.Add(pyLibraryEntrypointFilename)
}
appendPyLibrary(srcs, pyLibraryTargetName)
})
} else if !pyLibraryFilenames.Empty() {
Expand Down Expand Up @@ -468,6 +466,19 @@ func hasEntrypointFile(dir string) bool {
return false
}

// hasLibraryEntrypointFile returns if the given directory has the library
// entrypoint file, and if it is non-empty.
func hasLibraryEntrypointFile(dir string) (bool, bool) {
stat, err := os.Stat(filepath.Join(dir, pyLibraryEntrypointFilename))
if os.IsNotExist(err) {
return false, false
}
if err != nil {
log.Fatalf("ERROR: %v\n", err)
}
return true, stat.Size() != 0
}

// isEntrypointFile returns whether the given path is an entrypoint file. The
// given path can be absolute or relative.
func isEntrypointFile(path string) bool {
Expand Down
14 changes: 10 additions & 4 deletions gazelle/python/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,17 @@ func (py *Resolver) Imports(c *config.Config, r *rule.Rule, f *rule.File) []reso
provides := make([]resolve.ImportSpec, 0, len(srcs)+1)
for _, src := range srcs {
ext := filepath.Ext(src)
if ext == ".py" {
pythonProjectRoot := cfg.PythonProjectRoot()
provide := importSpecFromSrc(pythonProjectRoot, f.Pkg, src)
provides = append(provides, provide)
if ext != ".py" {
continue
}
if cfg.PerFileGeneration() && len(srcs) > 1 && src == pyLibraryEntrypointFilename {
// Do not provide import spec from __init__.py when it is being included as
// part of another module.
continue
}
pythonProjectRoot := cfg.PythonProjectRoot()
provide := importSpecFromSrc(pythonProjectRoot, f.Pkg, src)
provides = append(provides, provide)
}
if len(provides) == 0 {
return nil
Expand Down
1 change: 1 addition & 0 deletions gazelle/python/testdata/per_file_non_empty_init/BUILD.in
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
load("@rules_python//python:defs.bzl", "py_library")

# gazelle:python_generation_mode file
# gazelle:python_generation_mode_per_file_include_init true
6 changes: 5 additions & 1 deletion gazelle/python/testdata/per_file_non_empty_init/BUILD.out
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
load("@rules_python//python:defs.bzl", "py_library")

# gazelle:python_generation_mode file
# gazelle:python_generation_mode_per_file_include_init true

py_library(
name = "__init__",
Expand All @@ -11,6 +12,9 @@ py_library(

py_library(
name = "foo",
srcs = ["foo.py"],
srcs = [
"__init__.py",
siddharthab marked this conversation as resolved.
Show resolved Hide resolved
"foo.py",
],
visibility = ["//:__subpackages__"],
)
1 change: 1 addition & 0 deletions gazelle/python/testdata/per_file_subdirs/bar/BUILD.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# gazelle:python_generation_mode_per_file_include_init true
16 changes: 15 additions & 1 deletion gazelle/python/testdata/per_file_subdirs/bar/BUILD.out
Original file line number Diff line number Diff line change
@@ -1,14 +1,28 @@
load("@rules_python//python:defs.bzl", "py_library", "py_test")

# gazelle:python_generation_mode_per_file_include_init true

py_library(
siddharthab marked this conversation as resolved.
Show resolved Hide resolved
name = "__init__",
srcs = ["__init__.py"],
visibility = ["//:__subpackages__"],
)

py_library(
name = "bar",
srcs = [
"__init__.py",
"bar.py",
],
visibility = ["//:__subpackages__"],
)

py_library(
name = "foo",
srcs = ["foo.py"],
srcs = [
"__init__.py",
"foo.py",
],
visibility = ["//:__subpackages__"],
)

Expand Down
Empty file.
87 changes: 53 additions & 34 deletions gazelle/pythonconfig/pythonconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ const (
// GenerationMode represents the directive that controls the target generation
// mode. See below for the GenerationModeType constants.
GenerationMode = "python_generation_mode"
// GenerationModePerFileIncludeInit represents the directive that augments
// the "per_file" GenerationMode by including the package's __init__.py file.
// This is a boolean directive.
GenerationModePerFileIncludeInit = "python_generation_mode_per_file_include_init"
siddharthab marked this conversation as resolved.
Show resolved Hide resolved
// LibraryNamingConvention represents the directive that controls the
// py_library naming convention. It interpolates $package_name$ with the
// Bazel package name. E.g. if the Bazel package name is `foo`, setting this
Expand Down Expand Up @@ -122,15 +126,16 @@ type Config struct {
pythonProjectRoot string
gazelleManifest *manifest.Manifest

excludedPatterns *singlylinkedlist.List
ignoreFiles map[string]struct{}
ignoreDependencies map[string]struct{}
validateImportStatements bool
coarseGrainedGeneration bool
perFileGeneration bool
libraryNamingConvention string
binaryNamingConvention string
testNamingConvention string
excludedPatterns *singlylinkedlist.List
ignoreFiles map[string]struct{}
ignoreDependencies map[string]struct{}
validateImportStatements bool
coarseGrainedGeneration bool
perFileGeneration bool
perFileGenerationIncludeInit bool
libraryNamingConvention string
binaryNamingConvention string
testNamingConvention string
}

// New creates a new Config.
Expand All @@ -139,18 +144,19 @@ func New(
pythonProjectRoot string,
) *Config {
return &Config{
extensionEnabled: true,
repoRoot: repoRoot,
pythonProjectRoot: pythonProjectRoot,
excludedPatterns: singlylinkedlist.New(),
ignoreFiles: make(map[string]struct{}),
ignoreDependencies: make(map[string]struct{}),
validateImportStatements: true,
coarseGrainedGeneration: false,
perFileGeneration: false,
libraryNamingConvention: packageNameNamingConventionSubstitution,
binaryNamingConvention: fmt.Sprintf("%s_bin", packageNameNamingConventionSubstitution),
testNamingConvention: fmt.Sprintf("%s_test", packageNameNamingConventionSubstitution),
extensionEnabled: true,
repoRoot: repoRoot,
pythonProjectRoot: pythonProjectRoot,
excludedPatterns: singlylinkedlist.New(),
ignoreFiles: make(map[string]struct{}),
ignoreDependencies: make(map[string]struct{}),
validateImportStatements: true,
coarseGrainedGeneration: false,
perFileGeneration: false,
perFileGenerationIncludeInit: false,
libraryNamingConvention: packageNameNamingConventionSubstitution,
binaryNamingConvention: fmt.Sprintf("%s_bin", packageNameNamingConventionSubstitution),
testNamingConvention: fmt.Sprintf("%s_test", packageNameNamingConventionSubstitution),
}
}

Expand All @@ -163,19 +169,20 @@ func (c *Config) Parent() *Config {
// current Config and sets itself as the parent to the child.
func (c *Config) NewChild() *Config {
return &Config{
parent: c,
extensionEnabled: c.extensionEnabled,
repoRoot: c.repoRoot,
pythonProjectRoot: c.pythonProjectRoot,
excludedPatterns: c.excludedPatterns,
ignoreFiles: make(map[string]struct{}),
ignoreDependencies: make(map[string]struct{}),
validateImportStatements: c.validateImportStatements,
coarseGrainedGeneration: c.coarseGrainedGeneration,
perFileGeneration: c.perFileGeneration,
libraryNamingConvention: c.libraryNamingConvention,
binaryNamingConvention: c.binaryNamingConvention,
testNamingConvention: c.testNamingConvention,
parent: c,
extensionEnabled: c.extensionEnabled,
repoRoot: c.repoRoot,
pythonProjectRoot: c.pythonProjectRoot,
excludedPatterns: c.excludedPatterns,
ignoreFiles: make(map[string]struct{}),
ignoreDependencies: make(map[string]struct{}),
validateImportStatements: c.validateImportStatements,
coarseGrainedGeneration: c.coarseGrainedGeneration,
perFileGeneration: c.perFileGeneration,
perFileGenerationIncludeInit: c.perFileGenerationIncludeInit,
libraryNamingConvention: c.libraryNamingConvention,
binaryNamingConvention: c.binaryNamingConvention,
testNamingConvention: c.testNamingConvention,
}
}

Expand Down Expand Up @@ -344,6 +351,18 @@ func (c *Config) PerFileGeneration() bool {
return c.perFileGeneration
}

// SetPerFileGenerationIncludeInit sets whether py_library targets should
// include __init__.py files when PerFileGeneration() is true.
func (c *Config) SetPerFileGenerationIncludeInit(includeInit bool) {
c.perFileGenerationIncludeInit = includeInit
}

// PerFileGenerationIncludeInit returns whether py_library targets should
// include __init__.py files when PerFileGeneration() is true.
func (c *Config) PerFileGenerationIncludeInit() bool {
return c.perFileGenerationIncludeInit
}

// SetLibraryNamingConvention sets the py_library target naming convention.
func (c *Config) SetLibraryNamingConvention(libraryNamingConvention string) {
c.libraryNamingConvention = libraryNamingConvention
Expand Down