Open dougthor42 opened 3 months ago
I see two main options for fixing:
setup.py
. This is the easiest fix, but is potentially breaking for other users.
# python_ignore_files setup.py
in the root BUILD.bazel
. This can be done in a CHANGELOG entry.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.
python_ignore_files
directive.Thoughts? Other ideas?
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?
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:
rules_python_gazelle_plugin == 0.33.1
as that's what we're currently running.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:
Obviously you can put the patch file where ever you want.
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?
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
π bug report
Affected Rule
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 rootsetup.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:
Expected result:
A
//src/setup
target is made insrc/BUILD.bazel
.Actual result
A
//src/setup
target is not made insrc/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?