8000 feat: Creating one py_binary per main module (#1584) · tanan/rules_python@6ffb04e · GitHub
[go: up one dir, main page]
Skip to content {"props":{"docsUrl":"https://docs.github.com/get-started/accessibility/keyboard-shortcuts"}}

Commit 6ffb04e

Browse files
linzhplinzhp
authored
feat: Creating one py_binary per main module (bazel-contrib#1584)
Many existing Python repos don't use `__main__.py` to indicate the the main module. Instead, they put something like below in any Python files: ```python if __name__ == "__main__": main() ``` This PR makes the Gazelle extension able to recognize main modules like this, when `__main__.py` doesn't exist. This reduces the need to create `__main__.py` when enabling Gazelle extensions in existing Python repos. The current behavior of creating single `py_binary` for `__main__.py` is preserved and takes precedence. So this is a backward-compatible change. Closes bazel-contrib#1566.
1 parent 27450f9 commit 6ffb04e

File tree

17 files changed

+253
-60
lines changed

17 files changed

+253
-60
lines changed

‎examples/bzlmod/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ compile_pip_requirements_3_9(
2525
# with pip-compile.
2626
compile_pip_requirements_3_10(
2727
name = "requirements_3_10",
28+
timeout = "moderate",
2829
src = "requirements.in",
2930
requirements_txt = "requirements_lock_3_10.txt",
3031
requirements_windows = "requirements_windows_3_10.txt",

‎gazelle/python/BUILD.bazel

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
load("@bazel_gazelle//:def.bzl", "gazelle_binary")
22
load("@io_bazel_rules_go//go:def.bzl", "go_library")
3-
load("@rules_python//python:defs.bzl", "py_binary")
3+
load("@rules_python//python:defs.bzl", "py_binary", "py_test")
44
load(":gazelle_test.bzl", "gazelle_test")
55

66
go_library(
@@ -58,6 +58,15 @@ py_binary(
5858
visibility = ["//visibility:public"],
5959
)
6060

61+
py_test(
62+
name = "parse_test",
63+
srcs = [
64+
"parse.py",
65+
"parse_test.py",
66+
],
67+
imports = ["."],
68+
)
69+
6170
filegroup(
6271
name = "helper.zip",
6372
srcs = [":helper"],

‎gazelle/python/__main__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
if __name__ == "__main__":
2525
if len(sys.argv) < 2:
26-
sys.exit("Please provide subcommand, either print or std_modules")
26+
sys.exit("Please provide subcommand, either parse or std_modules")
2727
if sys.argv[1] == "parse":
2828
sys.exit(parse.main(sys.stdin, sys.stdout))
2929
elif sys.argv[1] == "std_modules":

‎gazelle/python/generate.go

Lines changed: 65 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"log"
2121
"os"
2222
"path/filepath"
23+
"sort"
2324
"strings"
2425

2526
"github.com/bazelbuild/bazel-gazelle/config"
@@ -89,9 +90,9 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
8990
pyTestFilenames := treeset.NewWith(godsutils.StringComparator)
9091
pyFileNames := treeset.NewWith(godsutils.StringComparator)
9192

92-
// hasPyBinary controls whether a py_binary target should be generated for
93+
// hasPyBinaryEntryPointFile controls whether a single py_binary target should be generated for
9394
// this package or not.
94-
hasPyBinary := false
95+
hasPyBinaryEntryPointFile := false
9596

9697
// hasPyTestEntryPointFile and hasPyTestEntryPointTarget control whether a py_test target should
9798
// be generated for this package or not.
@@ -106,8 +107,8 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
106107
ext := filepath.Ext(f)
107108
if ext == ".py" {
108109
pyFileNames.Add(f)
109-
if !hasPyBinary && f == pyBinaryEntrypointFilename {
110-
hasPyBinary = true
110+
if !hasPyBinaryEntryPointFile && f == pyBinaryEntrypointFilename {
111+
hasPyBinaryEntryPointFile = true
111112
} else if !hasPyTestEntryPointFile && f == pyTestEntrypointFilename {
112113
hasPyTestEntryPointFile = true
113114
} else if f == conftestFilename {
@@ -219,7 +220,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
219220
collisionErrors := singlylinkedlist.New()
220221

221222
appendPyLibrary := func(srcs *treeset.Set, pyLibraryTargetName string) {
222-
deps, err := parser.parse(srcs)
223+
deps, mainModules, err := parser.parse(srcs)
223224
if err != nil {
224225
log.Fatalf("ERROR: %v\n", err)
225226
}
@@ -228,16 +229,33 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
228229
// exists, and if it is of a different kind from the one we are
229230
// generating. If so, we have to throw an error since Gazelle won't
230231
// generate it correctly.
231-
if args.File != nil {
232-
for _, t := range args.File.Rules {
233-
if t.Name() == pyLibraryTargetName && t.Kind() != actualPyLibraryKind {
234-
fqTarget := label.New("", args.Rel, pyLibraryTargetName)
235-
err := fmt.Errorf("failed to generate target %q of kind %q: "+
236-
"a target of kind %q with the same name already exists. "+
237-
"Use the '# gazelle:%s' directive to change the naming convention.",
238-
fqTarget.String(), actualPyLibraryKind, t.Kind(), pythonconfig.LibraryNamingConvention)
239-
collisionErrors.Add(err)
232+
if err := ensureNoCollision(args.File, pyLibraryTargetName, actualPyLibraryKind); err != nil {
233+
fqTarget := label.New("", args.Rel, pyLibraryTargetName)
234+
err := fmt.Errorf("failed to generate target %q of kind %q: %w. "+
235+
"Use the '# gazelle:%s' directive to change the naming convention.",
236+
fqTarget.String(), actualPyLibraryKind, err, pythonconfig.LibraryNamingConvention)
237+
collisionErrors.Add(err)
238+
}
239+
240+
if !hasPyBinaryEntryPointFile {
241+
sort.Strings(mainModules)
242+
// Creating one py_binary target per main module when __main__.py doesn't exist.
243+
for _, filename := range mainModules {
244+
pyBinaryTargetName := strings.TrimSuffix(filepath.Base(filename), ".py")
245+
if err := ensureNoCollision(args.File, pyBinaryTargetName, actualPyBinaryKind); err != nil {
246+
fqTarget := label.New("", args.Rel, pyBinaryTargetName)
247+
log.Printf("failed to generate target %q of kind %q: %v",
248+
fqTarget.String(), actualPyBinaryKind, err)
249+
continue
240250
}
251+
srcs.Remove(filename)
252+
pyBinary := newTargetBuilder(pyBinaryKind, pyBinaryTargetName, pythonProjectRoot, args.Rel, pyFileNames).
253+
addVisibility(visibility).
254+
addSrc(filename).
255+
addModuleDependencies(deps).
256+
generateImportsAttribute().build()
257+
result.Gen = append(result.Gen, pyBinary)
258+
result.Imports = append(result.Imports, pyBinary.PrivateAttr(config.GazelleImportsKey))
241259
}
242260
}
243261

@@ -270,8 +288,8 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
270288
appendPyLibrary(pyLibraryFilenames, cfg.RenderLibraryName(packageName))
271289
}
272290

273-
if hasPyBinary {
274-
deps, err := parser.parseSingle(pyBinaryEntrypointFilename)
291+
if hasPyBinaryEntryPointFile {
292+
deps, _, err := parser.parseSingle(pyBinaryEntrypointFilename)
275293
if err != nil {
276294
log.Fatalf("ERROR: %v\n", err)
277295
}
@@ -282,17 +300,12 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
282300
// exists, and if it is of a different kind from the one we are
283301
// generating. If so, we have to throw an error since Gazelle won't
284302
// generate it correctly.
285-
if args.File != nil {
286-
for _, t := range args.File.Rules {
287-
if t.Name() == pyBinaryTargetName && t.Kind() != actualPyBinaryKind {
288-
fqTarget := label.New("", args.Rel, pyBinaryTargetName)
289-
err := fmt.Errorf("failed to generate target %q of kind %q: "+
290-
"a target of kind %q with the same name already exists. "+
291-
"Use the '# gazelle:%s' directive to change the naming convention.",
292-
fqTarget.String(), actualPyBinaryKind, t.Kind(), pythonconfig.BinaryNamingConvention)
293-
collisionErrors.Add(err)
294-
}
295-
}
303+
if err := ensureNoCollision(args.File, pyBinaryTargetName, actualPyBinaryKind); err != nil {
304+
fqTarget := label.New("", args.Rel, pyBinaryTargetName)
305+
err := fmt.Errorf("failed to generate target %q of kind %q: %w. "+
306+
"Use the '# gazelle:%s' directive to change the naming convention.",
307+
fqTarget.String(), actualPyBinaryKind, err, pythonconfig.BinaryNamingConvention)
308+
collisionErrors.Add(err)
296309
}
297310

298311
pyBinaryTarget := newTargetBuilder(pyBinaryKind, pyBinaryTargetName, pythonProjectRoot, args.Rel, pyFileNames).
@@ -310,7 +323,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
310323

311324
var conftest *rule.Rule
312325
if hasConftestFile {
313-
deps, err := parser.parseSingle(conftestFilename)
326+
deps, _, err := parser.parseSingle(conftestFilename)
314327
if err != nil {
315328
log.Fatalf("ERROR: %v\n", err)
316329
}
@@ -319,16 +332,11 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
319332
// exists, and if it is of a different kind from the one we are
320333
// generating. If so, we have to throw an error since Gazelle won't
321334
// generate it correctly.
322-
if args.File != nil {
323-
for _, t := range args.File.Rules {
324-
if t.Name() == conftestTargetname && t.Kind() != actualPyLibraryKind {
325-
fqTarget := label.New("", args.Rel, conftestTargetname)
326-
err := fmt.Errorf("failed to generate target %q of kind %q: "+
327-
"a target of kind %q with the same name already exists.",
328-
fqTarget.String(), actualPyLibraryKind, t.Kind())
329-
collisionErrors.Add(err)
330-
}
331-
}
335+
if err := ensureNoCollision(args.File, conftestTargetname, actualPyLibraryKind); err != nil {
336+
fqTarget := label.New("", args.Rel, conftestTargetname)
337+
err := fmt.Errorf("failed to generate target %q of kind %q: %w. ",
338+
fqTarget.String(), actualPyLibraryKind, err)
339+
collisionErrors.Add(err)
332340
}
333341

334342
conftestTarget := newTargetBuilder(pyLibraryKind, conftestTargetname, pythonProjectRoot, args.Rel, pyFileNames).
@@ -346,25 +354,20 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
346354

347355
var pyTestTargets []*targetBuilder
348356
newPyTestTargetBuilder := func(srcs *treeset.Set, pyTestTargetName string) *targetBuilder {
349-
deps, err := parser.parse(srcs)
357+
deps, _, err := parser.parse(srcs)
350358
if err != nil {
351359
log.Fatalf("ERROR: %v\n", err)
352360
}
353361
// Check if a target with the same name we are generating already
354362
// exists, and if it is of a different kind from the one we are
355363
// generating. If so, we have to throw an error since Gazelle won't
356364
// generate it correctly.
357-
if args.File != nil {
358-
for _, t := range args.File.Rules {
359-
if t.Name() == pyTestTargetName && t.Kind() != actualPyTestKind {
360-
fqTarget := label.New("", args.Rel, pyTestTargetName)
361-
err := fmt.Errorf("failed to generate target %q of kind %q: "+
362-
"a target of kind %q with the same name already exists. "+
363-
"Use the '# gazelle:%s' directive to change the naming convention.",
364-
fqTarget.String(), actualPyTestKind, t.Kind(), pythonconfig.TestNamingConvention)
365-
collisionErrors.Add(err)
366-
}
367-
}
365+
if err := ensureNoCollision(args.File, pyTestTargetName, actualPyTestKind); err != nil {
366+
fqTarget := label.New("", args.Rel, pyTestTargetName)
367+
err := fmt.Errorf("failed to generate target %q of kind %q: %w. "+
368+
"Use the '# gazelle:%s' directive to change the naming convention.",
369+
fqTarget.String(), actualPyTestKind, err, pythonconfig.TestNamingConvention)
370+
collisionErrors.Add(err)
368371
}
369372
return newTargetBuilder(pyTestKind, pyTestTargetName, pythonProjectRoot, args.Rel, pyFileNames).
370373
addSrcs(srcs).
@@ -476,3 +479,15 @@ func isEntrypointFile(path string) bool {
476479
return false
477480
}
478481
}
482+
483+
func ensureNoCollision(file *rule.File, targetName, kind string) error {
484+
if 10000 file == nil {
485+
return nil
486+
}
487+
for _, t := range file.Rules {
488+
if t.Name() == targetName && t.Kind() != kind {
489+
return fmt.Errorf("a target of kind %q with the same name already exists", t.Kind())
490+
}
491+
}
492+
return nil
493+
}

‎gazelle/python/parse.py

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import os
2323
import sys
2424
from io import BytesIO
25-
from tokenize import COMMENT, tokenize
25+
from tokenize import COMMENT, NAME, OP, STRING, tokenize
2626

2727

2828
def parse_import_statements(content, filepath):
@@ -59,6 +59,30 @@ def parse_comments(content):
5959
return comments
6060

6161

62+
def parse_main(content):
63+
g = tokenize(BytesIO(content.encode("utf-8")).readline)
64+
for token_type, token_val, start, _, _ in g:
65+
if token_type != NAME or token_val != "if" or start[1] != 0:
66+
continue
67+
try:
68+
token_type, token_val, start, _, _ = next(g)
69+
if token_type != NAME or token_val != "__name__":
70+
continue
71+
token_type, token_val, start, _, _ = next(g)
72+
if token_type != OP or token_val != "==":
73+
continue
74+
token_type, token_val, start, _, _ = next(g)
75+
if token_type != STRING or token_val.strip("\"'") != '__main__':
76+
continue
77+
token_type, token_val, start, _, _ = next(g)
78+
if token_type != OP or token_val != ":":
79+
continue
80+
return True
81+
except StopIteration:
82+
break
83+
return False
84+
85+
6286
def parse(repo_root, rel_package_path, filename):
6387
rel_filepath = os.path.join(rel_package_path, filename)
6488
abs_filepath = os.path.join(repo_root, rel_filepath)
@@ -70,11 +94,16 @@ def parse(repo_root, rel_package_path, filename):
7094
parse_import_statements, content, rel_filepath
7195
)
7296
comments_future = executor.submit(parse_comments, content)
97+
main_future = executor.submit(parse_main, content)
7398
modules = modules_future.result()
7499
comments = comments_future.result()
100+
has_main = main_future.result()
101+
75102
output = {
103+
"filename": filename,
76104
"modules": modules,
77105
"comments": comments,
106+
"has_main": has_main,
78107
}
79108
return output
80109

‎gazelle/python/parse_test.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import unittest
2+
import parse
3+
4+
class TestParse(unittest.TestCase):
5+
def test_not_has_main(self):
6+
content = "a = 1\nb = 2"
7+
self.assertFalse(parse.parse_main(content))
8+
9+
def test_has_main_in_function(self):
10+
content = """
11+
def foo():
12+
if __name__ == "__main__":
13+
a = 3
14+
"""
15+
self.assertFalse(parse.parse_main(content))
16+
17+
def test_has_main(self):
18+
content = """
19+
import unittest
20+
21+
from lib import main
22+
23+
24+
class ExampleTest(unittest.TestCase):
25+
def test_main(self):
26+
self.assertEqual(
27+
"",
28+
main([["A", 1], ["B", 2]]),
29+
)
30+
31+
32+
if __name__ == "__main__":
33+
unittest.main()
34+
"""
35+
self.assertTrue(parse.parse_main(content))
36+
37+
38+
if __name__ == "__main__":
39+
unittest.main()

0 commit comments

Comments
 (0)
0