Gazelle incorrectly ignores files called `setup.py`.

[dougthor42]

🐞 bug report

Affected Rule

• gazelle

Is this a regression?

No, seems like it's been present from the begining (a5a7ffbf4 / #514).

Description

Gazelle incorrectly ignores files called setup.py.

I believe this was added because the root setup.py is something that doesn't need a bazel target (I'm not actually sure if that's true...) or maybe it's because the root setup.py is annoying to run Gazelle on.

Anyway, the problem is that any file called setup.py will also be ignored.

I'm fine with the root setup.py being ignored, though I reserve the right to change my mind in the future :laughing:

This is the issue: https://github.com/bazelbuild/rules_python/blob/fa13b0138924245c55f79922509e4688f252ff71/gazelle/pythonconfig/pythonconfig.go#L128-L130

We can very easily update the test case to account for this.

🔬 Minimal Reproduction

Dir structure:

.
|-- src/
|   +-- setup.py
|
+-- MODULE.bazel

Expected result:

A //src/setup target is made in src/BUILD.bazel.

Actual result

A //src/setup target is not made in src/BUILD.bazel.

🔥 Exception or Error

N/A

🌍 Your Environment

Operating System:

gLinux

Output of bazel version:

7.2.0

Rules_python version:

0.31.0

Anything else relevant?

[dougthor42]

I see two main options for fixing:

1. Don't ignore any setup.py. This is the easiest fix, but is potentially breaking for other users.
   • The "potentially breaking" part has a pretty easy fix though: tell users to manually add # python_ignore_files setup.py in the root BUILD.bazel. This can be done in a CHANGELOG entry.
2. Update Config.ignoreFiles to support relative paths, and make the defaultIgnoreFiles specifically ./setup.py. This will be a larger, more complex change but less likely to break people.
   • IMO we don't need to, and probably shouldn't, support relative paths in the public python_ignore_files directive.

Thoughts? Other ideas?

[aignas]

I like the first approach more but would love to see if there are better options for the migration.

It could suddenly be the case where gazelle might start failing, but on the other hand pep621 is all about pyproject.toml files, so I would vote to mainly support that.

We could do a feature flag but then flipping it will not solve anything. We could add a warning though.

What about having 'default python ignore files' directive which is similar to what we did with ... some other ignore pattern options?

[dougthor42]

Sorry I haven't been able to follow up on this yet. I applied a patch and now implementing a proper fix has dropped down to a P3 :face_with_diagonal_mouth:.

Can you tell me what the original reasoning for ignoring setup.py was? I read through the comments in #514 but didn't see anything mentioning it. Maybe @f0rmiga or @alexeagle knows?

---

For anyone finding this before it gets officially fixed, this is what I did:

# in MODULE.bazel
single_version_override(
    module_name = "rules_python_gazelle_plugin",
    patch_strip = 2,  # remove "a/gazelle/"
    patches = ["//tools/bazel/patches:bazelbuild/rules_python/dont_ignore_setuppy_gh2108.patch"],
)

# tools/bazel/patches/bazelbuild/rules_python/dont_ignore_setuppy_gh2108.patch
diff --git a/gazelle/pythonconfig/pythonconfig.go b/gazelle/pythonconfig/pythonconfig.go
index aa925529..86a20c4f 100644
--- a/gazelle/pythonconfig/pythonconfig.go
+++ b/gazelle/pythonconfig/pythonconfig.go
@@ -109,7 +109,6 @@ const (
 // defaultIgnoreFiles is the list of default values used in the
 // python_ignore_files option.
 var defaultIgnoreFiles = map[string]struct{}{
-	"setup.py": {},
 }

 func SanitizeDistribution(distributionName string) string {

There are couple things to point out:

1. Obviously you can put the patch file where ever you want.
2. The patch file is only guaranteed to work for rules_python_gazelle_plugin == 0.33.1 as that's what we're currently running.

[f0rmiga]

Sorry I haven't been able to follow up on this yet. I applied a patch and now implementing a proper fix has dropped down to a P3 :face_with_diagonal_mouth:.

Can you tell me what the original reasoning for ignoring setup.py was? I read through the comments in #514 but didn't see anything mentioning it. Maybe @f0rmiga or @alexeagle knows?

---

For anyone finding this before it gets officially fixed, this is what I did:

# in MODULE.bazel

single_version_override(

    module_name = "rules_python_gazelle_plugin",

    patch_strip = 2,  # remove "a/gazelle/"

    patches = ["//tools/bazel/patches:bazelbuild/rules_python/dont_ignore_setuppy_gh2108.patch"],

)

# tools/bazel/patches/bazelbuild/rules_python/dont_ignore_setuppy_gh2108.patch

diff --git a/gazelle/pythonconfig/pythonconfig.go b/gazelle/pythonconfig/pythonconfig.go

index aa925529..86a20c4f 100644

--- a/gazelle/pythonconfig/pythonconfig.go

+++ b/gazelle/pythonconfig/pythonconfig.go

@@ -109,7 +109,6 @@ const (

 // defaultIgnoreFiles is the list of default values used in the

 // python_ignore_files option.

 var defaultIgnoreFiles = map[string]struct{}{

-	"setup.py": {},

 }



 func SanitizeDistribution(distributionName string) string {

There are couple things to point out:

1. Obviously you can put the patch file where ever you want.

2. The patch file is only guaranteed to work for rules_python_gazelle_plugin == 0.33.1 as that's what we're currently running.

I'm afraid I don't recall why we ignore it. On the other hand, why would you want to include a setup.py? Is it just a regular file that happens to have the same name as the well-known setup.py of a project?

[dougthor42]

Is it just a regular file that happens to have the same name as the well-known setup.py of a project?

Correct. We have a number of files that are called setup.py but do not live in the git (and bazel workspace) root like a normal "setup.py of a project" would:

$ find src/ -type f -name "setup.py"
src/pyle_xc/fab/hats/common_blocks/setup.py
src/pyle/sim/cirq/setup.py
src/pyle/cloud/bigquery/pipeline/setup.py
src/pyle/dataking/dc_measure/setup.py