Skip to content

Commit

Permalink
Don't immediately unwrap protoc vendored paths (#121)
Browse files Browse the repository at this point in the history
Summary:
The intention of using environment variables is to override these paths. With the code as written, this will unconditionally panic if protoc_bin_vendored doesn't know where the protobuff compiler/includes are, even if we're using an environment variable to set these paths.

With this change, we only unwrap when actually attempting to use the path from `protoc_bin_vendored`.

Found as part of #120.

Pull Request resolved: #121

Reviewed By: cjhopman

Differential Revision: D44799077

Pulled By: ndmitchell

fbshipit-source-id: 56db51961166df0870cb4ad07a220abfc21c5008
  • Loading branch information
steveklabnik authored and facebook-github-bot committed Apr 9, 2023
1 parent 47d13b2 commit b426e09
Showing 1 changed file with 16 additions and 10 deletions.
26 changes: 16 additions & 10 deletions app/buck2_protoc_dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,32 @@ use std::env;
use std::ffi::OsString;
use std::io;
use std::path::Path;
#[cfg(not(buck2_build))]
use std::path::PathBuf;

fn get_env(key: &str) -> Option<OsString> {
println!("cargo:rerun-if-env-changed={}", key);
env::var_os(key)
}

#[cfg(not(buck2_build))]
fn set_var(var: &str, override_var: &str, path: &Path) {
assert!(path.exists(), "Path does not exist: `{}`", path.display());

let path_buf;
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);
path_buf = std::path::PathBuf::from(override_var_value);
&path_buf
PathBuf::from(override_var_value)
} else {
path
match path {
Err(e) => {
panic!("{var} not available for platform {e:?}, set ${override_var} to override")
}
Ok(path) => {
assert!(path.exists(), "Path does not exist: `{}`", path.display());
path
}
}
};

let path = dunce::canonicalize(path).expect("Failed to canonicalize path");
let path = dunce::canonicalize(&path).expect("Failed to canonicalize path");
eprintln!("INFO: Variable ${} set to {:?}", var, path);
env::set_var(var, path);
}
Expand All @@ -50,7 +56,7 @@ fn maybe_set_protoc() {
set_var(
"PROTOC",
"BUCK2_BUILD_PROTOC",
&protoc_bin_vendored::protoc_bin_path().unwrap(),
protoc_bin_vendored::protoc_bin_path(),
);
}
}
Expand All @@ -62,7 +68,7 @@ fn maybe_set_protoc_include() {
set_var(
"PROTOC_INCLUDE",
"BUCK2_BUILD_PROTOC_INCLUDE",
&protoc_bin_vendored::include_path().unwrap(),
protoc_bin_vendored::include_path(),
);
}
}
Expand Down

0 comments on commit b426e09

Please sign in to comment.