Repository: tokio-rs/prost (language: rust)

Issue:
Don't invoke the bundled protoc binary on non-gnu Linux hosts
Revisiting the problem reported in #182. Unfortunately, it percolates to builds that use tower-grpc-build and troubleshooting clues are currently less than ideal.

Is it possible to avoid running the bundled protoc on non-glibc (e.g. musl) Linux host environment and report that a binary needs to be supplied? Notably this is different from the build target.


Discussion / hints:
Ah, is this coming up when using `prost` on Alpine Linux?  I don't think I fully grasped the significance of the report in #182; I thought it was a really oddball setup that I wouldn't be able to reproduce.  It should definitely be possible to hack support for dynamically detecting non-existence of glibc and falling back to the environment or path.  See https://github.com/danburkert/prost/blob/master/prost-build/build.rs for where this all happens.
Or, for a real challenge, convince upstream to ship fully static binaries :)
> It should definitely be possible to hack support for dynamically detecting non-existence of glibc

There may be way too many corner cases; this would need to check if the libraries exist in the library path where the `/lib*/ld*.so.?` loaded by the binary would expect to find them, which is configurable (both in `/etc` and via `${LD_LIBRARY_PATH}`). It may be easier to try to run `ldd` and parse its output.

I'd rather suggest checking the host tuple (available from ???) or `uname -o`. Unfortunately, `env::consts::OS` does not provide detailed information on the flavor of Linux.
I’m still somewhat confused why asking users to make `protoc` available in the environment and using it by default is an issue. Its a build too like any other.
I second @nagisa's question: why do we need to handle the case where `protoc` isn't installed? It's actually quite difficult to provide a reasonable error message when the loader is the problem, as opposed to `protoc` not being available, to which we could respond with `protoc not found, please install` or some such message.
A compromise might be to prefer the `protoc` from `PATH` _before_ the bundled one.
@danburkert Would you accept a PR to move the order of preference from the bundled `protoc` to the one found in `PATH`?
Just hit this, had to strace to track down the issue. I don't quite understand why prost is using a pre-compiled binary blob *at all*?
This choice has serious security implications. I can audit the packages installed on my system, they can all be traced back to sources. I can audit all my rust dependencies, again, the produced binaries are linked to sources. What I cannot do is audit a random binary blob, I have to trust both the upstream maintainers (and their build servers) and the prost maintainers. 

(Using the PROTOC/PROTOC_INCLUDE workaround now)
> @danburkert Would you accept a PR to move the order of preference from the bundled protoc to the one found in PATH?

No, but an additional option on `prost_build::Config` to override the `protoc` used would be fine.
I still consider it a bug that `prost` fails to use the bundled `protoc` on alpine without falling back the `$PATH`, but it's not something that's high on my list to fix, I will accept PRs fixing that as well though.
@danburkert Hit the bug, is it acceptable to do something like this? We can run the binary to test if it works on the platform.

```
@@ -47,7 +48,14 @@ fn bundled_protoc() -> Option<PathBuf> {
         _ => return None,
     };
 
-    Some(bundle_path().join(protoc_bin_name))
+    let path = bundle_path().join(protoc_bin_name);
+
+    let output = Command::new(&path).arg("--version").output();
+
+    match output.map_or(false, |o| o.status.success()) {
+        true => Some(path),
+        _ => None,
+    }
 }
 
 /// Returns the path to the `protoc` included on the `PATH`, if it exists.
```

I would like to file the PR and test.
@xhebox I recommend setting `env PROTOC=protoc` to get the behavior you want.
>  This choice has serious security implications. I can audit the packages installed on my system, they can all be traced back to sources. I can audit all my rust dependencies, again, the produced binaries are linked to sources. What I cannot do is audit a random binary blob, I have to trust both the upstream maintainers (and their build servers) and the prost maintainers.

