From 9e07542a03fefa2cf982ba093b099805362df05d Mon Sep 17 00:00:00 2001
From: Christian Blichmann <cblichmann@google.com>
Date: Mon, 7 Oct 2024 02:59:35 -0700
Subject: [PATCH] mounts: Remove `optional` from mount tree proto

Proto3 semantics are "optional" by default and now only control generation of `has_XXX()` presence checks. For the mount tree, we only need those for `node`.

PiperOrigin-RevId: 683103568
Change-Id: I3c83385b52431c135df518d21cb20267beb09bf0
---
 sandboxed_api/sandbox2/mount_tree.proto | 12 ++---
 sandboxed_api/sandbox2/mounts_test.cc   | 63 +++++++++++--------------
 2 files changed, 34 insertions(+), 41 deletions(-)

diff --git a/sandboxed_api/sandbox2/mount_tree.proto b/sandboxed_api/sandbox2/mount_tree.proto
index 64fc81156..2663c7f49 100644
--- a/sandboxed_api/sandbox2/mount_tree.proto
+++ b/sandboxed_api/sandbox2/mount_tree.proto
@@ -26,24 +26,24 @@ message MountTree {
   // FileNode represents a bind mount for a regular file using "outside" as the
   // source.
   message FileNode {
-    optional string outside = 2;
-    optional bool writable = 3;
+    string outside = 2;
+    bool writable = 3;
   }
 
   // DirNode is like FileNode but for directories.
   message DirNode {
-    optional string outside = 2;
-    optional bool writable = 3;
+    string outside = 2;
+    bool writable = 3;
   }
 
   // TmpfsNode mounts a tmpfs with given options.
   message TmpfsNode {
-    optional string tmpfs_options = 1;
+    string tmpfs_options = 1;
   }
 
   // RootNode is as special node for root of the MountTree
   message RootNode {
-    optional bool writable = 3;
+    bool writable = 3;
   }
 
   message Node {
diff --git a/sandboxed_api/sandbox2/mounts_test.cc b/sandboxed_api/sandbox2/mounts_test.cc
index 21490a2d5..14d10f6cd 100644
--- a/sandboxed_api/sandbox2/mounts_test.cc
+++ b/sandboxed_api/sandbox2/mounts_test.cc
@@ -25,6 +25,7 @@
 #include "absl/status/status.h"
 #include "absl/strings/match.h"
 #include "absl/strings/str_cat.h"
+#include "absl/strings/string_view.h"
 #include "sandboxed_api/testing.h"
 #include "sandboxed_api/util/path.h"
 #include "sandboxed_api/util/status_matchers.h"
@@ -183,7 +184,7 @@ TEST(MountTreeTest, TestMultipleInsertion) {
 
 TEST(MountTreeTest, TestEvilNullByte) {
   Mounts mounts;
-  // create the filename with a null byte this way as g4 fix forces newlines
+  // Create the filename with a null byte this way as g4 fix forces newlines
   // otherwise.
   std::string filename = "/a/b";
   filename[2] = '\0';
@@ -214,20 +215,18 @@ TEST(MountTreeTest, TestMinimalDynamicBinary) {
 
 TEST(MountTreeTest, TestList) {
   struct TestCase {
-    const char* path;
-    const bool is_ro;
+    absl::string_view path;
+    bool is_ro;
   };
-  // clang-format off
   constexpr TestCase kTestCases[] = {
       // NOTE: Directories have a trailing '/'; files don't.
-      {"/a/b", true},
-      {"/a/c/", true},
-      {"/a/c/d/e/f/g", true},
-      {"/h", true},
-      {"/i/j/k", false},
-      {"/i/l/", false},
+      {"/a/b", true},          // File
+      {"/a/c/", true},         // Directory
+      {"/a/c/d/e/f/g", true},  // File
+      {"/h", true},            // File
+      {"/i/j/k", false},       // File
+      {"/i/l/", false},        // Directory
   };
-  // clang-format on
 
   Mounts mounts;
 
@@ -254,30 +253,24 @@ TEST(MountTreeTest, TestList) {
   std::vector<std::string> inside_entries;
   mounts.RecursivelyListMounts(&outside_entries, &inside_entries);
 
-  // clang-format off
-  EXPECT_THAT(
-      inside_entries,
-      UnorderedElementsAreArray({
-          "R /a/b",
-          "R /a/c/",
-          "R /a/c/d/e/f/g",
-          "R /h",
-          "W /i/j/k",
-          "W /i/l/",
-          "/d",
-      }));
-  EXPECT_THAT(
-      outside_entries,
-      UnorderedElementsAreArray({
-          absl::StrCat("/some/dir/", "a/b"),
-          absl::StrCat("/some/dir/", "a/c/"),
-          absl::StrCat("/some/dir/", "a/c/d/e/f/g"),
-          absl::StrCat("/some/dir/", "h"),
-          absl::StrCat("/some/dir/", "i/j/k"),
-          absl::StrCat("/some/dir/", "i/l/"),
-          absl::StrCat("tmpfs: size=", 1024*1024),
-      }));
-  // clang-format on
+  EXPECT_THAT(inside_entries, UnorderedElementsAreArray({
+                                  "R /a/b",
+                                  "R /a/c/",
+                                  "R /a/c/d/e/f/g",
+                                  "R /h",
+                                  "W /i/j/k",
+                                  "W /i/l/",
+                                  "/d",
+                              }));
+  EXPECT_THAT(outside_entries, UnorderedElementsAreArray({
+                                   absl::StrCat("/some/dir/", "a/b"),
+                                   absl::StrCat("/some/dir/", "a/c/"),
+                                   absl::StrCat("/some/dir/", "a/c/d/e/f/g"),
+                                   absl::StrCat("/some/dir/", "h"),
+                                   absl::StrCat("/some/dir/", "i/j/k"),
+                                   absl::StrCat("/some/dir/", "i/l/"),
+                                   absl::StrCat("tmpfs: size=", 1024 * 1024),
+                               }));
 }
 
 TEST(MountTreeTest, TestIsWritable) {