Reverse last-minute switch to edition-lifetimes design.

We will be punting on actual implementation of lifetime verification during dynamic builds, but in the future leaning towards the simplified algorithm.

PiperOrigin-RevId: 624937514
pull/16497/head
Mike Kruskal 12 months ago committed by Copybara-Service
parent 3f493d9e52
commit cfba3a661d
  1. 185
      docs/design/editions/edition-lifetimes.md

@ -201,50 +201,54 @@ is to implement this ASAP, before we start rolling out 2024.
#### Runtimes with Dynamic Messages #### Runtimes with Dynamic Messages
None of the generators where editions have already been rolled out require any None of the generators where editions have already been rolled out require any
changes. We will need to add validation layers to runtimes that support dynamic changes. We likely will want to add validation layers to runtimes that support
messages though, to make sure there are no invalid descriptors floating around. dynamic messages though, to make sure there are no invalid descriptors floating
Any runtime that supports dynamic messages should have reflection, and the same around. Since they all have access to protoc's compiled defaults IR, we can pack
reflection-based algorithm will need to be duplicated everywhere. For each as much information in there as possible to minimize duplication. Specifically,
`FeatureSet` specified on a descriptor: we will add two new `FeatureSet` fields to `FeatureSetEditionDefault` in
addition to the existing `features` field.
``` * overridable_features - The default values that users **are** allowed to
absl::Status Validate(Edition edition, Message& features) { override in a given edition
std::vector<const FieldDescriptor*> fields; * fixed_features - The default values that users **are not** allowed to
features.GetReflection()->ListFields(features, &fields); override in a given edition
for (const FieldDescriptor* field : fields) {
// Recurse into message extension.
if (field->is_extension() &&
field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
CollectLifetimeResults(
edition, message.GetReflection()->GetMessage(message, field),
results);
continue;
}
// Skip fields that don't have feature support specified. We will keep the existing `features` field as a migration tool, to avoid
if (!field->options().has_feature_support()) continue; breaking plugins and runtimes that already use it to calculate defaults. We can
strip it from OSS prior to the 27.0 release though, and remove it once everyone
has been migrated.
// Check lifetime constrains In order to calculate the full defaults of any edition, each language will
const FieldOptions::FeatureSupport& support = simply need to merge the two `FeatureSet` objects. The advantage to splitting
field->options().feature_support(); them means that we can fairly easily implement validation checks in every
if (edition < support.edition_introduced()) { language that needs it for dynamic messages. The algorithm is as follows, for
return absl::FailedPrecondition(absl::StrCat( some incoming unresolved `FeatureSet` user_features:
"Feature ", field->full_name(), " wasn't introduced until edition ",
support.edition_introduced())); 1. Strip all unknown fields from user_features
} 2. Strip all extensions from user_features that the runtime doesn't handle
if (support.has_edition_removed() && edition >= support.edition_removed()) { 3. merged_features := user_features.Merge(overridable_defaults)
return absl::FailedPrecondition(absl::StrCat( 4. assert merged_features == overridable_defaults
"Feature ", field->full_name(), " has been removed in edition ",
support.edition_removed())); This will work as long as every feature is a scalar value (making merge a simple
} else if (support.has_edition_deprecated() && override). We already ban oneof and repeated features, and we plan to ban
edition >= support.edition_deprecated()) { message features before the OSS release.
ABSL_LOG(WARNING) << absl::StrCat(
"Feature ", field->full_name(), " has been deprecated in edition ", Note, that there is a slight gap here in that we perform no validation for
support.edition_deprecated(), ": ", support.deprecation_warning()); features owned by *other* languages. Dynamic messages in language A will naively
} be allowed to specify whatever language B features they want. This isn't
} optimal, but it is in line with our current situation where validation of
} dynamic messages is substantially more permissive than descriptors processed by
``` protoc.
On the other hand, owners of language A will have the *option* of easily adding
validation for language B's features, without having to reimplement the
reflective inspection of imports that protoc does. This can be done by simply
adding those features to the compilation of the defaults IR, and then not
stripping those extensions during validation. This will have the effect of tying
the edition support window of A to that of B though, and A won't be able to
extend its maximum edition until B does (at least for dynamic messages). For
generators in a monorepo like Protobuf's this seems fine, but may not be
desirable elsewhere.
### Patching Old Editions ### Patching Old Editions
@ -302,61 +306,68 @@ problems listed in the overview of this topic.
* High cognitive overhead for our users. They'd need to track the progress of * High cognitive overhead for our users. They'd need to track the progress of
every feature individually across releases. every feature individually across releases.
### "Simplified" Algorithm for Dynamic Messages ### Full Validation for Dynamic Messages
**Below is from the original proposal, and has been replaced with a simpler and
more accurate reflection-based approach.**
None of the generators where editions have already been rolled out require any None of the generators where editions have already been rolled out require any
changes. We likely will want to add validation layers to runtimes that support changes. We will need to add validation layers to runtimes that support dynamic
dynamic messages though, to make sure there are no invalid descriptors floating messages though, to make sure there are no invalid descriptors floating around.
around. Since they all have access to protoc's compiled defaults IR, we can pack Any runtime that supports dynamic messages should have reflection, and the same
as much information in there as possible to minimize duplication. Specifically, reflection-based algorithm will need to be duplicated everywhere. For each
we will add two new `FeatureSet` fields to `FeatureSetEditionDefault` in `FeatureSet` specified on a descriptor:
addition to the existing `features` field.
* overridable_features - The default values that users **are** allowed to ```
override in a given edition absl::Status Validate(Edition edition, Message& features) {
* fixed_features - The default values that users **are not** allowed to std::vector<const FieldDescriptor*> fields;
override in a given edition features.GetReflection()->ListFields(features, &fields);
for (const FieldDescriptor* field : fields) {
// Recurse into message extension.
if (field->is_extension() &&
field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
CollectLifetimeResults(
edition, message.GetReflection()->GetMessage(message, field),
results);
continue;
}
We will keep the existing `features` field as a migration tool, to avoid // Skip fields that don't have feature support specified.
breaking plugins and runtimes that already use it to calculate defaults. We can if (!field->options().has_feature_support()) continue;
strip it from OSS prior to the 27.0 release though, and remove it once everyone
has been migrated.
In order to calculate the full defaults of any edition, each language will // Check lifetime constrains
simply need to merge the two `FeatureSet` objects. The advantage to splitting const FieldOptions::FeatureSupport& support =
them means that we can fairly easily implement validation checks in every field->options().feature_support();
language that needs it for dynamic messages. The algorithm is as follows, for if (edition < support.edition_introduced()) {
some incoming unresolved `FeatureSet` user_features: return absl::FailedPrecondition(absl::StrCat(
"Feature ", field->full_name(), " wasn't introduced until edition ",
support.edition_introduced()));
}
if (support.has_edition_removed() && edition >= support.edition_removed()) {
return absl::FailedPrecondition(absl::StrCat(
"Feature ", field->full_name(), " has been removed in edition ",
support.edition_removed()));
} else if (support.has_edition_deprecated() &&
edition >= support.edition_deprecated()) {
ABSL_LOG(WARNING) << absl::StrCat(
"Feature ", field->full_name(), " has been deprecated in edition ",
support.edition_deprecated(), ": ", support.deprecation_warning());
}
}
}
```
1. Strip all unknown fields from user_features within our extensible range #### Pros
(fields 1000+)
2. Strip all extensions from user_features that the runtime doesn't handle
3. merged_features := user_features.Merge(overridable_defaults)
4. assert merged_features == overridable_defaults
This will work as long as every feature is a scalar value (making merge a simple * Prevents any feature lifetime violations for any language, in any language
override). We already ban oneof and repeated features, and we plan to ban * Easier to understand
message features before the OSS release. * Less error-prone
* Easy to test with fake features
Note, that there is a slight gap here in that we perform no validation for #### Cons
features owned by *other* languages. Dynamic messages in language A will naively
be allowed to specify whatever language B features they want. This isn't
optimal, but it is in line with our current situation where validation of
dynamic messages is substantially more permissive than descriptors processed by
protoc.
On the other hand, owners of language A will have the *option* of easily adding * Only works post-build, which requires a huge amount of code in every
validation for language B's features, without having to reimplement the language to walk the descriptor tree applying these checks
reflective inspection of imports that protoc does. This can be done by simply * Performance concerns, especially in upb
adding those features to the compilation of the defaults IR, and then not * Duplicates protoc validation, even though most languages perform
stripping those extensions during validation. This will have the effect of tying significantly looser checks on dynamic messages
the edition support window of A to that of B though, and A won't be able to
extend its maximum edition until B does (at least for dynamic messages). For
generators in a monorepo like Protobuf's this seems fine, but may not be
desirable elsewhere.
#### Pros #### Pros

Loading…
Cancel
Save