Just in case anyone stumbles across this and gets the wrong impression, the `prost` project is not shipping a 'random' `protoc` binary.  The bundled binaries are taken directly from the upstream protobuf project using [this script](https://github.com/danburkert/prost/blob/master/prost-build/third-party/update-bundled-protobuf.sh).  The binaries can thus be audited against the upstream release.  If there are security concerns with the upstream release, I suggest you take it there.  Ultimately if this is unsatisfactory, the env escape hatch is recommended.
@danburkert Yeah, I can set PROTOC somehow, github.com/AdelieLinux/gcompat can even get the executable runnable under musl-host linux. But that `No such file or directory` tip is just too confusing. E.g. tikv/pprof-rs is using this project, then that error poped up.

It is not working by default, and I don't even know how to fix it before digging deep. I would prefer adding some switches to move env-protoc before the bundled, or not to use the bundled one on musl(it is not working anyway).

Fallback is also a good alternative:

> I still consider it a bug that prost fails to use the bundled protoc on alpine without falling back the $PATH.

The most simple way I can come up with, is running the binary directly. Then we can know if we should fallback. Maybe you can check if there is `/lib{,32,64}/ld-linux-*.so.*` if `target_env = musl`, if not, it is definitely impossible to run these executables. But as @mzabaluev mentioned:

> It may be easier to try to run ldd and parse its output.

 Running binary is equaivalent to ldd method, since the first thing before running is loading. And musl host does not necessarily come with a ldd program(you may not know where it is due to PATH). So running binary is the easiest and most reliable way overall.

Which way do you think is acceptable? It is just not good if prost-build is not **working by default** on musl.

Fix patch:
diff --git a/prost-build/build.rs b/prost-build/build.rs
index 4bc1cc6ed..3ce4848b9 100644
--- a/prost-build/build.rs
+++ b/prost-build/build.rs
@@ -36,6 +36,7 @@ fn env_protoc() -> Option<PathBuf> {
 }
 
 /// Returns the path to the bundled `protoc`, if it is available for the host platform.
+#[cfg(not(target_env = "musl"))]
 fn bundled_protoc() -> Option<PathBuf> {
     let protoc_bin_name = match (env::consts::OS, env::consts::ARCH) {
         ("linux", "x86") => "protoc-linux-x86_32",
@@ -50,6 +51,14 @@ fn bundled_protoc() -> Option<PathBuf> {
     Some(bundle_path().join(protoc_bin_name))
 }
 
+/// `musl` build hosts do not have a bundled `protoc`.
+///
+/// Note: this checks the target of the `prost-build` build.rs, which is ultimately the host architecture.
+#[cfg(target_env = "musl")]
+fn bundled_protoc() -> Option<PathBuf> {
+    None
+}
+
 /// Returns the path to the `protoc` included on the `PATH`, if it exists.
 fn path_protoc() -> Option<PathBuf> {
     which::which("protoc").ok()
diff --git a/prost-build/src/lib.rs b/prost-build/src/lib.rs
index 7aa20ed47..4500a6068 100644
--- a/prost-build/src/lib.rs
+++ b/prost-build/src/lib.rs
@@ -97,14 +97,17 @@
 //! PROTOC_INCLUDE=/usr/include
 //! ```
 //!
-//! If `PROTOC` is not found in the environment, then a pre-compiled `protoc` binary bundled in
-//! the prost-build crate is used. Pre-compiled `protoc` binaries exist for Linux, macOS, and
-//! Windows systems. If no pre-compiled `protoc` is available for the host platform, then the
+//! If `PROTOC` is not found in the environment, then a pre-compiled `protoc` binary bundled in the
+//! prost-build crate is used. Pre-compiled `protoc` binaries exist for Linux (non-musl), macOS,
+//! and Windows systems. If no pre-compiled `protoc` is available for the host platform, then the
 //! `protoc` or `protoc.exe` binary on the `PATH` is used. If `protoc` is not available in any of
 //! these fallback locations, then the build fails.
 //!
-//! If `PROTOC_INCLUDE` is not found in the environment, then the Protobuf include directory bundled
-//! in the prost-build crate is be used.
+//! If `PROTOC_INCLUDE` is not found in the environment, then the Protobuf include directory
+//! bundled in the prost-build crate is be used.
+//!
+//! To force `prost-build` to use the `protoc` on the `PATH`, add `PROTOC=protoc` to the
+//! environment.
 
 mod ast;
 mod code_generator;
@@ -697,7 +700,13 @@ impl Config {
             cmd.arg(proto.as_ref());
         }
 
-        let output = cmd.output()?;
+        let output = cmd.output().map_err(|error| {
+            Error::new(
+                error.kind(),
+                format!("failed to invoke protoc (hint: https://docs.rs/prost-build/#sourcing-protoc): {}", error),
+            )
+        })?;
+
         if !output.status.success() {
             return Err(Error::new(
                 ErrorKind::Other,


Test patch: