Make setup_protoc() unsafe

Summary:
This function mutates environment variables using `env::set_var`, which is unsafe on some platforms if anything is simultaneously reading the environment from non-Rust code.

https://doc.rust-lang.org/edition-guide/rust-2024/newly-unsafe-functions.html#stdenvset_var-remove_var

Reviewed By: JakobDegen

Differential Revision: D74377239

fbshipit-source-id: 5e3dcef9276d9f37460a58dbcf3e557722243788

Author: David Tolnay

app/buck2_action_metadata_proto/build.rs

@@ -19,7 +19,6 @@ fn main() -> io::Result<()> {
         vec![".".to_owned()]
     };
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
-        .compile(proto_files, &includes)
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }.compile(proto_files, &includes)
 }

app/buck2_cli_proto/build.rs

@@ -24,8 +24,8 @@ fn main() -> io::Result<()> {
         ]
     };
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }
         .type_attribute(".", "#[derive(::serde::Serialize, ::serde::Deserialize)] #[serde(rename_all = \"snake_case\")]")
         .type_attribute(".", "#[derive(::allocative::Allocative)]")
         .field_attribute("start_time", "#[serde(with = \"serialize_timestamp\")]")

app/buck2_data/build.rs

@@ -19,8 +19,8 @@ fn main() -> io::Result<()> {
         vec![".".to_owned(), "../buck2_host_sharing_proto".to_owned()]
     };
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }
         .type_attribute(
             "buck.data.BuckEvent.data",
             "#[allow(clippy::large_enum_variant)]",

app/buck2_downward_api_proto/build.rs

@@ -19,7 +19,6 @@ fn main() -> io::Result<()> {
         vec![".".to_owned()]
     };
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
-        .compile(proto_files, &includes)
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }.compile(proto_files, &includes)
 }

app/buck2_forkserver_proto/build.rs

@@ -23,8 +23,8 @@ fn main() -> io::Result<()> {
         ]
     };
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }
         .type_attribute(
             "buck.forkserver.RequestEvent.data",
             "#[derive(::derive_more::From, ::gazebo::variants::VariantName, ::gazebo::variants::UnpackVariants)]",

app/buck2_health_check_proto/build.rs

@@ -23,8 +23,8 @@ fn main() -> io::Result<()> {
         ]
     };
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }
         .extern_path(".buck.data", "::buck2_data")
         .compile(proto_files, &includes)
 }

app/buck2_host_sharing_proto/build.rs

@@ -19,8 +19,8 @@ fn main() -> io::Result<()> {
         vec![".".to_owned()]
     };
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }
         .type_attribute(".", "#[derive(::allocative::Allocative)]")
         .type_attribute(".", "#[derive(::serde::Serialize, ::serde::Deserialize)]")
         .extern_path(".buck.data", "::buck2_data")

app/buck2_install_proto/build.rs

@@ -19,7 +19,6 @@ fn main() -> io::Result<()> {
         vec![".".to_owned()]
     };
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
-        .compile(proto_files, &includes)
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }.compile(proto_files, &includes)
 }

app/buck2_protoc_dev/src/lib.rs

@@ -22,7 +22,11 @@ fn get_env(key: &str) -> Option<OsString> {
 }
 
 #[cfg(not(buck2_build))]
-fn set_var(var: &str, override_var: &str, path: Result<PathBuf, protoc_bin_vendored::Error>) {
+unsafe fn set_var(
+    var: &str,
+    override_var: &str,
+    path: Result<PathBuf, protoc_bin_vendored::Error>,
+) {
     let path = if let Some(override_var_value) = env::var_os(override_var) {
         eprintln!("INFO: Variable ${} is overridden by ${}", var, override_var);
         PathBuf::from(override_var_value)
@@ -40,13 +44,13 @@ fn set_var(var: &str, override_var: &str, path: Result<PathBuf, protoc_bin_vendo
 
     let path = dunce::canonicalize(path).expect("Failed to canonicalize path");
     eprintln!("INFO: Variable ${} set to {:?}", var, path);
-    env::set_var(var, path);
+    unsafe { env::set_var(var, path) };
 }
 
 /// Set up $PROTOC to point to the in repo binary if available.
 ///
 /// Note: repo root is expected to be a relative or absolute path to the root of the repository.
-fn maybe_set_protoc() {
+unsafe fn maybe_set_protoc() {
     #[cfg(not(buck2_build))]
     {
         // `cargo build` of `buck2` does not require external `protoc` dependency
@@ -55,23 +59,27 @@ fn maybe_set_protoc() {
         // https://github.com/facebook/buck2/issues/65
         // So for NixOS builds path to `protoc` binary can be overridden with
         // `BUCK2_BUILD_PROTOC` environment variable.
-        set_var(
-            "PROTOC",
-            "BUCK2_BUILD_PROTOC",
-            protoc_bin_vendored::protoc_bin_path(),
-        );
+        unsafe {
+            set_var(
+                "PROTOC",
+                "BUCK2_BUILD_PROTOC",
+                protoc_bin_vendored::protoc_bin_path(),
+            );
+        }
     }
 }
 
 /// Set $PROTOC_INCLUDE.
-fn maybe_set_protoc_include() {
+unsafe fn maybe_set_protoc_include() {
     #[cfg(not(buck2_build))]
     {
-        set_var(
-            "PROTOC_INCLUDE",
-            "BUCK2_BUILD_PROTOC_INCLUDE",
-            protoc_bin_vendored::include_path(),
-        );
+        unsafe {
+            set_var(
+                "PROTOC_INCLUDE",
+                "BUCK2_BUILD_PROTOC_INCLUDE",
+                protoc_bin_vendored::include_path(),
+            );
+        }
     }
 }
 
@@ -112,10 +120,10 @@ impl Builder {
         }
     }
 
-    pub fn setup_protoc(self) -> Self {
+    pub unsafe fn setup_protoc(self) -> Self {
         // It would be great if there were on the config rather than an env variables...
-        maybe_set_protoc();
-        maybe_set_protoc_include();
+        unsafe { maybe_set_protoc() };
+        unsafe { maybe_set_protoc_include() };
         self
     }
 

app/buck2_subscription_proto/build.rs

@@ -19,8 +19,8 @@ fn main() -> io::Result<()> {
         vec![".".to_owned()]
     };
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }
         .type_attribute(".", "#[derive(::serde::Serialize, ::serde::Deserialize)]")
         .type_attribute(".", "#[derive(::allocative::Allocative)]")
         .type_attribute(

app/buck2_test_proto/build.rs

@@ -23,8 +23,8 @@ fn main() -> io::Result<()> {
         ]
     };
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }
         .type_attribute(
             "buck.test.ExecuteResponse2.response",
             "#[allow(clippy::large_enum_variant)]",

app/buck2_worker_proto/build.rs

@@ -19,7 +19,6 @@ fn main() -> io::Result<()> {
         vec![".".to_owned(), "../buck2_data".to_owned()]
     };
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
-        .compile(proto_files, &includes)
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }.compile(proto_files, &includes)
 }

remote_execution/oss/re_grpc_proto/build.rs

@@ -22,8 +22,8 @@ fn main() -> io::Result<()> {
         "proto/google/rpc/status.proto",
     ];
 
-    buck2_protoc_dev::configure()
-        .setup_protoc()
+    let builder = buck2_protoc_dev::configure();
+    unsafe { builder.setup_protoc() }
         .type_attribute(".", "#[derive(::serde::Serialize, ::serde::Deserialize)]")
         .field_attribute(
             "build.bazel.remote.execution.v2.Action.timeout",