From 480e327e47bea4201a322f1a105b192aa342bfec Mon Sep 17 00:00:00 2001 From: yushan Date: Thu, 24 Apr 2025 22:07:44 +0000 Subject: [PATCH 1/5] Skip indexing py_binary rules Update update --- gazelle/python/resolve.go | 29 +++++++++++++++++++ .../BUILD.in | 3 ++ .../BUILD.out | 3 ++ .../README.md | 2 ++ .../WORKSPACE | 1 + .../bar/BUILD.in | 8 +++++ .../bar/BUILD.out | 8 +++++ .../bar/bar.py | 1 + .../foo/BUILD.in | 0 .../foo/BUILD.out | 13 +++++++++ .../foo/script.py | 3 ++ .../test.yaml | 17 +++++++++++ 12 files changed, 88 insertions(+) create mode 100644 gazelle/python/testdata/import_with_same_srcs_library_and_binary/BUILD.in create mode 100644 gazelle/python/testdata/import_with_same_srcs_library_and_binary/BUILD.out create mode 100644 gazelle/python/testdata/import_with_same_srcs_library_and_binary/README.md create mode 100644 gazelle/python/testdata/import_with_same_srcs_library_and_binary/WORKSPACE create mode 100644 gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/BUILD.in create mode 100644 gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/BUILD.out create mode 100644 gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/bar.py create mode 100644 gazelle/python/testdata/import_with_same_srcs_library_and_binary/foo/BUILD.in create mode 100644 gazelle/python/testdata/import_with_same_srcs_library_and_binary/foo/BUILD.out create mode 100644 gazelle/python/testdata/import_with_same_srcs_library_and_binary/foo/script.py create mode 100644 gazelle/python/testdata/import_with_same_srcs_library_and_binary/test.yaml diff --git a/gazelle/python/resolve.go b/gazelle/python/resolve.go index 413e69b289..780c2d1bd6 100644 --- a/gazelle/python/resolve.go +++ b/gazelle/python/resolve.go @@ -55,6 +55,9 @@ func (*Resolver) Name() string { return languageName } // If nil is returned, the rule will not be indexed. If any non-nil slice is // returned, including an empty slice, the rule will be indexed. func (py *Resolver) Imports(c *config.Config, r *rule.Rule, f *rule.File) []resolve.ImportSpec { + if !indexPyBinaryImport(r, f) { + return nil + } cfgs := c.Exts[languageName].(pythonconfig.Configs) cfg := cfgs[f.Pkg] srcs := r.AttrStrings("srcs") @@ -366,3 +369,29 @@ func convertDependencySetToExpr(set *treeset.Set) bzl.Expr { } return &bzl.ListExpr{List: deps} } + +// indexPyBinaryImport returns whether the corresponding py_binary rule need to be indexed. +// To avoid multiple labels indexing the same import, +// check if there is a corresponding py_library rule with the same srcs. +func indexPyBinaryImport(r *rule.Rule, f *rule.File) bool { + // If the rule is not a py_binary, it should be indexed. + if r.Kind() != "py_binary" { + return true + } + pyBinarySrcs := r.AttrStrings("srcs") + if len(pyBinarySrcs) == 0 { + return false + } + for _, otherRule := range f.Rules { + if otherRule.Kind() != "py_library" { + continue + } + pyLibrarySrcs := otherRule.AttrStrings("srcs") + for _, src := range pyLibrarySrcs { + if src == pyBinarySrcs[0] { + return false + } + } + } + return true +} diff --git a/gazelle/python/testdata/import_with_same_srcs_library_and_binary/BUILD.in b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/BUILD.in new file mode 100644 index 0000000000..fc43208db2 --- /dev/null +++ b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/BUILD.in @@ -0,0 +1,3 @@ +# gazelle:python_extension enabled +# gazelle:python_library_naming_convention py_default_library +# gazelle:python_generation_mode package diff --git a/gazelle/python/testdata/import_with_same_srcs_library_and_binary/BUILD.out b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/BUILD.out new file mode 100644 index 0000000000..fc43208db2 --- /dev/null +++ b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/BUILD.out @@ -0,0 +1,3 @@ +# gazelle:python_extension enabled +# gazelle:python_library_naming_convention py_default_library +# gazelle:python_generation_mode package diff --git a/gazelle/python/testdata/import_with_same_srcs_library_and_binary/README.md b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/README.md new file mode 100644 index 0000000000..39095b5aff --- /dev/null +++ b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/README.md @@ -0,0 +1,2 @@ +# Import with same srcs for library and binary +This test case asserts a file with py_library and py_binary rules that include the same .py file in srcs will resolve to the py_library rule correctly. diff --git a/gazelle/python/testdata/import_with_same_srcs_library_and_binary/WORKSPACE b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/WORKSPACE new file mode 100644 index 0000000000..faff6af87a --- /dev/null +++ b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/WORKSPACE @@ -0,0 +1 @@ +# This is a Bazel workspace for the Gazelle test data. diff --git a/gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/BUILD.in b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/BUILD.in new file mode 100644 index 0000000000..f976ef1c58 --- /dev/null +++ b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/BUILD.in @@ -0,0 +1,8 @@ +load("@rules_python//python:defs.bzl", "py_library") + +py_library( + name = "bar", + srcs = ["bar.py"], + visibility = ["//:__subpackages__"], + deps = ["//foo"], +) diff --git a/gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/BUILD.out b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/BUILD.out new file mode 100644 index 0000000000..a93d3710cb --- /dev/null +++ b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/BUILD.out @@ -0,0 +1,8 @@ +load("@rules_python//python:defs.bzl", "py_library") + +py_library( + name = "bar", + srcs = ["bar.py"], + visibility = ["//:__subpackages__"], + deps = ["//foo:py_default_library"], +) diff --git a/gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/bar.py b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/bar.py new file mode 100644 index 0000000000..9935a64811 --- /dev/null +++ b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/bar/bar.py @@ -0,0 +1 @@ +import foo.script diff --git a/gazelle/python/testdata/import_with_same_srcs_library_and_binary/foo/BUILD.in b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/foo/BUILD.in new file mode 100644 index 0000000000..e69de29bb2 diff --git a/gazelle/python/testdata/import_with_same_srcs_library_and_binary/foo/BUILD.out b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/foo/BUILD.out new file mode 100644 index 0000000000..fe6482a492 --- /dev/null +++ b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/foo/BUILD.out @@ -0,0 +1,13 @@ +load("@rules_python//python:defs.bzl", "py_binary", "py_library") + +py_binary( + name = "script", + srcs = ["script.py"], + visibility = ["//:__subpackages__"], +) + +py_library( + name = "py_default_library", + srcs = ["script.py"], + visibility = ["//:__subpackages__"], +) diff --git a/gazelle/python/testdata/import_with_same_srcs_library_and_binary/foo/script.py b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/foo/script.py new file mode 100644 index 0000000000..165f69654f --- /dev/null +++ b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/foo/script.py @@ -0,0 +1,3 @@ + +if __name__ == "__main__": + print("Hello, world!") diff --git a/gazelle/python/testdata/import_with_same_srcs_library_and_binary/test.yaml b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/test.yaml new file mode 100644 index 0000000000..2410223e59 --- /dev/null +++ b/gazelle/python/testdata/import_with_same_srcs_library_and_binary/test.yaml @@ -0,0 +1,17 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +--- +expect: + exit_code: 0 From aaef24431a6a7ef5b8204e1a6f4729f2a6822904 Mon Sep 17 00:00:00 2001 From: yushan Date: Fri, 6 Jun 2025 20:37:03 +0000 Subject: [PATCH 2/5] Update --- gazelle/python/generate.go | 8 +++++--- gazelle/python/resolve.go | 28 +--------------------------- 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go index 5eedbd9601..8729850660 100644 --- a/gazelle/python/generate.go +++ b/gazelle/python/generate.go @@ -123,6 +123,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes pyFileNames.Add(f) if !hasPyBinaryEntryPointFile && f == pyBinaryEntrypointFilename { hasPyBinaryEntryPointFile = true + pyLibraryFilenames.Add(f) } else if !hasPyTestEntryPointFile && f == pyTestEntrypointFilename { hasPyTestEntryPointFile = true } else if f == conftestFilename { @@ -247,9 +248,9 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes // 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) - } + // if cfg.PerFileGeneration() { + // srcs.Remove(name) + // } } sort.Strings(mainFileNames) for _, filename := range mainFileNames { @@ -352,6 +353,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes collisionErrors.Add(err) } + // Create the py_binary target that depends on the py_library pyBinaryTarget := newTargetBuilder(pyBinaryKind, pyBinaryTargetName, pythonProjectRoot, args.Rel, pyFileNames). setMain(pyBinaryEntrypointFilename). addVisibility(visibility). diff --git a/gazelle/python/resolve.go b/gazelle/python/resolve.go index 780c2d1bd6..faba222153 100644 --- a/gazelle/python/resolve.go +++ b/gazelle/python/resolve.go @@ -55,7 +55,7 @@ func (*Resolver) Name() string { return languageName } // If nil is returned, the rule will not be indexed. If any non-nil slice is // returned, including an empty slice, the rule will be indexed. func (py *Resolver) Imports(c *config.Config, r *rule.Rule, f *rule.File) []resolve.ImportSpec { - if !indexPyBinaryImport(r, f) { + if r.Kind() == "py_binary" { return nil } cfgs := c.Exts[languageName].(pythonconfig.Configs) @@ -369,29 +369,3 @@ func convertDependencySetToExpr(set *treeset.Set) bzl.Expr { } return &bzl.ListExpr{List: deps} } - -// indexPyBinaryImport returns whether the corresponding py_binary rule need to be indexed. -// To avoid multiple labels indexing the same import, -// check if there is a corresponding py_library rule with the same srcs. -func indexPyBinaryImport(r *rule.Rule, f *rule.File) bool { - // If the rule is not a py_binary, it should be indexed. - if r.Kind() != "py_binary" { - return true - } - pyBinarySrcs := r.AttrStrings("srcs") - if len(pyBinarySrcs) == 0 { - return false - } - for _, otherRule := range f.Rules { - if otherRule.Kind() != "py_library" { - continue - } - pyLibrarySrcs := otherRule.AttrStrings("srcs") - for _, src := range pyLibrarySrcs { - if src == pyBinarySrcs[0] { - return false - } - } - } - return true -} From b1bd1e76fd470e4a3ca77d3371d02a8a0ceb8796 Mon Sep 17 00:00:00 2001 From: yushan Date: Tue, 24 Jun 2025 22:01:02 +0000 Subject: [PATCH 3/5] Update --- gazelle/python/generate.go | 10 +++++----- gazelle/python/resolve.go | 8 +++++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go index 8729850660..f37fb717d7 100644 --- a/gazelle/python/generate.go +++ b/gazelle/python/generate.go @@ -48,8 +48,8 @@ var ( buildFilenames = []string{"BUILD", "BUILD.bazel"} ) -func GetActualKindName(kind string, args language.GenerateArgs) string { - if kindOverride, ok := args.Config.KindMap[kind]; ok { +func GetActualKindName(kind string, c *config.Config) string { + if kindOverride, ok := c.KindMap[kind]; ok { return kindOverride.KindName } return kind @@ -90,9 +90,9 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes } } - actualPyBinaryKind := GetActualKindName(pyBinaryKind, args) - actualPyLibraryKind := GetActualKindName(pyLibraryKind, args) - actualPyTestKind := GetActualKindName(pyTestKind, args) + actualPyBinaryKind := GetActualKindName(pyBinaryKind, args.Config) + actualPyLibraryKind := GetActualKindName(pyLibraryKind, args.Config) + actualPyTestKind := GetActualKindName(pyTestKind, args.Config) pythonProjectRoot := cfg.PythonProjectRoot() diff --git a/gazelle/python/resolve.go b/gazelle/python/resolve.go index faba222153..28cdeb873f 100644 --- a/gazelle/python/resolve.go +++ b/gazelle/python/resolve.go @@ -55,11 +55,13 @@ func (*Resolver) Name() string { return languageName } // If nil is returned, the rule will not be indexed. If any non-nil slice is // returned, including an empty slice, the rule will be indexed. func (py *Resolver) Imports(c *config.Config, r *rule.Rule, f *rule.File) []resolve.ImportSpec { - if r.Kind() == "py_binary" { - return nil - } cfgs := c.Exts[languageName].(pythonconfig.Configs) cfg := cfgs[f.Pkg] + if !cfg.PerFileGeneration() && GetActualKindName(r.Kind(), c) == pyBinaryKind { + // Don't index py_binary in except in file mode, because all non-test Python files + // are in py_library already. + return nil + } srcs := r.AttrStrings("srcs") provides := make([]resolve.ImportSpec, 0, len(srcs)+1) for _, src := range srcs { From 443fd7c429ccc2d1cef4f5468db0ee2e9606ef96 Mon Sep 17 00:00:00 2001 From: yushan Date: Tue, 24 Jun 2025 22:10:55 +0000 Subject: [PATCH 4/5] Update --- gazelle/python/generate.go | 18 ++++++++---------- gazelle/python/resolve.go | 5 ----- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go index f37fb717d7..5eedbd9601 100644 --- a/gazelle/python/generate.go +++ b/gazelle/python/generate.go @@ -48,8 +48,8 @@ var ( buildFilenames = []string{"BUILD", "BUILD.bazel"} ) -func GetActualKindName(kind string, c *config.Config) string { - if kindOverride, ok := c.KindMap[kind]; ok { +func GetActualKindName(kind string, args language.GenerateArgs) string { + if kindOverride, ok := args.Config.KindMap[kind]; ok { return kindOverride.KindName } return kind @@ -90,9 +90,9 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes } } - actualPyBinaryKind := GetActualKindName(pyBinaryKind, args.Config) - actualPyLibraryKind := GetActualKindName(pyLibraryKind, args.Config) - actualPyTestKind := GetActualKindName(pyTestKind, args.Config) + actualPyBinaryKind := GetActualKindName(pyBinaryKind, args) + actualPyLibraryKind := GetActualKindName(pyLibraryKind, args) + actualPyTestKind := GetActualKindName(pyTestKind, args) pythonProjectRoot := cfg.PythonProjectRoot() @@ -123,7 +123,6 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes pyFileNames.Add(f) if !hasPyBinaryEntryPointFile && f == pyBinaryEntrypointFilename { hasPyBinaryEntryPointFile = true - pyLibraryFilenames.Add(f) } else if !hasPyTestEntryPointFile && f == pyTestEntrypointFilename { hasPyTestEntryPointFile = true } else if f == conftestFilename { @@ -248,9 +247,9 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes // 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) - // } + if cfg.PerFileGeneration() { + srcs.Remove(name) + } } sort.Strings(mainFileNames) for _, filename := range mainFileNames { @@ -353,7 +352,6 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes collisionErrors.Add(err) } - // Create the py_binary target that depends on the py_library pyBinaryTarget := newTargetBuilder(pyBinaryKind, pyBinaryTargetName, pythonProjectRoot, args.Rel, pyFileNames). setMain(pyBinaryEntrypointFilename). addVisibility(visibility). diff --git a/gazelle/python/resolve.go b/gazelle/python/resolve.go index 28cdeb873f..413e69b289 100644 --- a/gazelle/python/resolve.go +++ b/gazelle/python/resolve.go @@ -57,11 +57,6 @@ func (*Resolver) Name() string { return languageName } func (py *Resolver) Imports(c *config.Config, r *rule.Rule, f *rule.File) []resolve.ImportSpec { cfgs := c.Exts[languageName].(pythonconfig.Configs) cfg := cfgs[f.Pkg] - if !cfg.PerFileGeneration() && GetActualKindName(r.Kind(), c) == pyBinaryKind { - // Don't index py_binary in except in file mode, because all non-test Python files - // are in py_library already. - return nil - } srcs := r.AttrStrings("srcs") provides := make([]resolve.ImportSpec, 0, len(srcs)+1) for _, src := range srcs { From 373a0d19e972d710be7ae6edd642c4d8f674de74 Mon Sep 17 00:00:00 2001 From: yushan Date: Tue, 24 Jun 2025 22:12:07 +0000 Subject: [PATCH 5/5] Update --- gazelle/python/generate.go | 10 +++++----- gazelle/python/resolve.go | 5 +++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go index 5eedbd9601..52ab1245f3 100644 --- a/gazelle/python/generate.go +++ b/gazelle/python/generate.go @@ -48,8 +48,8 @@ var ( buildFilenames = []string{"BUILD", "BUILD.bazel"} ) -func GetActualKindName(kind string, args language.GenerateArgs) string { - if kindOverride, ok := args.Config.KindMap[kind]; ok { +func GetActualKindName(kind string, c *config.Config) string { + if kindOverride, ok := c.KindMap[kind]; ok { return kindOverride.KindName } return kind @@ -90,9 +90,9 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes } } - actualPyBinaryKind := GetActualKindName(pyBinaryKind, args) - actualPyLibraryKind := GetActualKindName(pyLibraryKind, args) - actualPyTestKind := GetActualKindName(pyTestKind, args) + actualPyBinaryKind := GetActualKindName(pyBinaryKind, args.Config) + actualPyLibraryKind := GetActualKindName(pyLibraryKind, args.Config) + actualPyTestKind := GetActualKindName(pyTestKind, args.Config) pythonProjectRoot := cfg.PythonProjectRoot() diff --git a/gazelle/python/resolve.go b/gazelle/python/resolve.go index 413e69b289..28cdeb873f 100644 --- a/gazelle/python/resolve.go +++ b/gazelle/python/resolve.go @@ -57,6 +57,11 @@ func (*Resolver) Name() string { return languageName } func (py *Resolver) Imports(c *config.Config, r *rule.Rule, f *rule.File) []resolve.ImportSpec { cfgs := c.Exts[languageName].(pythonconfig.Configs) cfg := cfgs[f.Pkg] + if !cfg.PerFileGeneration() && GetActualKindName(r.Kind(), c) == pyBinaryKind { + // Don't index py_binary in except in file mode, because all non-test Python files + // are in py_library already. + return nil + } srcs := r.AttrStrings("srcs") provides := make([]resolve.ImportSpec, 0, len(srcs)+1) for _, src := range srcs {