Fix `PROTO2_OPENSOURCE` annotations

These are supposed to control some code differences between open source and our
internal codebase, but some small errors or typos seem to have prevented a few
of them from working as intended.

I had to fix up some tests as well. Some test cases had to be tweaked to avoid
making assumptions about exact file paths in error messages, and there was also
a memory leak caused by `getcwd` allocating a buffer that was never freed.

Fixes #17507.

PiperOrigin-RevId: 682420154
pull/18614/head
Adam Cozzette 5 months ago committed by Copybara-Service
parent a270d56dcd
commit 1481ea06e0
  1. 4
      src/google/protobuf/compiler/command_line_interface.cc
  2. 4
      src/google/protobuf/compiler/command_line_interface_tester.cc
  3. 3
      src/google/protobuf/compiler/command_line_interface_tester.h
  4. 19
      src/google/protobuf/compiler/command_line_interface_unittest.cc
  5. 5
      src/google/protobuf/message_lite.h
  6. 4
      src/google/protobuf/testing/file.cc
  7. 2
      src/google/protobuf/testing/file.h

@ -394,10 +394,6 @@ class CommandLineInterface::ErrorPrinter
std::ostream& out) {
std::string dfile;
if (
#ifndef PROTOBUF_OPENSOURCE
// Print full path when running under MSVS
format_ == CommandLineInterface::ERROR_FORMAT_MSVS &&
#endif // !PROTOBUF_OPENSOURCE
tree_ != nullptr && tree_->VirtualFileToDiskFile(filename, &dfile)) {
out << dfile;
} else {

@ -122,8 +122,8 @@ void CommandLineInterfaceTester::ExpectNoErrors() {
void CommandLineInterfaceTester::ExpectErrorText(
absl::string_view expected_text) {
EXPECT_NE(0, return_code_);
EXPECT_EQ(absl::StrReplaceAll(expected_text, {{"$tmpdir", temp_directory_}}),
error_text_);
EXPECT_THAT(error_text_, HasSubstr(absl::StrReplaceAll(
expected_text, {{"$tmpdir", temp_directory_}})));
}
void CommandLineInterfaceTester::ExpectErrorSubstring(

@ -25,6 +25,7 @@
#include "absl/strings/string_view.h"
#include "google/protobuf/compiler/code_generator.h"
#include "google/protobuf/compiler/command_line_interface.h"
#include "google/protobuf/testing/file.h"
// Must be included last.
#include "google/protobuf/port_def.inc"
@ -76,12 +77,10 @@ class CommandLineInterfaceTester : public testing::Test {
// Creates a subdirectory within temp_directory_.
void CreateTempDir(absl::string_view name);
#ifdef PROTOBUF_OPENSOURCE
// Changes working directory to temp directory.
void SwitchToTempDirectory() {
File::ChangeWorkingDirectory(temp_directory_);
}
#endif // !PROTOBUF_OPENSOURCE
// -----------------------------------------------------------------
// Methods to check the test results (called after Run()).

@ -2618,7 +2618,6 @@ TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFileGivenTwoInputs) {
"Can only process one input file when using --dependency_out=FILE.\n");
}
#ifdef PROTOBUF_OPENSOURCE
TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFile) {
CreateTempFile("foo.proto",
"syntax = \"proto2\";\n"
@ -2630,7 +2629,8 @@ TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFile) {
" optional Foo foo = 1;\n"
"}\n");
std::string current_working_directory = getcwd(nullptr, 0);
char current_dir[PATH_MAX];
ASSERT_EQ(getcwd(current_dir, sizeof(current_dir)), current_dir);
SwitchToTempDirectory();
Run("protocol_compiler --dependency_out=manifest --test_out=. "
@ -2642,12 +2642,8 @@ TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFile) {
"bar.proto.MockCodeGenerator.test_generator: "
"foo.proto\\\n bar.proto");
File::ChangeWorkingDirectory(current_working_directory);
File::ChangeWorkingDirectory(current_dir);
}
#else // !PROTOBUF_OPENSOURCE
// TODO: Figure out how to change and get working directory in
// google3.
#endif // !PROTOBUF_OPENSOURCE
TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFileForAbsolutePath) {
CreateTempFile("foo.proto",
@ -2771,9 +2767,12 @@ TEST_F(CommandLineInterfaceTest, ParseErrorsMultipleFiles) {
"--proto_path=$tmpdir foo.proto");
ExpectErrorText(
"bar.proto:2:1: Expected top-level statement (e.g. \"message\").\n"
"baz.proto:2:1: Import \"bar.proto\" was not found or had errors.\n"
"foo.proto:2:1: Import \"bar.proto\" was not found or had errors.\n"
"bar.proto:2:1: Expected top-level statement (e.g. \"message\").\n");
ExpectErrorText(
"baz.proto:2:1: Import \"bar.proto\" was not found or had errors.\n");
ExpectErrorText(
"foo.proto:2:1: Import \"bar.proto\" was not found or had errors.\n");
ExpectErrorText(
"foo.proto:3:1: Import \"baz.proto\" was not found or had errors.\n");
}

@ -1206,11 +1206,6 @@ PROTOBUF_ALWAYS_INLINE inline MessageLite* MessageCreator::PlacementNew(
// - We know the minimum size is 16. We have a fallback for when it is not.
// - We can "underflow" the buffer because those are the MessageLite bytes
// we will set later.
#ifndef PROTO2_OPENSOURCE
// This manual handling shows a 1.85% improvement in the parsing
// microbenchmark.
// TODO: Verify this is still the case.
#endif // !PROTO2_OPENSOUCE
if (as_tag == kZeroInit) {
// Make sure the input is really all zeros.
ABSL_DCHECK(std::all_of(src + sizeof(MessageLite), src + size,

@ -196,8 +196,8 @@ void File::DeleteRecursively(const std::string& name, void* dummy1,
#endif
}
bool File::ChangeWorkingDirectory(const std::string& new_working_directory) {
return chdir(new_working_directory.c_str()) == 0;
bool File::ChangeWorkingDirectory(absl::string_view new_working_directory) {
return chdir(new_working_directory.data()) == 0;
}
} // namespace protobuf

@ -63,7 +63,7 @@ class File {
void* dummy2);
// Change working directory to given directory.
static bool ChangeWorkingDirectory(const std::string& new_working_directory);
static bool ChangeWorkingDirectory(absl::string_view new_working_directory);
static absl::Status GetContents(const std::string& name, std::string* output,
bool /*is_default*/) {

Loading…
Cancel
Save