From 7a0e10e0219fd753b97c2879eb9a92584a419105 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Mon, 11 Mar 2024 10:36:19 -0700 Subject: [PATCH] Release edition defaults CLI arguments. These are no longer experimental, and are the preferred way to generate edition defaults IR for feature resolution. PiperOrigin-RevId: 614715815 --- .../compiler/command_line_interface.cc | 60 +++++++++---------- .../compiler/command_line_interface.h | 10 ++-- .../command_line_interface_unittest.cc | 56 ++++++++--------- src/google/protobuf/editions/defaults.bzl | 6 +- 4 files changed, 63 insertions(+), 69 deletions(-) diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 3395f6be7747..26ef835479f7 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -1386,8 +1386,8 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { } } - if (!experimental_edition_defaults_out_name_.empty()) { - if (!WriteExperimentalEditionDefaults(*descriptor_pool)) { + if (!edition_defaults_out_name_.empty()) { + if (!WriteEditionDefaults(*descriptor_pool)) { return 1; } } @@ -1681,9 +1681,9 @@ void CommandLineInterface::Clear() { dependency_out_name_.clear(); experimental_editions_ = false; - experimental_edition_defaults_out_name_.clear(); - experimental_edition_defaults_minimum_ = EDITION_UNKNOWN; - experimental_edition_defaults_maximum_ = EDITION_UNKNOWN; + edition_defaults_out_name_.clear(); + edition_defaults_minimum_ = EDITION_UNKNOWN; + edition_defaults_maximum_ = EDITION_UNKNOWN; mode_ = MODE_COMPILE; print_mode_ = PRINT_NONE; @@ -1928,8 +1928,7 @@ CommandLineInterface::ParseArgumentStatus CommandLineInterface::ParseArguments( return PARSE_ARGUMENT_FAIL; } if (mode_ == MODE_COMPILE && output_directives_.empty() && - descriptor_set_out_name_.empty() && - experimental_edition_defaults_out_name_.empty()) { + descriptor_set_out_name_.empty() && edition_defaults_out_name_.empty()) { std::cerr << "Missing output directives." << std::endl; return PARSE_ARGUMENT_FAIL; } @@ -2351,8 +2350,8 @@ CommandLineInterface::InterpretArgument(const std::string& name, // experimental, undocumented, unsupported flag. Enable it at your own risk // (or, just don't!). experimental_editions_ = true; - } else if (name == "--experimental_edition_defaults_out") { - if (!experimental_edition_defaults_out_name_.empty()) { + } else if (name == "--edition_defaults_out") { + if (!edition_defaults_out_name_.empty()) { std::cerr << name << " may only be passed once." << std::endl; return PARSE_ARGUMENT_FAIL; } @@ -2367,24 +2366,24 @@ CommandLineInterface::InterpretArgument(const std::string& name, << std::endl; return PARSE_ARGUMENT_FAIL; } - experimental_edition_defaults_out_name_ = value; - } else if (name == "--experimental_edition_defaults_minimum") { - if (experimental_edition_defaults_minimum_ != EDITION_UNKNOWN) { + edition_defaults_out_name_ = value; + } else if (name == "--edition_defaults_minimum") { + if (edition_defaults_minimum_ != EDITION_UNKNOWN) { std::cerr << name << " may only be passed once." << std::endl; return PARSE_ARGUMENT_FAIL; } if (!Edition_Parse(absl::StrCat("EDITION_", value), - &experimental_edition_defaults_minimum_)) { + &edition_defaults_minimum_)) { std::cerr << name << " unknown edition \"" << value << "\"." << std::endl; return PARSE_ARGUMENT_FAIL; } - } else if (name == "--experimental_edition_defaults_maximum") { - if (experimental_edition_defaults_maximum_ != EDITION_UNKNOWN) { + } else if (name == "--edition_defaults_maximum") { + if (edition_defaults_maximum_ != EDITION_UNKNOWN) { std::cerr << name << " may only be passed once." << std::endl; return PARSE_ARGUMENT_FAIL; } if (!Edition_Parse(absl::StrCat("EDITION_", value), - &experimental_edition_defaults_maximum_)) { + &edition_defaults_maximum_)) { std::cerr << name << " unknown edition \"" << value << "\"." << std::endl; return PARSE_ARGUMENT_FAIL; } @@ -2724,8 +2723,8 @@ bool CommandLineInterface::GenerateDependencyManifestFile( output_filenames.push_back(descriptor_set_out_name_); } - if (!experimental_edition_defaults_out_name_.empty()) { - output_filenames.push_back(experimental_edition_defaults_out_name_); + if (!edition_defaults_out_name_.empty()) { + output_filenames.push_back(edition_defaults_out_name_); } // Create the depfile, even if it will be empty. @@ -3029,12 +3028,11 @@ bool CommandLineInterface::WriteDescriptorSet( return true; } -bool CommandLineInterface::WriteExperimentalEditionDefaults( - const DescriptorPool& pool) { +bool CommandLineInterface::WriteEditionDefaults(const DescriptorPool& pool) { const Descriptor* feature_set = pool.FindMessageTypeByName("google.protobuf.FeatureSet"); if (feature_set == nullptr) { - std::cerr << experimental_edition_defaults_out_name_ + std::cerr << edition_defaults_out_name_ << ": Could not find FeatureSet in descriptor pool. Please make " "sure descriptor.proto is in your import path" << std::endl; @@ -3044,31 +3042,31 @@ bool CommandLineInterface::WriteExperimentalEditionDefaults( pool.FindAllExtensions(feature_set, &extensions); Edition minimum = PROTOBUF_MINIMUM_EDITION; - if (experimental_edition_defaults_minimum_ != EDITION_UNKNOWN) { - minimum = experimental_edition_defaults_minimum_; + if (edition_defaults_minimum_ != EDITION_UNKNOWN) { + minimum = edition_defaults_minimum_; } Edition maximum = PROTOBUF_MAXIMUM_EDITION; - if (experimental_edition_defaults_maximum_ != EDITION_UNKNOWN) { - maximum = experimental_edition_defaults_maximum_; + if (edition_defaults_maximum_ != EDITION_UNKNOWN) { + maximum = edition_defaults_maximum_; } absl::StatusOr defaults = FeatureResolver::CompileDefaults(feature_set, extensions, minimum, maximum); if (!defaults.ok()) { - std::cerr << experimental_edition_defaults_out_name_ << ": " + std::cerr << edition_defaults_out_name_ << ": " << defaults.status().message() << std::endl; return false; } int fd; do { - fd = open(experimental_edition_defaults_out_name_.c_str(), + fd = open(edition_defaults_out_name_.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666); } while (fd < 0 && errno == EINTR); if (fd < 0) { - perror(experimental_edition_defaults_out_name_.c_str()); + perror(edition_defaults_out_name_.c_str()); return false; } @@ -3080,7 +3078,7 @@ bool CommandLineInterface::WriteExperimentalEditionDefaults( // into version control. coded_out.SetSerializationDeterministic(true); if (!defaults->SerializeToCodedStream(&coded_out)) { - std::cerr << experimental_edition_defaults_out_name_ << ": " + std::cerr << edition_defaults_out_name_ << ": " << strerror(out.GetErrno()) << std::endl; out.Close(); return false; @@ -3088,8 +3086,8 @@ bool CommandLineInterface::WriteExperimentalEditionDefaults( } if (!out.Close()) { - std::cerr << experimental_edition_defaults_out_name_ << ": " - << strerror(out.GetErrno()) << std::endl; + std::cerr << edition_defaults_out_name_ << ": " << strerror(out.GetErrno()) + << std::endl; return false; } diff --git a/src/google/protobuf/compiler/command_line_interface.h b/src/google/protobuf/compiler/command_line_interface.h index 0828497d6a71..e99deada8e78 100644 --- a/src/google/protobuf/compiler/command_line_interface.h +++ b/src/google/protobuf/compiler/command_line_interface.h @@ -294,8 +294,8 @@ class PROTOC_EXPORT CommandLineInterface { bool WriteDescriptorSet( const std::vector& parsed_files); - // Implements the --experimental_edition_defaults_out option. - bool WriteExperimentalEditionDefaults(const DescriptorPool& pool); + // Implements the --edition_defaults_out option. + bool WriteEditionDefaults(const DescriptorPool& pool); // Implements the --dependency_out option bool GenerateDependencyManifestFile( @@ -434,9 +434,9 @@ class PROTOC_EXPORT CommandLineInterface { // FileDescriptorSet should be written. Otherwise, empty. std::string descriptor_set_out_name_; - std::string experimental_edition_defaults_out_name_; - Edition experimental_edition_defaults_minimum_; - Edition experimental_edition_defaults_maximum_; + std::string edition_defaults_out_name_; + Edition edition_defaults_minimum_; + Edition edition_defaults_maximum_; // If --dependency_out was given, this is the path to the file where the // dependency file will be written. Otherwise, empty. diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index 8c0dfe17d88d..f351edc06248 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -1808,7 +1808,7 @@ TEST_F(CommandLineInterfaceTest, EditionDefaults) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_out=$tmpdir/defaults " + "--edition_defaults_out=$tmpdir/defaults " "google/protobuf/descriptor.proto"); ExpectNoErrors(); @@ -1856,8 +1856,8 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsWithMaximum) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_out=$tmpdir/defaults " - "--experimental_edition_defaults_maximum=99997_TEST_ONLY " + "--edition_defaults_out=$tmpdir/defaults " + "--edition_defaults_maximum=99997_TEST_ONLY " "google/protobuf/descriptor.proto"); ExpectNoErrors(); @@ -1905,9 +1905,9 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsWithMinimum) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_out=$tmpdir/defaults " - "--experimental_edition_defaults_minimum=99997_TEST_ONLY " - "--experimental_edition_defaults_maximum=99999_TEST_ONLY " + "--edition_defaults_out=$tmpdir/defaults " + "--edition_defaults_minimum=99997_TEST_ONLY " + "--edition_defaults_maximum=99999_TEST_ONLY " "google/protobuf/descriptor.proto"); ExpectNoErrors(); @@ -1957,8 +1957,8 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsWithExtension) { CreateTempFile("features.proto", pb::TestFeatures::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_out=$tmpdir/defaults " - "--experimental_edition_defaults_maximum=99999_TEST_ONLY " + "--edition_defaults_out=$tmpdir/defaults " + "--edition_defaults_maximum=99999_TEST_ONLY " "features.proto google/protobuf/descriptor.proto"); ExpectNoErrors(); @@ -1996,7 +1996,7 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsDependencyManifest) { pb::TestFeatures::descriptor()->file()->DebugString()); Run("protocol_compiler --dependency_out=$tmpdir/manifest " - "--experimental_edition_defaults_out=$tmpdir/defaults " + "--edition_defaults_out=$tmpdir/defaults " "--proto_path=$tmpdir features.proto"); ExpectNoErrors(); @@ -2014,7 +2014,7 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidMissingDescriptor) { message Foo {} )schema"); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_out=$tmpdir/defaults " + "--edition_defaults_out=$tmpdir/defaults " "features.proto"); ExpectErrorSubstring("Could not find FeatureSet in descriptor pool"); } @@ -2023,21 +2023,19 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidTwice) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_out=$tmpdir/defaults " - "--experimental_edition_defaults_out=$tmpdir/defaults " + "--edition_defaults_out=$tmpdir/defaults " + "--edition_defaults_out=$tmpdir/defaults " "google/protobuf/descriptor.proto"); - ExpectErrorSubstring( - "experimental_edition_defaults_out may only be passed once"); + ExpectErrorSubstring("edition_defaults_out may only be passed once"); } TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidEmpty) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_out= " + "--edition_defaults_out= " "google/protobuf/descriptor.proto"); - ExpectErrorSubstring( - "experimental_edition_defaults_out requires a non-empty value"); + ExpectErrorSubstring("edition_defaults_out requires a non-empty value"); } TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidCompile) { @@ -2045,7 +2043,7 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidCompile) { google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " "--encode=pb.CppFeatures " - "--experimental_edition_defaults_out=$tmpdir/defaults " + "--edition_defaults_out=$tmpdir/defaults " "google/protobuf/descriptor.proto"); ExpectErrorSubstring("Cannot use --encode or --decode and generate defaults"); } @@ -2054,18 +2052,17 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidMinimumTwice) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_minimum=2023 " - "--experimental_edition_defaults_minimum=2023 " + "--edition_defaults_minimum=2023 " + "--edition_defaults_minimum=2023 " "google/protobuf/descriptor.proto"); - ExpectErrorSubstring( - "experimental_edition_defaults_minimum may only be passed once"); + ExpectErrorSubstring("edition_defaults_minimum may only be passed once"); } TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidMinimumEmpty) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_minimum= " + "--edition_defaults_minimum= " "google/protobuf/descriptor.proto"); ExpectErrorSubstring("unknown edition \"\""); } @@ -2074,7 +2071,7 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidMinimumUnknown) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_minimum=2022 " + "--edition_defaults_minimum=2022 " "google/protobuf/descriptor.proto"); ExpectErrorSubstring("unknown edition \"2022\""); } @@ -2083,18 +2080,17 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidMaximumTwice) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_maximum=2023 " - "--experimental_edition_defaults_maximum=2023 " + "--edition_defaults_maximum=2023 " + "--edition_defaults_maximum=2023 " "google/protobuf/descriptor.proto"); - ExpectErrorSubstring( - "experimental_edition_defaults_maximum may only be passed once"); + ExpectErrorSubstring("edition_defaults_maximum may only be passed once"); } TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidMaximumEmpty) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_maximum= " + "--edition_defaults_maximum= " "google/protobuf/descriptor.proto"); ExpectErrorSubstring("unknown edition \"\""); } @@ -2103,7 +2099,7 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidMaximumUnknown) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); Run("protocol_compiler --proto_path=$tmpdir " - "--experimental_edition_defaults_maximum=2022 " + "--edition_defaults_maximum=2022 " "google/protobuf/descriptor.proto"); ExpectErrorSubstring("unknown edition \"2022\""); } diff --git a/src/google/protobuf/editions/defaults.bzl b/src/google/protobuf/editions/defaults.bzl index 9ab7b318373c..e20b9222b855 100644 --- a/src/google/protobuf/editions/defaults.bzl +++ b/src/google/protobuf/editions/defaults.bzl @@ -20,10 +20,10 @@ def _compile_edition_defaults_impl(ctx): paths.extend(src[ProtoInfo].transitive_proto_path.to_list()) args = ctx.actions.args() - args.add("--experimental_edition_defaults_out", out_file) + args.add("--edition_defaults_out", out_file) - args.add("--experimental_edition_defaults_minimum", ctx.attr.minimum_edition) - args.add("--experimental_edition_defaults_maximum", ctx.attr.maximum_edition) + args.add("--edition_defaults_minimum", ctx.attr.minimum_edition) + args.add("--edition_defaults_maximum", ctx.attr.maximum_edition) for p in paths: args.add("--proto_path", p) for source in sources: