Skip to content

Commit

Permalink
Implement feature lifetime validation in protoc and the C++ runtime.
Browse files Browse the repository at this point in the history
Features need to be validated within the pool being built, since the generated pool only contains extensions linked into the binary (e.g. protoc or a runtime building dynamic protos).  The generated pool may be missing extensions used in this proto or it may have version skew.  Moving to the build pool requires reflective parsing, which in general can't be done from inside the pool's database lock.  This required some refactoring to add a post-build validation phase outside of the lock.

For now, the feature support spec is optional and the checks only are only applied when it's present.  Follow-up changes will add these specs to our existing features and then require them for all FeatureSet extensions.

PiperOrigin-RevId: 623630219
  • Loading branch information
mkruskal-google authored and copybara-github committed Apr 10, 2024
1 parent b267cd4 commit b3b4497
Show file tree
Hide file tree
Showing 10 changed files with 1,205 additions and 172 deletions.
8 changes: 8 additions & 0 deletions python/google/protobuf/internal/descriptor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,7 @@ class ReturnObject:

ret = ReturnObject()
ret.pool = descriptor_pool.DescriptorPool()

defaults = descriptor_pb2.FeatureSetDefaults(
defaults=[
descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault(
Expand All @@ -1465,6 +1466,13 @@ class ReturnObject:
].multiple_feature = 1
ret.pool.SetFeatureSetDefaults(defaults)

# Add dependencies
file = descriptor_pb2.FileDescriptorProto()
descriptor_pb2.DESCRIPTOR.CopyToProto(file)
ret.pool.Add(file)
unittest_features_pb2.DESCRIPTOR.CopyToProto(file)
ret.pool.Add(file)

ret.file = ret.pool.AddSerializedFile(self.file_proto.SerializeToString())
ret.top_message = ret.pool.FindMessageTypeByName(
'protobuf_unittest.TopMessage'
Expand Down
73 changes: 65 additions & 8 deletions src/google/protobuf/compiler/command_line_interface_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1761,6 +1761,63 @@ TEST_F(CommandLineInterfaceTest, Plugin_SourceFeatures) {
}
}

TEST_F(CommandLineInterfaceTest, GeneratorFeatureLifetimeError) {
CreateTempFile("google/protobuf/descriptor.proto",
google::protobuf::DescriptorProto::descriptor()->file()->DebugString());
CreateTempFile("google/protobuf/unittest_features.proto",
pb::TestFeatures::descriptor()->file()->DebugString());
CreateTempFile("foo.proto",
R"schema(
edition = "2024";
import "google/protobuf/unittest_features.proto";
package foo;
message Foo {
int32 b = 1 [
features.(pb.test).removed_feature = VALUE6
];
}
)schema");

Run("protocol_compiler --experimental_editions --proto_path=$tmpdir "
"--test_out=$tmpdir foo.proto");
ExpectErrorSubstring(
"foo.proto:6:13: Feature pb.TestFeatures.removed_feature has been "
"removed in edition 2024");
}

TEST_F(CommandLineInterfaceTest, PluginFeatureLifetimeError) {
CreateTempFile("google/protobuf/descriptor.proto",
google::protobuf::DescriptorProto::descriptor()->file()->DebugString());
CreateTempFile("google/protobuf/unittest_features.proto",
pb::TestFeatures::descriptor()->file()->DebugString());
CreateTempFile("foo.proto",
R"schema(
edition = "2023";
import "google/protobuf/unittest_features.proto";
package foo;
message Foo {
int32 b = 1 [
features.(pb.test).future_feature = VALUE6
];
}
)schema");

#ifdef GOOGLE_PROTOBUF_FAKE_PLUGIN_PATH
std::string plugin_path = GOOGLE_PROTOBUF_FAKE_PLUGIN_PATH;
#else
std::string plugin_path = absl::StrCat(
TestUtil::TestSourceDir(), "/google/protobuf/compiler/fake_plugin");
#endif

Run(absl::StrCat(
"protocol_compiler --fake_plugin_out=$tmpdir --proto_path=$tmpdir "
"foo.proto --plugin=prefix-gen-fake_plugin=",
plugin_path));
ExpectErrorSubstring(
"foo.proto:6:13: Feature pb.TestFeatures.future_feature wasn't "
"introduced until edition 2024");
}

TEST_F(CommandLineInterfaceTest, GeneratorNoEditionsSupport) {
CreateTempFile("foo.proto", R"schema(
edition = "2023";
Expand Down Expand Up @@ -1958,26 +2015,26 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsWithExtension) {
FeatureSetDefaults defaults = ReadEditionDefaults("defaults");
EXPECT_EQ(defaults.minimum_edition(), EDITION_PROTO2);
EXPECT_EQ(defaults.maximum_edition(), EDITION_99999_TEST_ONLY);
ASSERT_EQ(defaults.defaults_size(), 5);
ASSERT_EQ(defaults.defaults_size(), 6);
EXPECT_EQ(defaults.defaults(0).edition(), EDITION_PROTO2);
EXPECT_EQ(defaults.defaults(1).edition(), EDITION_PROTO3);
EXPECT_EQ(defaults.defaults(2).edition(), EDITION_2023);
EXPECT_EQ(defaults.defaults(3).edition(), EDITION_99997_TEST_ONLY);
EXPECT_EQ(defaults.defaults(4).edition(), EDITION_99998_TEST_ONLY);
EXPECT_EQ(defaults.defaults(3).edition(), EDITION_2024);
EXPECT_EQ(defaults.defaults(4).edition(), EDITION_99997_TEST_ONLY);
EXPECT_EQ(defaults.defaults(5).edition(), EDITION_99998_TEST_ONLY);
EXPECT_EQ(
defaults.defaults(0).features().GetExtension(pb::test).file_feature(),
pb::EnumFeature::VALUE1);
EXPECT_EQ(
defaults.defaults(1).features().GetExtension(pb::test).file_feature(),
pb::EnumFeature::VALUE2);
EXPECT_EQ(
defaults.defaults(2).features().GetExtension(pb::test).file_feature(),
pb::EnumFeature::VALUE3);
EXPECT_EQ(
defaults.defaults(3).features().GetExtension(pb::test).file_feature(),
pb::EnumFeature::VALUE4);
pb::EnumFeature::VALUE3);
EXPECT_EQ(
defaults.defaults(4).features().GetExtension(pb::test).file_feature(),
pb::EnumFeature::VALUE4);
EXPECT_EQ(
defaults.defaults(5).features().GetExtension(pb::test).file_feature(),
pb::EnumFeature::VALUE5);
}

Expand Down
Loading

0 comments on commit b3b4497

Please sign in to comment.