From b4efd85118f36f9750ddb4336d9343ff415058e8 Mon Sep 17 00:00:00 2001 From: Nikolaus Wittenstein Date: Tue, 2 Jan 2024 12:10:25 -0800 Subject: [PATCH 1/3] Fix per-file config interaction with one py_binary per main --- gazelle/python/generate.go | 35 ++++++++++++------ .../BUILD.in | 4 ++ .../BUILD.out | 37 +++++++++++++++++++ .../README.md | 4 ++ .../WORKSPACE | 1 + .../lib.py | 2 + .../lib2.py | 1 + .../main.py | 4 ++ .../main2.py | 4 ++ .../test.yaml | 17 +++++++++ 10 files changed, 97 insertions(+), 12 deletions(-) create mode 100644 gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.in create mode 100644 gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.out create mode 100644 gazelle/python/testdata/binary_without_entrypoint_per_file_generation/README.md create mode 100644 gazelle/python/testdata/binary_without_entrypoint_per_file_generation/WORKSPACE create mode 100644 gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib.py create mode 100644 gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib2.py create mode 100644 gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main.py create mode 100644 gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main2.py create mode 100644 gazelle/python/testdata/binary_without_entrypoint_per_file_generation/test.yaml diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go index 95f5396f78..ba273be2b7 100644 --- a/gazelle/python/generate.go +++ b/gazelle/python/generate.go @@ -225,23 +225,17 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes log.Fatalf("ERROR: %v\n", err) } - // Check if a target with the same name we are generating already - // exists, and if it is of a different kind from the one we are - // generating. If so, we have to throw an error since Gazelle won't - // generate it correctly. - if err := ensureNoCollision(args.File, pyLibraryTargetName, actualPyLibraryKind); err != nil { - fqTarget := label.New("", args.Rel, pyLibraryTargetName) - err := fmt.Errorf("failed to generate target %q of kind %q: %w. "+ - "Use the '# gazelle:%s' directive to change the naming convention.", - fqTarget.String(), actualPyLibraryKind, err, pythonconfig.LibraryNamingConvention) - collisionErrors.Add(err) - } - if !hasPyBinaryEntryPointFile { // Creating one py_binary target per main module when __main__.py doesn't exist. mainFileNames := make([]string, 0, len(mainModules)) for name := range mainModules { mainFileNames = append(mainFileNames, name) + + // Remove the file from srcs if we're doing per-file library generation so + // that we don't also generate a py_library target for it. + if cfg.PerFileGeneration() { + srcs.Remove(name) + } } sort.Strings(mainFileNames) for _, filename := range mainFileNames { @@ -262,6 +256,23 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes } } + // If we're doing per-file generation, srcs could be empty at this point, meaning we shouldn't make a py_library. + if srcs.Empty() { + return + } + + // Check if a target with the same name we are generating already + // exists, and if it is of a different kind from the one we are + // generating. If so, we have to throw an error since Gazelle won't + // generate it correctly. + if err := ensureNoCollision(args.File, pyLibraryTargetName, actualPyLibraryKind); err != nil { + fqTarget := label.New("", args.Rel, pyLibraryTargetName) + err := fmt.Errorf("failed to generate target %q of kind %q: %w. "+ + "Use the '# gazelle:%s' directive to change the naming convention.", + fqTarget.String(), actualPyLibraryKind, err, pythonconfig.LibraryNamingConvention) + collisionErrors.Add(err) + } + pyLibrary := newTargetBuilder(pyLibraryKind, pyLibraryTargetName, pythonProjectRoot, args.Rel, pyFileNames). addVisibility(visibility). addSrcs(srcs). diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.in b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.in new file mode 100644 index 0000000000..b24a82339d --- /dev/null +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.in @@ -0,0 +1,4 @@ +# gazelle:python_generation_mode file + +# gazelle:resolve py numpy @pip//:numpy +# gazelle:resolve py pandas @pip//:pandas diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.out b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.out new file mode 100644 index 0000000000..1489832396 --- /dev/null +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.out @@ -0,0 +1,37 @@ +load("@rules_python//python:defs.bzl", "py_binary", "py_library") + +# gazelle:python_generation_mode file + +# gazelle:resolve py numpy @pip//:numpy +# gazelle:resolve py pandas @pip//:pandas + +py_library( + name = "lib", + srcs = ["lib.py"], + visibility = ["//:__subpackages__"], + deps = [ + "@pip//:numpy", + "@pip//:pandas", + ], +) + +py_library( + name = "lib2", + srcs = ["lib2.py"], + visibility = ["//:__subpackages__"], + deps = [":lib"], +) + +py_binary( + name = "main", + srcs = ["main.py"], + visibility = ["//:__subpackages__"], + deps = ["@pip//:pandas"], +) + +py_binary( + name = "main2", + srcs = ["main2.py"], + visibility = ["//:__subpackages__"], + deps = [":lib2"], +) diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/README.md b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/README.md new file mode 100644 index 0000000000..9cbe3e9e72 --- /dev/null +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/README.md @@ -0,0 +1,4 @@ +# Binary without entrypoint + +This test case asserts that when there is no __main__.py, a py_binary is generated per file main module, and that this +py_binary is instead of (not in addition to) any py_library target. diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/WORKSPACE b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/WORKSPACE new file mode 100644 index 0000000000..faff6af87a --- /dev/null +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/WORKSPACE @@ -0,0 +1 @@ +# This is a Bazel workspace for the Gazelle test data. diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib.py b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib.py new file mode 100644 index 0000000000..3e1e6b8dd2 --- /dev/null +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib.py @@ -0,0 +1,2 @@ +import numpy +import pandas diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib2.py b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib2.py new file mode 100644 index 0000000000..153e65dab7 --- /dev/null +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib2.py @@ -0,0 +1 @@ +import lib diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main.py b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main.py new file mode 100644 index 0000000000..a068203844 --- /dev/null +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main.py @@ -0,0 +1,4 @@ +import pandas + +if __name__ == "__main__": + run() diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main2.py b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main2.py new file mode 100644 index 0000000000..98b3e83a40 --- /dev/null +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main2.py @@ -0,0 +1,4 @@ +import lib2 + +if __name__ == "__main__": + run() diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/test.yaml b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/test.yaml new file mode 100644 index 0000000000..2410223e59 --- /dev/null +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/test.yaml @@ -0,0 +1,17 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +--- +expect: + exit_code: 0 From 43db643202f56157dc1c6a8ee3e8bcae98c54577 Mon Sep 17 00:00:00 2001 From: Nikolaus Wittenstein Date: Wed, 3 Jan 2024 09:14:01 -0800 Subject: [PATCH 2/3] Add lib_and_main.py test --- .../BUILD.out | 11 ++++++++++- .../lib2.py | 1 + .../lib_and_main.py | 6 ++++++ .../main2.py | 2 +- 4 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib_and_main.py diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.out b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.out index 1489832396..bffedb1e27 100644 --- a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.out +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.out @@ -19,7 +19,16 @@ py_library( name = "lib2", srcs = ["lib2.py"], visibility = ["//:__subpackages__"], - deps = [":lib"], + deps = [ + ":lib", + ":lib_and_main", + ], +) + +py_binary( + name = "lib_and_main", + srcs = ["lib_and_main.py"], + visibility = ["//:__subpackages__"], ) py_binary( diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib2.py b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib2.py index 153e65dab7..592a2dab8f 100644 --- a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib2.py +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib2.py @@ -1 +1,2 @@ import lib +import lib_and_main diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib_and_main.py b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib_and_main.py new file mode 100644 index 0000000000..c6e2d49c94 --- /dev/null +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib_and_main.py @@ -0,0 +1,6 @@ +def library_func(): + print("library_func") + + +if __name__ == "__main__": + library_func() diff --git a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main2.py b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main2.py index 98b3e83a40..6f923b82c0 100644 --- a/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main2.py +++ b/gazelle/python/testdata/binary_without_entrypoint_per_file_generation/main2.py @@ -1,4 +1,4 @@ import lib2 if __name__ == "__main__": - run() + lib2.lib_and_main.library_func() From 957345e5506aa23dfa3ae186ed5e2fb1fc11caff Mon Sep 17 00:00:00 2001 From: Nikolaus Wittenstein Date: Mon, 8 Jan 2024 12:17:48 -0800 Subject: [PATCH 3/3] Update documentation for py_binary generation --- CHANGELOG.md | 4 ++++ gazelle/README.md | 16 ++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56013267e5..e6af80f76b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,10 @@ A brief description of the categories of changes: * (toolchains) `py_runtime` can now take an executable target. Note: runfiles from the target are not supported yet. +* (gazelle) When `python_generation_mode` is set to `file`, create one `py_binary` + target for each file with `if __name__ == "__main__"` instead of just one + `py_binary` for the whole module. + ### Fixed * (gazelle) The gazelle plugin helper was not working with Python toolchains 3.11 diff --git a/gazelle/README.md b/gazelle/README.md index a9a69ccf22..2e2337ab5d 100644 --- a/gazelle/README.md +++ b/gazelle/README.md @@ -250,8 +250,20 @@ When no such entry point exists, Gazelle will look for a line like this in the t if __name == "__main__": ``` -Gazelle will create `py_binary` target will be created for every module with such line, with the target name -being the same as module name. +Gazelle will create a `py_binary` target for every module with such a line, with +the target name the same as the module name. + +If `python_generation_mode` is set to `file`, then instead of one `py_binary` +target per module, Gazelle will create one `py_binary` target for each file with +such a line, and the name of the target will match the name of the script. + +Note that it's possible for another script to depend on a `py_binary` target and +import from the `py_binary`'s scripts. This can have possible negative effects on +Bazel analysis time and runfiles size compared to depending on a `py_library` +target. The simplest way to avoid these negative effects is to extract library +code into a separate script without a `main` line. Gazelle will then create a +`py_library` target for that library code, and other scripts can depend on that +`py_library` target. ## Developer Notes