Merge #1619

1619: feat(native-engine) Panic if `gcc`/`clang-10` is missing r=syrusakbary a=Hywan When `gcc` or `clang-10` is absent, we get a non-obvious error: > Compilation error: No such file or directory (os error 2) It comes from https://github.com/wasmerio/wasmer/blob/2cd12213fb256f475c4e1dc87aefd422115665f7/lib/engine-native/src/artifact.rs#L288-L298. This PR improves that by panicking when `gcc` or `clang` isn't present. Co-authored-by: Ivan Enderlin <[email protected]>
wasmerio · Sep 14, 2020 · 850fef7 · 850fef7
2 parents 2cd1221 + cd87983
commit 850fef7
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 10 deletions.
diff --git a/lib/engine-native/Cargo.toml b/lib/engine-native/Cargo.toml
@@ -23,6 +23,7 @@ bincode = "1.3"
 leb128 = "0.2"
 libloading = "0.6"
 tempfile = "3.1"
+which = "4.0"
 
 [features]
 # Enable the `compiler` feature if you want the engine to compile

diff --git a/lib/engine-native/src/artifact.rs b/lib/engine-native/src/artifact.rs
@@ -256,8 +256,7 @@ impl NativeArtifact {
                 .map_err(to_compile_error)?
         };
 
-        let host_target = Triple::host();
-        let is_cross_compiling = target_triple != &host_target;
+        let is_cross_compiling = engine_inner.is_cross_compiling();
         let cross_compiling_args: Vec<String> = if is_cross_compiling {
             vec![
                 format!("--target={}", target_triple),
@@ -276,15 +275,10 @@ impl NativeArtifact {
         trace!(
             "Compiling for target {} from host {}",
             target_triple.to_string(),
-            host_target.to_string()
+            Triple::host().to_string(),
         );
 
-        let linker = if is_cross_compiling {
-            "clang-10"
-        } else {
-            "gcc"
-        };
-
+        let linker: &'static str = engine_inner.linker().into();
         let output = Command::new(linker)
             .arg(&filepath)
             .arg("-o")

diff --git a/lib/engine-native/src/engine.rs b/lib/engine-native/src/engine.rs
@@ -7,12 +7,14 @@ use std::sync::Arc;
 use std::sync::Mutex;
 #[cfg(feature = "compiler")]
 use wasmer_compiler::Compiler;
-use wasmer_compiler::{CompileError, Target};
+use wasmer_compiler::{CompileError, Target, Triple};
 use wasmer_engine::{Artifact, DeserializeError, Engine, EngineId, Tunables};
 #[cfg(feature = "compiler")]
 use wasmer_types::Features;
 use wasmer_types::FunctionType;
 use wasmer_vm::{SignatureRegistry, VMSharedSignatureIndex, VMTrampoline};
+#[cfg(feature = "compiler")]
+use which::which;
 
 /// A WebAssembly `Native` Engine.
 #[derive(Clone)]
@@ -27,13 +29,32 @@ impl NativeEngine {
     /// Create a new `NativeEngine` with the given config
     #[cfg(feature = "compiler")]
     pub fn new(compiler: Box<dyn Compiler + Send>, target: Target, features: Features) -> Self {
+        let host_target = Triple::host();
+        let is_cross_compiling = target.triple() != &host_target;
+
+        let linker = if is_cross_compiling {
+            which(Into::<&'static str>::into(Linker::Clang10))
+                .map(|_| Linker::Clang10)
+                .or_else(|_| {
+                    which(Into::<&'static str>::into(Linker::Clang))
+                        .map(|_| Linker::Clang)
+                })
+                .expect("Nor `clang-10` or `clang` has been found, at least one of them is required for the `NativeEngine`")
+        } else {
+            which(Into::<&'static str>::into(Linker::Gcc))
+                .map(|_| Linker::Gcc)
+                .expect("`gcc` has not been found, it is required for the `NativeEngine`")
+        };
+
         Self {
             inner: Arc::new(Mutex::new(NativeEngineInner {
                 compiler: Some(compiler),
                 trampolines: HashMap::new(),
                 signatures: SignatureRegistry::new(),
                 prefixer: None,
                 features,
+                is_cross_compiling,
+                linker,
             })),
             target: Arc::new(target),
             engine_id: EngineId::default(),
@@ -63,6 +84,8 @@ impl NativeEngine {
                 trampolines: HashMap::new(),
                 signatures: SignatureRegistry::new(),
                 prefixer: None,
+                is_cross_compiling: false,
+                linker: Linker::None,
             })),
             target: Arc::new(Target::default()),
             engine_id: EngineId::default(),
@@ -172,6 +195,25 @@ impl Engine for NativeEngine {
     }
 }
 
+#[derive(Clone, Copy)]
+pub(crate) enum Linker {
+    None,
+    Clang10,
+    Clang,
+    Gcc,
+}
+
+impl Into<&'static str> for Linker {
+    fn into(self) -> &'static str {
+        match self {
+            Self::None => "",
+            Self::Clang10 => "clang-10",
+            Self::Clang => "clang",
+            Self::Gcc => "gcc",
+        }
+    }
+}
+
 /// The inner contents of `NativeEngine`
 pub struct NativeEngineInner {
     /// The compiler
@@ -189,6 +231,10 @@ pub struct NativeEngineInner {
     /// the functions in the shared object generated by the `NativeEngine`,
     /// so we can assure no collisions.
     prefixer: Option<Box<dyn Fn(&[u8]) -> String + Send>>,
+    /// Whether the native engine will cross-compile.
+    is_cross_compiling: bool,
+    /// The linker to use.
+    linker: Linker,
 }
 
 impl NativeEngineInner {
@@ -249,4 +295,12 @@ impl NativeEngineInner {
         // where they belong become unallocated.
         self.trampolines.insert(index, trampoline);
     }
+
+    pub(crate) fn is_cross_compiling(&self) -> bool {
+        self.is_cross_compiling
+    }
+
+    pub(crate) fn linker(&self) -> Linker {
+        self.linker
+    }
 }