From af2eedb1e777b02c54cfaed52921490f6511aa03 Mon Sep 17 00:00:00 2001 From: oliversun09 <112964599+oliversun09@users.noreply.github.com> Date: Mon, 12 Dec 2022 10:09:14 -0500 Subject: [PATCH] BSR-704/default except override for optimize_for (#1625) * Add default, except override to managed mode `optimize_for` option * preserves backward compatibility with the current `optimize_for` option being string --- CHANGELOG.md | 4 + private/buf/bufgen/bufgen.go | 59 +++- private/buf/bufgen/config.go | 89 +++-- private/buf/bufgen/config_test.go | 124 ++++++- private/buf/bufgen/generator.go | 6 +- .../buf/bufgen/testdata/v1/gen_error10.yaml | 14 + .../buf/bufgen/testdata/v1/gen_error11.yaml | 14 + .../buf/bufgen/testdata/v1/gen_error12.yaml | 14 + .../buf/bufgen/testdata/v1/gen_success9.json | 23 ++ .../buf/bufgen/testdata/v1/gen_success9.yaml | 14 + .../buf/bufgen/testdata/v1/gen_success9.yml | 14 + private/buf/bufmigrate/v1beta1_migrator.go | 2 +- .../success/complex/output/buf.gen.yaml | 3 +- .../bufimage/bufimagemodify/bufimagemodify.go | 6 +- .../bufimage/bufimagemodify/optimize_for.go | 47 ++- .../bufimagemodify/optimize_for_test.go | 306 +++++++++++++++++- 16 files changed, 683 insertions(+), 56 deletions(-) create mode 100644 private/buf/bufgen/testdata/v1/gen_error10.yaml create mode 100644 private/buf/bufgen/testdata/v1/gen_error11.yaml create mode 100644 private/buf/bufgen/testdata/v1/gen_error12.yaml create mode 100644 private/buf/bufgen/testdata/v1/gen_success9.json create mode 100644 private/buf/bufgen/testdata/v1/gen_success9.yaml create mode 100644 private/buf/bufgen/testdata/v1/gen_success9.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index b7d6b5c8ff..858d04d491 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ ## [UNRELEASED] - `buf generate` now batches remote plugin generation calls for improved performance. +- Update `optimize_for` option in managed mode, allowing a `default` value for `optimize_for` + for all files, `except` and `override`, which both behave similarly to other `except` + and `override` options. Specifying an `optimize_for` value in the earlier versions is + equivalent to having a `optimize_for` with that value as default. ## [v1.10.0] - 2022-12-07 diff --git a/private/buf/bufgen/bufgen.go b/private/buf/bufgen/bufgen.go index f5cd79bd2c..28f9e210ad 100644 --- a/private/buf/bufgen/bufgen.go +++ b/private/buf/bufgen/bufgen.go @@ -229,7 +229,7 @@ type ManagedConfig struct { JavaStringCheckUtf8 *bool JavaPackagePrefix *JavaPackagePrefixConfig CsharpNameSpaceConfig *CsharpNameSpaceConfig - OptimizeFor *descriptorpb.FileOptions_OptimizeMode + OptimizeForConfig *OptimizeForConfig GoPackagePrefixConfig *GoPackagePrefixConfig Override map[string]map[string]string } @@ -242,6 +242,13 @@ type JavaPackagePrefixConfig struct { Override map[bufmoduleref.ModuleIdentity]string } +type OptimizeForConfig struct { + Default descriptorpb.FileOptions_OptimizeMode + Except []bufmoduleref.ModuleIdentity + // bufmoduleref.ModuleIdentity -> optimize_for. + Override map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode +} + // GoPackagePrefixConfig is the go_package prefix configuration. type GoPackagePrefixConfig struct { Default string @@ -332,7 +339,7 @@ type ExternalManagedConfigV1 struct { JavaStringCheckUtf8 *bool `json:"java_string_check_utf8,omitempty" yaml:"java_string_check_utf8,omitempty"` JavaPackagePrefix ExternalJavaPackagePrefixConfigV1 `json:"java_package_prefix,omitempty" yaml:"java_package_prefix,omitempty"` CsharpNamespace ExternalCsharpNamespaceConfigV1 `json:"csharp_namespace,omitempty" yaml:"csharp_namespace,omitempty"` - OptimizeFor string `json:"optimize_for,omitempty" yaml:"optimize_for,omitempty"` + OptimizeFor ExternalOptimizeForConfigV1 `json:"optimize_for,omitempty" yaml:"optimize_for,omitempty"` GoPackagePrefix ExternalGoPackagePrefixConfigV1 `json:"go_package_prefix,omitempty" yaml:"go_package_prefix,omitempty"` Override map[string]map[string]string `json:"override,omitempty" yaml:"override,omitempty"` } @@ -343,7 +350,7 @@ func (e ExternalManagedConfigV1) IsEmpty() bool { e.JavaMultipleFiles == nil && e.JavaStringCheckUtf8 == nil && e.JavaPackagePrefix.IsEmpty() && - e.OptimizeFor == "" && + e.OptimizeFor.IsEmpty() && e.GoPackagePrefix.IsEmpty() && len(e.Override) == 0 } @@ -394,6 +401,52 @@ func (e *ExternalJavaPackagePrefixConfigV1) unmarshalWith(unmarshal func(interfa return nil } +// ExternalOptimizeForConfigV1 is the external optimize_for configuration. +type ExternalOptimizeForConfigV1 struct { + Default string `json:"default,omitempty" yaml:"default,omitempty"` + Except []string `json:"except,omitempty" yaml:"except,omitempty"` + Override map[string]string `json:"override,omitempty" yaml:"override,omitempty"` +} + +// IsEmpty returns true if the config is empty +func (e ExternalOptimizeForConfigV1) IsEmpty() bool { + return e.Default == "" && + len(e.Except) == 0 && + len(e.Override) == 0 +} + +// UnmarshalYAML satisfies the yaml.Unmarshaler interface. This is done to maintain backward compatibility +// of accepting a plain string value for optimize_for. +func (e *ExternalOptimizeForConfigV1) UnmarshalYAML(unmarshal func(interface{}) error) error { + return e.unmarshalWith(unmarshal) +} + +// UnmarshalJSON satisfies the json.Unmarshaler interface. This is done to maintain backward compatibility +// of accepting a plain string value for optimize_for. +func (e *ExternalOptimizeForConfigV1) UnmarshalJSON(data []byte) error { + unmarshal := func(v interface{}) error { + return json.Unmarshal(data, v) + } + + return e.unmarshalWith(unmarshal) +} + +// unmarshalWith is used to unmarshal into json/yaml. See https://abhinavg.net/posts/flexible-yaml for details. +func (e *ExternalOptimizeForConfigV1) unmarshalWith(unmarshal func(interface{}) error) error { + var optimizeFor string + if err := unmarshal(&optimizeFor); err == nil { + e.Default = optimizeFor + return nil + } + + type rawExternalOptimizeForConfigV1 ExternalOptimizeForConfigV1 + if err := unmarshal((*rawExternalOptimizeForConfigV1)(e)); err != nil { + return err + } + + return nil +} + // ExternalGoPackagePrefixConfigV1 is the external go_package prefix configuration. type ExternalGoPackagePrefixConfigV1 struct { Default string `json:"default,omitempty" yaml:"default,omitempty"` diff --git a/private/buf/bufgen/config.go b/private/buf/bufgen/config.go index 96e391b629..9d3002fac4 100644 --- a/private/buf/bufgen/config.go +++ b/private/buf/bufgen/config.go @@ -252,16 +252,9 @@ func newManagedConfigV1(logger *zap.Logger, externalManagedConfig ExternalManage if err != nil { return nil, err } - var optimizeFor *descriptorpb.FileOptions_OptimizeMode - if externalManagedConfig.OptimizeFor != "" { - value, ok := descriptorpb.FileOptions_OptimizeMode_value[externalManagedConfig.OptimizeFor] - if !ok { - return nil, fmt.Errorf( - "invalid optimize_for value; expected one of %v", - enumMapToStringSlice(descriptorpb.FileOptions_OptimizeMode_value), - ) - } - optimizeFor = optimizeModePtr(descriptorpb.FileOptions_OptimizeMode(value)) + optimizeForConfig, err := newOptimizeForConfigV1(externalManagedConfig.OptimizeFor) + if err != nil { + return nil, err } goPackagePrefixConfig, err := newGoPackagePrefixConfigV1(externalManagedConfig.GoPackagePrefix) if err != nil { @@ -293,7 +286,7 @@ func newManagedConfigV1(logger *zap.Logger, externalManagedConfig ExternalManage JavaStringCheckUtf8: externalManagedConfig.JavaStringCheckUtf8, JavaPackagePrefix: javaPackagePrefixConfig, CsharpNameSpaceConfig: csharpNamespaceConfig, - OptimizeFor: optimizeFor, + OptimizeForConfig: optimizeForConfig, GoPackagePrefixConfig: goPackagePrefixConfig, Override: override, }, nil @@ -338,6 +331,63 @@ func newJavaPackagePrefixConfigV1(externalJavaPackagePrefixConfig ExternalJavaPa }, nil } +func newOptimizeForConfigV1(externalOptimizeForConfigV1 ExternalOptimizeForConfigV1) (*OptimizeForConfig, error) { + if externalOptimizeForConfigV1.IsEmpty() { + return nil, nil + } + if externalOptimizeForConfigV1.Default == "" { + return nil, errors.New("optimize_for setting requires a default value") + } + value, ok := descriptorpb.FileOptions_OptimizeMode_value[externalOptimizeForConfigV1.Default] + if !ok { + return nil, fmt.Errorf( + "invalid optimize_for default value; expected one of %v", + enumMapToStringSlice(descriptorpb.FileOptions_OptimizeMode_value), + ) + } + defaultOptimizeFor := descriptorpb.FileOptions_OptimizeMode(value) + seenModuleIdentities := make(map[string]struct{}, len(externalOptimizeForConfigV1.Except)) + except := make([]bufmoduleref.ModuleIdentity, 0, len(externalOptimizeForConfigV1.Except)) + for _, moduleName := range externalOptimizeForConfigV1.Except { + moduleIdentity, err := bufmoduleref.ModuleIdentityForString(moduleName) + if err != nil { + return nil, fmt.Errorf("invalid optimize_for except: %w", err) + } + if _, ok := seenModuleIdentities[moduleIdentity.IdentityString()]; ok { + return nil, fmt.Errorf("invalid optimize_for except: %q is defined multiple times", moduleIdentity.IdentityString()) + } + seenModuleIdentities[moduleIdentity.IdentityString()] = struct{}{} + except = append(except, moduleIdentity) + } + override := make(map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode, len(externalOptimizeForConfigV1.Override)) + for moduleName, optimizeFor := range externalOptimizeForConfigV1.Override { + moduleIdentity, err := bufmoduleref.ModuleIdentityForString(moduleName) + if err != nil { + return nil, fmt.Errorf("invalid optimize_for override key: %w", err) + } + value, ok := descriptorpb.FileOptions_OptimizeMode_value[optimizeFor] + if !ok { + return nil, fmt.Errorf( + "invalid optimize_for override value; expected one of %v", + enumMapToStringSlice(descriptorpb.FileOptions_OptimizeMode_value), + ) + } + if _, ok := seenModuleIdentities[moduleIdentity.IdentityString()]; ok { + return nil, fmt.Errorf( + "invalid optimize_for override: %q is already defined as an except", + moduleIdentity.IdentityString(), + ) + } + seenModuleIdentities[moduleIdentity.IdentityString()] = struct{}{} + override[moduleIdentity] = descriptorpb.FileOptions_OptimizeMode(value) + } + return &OptimizeForConfig{ + Default: defaultOptimizeFor, + Except: except, + Override: override, + }, nil +} + func newGoPackagePrefixConfigV1(externalGoPackagePrefixConfig ExternalGoPackagePrefixConfigV1) (*GoPackagePrefixConfig, error) { if externalGoPackagePrefixConfig.IsEmpty() { return nil, nil @@ -473,7 +523,7 @@ func newManagedConfigV1Beta1(externalOptionsConfig ExternalOptionsConfigV1Beta1, if !enabled || externalOptionsConfig == (ExternalOptionsConfigV1Beta1{}) { return nil, nil } - var optimizeFor *descriptorpb.FileOptions_OptimizeMode + var optimizeForConfig *OptimizeForConfig if externalOptionsConfig.OptimizeFor != "" { value, ok := descriptorpb.FileOptions_OptimizeMode_value[externalOptionsConfig.OptimizeFor] if !ok { @@ -482,12 +532,16 @@ func newManagedConfigV1Beta1(externalOptionsConfig ExternalOptionsConfigV1Beta1, enumMapToStringSlice(descriptorpb.FileOptions_OptimizeMode_value), ) } - optimizeFor = optimizeModePtr(descriptorpb.FileOptions_OptimizeMode(value)) + optimizeForConfig = &OptimizeForConfig{ + Default: descriptorpb.FileOptions_OptimizeMode(value), + Except: make([]bufmoduleref.ModuleIdentity, 0), + Override: make(map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode), + } } return &ManagedConfig{ CcEnableArenas: externalOptionsConfig.CcEnableArenas, JavaMultipleFiles: externalOptionsConfig.JavaMultipleFiles, - OptimizeFor: optimizeFor, + OptimizeForConfig: optimizeForConfig, }, nil } @@ -501,13 +555,6 @@ func enumMapToStringSlice(enums map[string]int32) []string { return slice } -// optimizeModePtr is a convenience function for initializing the -// *descriptorpb.FileOptions_OptimizeMode type in-line. This is -// also useful in unit tests. -func optimizeModePtr(value descriptorpb.FileOptions_OptimizeMode) *descriptorpb.FileOptions_OptimizeMode { - return &value -} - type readConfigOptions struct { override string } diff --git a/private/buf/bufgen/config_test.go b/private/buf/bufgen/config_test.go index bf951a9957..56b8c930b0 100644 --- a/private/buf/bufgen/config_test.go +++ b/private/buf/bufgen/config_test.go @@ -45,12 +45,20 @@ func TestReadConfigV1Beta1(t *testing.T) { CcEnableArenas: &truth, JavaMultipleFiles: &truth, JavaStringCheckUtf8: nil, - OptimizeFor: optimizeModePtr(descriptorpb.FileOptions_CODE_SIZE), + OptimizeForConfig: &OptimizeForConfig{ + Default: descriptorpb.FileOptions_CODE_SIZE, + Except: make([]bufmoduleref.ModuleIdentity, 0), + Override: make(map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode), + }, }, } successConfig2 := &Config{ ManagedConfig: &ManagedConfig{ - OptimizeFor: optimizeModePtr(descriptorpb.FileOptions_SPEED), + OptimizeForConfig: &OptimizeForConfig{ + Default: descriptorpb.FileOptions_SPEED, + Except: make([]bufmoduleref.ModuleIdentity, 0), + Override: make(map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode), + }, }, PluginConfigs: []*PluginConfig{ { @@ -64,7 +72,11 @@ func TestReadConfigV1Beta1(t *testing.T) { } successConfig3 := &Config{ ManagedConfig: &ManagedConfig{ - OptimizeFor: optimizeModePtr(descriptorpb.FileOptions_LITE_RUNTIME), + OptimizeForConfig: &OptimizeForConfig{ + Default: descriptorpb.FileOptions_LITE_RUNTIME, + Except: make([]bufmoduleref.ModuleIdentity, 0), + Override: make(map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode), + }, }, PluginConfigs: []*PluginConfig{ { @@ -164,7 +176,11 @@ func TestReadConfigV1(t *testing.T) { Except: make([]bufmoduleref.ModuleIdentity, 0), Override: make(map[bufmoduleref.ModuleIdentity]string), }, - OptimizeFor: optimizeModePtr(descriptorpb.FileOptions_CODE_SIZE), + OptimizeForConfig: &OptimizeForConfig{ + Default: descriptorpb.FileOptions_CODE_SIZE, + Except: make([]bufmoduleref.ModuleIdentity, 0), + Override: make(map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode), + }, Override: map[string]map[string]string{ bufimagemodify.JavaPackageID: {"a.proto": "override"}, }, @@ -177,7 +193,11 @@ func TestReadConfigV1(t *testing.T) { } successConfig2 := &Config{ ManagedConfig: &ManagedConfig{ - OptimizeFor: optimizeModePtr(descriptorpb.FileOptions_SPEED), + OptimizeForConfig: &OptimizeForConfig{ + Default: descriptorpb.FileOptions_SPEED, + Except: make([]bufmoduleref.ModuleIdentity, 0), + Override: make(map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode), + }, }, PluginConfigs: []*PluginConfig{ { @@ -191,7 +211,11 @@ func TestReadConfigV1(t *testing.T) { } successConfig3 := &Config{ ManagedConfig: &ManagedConfig{ - OptimizeFor: optimizeModePtr(descriptorpb.FileOptions_LITE_RUNTIME), + OptimizeForConfig: &OptimizeForConfig{ + Default: descriptorpb.FileOptions_LITE_RUNTIME, + Except: make([]bufmoduleref.ModuleIdentity, 0), + Override: make(map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode), + }, }, PluginConfigs: []*PluginConfig{ { @@ -295,6 +319,48 @@ func TestReadConfigV1(t *testing.T) { }, }, } + successConfig9 := &Config{ + ManagedConfig: &ManagedConfig{ + OptimizeForConfig: &OptimizeForConfig{ + Default: descriptorpb.FileOptions_CODE_SIZE, + Except: []bufmoduleref.ModuleIdentity{ + mustCreateModuleIdentity( + t, + "someremote.com", + "owner", + "repo", + ), + mustCreateModuleIdentity( + t, + "someremote.com", + "owner", + "foo", + ), + }, + Override: map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode{ + mustCreateModuleIdentity( + t, + "someremote.com", + "owner", + "bar", + ): descriptorpb.FileOptions_SPEED, + mustCreateModuleIdentity( + t, + "someremote.com", + "owner", + "baz", + ): descriptorpb.FileOptions_LITE_RUNTIME, + }, + }, + }, + PluginConfigs: []*PluginConfig{ + { + Remote: "someremote.com/owner/plugins/myplugin", + Out: "gen/go", + Strategy: StrategyAll, + }, + }, + } ctx := context.Background() nopLogger := zap.NewNop() @@ -478,6 +544,30 @@ func TestReadConfigV1(t *testing.T) { config, err = ReadConfig(ctx, nopLogger, provider, readBucket, ReadConfigWithOverride(string(data))) require.NoError(t, err) assertConfigsWithEqualCsharpnamespace(t, successConfig8, config) + config, err = ReadConfig(ctx, nopLogger, provider, readBucket, ReadConfigWithOverride(filepath.Join("testdata", "v1", "gen_success9.yaml"))) + require.NoError(t, err) + assertConfigsWithEqualOptimizeFor(t, successConfig9, config) + data, err = os.ReadFile(filepath.Join("testdata", "v1", "gen_success9.yaml")) + require.NoError(t, err) + config, err = ReadConfig(ctx, nopLogger, provider, readBucket, ReadConfigWithOverride((string(data)))) + require.NoError(t, err) + assertConfigsWithEqualOptimizeFor(t, successConfig9, config) + config, err = ReadConfig(ctx, nopLogger, provider, readBucket, ReadConfigWithOverride(filepath.Join("testdata", "v1", "gen_success9.json"))) + require.NoError(t, err) + assertConfigsWithEqualOptimizeFor(t, successConfig9, config) + data, err = os.ReadFile(filepath.Join("testdata", "v1", "gen_success9.json")) + require.NoError(t, err) + config, err = ReadConfig(ctx, nopLogger, provider, readBucket, ReadConfigWithOverride(string(data))) + require.NoError(t, err) + assertConfigsWithEqualOptimizeFor(t, successConfig9, config) + config, err = ReadConfig(ctx, nopLogger, provider, readBucket, ReadConfigWithOverride(filepath.Join("testdata", "v1", "gen_success9.yml"))) + require.NoError(t, err) + assertConfigsWithEqualOptimizeFor(t, successConfig9, config) + data, err = os.ReadFile(filepath.Join("testdata", "v1", "gen_success9.yml")) + require.NoError(t, err) + config, err = ReadConfig(ctx, nopLogger, provider, readBucket, ReadConfigWithOverride(string(data))) + require.NoError(t, err) + assertConfigsWithEqualOptimizeFor(t, successConfig9, config) testReadConfigError(t, nopLogger, provider, readBucket, filepath.Join("testdata", "v1", "gen_error1.yaml")) testReadConfigError(t, nopLogger, provider, readBucket, filepath.Join("testdata", "v1", "gen_error2.yaml")) @@ -488,6 +578,9 @@ func TestReadConfigV1(t *testing.T) { testReadConfigError(t, nopLogger, provider, readBucket, filepath.Join("testdata", "v1", "gen_error7.yaml")) testReadConfigError(t, nopLogger, provider, readBucket, filepath.Join("testdata", "v1", "gen_error8.yaml")) testReadConfigError(t, nopLogger, provider, readBucket, filepath.Join("testdata", "v1", "gen_error9.yaml")) + testReadConfigError(t, nopLogger, provider, readBucket, filepath.Join("testdata", "v1", "gen_error10.yaml")) + testReadConfigError(t, nopLogger, provider, readBucket, filepath.Join("testdata", "v1", "gen_error11.yaml")) + testReadConfigError(t, nopLogger, provider, readBucket, filepath.Join("testdata", "v1", "gen_error12.yaml")) successConfig = &Config{ PluginConfigs: []*PluginConfig{ @@ -566,10 +659,23 @@ func assertConfigsWithEqualCsharpnamespace(t *testing.T, successConfig *Config, assertEqualModuleIdentityKeyedMaps(t, successCsharpConfig.Override, csharpConfig.Override) } -func assertEqualModuleIdentityKeyedMaps(t *testing.T, m1 map[bufmoduleref.ModuleIdentity]string, m2 map[bufmoduleref.ModuleIdentity]string) { +func assertConfigsWithEqualOptimizeFor(t *testing.T, successConfig *Config, config *Config) { + require.Equal(t, successConfig.PluginConfigs, config.PluginConfigs) + require.NotNil(t, successConfig.ManagedConfig) + require.NotNil(t, config.ManagedConfig) + successOptimizeForConfig := successConfig.ManagedConfig.OptimizeForConfig + require.NotNil(t, successOptimizeForConfig) + optimizeForConfig := config.ManagedConfig.OptimizeForConfig + require.NotNil(t, optimizeForConfig) + require.Equal(t, successOptimizeForConfig.Default, optimizeForConfig.Default) + require.Equal(t, successOptimizeForConfig.Except, optimizeForConfig.Except) + assertEqualModuleIdentityKeyedMaps(t, optimizeForConfig.Override, optimizeForConfig.Override) +} + +func assertEqualModuleIdentityKeyedMaps[V any](t *testing.T, m1 map[bufmoduleref.ModuleIdentity]V, m2 map[bufmoduleref.ModuleIdentity]V) { require.Equal(t, len(m1), len(m2)) - keyedM1 := make(map[string]string, len(m1)) - keyedM2 := make(map[string]string, len(m2)) + keyedM1 := make(map[string]V, len(m1)) + keyedM2 := make(map[string]V, len(m2)) for k, v := range m1 { keyedM1[k.IdentityString()] = v } diff --git a/private/buf/bufgen/generator.go b/private/buf/bufgen/generator.go index 4ed4c47201..e66369146b 100644 --- a/private/buf/bufgen/generator.go +++ b/private/buf/bufgen/generator.go @@ -589,11 +589,13 @@ func newModifier( managedConfig.Override[bufimagemodify.CsharpNamespaceID], ) modifier = bufimagemodify.Merge(modifier, csharpNamespaceModifier) - if managedConfig.OptimizeFor != nil { + if managedConfig.OptimizeForConfig != nil { optimizeFor, err := bufimagemodify.OptimizeFor( logger, sweeper, - *managedConfig.OptimizeFor, + managedConfig.OptimizeForConfig.Default, + managedConfig.OptimizeForConfig.Except, + managedConfig.OptimizeForConfig.Override, managedConfig.Override[bufimagemodify.OptimizeForID], ) if err != nil { diff --git a/private/buf/bufgen/testdata/v1/gen_error10.yaml b/private/buf/bufgen/testdata/v1/gen_error10.yaml new file mode 100644 index 0000000000..f138f1122f --- /dev/null +++ b/private/buf/bufgen/testdata/v1/gen_error10.yaml @@ -0,0 +1,14 @@ +version: v1 +managed: + enabled: true + optimize_for: + # missing default + except: + - someremote.com/owner/repo + - someremote.com/owner/foo + override: + someremote.com/owner/bar: SPEED + someremote.com/owner/baz: LITE_RUNTIME +plugins: + - remote: someremote.com/owner/plugins/myplugin + out: gen/go diff --git a/private/buf/bufgen/testdata/v1/gen_error11.yaml b/private/buf/bufgen/testdata/v1/gen_error11.yaml new file mode 100644 index 0000000000..50ff447e7b --- /dev/null +++ b/private/buf/bufgen/testdata/v1/gen_error11.yaml @@ -0,0 +1,14 @@ +version: v1 +managed: + enabled: true + optimize_for: + default: INVALID + except: + - someremote.com/owner/repo + - someremote.com/owner/foo + override: + someremote.com/owner/bar: SPEED + someremote.com/owner/baz: LITE_RUNTIME +plugins: + - remote: someremote.com/owner/plugins/myplugin + out: gen/go diff --git a/private/buf/bufgen/testdata/v1/gen_error12.yaml b/private/buf/bufgen/testdata/v1/gen_error12.yaml new file mode 100644 index 0000000000..22795c679e --- /dev/null +++ b/private/buf/bufgen/testdata/v1/gen_error12.yaml @@ -0,0 +1,14 @@ +version: v1 +managed: + enabled: true + optimize_for: + default: INVALID + except: + - someremote.com/owner/repo + - someremote.com/owner/foo + override: + someremote.com/owner/bar: INVALID + someremote.com/owner/baz: LITE_RUNTIME +plugins: + - remote: someremote.com/owner/plugins/myplugin + out: gen/go \ No newline at end of file diff --git a/private/buf/bufgen/testdata/v1/gen_success9.json b/private/buf/bufgen/testdata/v1/gen_success9.json new file mode 100644 index 0000000000..c87bb32017 --- /dev/null +++ b/private/buf/bufgen/testdata/v1/gen_success9.json @@ -0,0 +1,23 @@ +{ + "version": "v1", + "managed": { + "enabled": true, + "optimize_for": { + "default": "CODE_SIZE", + "except": [ + "someremote.com/owner/repo", + "someremote.com/owner/foo" + ], + "override": { + "someremote.com/owner/bar": "SPEED", + "someremote.com/owner/baz": "LITE_RUNTIME" + } + } + }, + "plugins": [ + { + "remote": "someremote.com/owner/plugins/myplugin", + "out": "gen/go" + } + ] +} diff --git a/private/buf/bufgen/testdata/v1/gen_success9.yaml b/private/buf/bufgen/testdata/v1/gen_success9.yaml new file mode 100644 index 0000000000..5a3ba0e202 --- /dev/null +++ b/private/buf/bufgen/testdata/v1/gen_success9.yaml @@ -0,0 +1,14 @@ +version: v1 +managed: + enabled: true + optimize_for: + default: CODE_SIZE + except: + - someremote.com/owner/repo + - someremote.com/owner/foo + override: + someremote.com/owner/bar: SPEED + someremote.com/owner/baz: LITE_RUNTIME +plugins: + - remote: someremote.com/owner/plugins/myplugin + out: gen/go diff --git a/private/buf/bufgen/testdata/v1/gen_success9.yml b/private/buf/bufgen/testdata/v1/gen_success9.yml new file mode 100644 index 0000000000..5a3ba0e202 --- /dev/null +++ b/private/buf/bufgen/testdata/v1/gen_success9.yml @@ -0,0 +1,14 @@ +version: v1 +managed: + enabled: true + optimize_for: + default: CODE_SIZE + except: + - someremote.com/owner/repo + - someremote.com/owner/foo + override: + someremote.com/owner/bar: SPEED + someremote.com/owner/baz: LITE_RUNTIME +plugins: + - remote: someremote.com/owner/plugins/myplugin + out: gen/go diff --git a/private/buf/bufmigrate/v1beta1_migrator.go b/private/buf/bufmigrate/v1beta1_migrator.go index 7c45e31bcd..c87e34b02f 100644 --- a/private/buf/bufmigrate/v1beta1_migrator.go +++ b/private/buf/bufmigrate/v1beta1_migrator.go @@ -356,7 +356,7 @@ func (m *v1beta1Migrator) maybeMigrateGenTemplate(dirPath string) (bool, error) Enabled: v1beta1GenTemplate.Managed, CcEnableArenas: v1beta1GenTemplate.Options.CcEnableArenas, JavaMultipleFiles: v1beta1GenTemplate.Options.JavaMultipleFiles, - OptimizeFor: v1beta1GenTemplate.Options.OptimizeFor, + OptimizeFor: bufgen.ExternalOptimizeForConfigV1{Default: v1beta1GenTemplate.Options.OptimizeFor}, }, } for _, plugin := range v1beta1GenTemplate.Plugins { diff --git a/private/buf/cmd/buf/testdata/migrate-v1beta1/success/complex/output/buf.gen.yaml b/private/buf/cmd/buf/testdata/migrate-v1beta1/success/complex/output/buf.gen.yaml index ee81900056..6627854f3a 100644 --- a/private/buf/cmd/buf/testdata/migrate-v1beta1/success/complex/output/buf.gen.yaml +++ b/private/buf/cmd/buf/testdata/migrate-v1beta1/success/complex/output/buf.gen.yaml @@ -7,4 +7,5 @@ plugins: opt: paths=source_relative managed: enabled: true - optimize_for: CODE_SIZE + optimize_for: + default: CODE_SIZE diff --git a/private/bufpkg/bufimage/bufimagemodify/bufimagemodify.go b/private/bufpkg/bufimage/bufimagemodify/bufimagemodify.go index dd09480072..41d760d361 100644 --- a/private/bufpkg/bufimage/bufimagemodify/bufimagemodify.go +++ b/private/bufpkg/bufimage/bufimagemodify/bufimagemodify.go @@ -194,14 +194,16 @@ func JavaStringCheckUtf8( func OptimizeFor( logger *zap.Logger, sweeper Sweeper, - value descriptorpb.FileOptions_OptimizeMode, + defaultOptimizeFor descriptorpb.FileOptions_OptimizeMode, + except []bufmoduleref.ModuleIdentity, + moduleOverrides map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode, overrides map[string]string, ) (Modifier, error) { validatedOverrides, err := stringOverridesToOptimizeModeOverrides(overrides) if err != nil { return nil, fmt.Errorf("invalid override for %s: %w", OptimizeForID, err) } - return optimizeFor(logger, sweeper, value, validatedOverrides), nil + return optimizeFor(logger, sweeper, defaultOptimizeFor, except, moduleOverrides, validatedOverrides), nil } // GoPackageImportPathForFile returns the go_package import path for the given diff --git a/private/bufpkg/bufimage/bufimagemodify/optimize_for.go b/private/bufpkg/bufimage/bufimagemodify/optimize_for.go index b76768dadc..190b5145b3 100644 --- a/private/bufpkg/bufimage/bufimagemodify/optimize_for.go +++ b/private/bufpkg/bufimage/bufimagemodify/optimize_for.go @@ -18,6 +18,7 @@ import ( "context" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleref" "go.uber.org/zap" "google.golang.org/protobuf/types/descriptorpb" ) @@ -32,22 +33,56 @@ var optimizeForPath = []int32{8, 9} func optimizeFor( logger *zap.Logger, sweeper Sweeper, - value descriptorpb.FileOptions_OptimizeMode, + defaultOptimizeFor descriptorpb.FileOptions_OptimizeMode, + except []bufmoduleref.ModuleIdentity, + moduleOverrides map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode, overrides map[string]descriptorpb.FileOptions_OptimizeMode, ) Modifier { + // Convert the bufmoduleref.ModuleIdentity types into + // strings so that they're comparable. + exceptModuleIdentityStrings := make(map[string]struct{}, len(except)) + for _, moduleIdentity := range except { + exceptModuleIdentityStrings[moduleIdentity.IdentityString()] = struct{}{} + } + overrideModuleIdentityStrings := make( + map[string]descriptorpb.FileOptions_OptimizeMode, + len(moduleOverrides), + ) + for moduleIdentity, optimizeFor := range moduleOverrides { + overrideModuleIdentityStrings[moduleIdentity.IdentityString()] = optimizeFor + } return ModifierFunc( func(ctx context.Context, image bufimage.Image) error { + seenModuleIdentityStrings := make(map[string]struct{}, len(overrideModuleIdentityStrings)) seenOverrideFiles := make(map[string]struct{}, len(overrides)) for _, imageFile := range image.Files() { - modifierValue := value + modifierValue := defaultOptimizeFor + if moduleIdentity := imageFile.ModuleIdentity(); moduleIdentity != nil { + moduleIdentityString := moduleIdentity.IdentityString() + if optimizeForOverrdie, ok := overrideModuleIdentityStrings[moduleIdentityString]; ok { + modifierValue = optimizeForOverrdie + seenModuleIdentityStrings[moduleIdentityString] = struct{}{} + } + } if overrideValue, ok := overrides[imageFile.Path()]; ok { modifierValue = overrideValue seenOverrideFiles[imageFile.Path()] = struct{}{} } - if err := optimizeForForFile(ctx, sweeper, imageFile, modifierValue); err != nil { + if err := optimizeForForFile( + ctx, + sweeper, + imageFile, + modifierValue, + exceptModuleIdentityStrings, + ); err != nil { return err } } + for moduleIdentityString := range overrideModuleIdentityStrings { + if _, ok := seenModuleIdentityStrings[moduleIdentityString]; !ok { + logger.Sugar().Warnf("optimize_for override for %q was unused", moduleIdentityString) + } + } for overrideFile := range overrides { if _, ok := seenOverrideFiles[overrideFile]; !ok { logger.Sugar().Warnf("%s override for %q was unused", OptimizeForID, overrideFile) @@ -63,6 +98,7 @@ func optimizeForForFile( sweeper Sweeper, imageFile bufimage.ImageFile, value descriptorpb.FileOptions_OptimizeMode, + exceptModuleIdentityStrings map[string]struct{}, ) error { descriptor := imageFile.Proto() options := descriptor.GetOptions() @@ -78,6 +114,11 @@ func optimizeForForFile( // same as the default, don't do anything. return nil } + if moduleIdentity := imageFile.ModuleIdentity(); moduleIdentity != nil { + if _, ok := exceptModuleIdentityStrings[moduleIdentity.IdentityString()]; ok { + return nil + } + } if options == nil { descriptor.Options = &descriptorpb.FileOptions{} } diff --git a/private/bufpkg/bufimage/bufimagemodify/optimize_for_test.go b/private/bufpkg/bufimage/bufimagemodify/optimize_for_test.go index 0772ff8110..5d5dd275a6 100644 --- a/private/bufpkg/bufimage/bufimagemodify/optimize_for_test.go +++ b/private/bufpkg/bufimage/bufimagemodify/optimize_for_test.go @@ -19,6 +19,7 @@ import ( "path/filepath" "testing" + "github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleref" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap" @@ -34,7 +35,7 @@ func TestOptimizeForEmptyOptions(t *testing.T) { assertFileOptionSourceCodeInfoEmpty(t, image, optimizeForPath, true) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil, nil, nil) require.NoError(t, err) modifier := NewMultiModifier( optimizeForModifier, @@ -59,7 +60,7 @@ func TestOptimizeForEmptyOptions(t *testing.T) { assertFileOptionSourceCodeInfoEmpty(t, image, optimizeForPath, false) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil, nil, nil) require.NoError(t, err) err = optimizeForModifier.Modify( context.Background(), @@ -80,7 +81,7 @@ func TestOptimizeForEmptyOptions(t *testing.T) { assertFileOptionSourceCodeInfoEmpty(t, image, optimizeForPath, true) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, map[string]string{"a.proto": "SPEED"}) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil, nil, map[string]string{"a.proto": "SPEED"}) require.NoError(t, err) modifier := NewMultiModifier( optimizeForModifier, @@ -109,7 +110,7 @@ func TestOptimizeForEmptyOptions(t *testing.T) { assertFileOptionSourceCodeInfoEmpty(t, image, optimizeForPath, false) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, map[string]string{"a.proto": "SPEED"}) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil, nil, map[string]string{"a.proto": "SPEED"}) require.NoError(t, err) err = optimizeForModifier.Modify( context.Background(), @@ -138,7 +139,7 @@ func TestOptimizeForAllOptions(t *testing.T) { assertFileOptionSourceCodeInfoNotEmpty(t, image, optimizeForPath) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil, nil, nil) require.NoError(t, err) modifier := NewMultiModifier( optimizeForModifier, @@ -164,7 +165,7 @@ func TestOptimizeForAllOptions(t *testing.T) { assertFileOptionSourceCodeInfoEmpty(t, image, optimizeForPath, false) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil, nil, nil) require.NoError(t, err) err = optimizeForModifier.Modify( context.Background(), @@ -185,7 +186,7 @@ func TestOptimizeForAllOptions(t *testing.T) { assertFileOptionSourceCodeInfoNotEmpty(t, image, optimizeForPath) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, map[string]string{"a.proto": "SPEED"}) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil, nil, map[string]string{"a.proto": "SPEED"}) require.NoError(t, err) modifier := NewMultiModifier( optimizeForModifier, @@ -214,7 +215,7 @@ func TestOptimizeForAllOptions(t *testing.T) { assertFileOptionSourceCodeInfoEmpty(t, image, optimizeForPath, false) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, map[string]string{"a.proto": "SPEED"}) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_LITE_RUNTIME, nil, nil, map[string]string{"a.proto": "SPEED"}) require.NoError(t, err) err = optimizeForModifier.Modify( context.Background(), @@ -243,7 +244,7 @@ func TestOptimizeForCcOptions(t *testing.T) { assertFileOptionSourceCodeInfoNotEmpty(t, image, optimizeForPath) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, nil) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, nil, nil, nil) require.NoError(t, err) modifier := NewMultiModifier( optimizeForModifier, @@ -269,7 +270,7 @@ func TestOptimizeForCcOptions(t *testing.T) { assertFileOptionSourceCodeInfoEmpty(t, image, optimizeForPath, false) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, nil) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, nil, nil, nil) require.NoError(t, err) err = optimizeForModifier.Modify( context.Background(), @@ -291,7 +292,7 @@ func TestOptimizeForCcOptions(t *testing.T) { assertFileOptionSourceCodeInfoNotEmpty(t, image, optimizeForPath) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, map[string]string{"a.proto": "LITE_RUNTIME"}) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, nil, nil, map[string]string{"a.proto": "LITE_RUNTIME"}) require.NoError(t, err) modifier := NewMultiModifier( optimizeForModifier, @@ -319,7 +320,7 @@ func TestOptimizeForCcOptions(t *testing.T) { assertFileOptionSourceCodeInfoEmpty(t, image, optimizeForPath, false) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, map[string]string{"a.proto": "LITE_RUNTIME"}) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, nil, nil, map[string]string{"a.proto": "LITE_RUNTIME"}) require.NoError(t, err) err = optimizeForModifier.Modify( context.Background(), @@ -347,7 +348,7 @@ func TestOptimizeForWellKnownTypes(t *testing.T) { image := testGetImage(t, dirPath, true) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, nil) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, nil, nil, nil) require.NoError(t, err) modifier := NewMultiModifier( optimizeForModifier, @@ -370,7 +371,7 @@ func TestOptimizeForWellKnownTypes(t *testing.T) { image := testGetImage(t, dirPath, false) sweeper := NewFileOptionSweeper() - optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, nil) + optimizeForModifier, err := OptimizeFor(zap.NewNop(), sweeper, descriptorpb.FileOptions_SPEED, nil, nil, nil) require.NoError(t, err) err = optimizeForModifier.Modify( context.Background(), @@ -384,3 +385,280 @@ func TestOptimizeForWellKnownTypes(t *testing.T) { } }) } + +func TestOptimizeForWithExcept(t *testing.T) { + t.Parallel() + dirPath := filepath.Join("testdata", "emptyoptions") + testModuleIdentity, err := bufmoduleref.NewModuleIdentity( + testRemote, + testRepositoryOwner, + testRepositoryName, + ) + require.NoError(t, err) + + t.Run("with SourceCodeInfo", func(t *testing.T) { + t.Parallel() + image := testGetImage(t, dirPath, true) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, true) + + sweeper := NewFileOptionSweeper() + optimizeForModifier, err := OptimizeFor( + zap.NewNop(), + sweeper, + descriptorpb.FileOptions_CODE_SIZE, + []bufmoduleref.ModuleIdentity{testModuleIdentity}, + nil, + nil, + ) + require.NoError(t, err) + + modifier := NewMultiModifier(optimizeForModifier, ModifierFunc(sweeper.Sweep)) + err = modifier.Modify( + context.Background(), + image, + ) + require.NoError(t, err) + assert.Equal(t, testGetImage(t, dirPath, true), image) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, true) + }) + + t.Run("without SourceCodeInfo", func(t *testing.T) { + t.Parallel() + image := testGetImage(t, dirPath, false) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, false) + + sweeper := NewFileOptionSweeper() + optimizeForModifier, err := OptimizeFor( + zap.NewNop(), + sweeper, + descriptorpb.FileOptions_CODE_SIZE, + []bufmoduleref.ModuleIdentity{testModuleIdentity}, + nil, + nil, + ) + require.NoError(t, err) + + modifier := NewMultiModifier(optimizeForModifier, ModifierFunc(sweeper.Sweep)) + err = modifier.Modify( + context.Background(), + image, + ) + require.NoError(t, err) + assert.Equal(t, testGetImage(t, dirPath, false), image) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, false) + }) + + t.Run("with SourceCodeInfo and per-file overrides", func(t *testing.T) { + t.Parallel() + image := testGetImage(t, dirPath, true) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, true) + + sweeper := NewFileOptionSweeper() + optimizeForModifier, err := OptimizeFor( + zap.NewNop(), + sweeper, + descriptorpb.FileOptions_CODE_SIZE, + []bufmoduleref.ModuleIdentity{testModuleIdentity}, + nil, + map[string]string{ + "a.proto": "LITE_RUNTIME", + }, + ) + require.NoError(t, err) + + modifier := NewMultiModifier(optimizeForModifier, ModifierFunc(sweeper.Sweep)) + err = modifier.Modify( + context.Background(), + image, + ) + require.NoError(t, err) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, true) + }) + + t.Run("without SourceCodeInfo and with per-file overrides", func(t *testing.T) { + t.Parallel() + image := testGetImage(t, dirPath, false) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, false) + + sweeper := NewFileOptionSweeper() + optimizeForModifier, err := OptimizeFor( + zap.NewNop(), + sweeper, + descriptorpb.FileOptions_CODE_SIZE, + []bufmoduleref.ModuleIdentity{testModuleIdentity}, + nil, + map[string]string{ + "a.proto": "SPEED", + }, + ) + require.NoError(t, err) + + modifier := NewMultiModifier(optimizeForModifier, ModifierFunc(sweeper.Sweep)) + err = modifier.Modify( + context.Background(), + image, + ) + require.NoError(t, err) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, false) + }) +} + +func TestOptimizeForWithOverride(t *testing.T) { + t.Parallel() + dirPath := filepath.Join("testdata", "emptyoptions") + overrideOptimizeFor := descriptorpb.FileOptions_LITE_RUNTIME + testModuleIdentity, err := bufmoduleref.NewModuleIdentity( + testRemote, + testRepositoryOwner, + testRepositoryName, + ) + require.NoError(t, err) + + t.Run("with SourceCodeInfo", func(t *testing.T) { + t.Parallel() + image := testGetImage(t, dirPath, true) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, true) + + sweeper := NewFileOptionSweeper() + optimizeForModifier, err := OptimizeFor( + zap.NewNop(), + sweeper, + descriptorpb.FileOptions_CODE_SIZE, + nil, + map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode{ + testModuleIdentity: overrideOptimizeFor, + }, + nil, + ) + require.NoError(t, err) + + modifier := NewMultiModifier(optimizeForModifier, ModifierFunc(sweeper.Sweep)) + err = modifier.Modify( + context.Background(), + image, + ) + require.NoError(t, err) + assert.NotEqual(t, testGetImage(t, dirPath, true), image) + + for _, imageFile := range image.Files() { + descriptor := imageFile.Proto() + assert.Equal(t, + overrideOptimizeFor, + descriptor.GetOptions().GetOptimizeFor(), + ) + } + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, true) + }) + + t.Run("without SourceCodeInfo", func(t *testing.T) { + t.Parallel() + image := testGetImage(t, dirPath, false) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, false) + + sweeper := NewFileOptionSweeper() + optimizeForModifier, err := OptimizeFor( + zap.NewNop(), + sweeper, + descriptorpb.FileOptions_CODE_SIZE, + nil, + map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode{ + testModuleIdentity: overrideOptimizeFor, + }, + nil, + ) + require.NoError(t, err) + + modifier := NewMultiModifier(optimizeForModifier, ModifierFunc(sweeper.Sweep)) + err = modifier.Modify( + context.Background(), + image, + ) + require.NoError(t, err) + assert.NotEqual(t, testGetImage(t, dirPath, true), image) + + for _, imageFile := range image.Files() { + descriptor := imageFile.Proto() + assert.Equal(t, + overrideOptimizeFor, + descriptor.GetOptions().GetOptimizeFor(), + ) + } + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, false) + }) + + t.Run("with SourceCodeInfo and per-file overrides", func(t *testing.T) { + t.Parallel() + image := testGetImage(t, dirPath, true) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, true) + + sweeper := NewFileOptionSweeper() + optimizeForModifier, err := OptimizeFor( + zap.NewNop(), + sweeper, + descriptorpb.FileOptions_CODE_SIZE, + nil, + map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode{ + testModuleIdentity: overrideOptimizeFor, + }, + map[string]string{ + "a.proto": "CODE_SIZE", + }, + ) + require.NoError(t, err) + + modifier := NewMultiModifier(optimizeForModifier, ModifierFunc(sweeper.Sweep)) + err = modifier.Modify( + context.Background(), + image, + ) + require.NoError(t, err) + assert.NotEqual(t, testGetImage(t, dirPath, true), image) + + for _, imageFile := range image.Files() { + descriptor := imageFile.Proto() + assert.Equal(t, + descriptorpb.FileOptions_CODE_SIZE, + descriptor.GetOptions().GetOptimizeFor(), + ) + } + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, true) + }) + + t.Run("without SourceCodeInfo and with per-file overrides", func(t *testing.T) { + t.Parallel() + image := testGetImage(t, dirPath, false) + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, false) + + sweeper := NewFileOptionSweeper() + optimizeForModifier, err := OptimizeFor( + zap.NewNop(), + sweeper, + descriptorpb.FileOptions_CODE_SIZE, + nil, + map[bufmoduleref.ModuleIdentity]descriptorpb.FileOptions_OptimizeMode{ + testModuleIdentity: overrideOptimizeFor, + }, + map[string]string{ + "a.proto": "CODE_SIZE", + }, + ) + require.NoError(t, err) + + modifier := NewMultiModifier(optimizeForModifier, ModifierFunc(sweeper.Sweep)) + err = modifier.Modify( + context.Background(), + image, + ) + require.NoError(t, err) + assert.NotEqual(t, testGetImage(t, dirPath, true), image) + + for _, imageFile := range image.Files() { + descriptor := imageFile.Proto() + assert.Equal(t, + descriptorpb.FileOptions_CODE_SIZE, + descriptor.GetOptions().GetOptimizeFor(), + ) + } + assertFileOptionSourceCodeInfoEmpty(t, image, goPackagePath, false) + }) +}