feat(python): make Python cleaning more selective (#4108)

Take a more selective approach to cleaning, removing files that are
*expected* to be generated, rather than removing everything except
explicitly-listed "keep" files. Files *can* still be listed to be kept,
but don't have to be unless they're expected to be generated. This will
allow libraries with a mixture of handwritten and generated code to add
new handwritten files without the configuration file being updated,
except for the (hopefully rare) cases where a new handwritten file
"looks like" a generated file (in terms of which directory it's in and
its name).

This change does not modify the migration tool to reduce the size of the
"keep" list yet, but that can happen later if and when we want it to.
(There's a balance between removing unnecessary entries by hand, and
putting the work into figuring out what's unnecessary automatically.)

Author: jskeet

doc/config-schema.md

@@ -63,6 +63,7 @@ This document describes the schema for the librarian.yaml.
 | `transport` | string | Transport is the transport protocol, such as "grpc+rest" or "grpc". |
 | `dart` | [DartPackage](#dartpackage-configuration) (optional) | Dart contains Dart-specific default configuration. |
 | `rust` | [RustDefault](#rustdefault-configuration) (optional) | Rust contains Rust-specific default configuration. |
+| `python` | [PythonDefault](#pythondefault-configuration) (optional) | Python contains Python-specific default configuration. |
 
 ## Library Configuration
 
@@ -136,10 +137,17 @@ This document describes the schema for the librarian.yaml.
 | `module_path_version` | string | ModulePathVersion is the version of the Go module path. |
 | `nested_module` | string | NestedModule is the name of a nested module directory. |
 
+## PythonDefault Configuration
+
+| Field | Type | Description |
+| :--- | :--- | :--- |
+| `common_gapic_paths` | list of string | CommonGAPICPaths contains paths which are generated for any package containing a GAPIC API. These are relative to the package's output directory, and the string "{neutral-source}" is replaced with the path to the version-neutral source code (e.g. "google/cloud/run"). If a library defines its own common_gapic_paths, they will be appended to the defaults. |
+
 ## PythonPackage Configuration
 
 | Field | Type | Description |
 | :--- | :--- | :--- |
+| (embedded) | [PythonDefault](#pythondefault-configuration) |  |
 | `opt_args` | list of string | OptArgs contains additional options passed to the generator, where the options are common to all apis. Example: ["warehouse-package-name=google-cloud-batch"] |
 | `opt_args_by_api` | map[string][]string | OptArgsByAPI contains additional options passed to the generator, where the options vary by api. In each entry, the key is the api (API path) and the value is the list of options to pass when generating that API. Example: {"google/cloud/secrets/v1beta": ["python-gapic-name=secretmanager"]} |
 | `proto_only_apis` | list of string | ProtoOnlyAPIs contains the list of API paths which are proto-only, so should use regular protoc Python generation instead of GAPIC. |

internal/config/config.go

@@ -144,6 +144,9 @@ type Default struct {
 
 	// Rust contains Rust-specific default configuration.
 	Rust *RustDefault `yaml:"rust,omitempty"`
+
+	// Python contains Python-specific default configuration.
+	Python *PythonDefault `yaml:"python,omitempty"`
 }
 
 // Library represents a library configuration.

internal/config/language.go

@@ -280,8 +280,11 @@ type RustPoller struct {
 	MethodID string `yaml:"method_id"`
 }
 
-// PythonPackage contains Python-specific library configuration.
+// PythonPackage contains Python-specific library configuration. It inherits
+// from PythonDefault, allowing library-specific overrides of global settings.
 type PythonPackage struct {
+	PythonDefault `yaml:",inline"`
+
 	// OptArgs contains additional options passed to the generator, where
 	// the options are common to all apis.
 	// Example: ["warehouse-package-name=google-cloud-batch"]
@@ -299,6 +302,17 @@ type PythonPackage struct {
 	ProtoOnlyAPIs []string `yaml:"proto_only_apis,omitempty"`
 }
 
+// PythonDefault contains Python-specific default configuration.
+type PythonDefault struct {
+	// CommonGAPICPaths contains paths which are generated for any package
+	// containing a GAPIC API. These are relative to the package's output
+	// directory, and the string "{neutral-source}" is replaced with the path
+	// to the version-neutral source code (e.g. "google/cloud/run"). If a
+	// library defines its own common_gapic_paths, they will be appended to
+	// the defaults.
+	CommonGAPICPaths []string `yaml:"common_gapic_paths,omitempty"`
+}
+
 // DartPackage contains Dart-specific library configuration.
 type DartPackage struct {
 	// APIKeysEnvironmentVariables is a comma-separated list of environment variable names

internal/librarian/generate.go

@@ -156,10 +156,14 @@ func cleanLibraries(language string, libraries []*config.Library) error {
 		switch language {
 		case languageFake:
 			// No cleaning needed.
-		case languageDart, languagePython:
+		case languageDart:
 			if err := checkAndClean(library.Output, library.Keep); err != nil {
 				return err
 			}
+		case languagePython:
+			if err := python.CleanLibrary(library); err != nil {
+				return err
+			}
 		case languageGo:
 			if err := golang.Clean(library); err != nil {
 				return err

internal/librarian/library.go

@@ -46,6 +46,9 @@ func fillDefaults(lib *config.Library, d *config.Default) *config.Library {
 	if d.Dart != nil {
 		return fillDart(lib, d)
 	}
+	if d.Python != nil {
+		return fillPython(lib, d)
+	}
 	return lib
 }
 
@@ -98,6 +101,16 @@ func fillDart(lib *config.Library, d *config.Default) *config.Library {
 	return lib
 }
 
+// fillPython populates empty Python-specific fields in lib from the provided
+// default.
+func fillPython(lib *config.Library, d *config.Default) *config.Library {
+	if lib.Python == nil {
+		lib.Python = &config.PythonPackage{}
+	}
+	lib.Python.CommonGAPICPaths = append(d.Python.CommonGAPICPaths, lib.Python.CommonGAPICPaths...)
+	return lib
+}
+
 // mergeDartDependencies merges library dependencies with default dependencies.
 // Duplicate dependencies in defaults will be ignored.
 func mergeDartDependencies(libDeps, defaultDeps string) string {

internal/librarian/library_test.go

@@ -353,6 +353,78 @@ func TestFillDefaults_Rust(t *testing.T) {
 	}
 }
 
+func TestFillDefaults_Python(t *testing.T) {
+	for _, test := range []struct {
+		name     string
+		lib      *config.Library
+		defaults *config.PythonDefault
+		want     *config.Library
+	}{
+		{
+			name: "common_gapic_paths only in defaults",
+			lib:  &config.Library{},
+			defaults: &config.PythonDefault{
+				CommonGAPICPaths: []string{"a", "b"},
+			},
+			want: &config.Library{
+				Python: &config.PythonPackage{
+					PythonDefault: config.PythonDefault{
+						CommonGAPICPaths: []string{"a", "b"},
+					},
+				},
+			},
+		},
+		{
+			name: "common_gapic_paths only in package",
+			lib: &config.Library{
+				Python: &config.PythonPackage{
+					PythonDefault: config.PythonDefault{
+						CommonGAPICPaths: []string{"a", "b"},
+					},
+				},
+			},
+			defaults: &config.PythonDefault{},
+			want: &config.Library{
+				Python: &config.PythonPackage{
+					PythonDefault: config.PythonDefault{
+						CommonGAPICPaths: []string{"a", "b"},
+					},
+				},
+			},
+		},
+		{
+			name: "common_gapic_paths merged",
+			lib: &config.Library{
+				Python: &config.PythonPackage{
+					PythonDefault: config.PythonDefault{
+						CommonGAPICPaths: []string{"c", "d"},
+					},
+				},
+			},
+			defaults: &config.PythonDefault{
+				CommonGAPICPaths: []string{"a", "b"},
+			},
+			want: &config.Library{
+				Python: &config.PythonPackage{
+					PythonDefault: config.PythonDefault{
+						CommonGAPICPaths: []string{"a", "b", "c", "d"},
+					},
+				},
+			},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			defaults := &config.Default{
+				Python: test.defaults,
+			}
+			got := fillDefaults(test.lib, defaults)
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
+
 func TestPrepareLibrary(t *testing.T) {
 	for _, test := range []struct {
 		name        string