fix: Fix per-file config interaction with one py_binary per main (#1664)

[This previous PR](#1584)
added the ability to make a `py_binary` target per file if `if __name__
== "__main__"` tokens were found in the file. This works great in the
default case, but when `python_generation_mode` is set to `file`, the
plugin now attempts to make both a `py_binary` and a `py_library` target
for each main file, which results in an error.

This PR modifies the behavior to work properly with per-file target
generation, and adds tests for this case.

Author: adzenith

CHANGELOG.md

@@ -57,6 +57,10 @@ A brief description of the categories of changes:
   from the target are not supported yet.
   ([#1612](https://github.com/bazelbuild/rules_python/issues/1612))
 
+* (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

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
 

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).

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

gazelle/python/testdata/binary_without_entrypoint_per_file_generation/BUILD.out

@@ -0,0 +1,46 @@
+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",
+        ":lib_and_main",
+    ],
+)
+
+py_binary(
+    name = "lib_and_main",
+    srcs = ["lib_and_main.py"],
+    visibility = ["//:__subpackages__"],
+)
+
+py_binary(
+    name = "main",
+    srcs = ["main.py"],
+    visibility = ["//:__subpackages__"],
+    deps = ["@pip//:pandas"],
+)
+
+py_binary(
+    name = "main2",
+    srcs = ["main2.py"],
+    visibility = ["//:__subpackages__"],
+    deps = [":lib2"],
+)

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.

gazelle/python/testdata/binary_without_entrypoint_per_file_generation/WORKSPACE

@@ -0,0 +1 @@
+# This is a Bazel workspace for the Gazelle test data.

gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib.py

@@ -0,0 +1,2 @@
+import numpy
+import pandas

gazelle/python/testdata/binary_without_entrypoint_per_file_generation/lib2.py

@@ -0,0 +1,2 @@
+import lib
+import lib_and_main

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()