Skip to content

Commit

Permalink
BSR-704/default except override for optimize_for (bufbuild#1625)
Browse files Browse the repository at this point in the history 
* Add default, except override to managed mode `optimize_for` option
* preserves backward compatibility with the current `optimize_for`
option being string
  • Loading branch information
@oliversun09
oliversun09 committed Dec 12, 2022
1 parent 631caf7 commit a79d156
Show file tree
Hide file tree
Showing 16 changed files with 683 additions and 56 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -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

Expand Down
59 changes: 56 additions & 3 deletions private/buf/bufgen/bufgen.go
View file
Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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"`
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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"`
Expand Down
89 changes: 68 additions & 21 deletions private/buf/bufgen/config.go
View file
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}

Expand All @@ -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
}
Expand Down

0 comments on commit a79d156

Please sign in to comment.