fix(gazelle) Delete python targets with invalid srcs

[yushan26]

When running Gazelle, it generated the following target:

py_binary(
    name = "remove_py_binary",
    srcs = ["__main__.py"],
    main = "__main__.py",
    visibility = ["//visibility:public"],
)

After __main__.py was deleted and the change committed, re-running Gazelle did not remove the file from the srcs list.
This change introduces logic to check whether all entries in a Python target’s srcs attribute correspond to valid files. If none of them exist, the target is added to result.Empty to signal that it should be cleaned up. This cleanup behavior applies to when python_generation mode is package or file, as all srcs are expected to reside directly within the current directory.

[linzhp]

nit: we can inline this variable because it's only used once

[linzhp]

We also need to check:

1. whether it's in args.GenFiles
2. whether it's a target. If it starts with either @, // or :, we can assume it's a target. If not, we can see if it ends with ".py"
3. whether it is a glob

[dougthor42]

IMO this should be handled by properly supporting fix.

Specifically (emphasis mine):

update performs a safe set of tranformations [sic], while fix performs some additional transformations that may delete or rename rules.

We should not be deleting rules when running in update mode.

I know it's a PITA - my users complain about it too - but there are too many legitimate cases where a py_library rule exists with empty srcs value or the label in srcs doesn't point to anything (for example, maybe that src is created from a genrule? Or maybe it's created during CI outside of Bazel?)

[jayconrod]

@linzhp tagged me on this.

update should delete rules with empty srcs, if that makes sense for the language. That's intended. For Go, such a library cannot be built. If all sources are gone (deleted by the user?), then it makes sense for update to remove the rules.

I don't know Python well enough to say whether that same behavior makes sense here. What does a py_library do if srcs are empty?

The purpose of fix was to help migrate through incompatible changes in the ruleset or in Bazel. For example, Go used to have a cgo_library macro that was used on cgo code, which needed to be separate from the go_library. Eventually, that was deprecated and removed, so fix helped by squashing the cgo_library and go_library targets together. That had potential to break anything not managed by Gazelle that depended on cgo_library targets directly, so the distinction was made between fix and update.

These days, rule sets are a lot more stable, so fix doesn't need to do very much.

[dougthor42]

Ah, well I stand corrected then! Thanks for the background and info - I misunderstood the difference between update and fix (if I remember to, I'll update the bazel-gazelle docs).

What does a py_library do if srcs are empty?

It's valid. For example, we use it to handle circular dependencies:

# ########## START Autogenerated cycle targets and directives ##########
# See go/qos-doc-2024-24 for more info.
# cycle_c63218a5 (2 targets):
# gazelle:resolve py labrad.client //src:cycle_c63218a5
# gazelle:resolve py labrad.proto_over_labrad //src:cycle_c63218a5
py_library(
    name = "cycle_c63218a5",
    srcs = [],
    imports = ["."],
    tags = ["cycle"],
    visibility = ["//visibility:public"],
    deps = [
        "//src/labrad:client",
        "//src/labrad:proto_over_labrad",
    ],
)

In the above case, :client depends on :proto_over_labrad which depends on :client.

Why do we put the cycles in deps and not in srcs? With srcs, all our internal dependencies are pulled in, but anything from @pypi// doesn't get pulled in to the target's dependencies. With deps, everything gets pulled in.

So Gazelle deleting py_library with empty srcs would break things for us. It might be easy enough to # gazelle:keep them though.

[linzhp]

Currently, Gazelle is already cleaning up py_library when the srcs is empty in some cases:

rules_python/gazelle/python/generate.go

Lines 269 to 271 in 4e22d25

	// If we're doing per-file generation, srcs could be empty at this point, meaning we shouldn't make a py_library.
	// If there is already a package named py_library target before, we should generate an empty py_library.
	if srcs.Empty() {

The situation we are trying to fix is the per-file py_binary targets (no entry point): when the source file is removed, we would like to py_binary to be removed too, or the build would fail.

There are two ways to move forward with this PR:

1. keep the current approach and ask people to # keep those py_library targets with empty sources, like you suggested.
2. scope it down to py_binary only for now (maybe think about py_test in the future)

Let us know which approach you prefer.