fix: Gazelle bug with merging py_binary targets in per-file mode and partial update

[jimmyt857]

This PR adds a new unit test. Currently, this is just a failing test without a fix, and I am still trying to understand the code well enough to find the root cause of the issue.

Our team uses Python+Gazelle in a monorepo, and we have a handful of directories with multiple .py files containing if __name__ == "__main__". Most of the time these are present for convenience or ad-hoc invocation. We're aware of the recommendation to split these into separate files, but that can cause clutter, and it is non-obvious to most engineers what to do when encountering this issue, which presents either as a misleading error message or a no-op without creating the appropriate targets.

Update
This bug occurs when ALL of the following are true:

• python_generation_mode is set to file.
• Multiple python binary files (files with if __name__ == "__main__") exist in the same directory.
• The directory has no __main__.py file.
• The BUILD file in the directory is partially complete, i.e. it contains py_binary targets for some of the python files, but not others.

In this situation, previously absent py_binary targets are merged into existing py_binary targets instead of being created as new targets.

[dougthor42]

Is there an Issue associated with this? We have been running the same setup you describe for quite some time now and haven't see any issues related to Gazelle-generated py_binary targets.

[jimmyt857]

There isn't an issue, but I'd be happy to open one if you like. I thought a PR with a testcase would be more helpful, because you can see the failing test artifact. Instead of the expected 2 py_binary targets, no change is made to the initial file (which starts with 1 of the py_binarys).

Also want to add how much our team appreciates the Gazelle tool for python. We have quite a few members who aren't as experienced with Bazel, and this was one of the main reasons we felt comfortable adopting Bazel.

[dougthor42]

Ah yes, very interesting!

Gazelle is merging the two rules, generating

py_binary(
    name = "a",
    srcs = ["b.py"],
    visibility = ["//:__subpackages__"],
)

where the name is a but the source file is b.py.

I'm able to reproduce this both with your branch and separately 👍.

---

It looks like Gazelle is correctly making the two pyBinary targets at this line (confirmation: injecting some logging prints both the a and b targets with the correct srcs values). That code is part of GenerateRules which is part of the interface for extending Gazelle.

Additionally, the returned GenerateResult has two values:

            gazelle: result.Gen: [0xc0000c6f00 0xc0000c7000]
            gazelle: result.Gen[0]: &{stmt:{index:0 deleted:false inserted:false updated:true comments:[] commentsUpdated:false expr:0xc0001fe1e0} kind:0xc0001bd500 args:[] attrs:map[name:{expr:0xc0001c4be0 val:a} srcs:{expr:0xc0001c4d20 val:[a.py]} visibility:{expr:0xc0001c4e60 val:[//:__subpackages__]}] private:map[_gazelle_python_resolved_deps:0xc000014148]}
            gazelle: result.Gen[1]: &{stmt:{index:0 deleted:false inserted:false updated:true comments:[] commentsUpdated:false expr:0xc0001fe2d0} kind:0xc0001bd7a0 args:[] attrs:map[name:{expr:0xc0001c4fa0 val:b} srcs:{expr:0xc0001c50e0 val:[b.py]} visibility:{expr:0xc0001c5220 val:[//:__subpackages__]}] private:map[_gazelle_python_resolved_deps:0xc0000141a0]}

So I wonder if it's something outside of our (rules_python's) control. I'll have to do more digging (though admittedly I don't know when I'll find the time...)

[jimmyt857]

@dougthor42 I came to the same conclusion as you, GenerateRules already provides the correct result. Digging a bit further, I figured it out!

The problem is here: https://github.com/bazelbuild/rules_python/blob/main/gazelle/python/kinds.go#L35

The documentation for this field confirms that this causes targets of this kind to be matched together, which must be doing some sort of merge operation.

Changing this to false fixes the test and all other tests still pass. This seems like a safe fix; AFAICT, this would only affect behavior in cases where there are multiple py_binary targets in 1 directory, which is the exact case where this bug is triggered.

[dougthor42]

Thanks for this! Things mostly LGTM, just a couple of nits.

[dougthor42]

LGTM!

[michael-christen]

Thanks for the fix!