feat(gazelle): Add "include_dep" Python comment annotation (#1863)

Add a new Python comment annotation for Gazelle: `include_dep`. This annotation accepts a comma-separated string of values. Values _should_ be targets names, but no validation is done. The annotation can be added multiple times, and all values are combined and de-duplicated. For `python_generation_mode = "package"`, the `include_dep` annotations found across all files included in the generated target. The `parser.annotations` struct is updated to include a new `includeDep` field, and `parser.parse` is updated to return the `annotations` struct. All target builders then add the resolved dependencies. Fixes #1862. Example: ```python # gazelle:include_dep //foo:bar,:hello_world,//:abc # gazelle:include_dep //:def,//foo:bar ``` will cause gazelle to generate: ```starlark deps = [ ":hello_world", "//:abc", "//:def", "//foo:bar", ] ``` --------- Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
bazelbuild · May 9, 2024 · a1d3c0d · a1d3c0d
1 parent d3cec48
commit a1d3c0d
Show file tree

Hide file tree

Showing 35 changed files with 379 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -73,6 +73,8 @@ A brief description of the categories of changes:
   the whl and sdist files will be written to the lock file. Controlling whether
   the downloading of metadata is done in parallel can be done using
   `parallel_download` attribute.
+* (gazelle) Add a new annotation `include_deps`. Also add documentation for
+  annotations to `gazelle/README.md`.
 * (deps): `rules_python` depends now on `rules_cc` 0.0.9
 * (pip_parse): A new flag `use_hub_alias_dependencies` has been added that is going
   to become default in the next release. This makes use of `dep_template` flag

diff --git a/gazelle/README.md b/gazelle/README.md
@@ -425,6 +425,108 @@ py_library(
 [issue-1826]: https://github.com/bazelbuild/rules_python/issues/1826
 
 
+### Annotations
+
+*Annotations* refer to comments found _within Python files_ that configure how
+Gazelle acts for that particular file.
+
+Annotations have the form:
+
+```python
+# gazelle:annotation_name value
+```
+
+and can reside anywhere within a Python file where comments are valid. For example:
+
+```python
+import foo
+# gazelle:annotation_name value
+
+def bar():  # gazelle:annotation_name value
+    pass
+```
+
+The annotations are:
+
+| **Annotation**                                                | **Default value** |
+|---------------------------------------------------------------|-------------------|
+| [`# gazelle:ignore imports`](#annotation-ignore)              | N/A               |
+| Tells Gazelle to ignore import statements. `imports` is a comma-separated list of imports to ignore. | |
+| [`# gazelle:include_dep targets`](#annotation-include_dep)    | N/A               |
+| Tells Gazelle to include a set of dependencies, even if they are not imported in a Python module. `targets` is a comma-separated list of target names to include as dependencies. | |
+
+
+#### Annotation: `ignore`
+
+This annotation accepts a comma-separated string of values. Values are names of Python
+imports that Gazelle should _not_ include in target dependencies.
+
+The annotation can be added multiple times, and all values are combined and
+de-duplicated.
+
+For `python_generation_mode = "package"`, the `ignore` annotations
+found across all files included in the generated target are removed from `deps`.
+
+Example:
+
+```python
+import numpy  # a pypi package
+
+# gazelle:ignore bar.baz.hello,foo
+import bar.baz.hello
+import foo
+
+# Ignore this import because _reasons_
+import baz  # gazelle:ignore baz
+```
+
+will cause Gazelle to generate:
+
+```starlark
+deps = ["@pypi//numpy"],
+```
+
+
+#### Annotation: `include_dep`
+
+This annotation accepts a comma-separated string of values. Values _must_
+be Python targets, but _no validation is done_. If a value is not a Python
+target, building will result in an error saying:
+
+```
+<target> does not have mandatory providers: 'PyInfo' or 'CcInfo' or 'PyInfo'.
+```
+
+Adding non-Python targets to the generated target is a feature request being
+tracked in [Issue #1865](https://github.com/bazelbuild/rules_python/issues/1865).
+
+The annotation can be added multiple times, and all values are combined
+and de-duplicated.
+
+For `python_generation_mode = "package"`, the `include_dep` annotations
+found across all files included in the generated target are included in `deps`.
+
+Example:
+
+```python
+# gazelle:include_dep //foo:bar,:hello_world,//:abc
+# gazelle:include_dep //:def,//foo:bar
+import numpy  # a pypi package
+```
+
+will cause Gazelle to generate:
+
+```starlark
+deps = [
+    ":hello_world",
+    "//:abc",
+    "//:def",
+    "//foo:bar",
+    "@pypi//numpy",
+]
+```
+
+
 ### Libraries
 
 Python source files are those ending in `.py` but not ending in `_test.py`.

diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go
@@ -233,7 +233,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 	collisionErrors := singlylinkedlist.New()
 
 	appendPyLibrary := func(srcs *treeset.Set, pyLibraryTargetName string) {
-		allDeps, mainModules, err := parser.parse(srcs)
+		allDeps, mainModules, annotations, err := parser.parse(srcs)
 		if err != nil {
 			log.Fatalf("ERROR: %v\n", err)
 		}
@@ -263,6 +263,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 					addVisibility(visibility).
 					addSrc(filename).
 					addModuleDependencies(mainModules[filename]).
+					addResolvedDependencies(annotations.includeDeps).
 					generateImportsAttribute().build()
 				result.Gen = append(result.Gen, pyBinary)
 				result.Imports = append(result.Imports, pyBinary.PrivateAttr(config.GazelleImportsKey))
@@ -290,6 +291,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 			addVisibility(visibility).
 			addSrcs(srcs).
 			addModuleDependencies(allDeps).
+			addResolvedDependencies(annotations.includeDeps).
 			generateImportsAttribute().
 			build()
 
@@ -314,7 +316,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 	}
 
 	if hasPyBinaryEntryPointFile {
-		deps, _, err := parser.parseSingle(pyBinaryEntrypointFilename)
+		deps, _, annotations, err := parser.parseSingle(pyBinaryEntrypointFilename)
 		if err != nil {
 			log.Fatalf("ERROR: %v\n", err)
 		}
@@ -338,6 +340,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 			addVisibility(visibility).
 			addSrc(pyBinaryEntrypointFilename).
 			addModuleDependencies(deps).
+			addResolvedDependencies(annotations.includeDeps).
 			generateImportsAttribute()
 
 		pyBinary := pyBinaryTarget.build()
@@ -348,7 +351,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 
 	var conftest *rule.Rule
 	if hasConftestFile {
-		deps, _, err := parser.parseSingle(conftestFilename)
+		deps, _, annotations, err := parser.parseSingle(conftestFilename)
 		if err != nil {
 			log.Fatalf("ERROR: %v\n", err)
 		}
@@ -367,6 +370,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 		conftestTarget := newTargetBuilder(pyLibraryKind, conftestTargetname, pythonProjectRoot, args.Rel, pyFileNames).
 			addSrc(conftestFilename).
 			addModuleDependencies(deps).
+			addResolvedDependencies(annotations.includeDeps).
 			addVisibility(visibility).
 			setTestonly().
 			generateImportsAttribute()
@@ -379,7 +383,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 
 	var pyTestTargets []*targetBuilder
 	newPyTestTargetBuilder := func(srcs *treeset.Set, pyTestTargetName string) *targetBuilder {
-		deps, _, err := parser.parse(srcs)
+		deps, _, annotations, err := parser.parse(srcs)
 		if err != nil {
 			log.Fatalf("ERROR: %v\n", err)
 		}
@@ -397,6 +401,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
 		return newTargetBuilder(pyTestKind, pyTestTargetName, pythonProjectRoot, args.Rel, pyFileNames).
 			addSrcs(srcs).
 			addModuleDependencies(deps).
+			addResolvedDependencies(annotations.includeDeps).
 			generateImportsAttribute()
 	}
 	if (hasPyTestEntryPointFile || hasPyTestEntryPointTarget || cfg.CoarseGrainedGeneration()) && !cfg.PerFileGeneration() {

diff --git a/gazelle/python/parser.go b/gazelle/python/parser.go
@@ -101,15 +101,15 @@ func newPython3Parser(
 
 // parseSingle parses a single Python file and returns the extracted modules
 // from the import statements as well as the parsed comments.
-func (p *python3Parser) parseSingle(pyFilename string) (*treeset.Set, map[string]*treeset.Set, error) {
+func (p *python3Parser) parseSingle(pyFilename string) (*treeset.Set, map[string]*treeset.Set, *annotations, error) {
 	pyFilenames := treeset.NewWith(godsutils.StringComparator)
 	pyFilenames.Add(pyFilename)
 	return p.parse(pyFilenames)
 }
 
 // parse parses multiple Python files and returns the extracted modules from
 // the import statements as well as the parsed comments.
-func (p *python3Parser) parse(pyFilenames *treeset.Set) (*treeset.Set, map[string]*treeset.Set, error) {
+func (p *python3Parser) parse(pyFilenames *treeset.Set) (*treeset.Set, map[string]*treeset.Set, *annotations, error) {
 	parserMutex.Lock()
 	defer parserMutex.Unlock()
 
@@ -122,28 +122,30 @@ func (p *python3Parser) parse(pyFilenames *treeset.Set) (*treeset.Set, map[strin
 	}
 	encoder := json.NewEncoder(parserStdin)
 	if err := encoder.Encode(&req); err != nil {
-		return nil, nil, fmt.Errorf("failed to parse: %w", err)
+		return nil, nil, nil, fmt.Errorf("failed to parse: %w", err)
 	}
 
 	reader := bufio.NewReader(parserStdout)
 	data, err := reader.ReadBytes(0)
 	if err != nil {
-		return nil, nil, fmt.Errorf("failed to parse: %w", err)
+		return nil, nil, nil, fmt.Errorf("failed to parse: %w", err)
 	}
 	data = data[:len(data)-1]
 	var allRes []parserResponse
 	if err := json.Unmarshal(data, &allRes); err != nil {
-		return nil, nil, fmt.Errorf("failed to parse: %w", err)
+		return nil, nil, nil, fmt.Errorf("failed to parse: %w", err)
 	}
 
 	mainModules := make(map[string]*treeset.Set, len(allRes))
+	allAnnotations := new(annotations)
+	allAnnotations.ignore = make(map[string]struct{})
 	for _, res := range allRes {
 		if res.HasMain {
 			mainModules[res.FileName] = treeset.NewWith(moduleComparator)
 		}
 		annotations, err := annotationsFromComments(res.Comments)
 		if err != nil {
-			return nil, nil, fmt.Errorf("failed to parse annotations: %w", err)
+			return nil, nil, nil, fmt.Errorf("failed to parse annotations: %w", err)
 		}
 
 		for _, m := range res.Modules {
@@ -164,9 +166,32 @@ func (p *python3Parser) parse(pyFilenames *treeset.Set) (*treeset.Set, map[strin
 				mainModules[res.FileName].Add(m)
 			}
 		}
+
+		// Collect all annotations from each file into a single annotations struct.
+		for k, v := range annotations.ignore {
+			allAnnotations.ignore[k] = v
+		}
+		allAnnotations.includeDeps = append(allAnnotations.includeDeps, annotations.includeDeps...)
 	}
 
-	return modules, mainModules, nil
+	allAnnotations.includeDeps = removeDupesFromStringTreeSetSlice(allAnnotations.includeDeps)
+
+	return modules, mainModules, allAnnotations, nil
+}
+
+// removeDupesFromStringTreeSetSlice takes a []string, makes a set out of the
+// elements, and then returns a new []string with all duplicates removed. Order
+// is preserved.
+func removeDupesFromStringTreeSetSlice(array []string) []string {
+	s := treeset.NewWith(godsutils.StringComparator)
+	for _, v := range array {
+		s.Add(v)
+	}
+	dedupe := make([]string, s.Size())
+	for i, v := range s.Values() {
+		dedupe[i] = fmt.Sprint(v)
+	}
+	return dedupe
 }
 
 // parserResponse represents a response returned by the parser.py for a given
@@ -211,7 +236,8 @@ const (
 	// The Gazelle annotation prefix.
 	annotationPrefix string = "gazelle:"
 	// The ignore annotation kind. E.g. '# gazelle:ignore <module_name>'.
-	annotationKindIgnore annotationKind = "ignore"
+	annotationKindIgnore     annotationKind = "ignore"
+	annotationKindIncludeDep annotationKind = "include_dep"
 )
 
 // comment represents a Python comment.
@@ -247,12 +273,15 @@ type annotation struct {
 type annotations struct {
 	// The parsed modules to be ignored by Gazelle.
 	ignore map[string]struct{}
+	// Labels that Gazelle should include as deps of the generated target.
+	includeDeps []string
 }
 
 // annotationsFromComments returns all the annotations parsed out of the
 // comments of a Python module.
 func annotationsFromComments(comments []comment) (*annotations, error) {
 	ignore := make(map[string]struct{})
+	includeDeps := []string{}
 	for _, comment := range comments {
 		annotation, err := comment.asAnnotation()
 		if err != nil {
@@ -269,10 +298,21 @@ func annotationsFromComments(comments []comment) (*annotations, error) {
 					ignore[m] = struct{}{}
 				}
 			}
+			if annotation.kind == annotationKindIncludeDep {
+				targets := strings.Split(annotation.value, ",")
+				for _, t := range targets {
+					if t == "" {
+						continue
+					}
+					t = strings.TrimSpace(t)
+					includeDeps = append(includeDeps, t)
+				}
+			}
 		}
 	}
 	return &annotations{
-		ignore: ignore,
+		ignore:      ignore,
+		includeDeps: includeDeps,
 	}, nil
 }
 

diff --git a/gazelle/python/target.go b/gazelle/python/target.go
@@ -99,6 +99,15 @@ func (t *targetBuilder) addResolvedDependency(dep string) *targetBuilder {
 	return t
 }
 
+// addResolvedDependencies adds multiple dependencies, that have already been
+// resolved or generated, to the target.
+func (t *targetBuilder) addResolvedDependencies(deps []string) *targetBuilder {
+	for _, dep := range deps {
+		t.addResolvedDependency(dep)
+	}
+	return t
+}
+
 // addVisibility adds visibility labels to the target.
 func (t *targetBuilder) addVisibility(visibility []string) *targetBuilder {
 	for _, item := range visibility {

diff --git a/gazelle/python/testdata/annotation_include_dep/BUILD.in b/gazelle/python/testdata/annotation_include_dep/BUILD.in
@@ -0,0 +1 @@
+# gazelle:python_generation_mode file