فهرست منبع

fix: java_home and cli swallowing logs (#3221)

* fix: http examples with android, add more important flags
- set java_home from assumed installations
- fix tracing redirection bug
Jonathan Kelley 7 ماه پیش
والد
کامیت
fb7c568275

+ 2 - 1
.cargo/config.toml

@@ -10,4 +10,5 @@ opt-level = 2
 
 [profile.dioxus-android]
 inherits = "dev"
-opt-level = 2
+opt-level = 1
+debug = 0

+ 12 - 0
Cargo.lock

@@ -3219,6 +3219,7 @@ dependencies = [
  "clap",
  "console",
  "console-subscriber",
+ "convert_case 0.6.0",
  "crossterm",
  "ctrlc",
  "dioxus-autofmt",
@@ -3469,6 +3470,7 @@ dependencies = [
  "futures-util",
  "getrandom 0.2.15",
  "http-range",
+ "openssl",
  "rand 0.8.5",
  "reqwest 0.12.9",
  "separator",
@@ -8003,6 +8005,15 @@ version = "0.1.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "ff011a302c396a5197692431fc1948019154afc178baf7d8e37367442a4601cf"
 
+[[package]]
+name = "openssl-src"
+version = "300.3.2+3.3.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a211a18d945ef7e648cc6e0058f4c548ee46aab922ea203e0d30e966ea23647b"
+dependencies = [
+ "cc",
+]
+
 [[package]]
 name = "openssl-sys"
 version = "0.9.104"
@@ -8011,6 +8022,7 @@ checksum = "45abf306cbf99debc8195b66b7346498d7b10c210de50418b5ccd7ceba08c741"
 dependencies = [
  "cc",
  "libc",
+ "openssl-src",
  "pkg-config",
  "vcpkg",
 ]

+ 4 - 0
Cargo.toml

@@ -297,6 +297,10 @@ tokio = { version = "1.40", default-features = false, features = [
 [target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
 tokio = { version = "1.40", features = ["full"] }
 
+# force vendored openssl on android
+[target.'cfg(target_os = "android")'.dev-dependencies]
+openssl = { version = "0.10", features = ["vendored"] }
+
 # To make most examples faster to compile, we split out assets and http-related stuff
 # This trims off like 270 dependencies, leading to a significant speedup in compilation time
 [features]

+ 1 - 0
packages/cli/Cargo.toml

@@ -24,6 +24,7 @@ dioxus-fullstack = { workspace = true }
 dioxus-dx-wire-format = { workspace = true }
 
 clap = { workspace = true, features = ["derive", "cargo"] }
+convert_case = { workspace = true }
 thiserror = { workspace = true }
 wasm-bindgen-cli-support = { workspace = true }
 wasm-bindgen-shared = { workspace = true }

+ 5 - 5
packages/cli/src/build/bundle.rs

@@ -449,7 +449,7 @@ impl AppBundle {
 
             std::fs::create_dir_all(self.server_exe().unwrap().parent().unwrap())?;
 
-            tracing::debug!("Copying server executable from {server:?} to {to:?}");
+            tracing::debug!("Copying server executable to: {to:?} {server:#?}");
 
             // Remove the old server executable if it exists, since copying might corrupt it :(
             // todo(jon): do this in more places, I think
@@ -712,8 +712,8 @@ impl AppBundle {
             .render_template(
                 include_str!("../../assets/macos/mac.plist.hbs"),
                 &InfoPlistData {
-                    display_name: self.build.platform_exe_name(),
-                    bundle_name: self.build.platform_exe_name(),
+                    display_name: self.build.krate.bundled_app_name(),
+                    bundle_name: self.build.krate.bundled_app_name(),
                     executable_name: self.build.platform_exe_name(),
                     bundle_identifier: self.build.krate.bundle_identifier(),
                 },
@@ -726,8 +726,8 @@ impl AppBundle {
             .render_template(
                 include_str!("../../assets/ios/ios.plist.hbs"),
                 &InfoPlistData {
-                    display_name: self.build.platform_exe_name(),
-                    bundle_name: self.build.platform_exe_name(),
+                    display_name: self.build.krate.bundled_app_name(),
+                    bundle_name: self.build.krate.bundled_app_name(),
                     executable_name: self.build.platform_exe_name(),
                     bundle_identifier: self.build.krate.bundle_identifier(),
                 },

+ 74 - 17
packages/cli/src/build/request.rs

@@ -100,7 +100,9 @@ impl BuildRequest {
             .arg("--message-format")
             .arg("json-diagnostic-rendered-ansi")
             .args(self.build_arguments())
-            .envs(self.env_vars());
+            .envs(self.env_vars()?);
+
+        // todo(jon): save the temps into a file that we use for asset extraction instead of the weird double compile.
         // .args(["--", "-Csave-temps=y"]);
 
         if let Some(target_dir) = self.custom_target_dir.as_ref() {
@@ -118,21 +120,19 @@ impl BuildRequest {
                 .krate
                 .android_ndk()
                 .context("Could not autodetect android linker")?;
-            let linker = self.build.target_args.arch().android_linker(&ndk);
+            let arch = self.build.target_args.arch();
+            let linker = arch.android_linker(&ndk);
 
-            tracing::trace!("Using android linker: {linker:?}");
+            let link_action = LinkAction::LinkAndroid {
+                linker,
+                extra_flags: vec![],
+            }
+            .to_json();
 
-            cmd.env(
-                LinkAction::ENV_VAR_NAME,
-                LinkAction::LinkAndroid {
-                    linker,
-                    extra_flags: vec![],
-                }
-                .to_json(),
-            );
+            cmd.env(LinkAction::ENV_VAR_NAME, link_action);
         }
 
-        tracing::trace!(dx_src = ?TraceSrc::Build, "Rust cargo args: {:?}", cmd);
+        tracing::trace!(dx_src = ?TraceSrc::Build, "Rust cargo args: {:#?}", cmd);
 
         let mut child = cmd
             .stdout(Stdio::piped())
@@ -368,7 +368,7 @@ impl BuildRequest {
             .arg("-Z")
             .arg("unstable-options")
             .args(self.build_arguments())
-            .envs(self.env_vars())
+            .envs(self.env_vars()?)
             .stdout(Stdio::piped())
             .stderr(Stdio::piped())
             .output()
@@ -421,7 +421,7 @@ impl BuildRequest {
         Command::new("cargo")
             .arg("rustc")
             .args(self.build_arguments())
-            .envs(self.env_vars())
+            .envs(self.env_vars()?)
             .arg("--offline") /* don't use the network, should already be resolved */
             .arg("--")
             .arg(format!(
@@ -454,10 +454,47 @@ impl BuildRequest {
         Ok(manifest)
     }
 
-    fn env_vars(&self) -> Vec<(&str, String)> {
+    fn env_vars(&self) -> Result<Vec<(&str, String)>> {
         let mut env_vars = vec![];
 
         if self.build.platform() == Platform::Android {
+            let ndk = self
+                .krate
+                .android_ndk()
+                .context("Could not autodetect android linker")?;
+            let arch = self.build.target_args.arch();
+            let linker = arch.android_linker(&ndk);
+            let min_sdk_version = arch.android_min_sdk_version();
+            let ar_path = arch.android_ar_path(&ndk);
+            let target_cc = arch.target_cc(&ndk);
+            let target_cxx = arch.target_cxx(&ndk);
+            let java_home = arch.java_home();
+
+            tracing::debug!(
+                r#"Using android:
+            min_sdk_version: {min_sdk_version}
+            linker: {linker:?}
+            ar_path: {ar_path:?}
+            target_cc: {target_cc:?}
+            target_cxx: {target_cxx:?}
+            java_home: {java_home:?}
+            "#
+            );
+
+            env_vars.push(("ANDROID_NATIVE_API_LEVEL", min_sdk_version.to_string()));
+            env_vars.push(("TARGET_AR", ar_path.display().to_string()));
+            env_vars.push(("TARGET_CC", target_cc.display().to_string()));
+            env_vars.push(("TARGET_CXX", target_cxx.display().to_string()));
+            env_vars.push(("ANDROID_NDK_ROOT", ndk.display().to_string()));
+
+            // attempt to set java_home to the android studio java home if it exists.
+            // https://stackoverflow.com/questions/71381050/java-home-is-set-to-an-invalid-directory-android-studio-flutter
+            // attempt to set java_home to the android studio java home if it exists and java_home was not already set
+            if let Some(java_home) = java_home {
+                tracing::debug!("Setting JAVA_HOME to {java_home:?}");
+                env_vars.push(("JAVA_HOME", java_home.display().to_string()));
+            }
+
             env_vars.push(("WRY_ANDROID_PACKAGE", "dev.dioxus.main".to_string()));
             env_vars.push(("WRY_ANDROID_LIBRARY", "dioxusmain".to_string()));
             env_vars.push((
@@ -468,9 +505,29 @@ impl BuildRequest {
             ));
 
             env_vars.push(("RUSTFLAGS", self.android_rust_flags()))
+
+            // todo(jon): the guide for openssl recommends extending the path to include the tools dir
+            //            in practice I couldn't get this to work, but this might eventually become useful.
+            //
+            // https://github.com/openssl/openssl/blob/master/NOTES-ANDROID.md#configuration
+            //
+            // They recommend a configuration like this:
+            //
+            // // export ANDROID_NDK_ROOT=/home/whoever/Android/android-sdk/ndk/20.0.5594570
+            // PATH=$ANDROID_NDK_ROOT/toolchains/llvm/prebuilt/linux-x86_64/bin:$ANDROID_NDK_ROOT/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin:$PATH
+            // ./Configure android-arm64 -D__ANDROID_API__=29
+            // make
+            //
+            // let tools_dir = arch.android_tools_dir(&ndk);
+            // let extended_path = format!(
+            //     "{}:{}",
+            //     tools_dir.display(),
+            //     std::env::var("PATH").unwrap_or_default()
+            // );
+            // env_vars.push(("PATH", extended_path));
         };
 
-        env_vars
+        Ok(env_vars)
     }
 
     /// We only really currently care about:
@@ -664,7 +721,7 @@ impl BuildRequest {
         }
         let hbs_data = HbsTypes {
             application_id: self.krate.full_mobile_app_name(),
-            app_name: self.krate.mobile_app_name(),
+            app_name: self.krate.bundled_app_name(),
         };
 
         // Top-level gradle config

+ 2 - 2
packages/cli/src/cli/build.rs

@@ -161,7 +161,7 @@ impl BuildArgs {
             // Some extra logs
             let arch = match arch {
                 Some(a) => {
-                    tracing::info!(
+                    tracing::debug!(
                         "Autodetected `{}` Android arch.",
                         a.android_target_triplet()
                     );
@@ -169,7 +169,7 @@ impl BuildArgs {
                 }
                 None => {
                     let a = Arch::default();
-                    tracing::info!(
+                    tracing::debug!(
                         "Could not detect Android arch, defaulting to `{}`",
                         a.android_target_triplet()
                     );

+ 1 - 1
packages/cli/src/cli/bundle.rs

@@ -198,7 +198,7 @@ impl Bundle {
         let mut settings = SettingsBuilder::new()
             .project_out_directory(krate.bundle_dir(self.build_arguments.platform()))
             .package_settings(PackageSettings {
-                product_name: krate.executable_name().to_string(),
+                product_name: krate.bundled_app_name(),
                 version: package.version.to_string(),
                 description: package.description.clone().unwrap_or_default(),
                 homepage: Some(package.homepage.clone().unwrap_or_default()),

+ 94 - 19
packages/cli/src/cli/target.rs

@@ -140,36 +140,111 @@ impl Arch {
         }
     }
 
-    pub(crate) fn android_linker(&self, ndk: &Path) -> PathBuf {
-        // "/Users/jonkelley/Library/Android/sdk/ndk/25.2.9519653/toolchains/llvm/prebuilt/darwin-x86_64/bin/aarch64-linux-android24-clang"
-
-        let toolchain_dir = ndk.join("toolchains").join("llvm").join("prebuilt");
-        let triplet = self.android_clang_triplet();
-        let clang_exec = format!("{}24-clang", triplet);
+    pub(crate) fn android_tools_dir(&self, ndk: &Path) -> PathBuf {
+        let prebuilt = ndk.join("toolchains").join("llvm").join("prebuilt");
 
         if cfg!(target_os = "macos") {
             // for whatever reason, even on aarch64 macos, the linker is under darwin-x86_64
-            return toolchain_dir
-                .join("darwin-x86_64")
-                .join("bin")
-                .join(clang_exec);
+            return prebuilt.join("darwin-x86_64").join("bin");
         }
 
         if cfg!(target_os = "linux") {
-            return toolchain_dir
-                .join("linux-x86_64")
-                .join("bin")
-                .join(clang_exec);
+            return prebuilt.join("linux-x86_64").join("bin");
         }
 
         if cfg!(target_os = "windows") {
-            return toolchain_dir
-                .join("windows-x86_64")
-                .join("bin")
-                .join(format!("{}.cmd", clang_exec));
+            return prebuilt.join("windows-x86_64").join("bin");
         }
 
-        unimplemented!("Unsupported target os for android toolchain auodetection")
+        unimplemented!("Unsupported target os for android toolchain autodetection")
+    }
+
+    pub(crate) fn android_linker(&self, ndk: &Path) -> PathBuf {
+        // "/Users/jonkelley/Library/Android/sdk/ndk/25.2.9519653/toolchains/llvm/prebuilt/darwin-x86_64/bin/aarch64-linux-android24-clang"
+        let triplet = self.android_clang_triplet();
+        let suffix = if cfg!(target_os = "windows") {
+            ".cmd"
+        } else {
+            ""
+        };
+
+        self.android_tools_dir(ndk)
+            .join(format!("{}24-clang{}", triplet, suffix))
+    }
+
+    pub(crate) fn android_min_sdk_version(&self) -> u32 {
+        // todo(jon): this should be configurable
+        24
+    }
+
+    pub(crate) fn android_ar_path(&self, ndk: &Path) -> PathBuf {
+        self.android_tools_dir(ndk).join("llvm-ar")
+    }
+
+    pub(crate) fn target_cc(&self, ndk: &Path) -> PathBuf {
+        self.android_tools_dir(ndk).join("clang")
+    }
+
+    pub(crate) fn target_cxx(&self, ndk: &Path) -> PathBuf {
+        self.android_tools_dir(ndk).join("clang++")
+    }
+
+    pub(crate) fn java_home(&self) -> Option<PathBuf> {
+        // wrap in a lazy so we don't accidentally keep probing for java home and potentially thrash env vars
+        once_cell::sync::Lazy::new(|| {
+            // https://stackoverflow.com/questions/71381050/java-home-is-set-to-an-invalid-directory-android-studio-flutter
+            // always respect the user's JAVA_HOME env var above all other options
+            //
+            // we only attempt autodetection if java_home is not set
+            //
+            // this is a better fallback than falling onto the users' system java home since many users might
+            // not even know which java that is - they just know they have android studio installed
+            if let Some(java_home) = std::env::var_os("JAVA_HOME") {
+                return Some(PathBuf::from(java_home));
+            }
+
+            // Attempt to autodetect java home from the android studio path or jdk path on macos
+            #[cfg(target_os = "macos")]
+            {
+                let jbr_home =
+                    PathBuf::from("/Applications/Android Studio.app/Contents/jbr/Contents/Home/");
+                if jbr_home.exists() {
+                    return Some(jbr_home);
+                }
+
+                let jre_home =
+                    PathBuf::from("/Applications/Android Studio.app/Contents/jre/Contents/Home");
+                if jre_home.exists() {
+                    return Some(jre_home);
+                }
+
+                let jdk_home =
+                    PathBuf::from("/Library/Java/JavaVirtualMachines/openjdk.jdk/Contents/Home/");
+                if jdk_home.exists() {
+                    return Some(jdk_home);
+                }
+            }
+
+            #[cfg(target_os = "windows")]
+            {
+                let jbr_home = PathBuf::from("C:\\Program Files\\Android\\Android Studio\\jbr");
+                if jbr_home.exists() {
+                    return Some(jbr_home);
+                }
+            }
+
+            // todo(jon): how do we detect java home on linux?
+            #[cfg(target_os = "linux")]
+            {
+                let jbr_home = PathBuf::from("/usr/lib/jvm/java-11-openjdk-amd64");
+                if jbr_home.exists() {
+                    return Some(jbr_home);
+                }
+            }
+
+            None
+        })
+        .clone()
     }
 }
 

+ 5 - 4
packages/cli/src/dioxus_crate.rs

@@ -608,12 +608,13 @@ impl DioxusCrate {
         format!("{}.{}", sub, tld)
     }
 
-    pub(crate) fn mobile_app_name(&self) -> String {
-        self.executable_name().to_string()
+    pub(crate) fn bundled_app_name(&self) -> String {
+        use convert_case::{Case, Casing};
+        self.executable_name().to_case(Case::Pascal)
     }
 
     pub(crate) fn full_mobile_app_name(&self) -> String {
-        format!("{}.{}", self.mobile_org(), self.mobile_app_name())
+        format!("{}.{}", self.mobile_org(), self.bundled_app_name())
     }
 
     pub(crate) fn bundle_identifier(&self) -> String {
@@ -621,7 +622,7 @@ impl DioxusCrate {
             return identifier.clone();
         }
 
-        format!("com.example.{}", self.executable_name())
+        format!("com.example.{}", self.bundled_app_name())
     }
 }
 

+ 31 - 16
packages/cli/src/logging.rs

@@ -25,7 +25,10 @@ use std::{
     fmt::{Debug, Display, Write as _},
     fs,
     path::PathBuf,
-    sync::Mutex,
+    sync::{
+        atomic::{AtomicBool, Ordering},
+        Mutex,
+    },
     time::Instant,
 };
 use tracing::{field::Visit, Level, Subscriber};
@@ -43,6 +46,7 @@ const LOG_ENV: &str = "DIOXUS_LOG";
 const LOG_FILE_NAME: &str = "dx.log";
 const DX_SRC_FLAG: &str = "dx_src";
 
+static TUI_ACTIVE: AtomicBool = AtomicBool::new(false);
 static TUI_TX: OnceCell<UnboundedSender<TraceMsg>> = OnceCell::new();
 pub static VERBOSITY: OnceCell<Verbosity> = OnceCell::new();
 
@@ -59,17 +63,12 @@ impl TraceController {
             .set(args.verbosity)
             .expect("verbosity should only be set once");
 
-        // When running in interactive mode (of which serve is the only one), we want to do things slightly differently
-        // This involves no fmt layer or file logging
-        if matches!(args.action, Commands::Serve(_)) {
-            Self::initialize_for_serve();
-            return args;
-        }
-
         // By default we capture ourselves at a higher tracing level when serving
         // This ensures we're tracing ourselves even if we end up tossing the logs
         let filter = if env::var(LOG_ENV).is_ok() {
             EnvFilter::from_env(LOG_ENV)
+        } else if matches!(args.action, Commands::Serve(_)) {
+            EnvFilter::new("error,dx=trace,dioxus-cli=trace,manganis-cli-support=trace")
         } else {
             EnvFilter::new(format!(
                 "error,dx={our_level},dioxus-cli={our_level},manganis-cli-support={our_level}",
@@ -108,11 +107,16 @@ impl TraceController {
             fmt_layer.boxed()
         };
 
+        // When running in interactive mode (of which serve is the only one), we don't want to log to console directly
+        let print_fmts_filter =
+            tracing_subscriber::filter::filter_fn(|_| !TUI_ACTIVE.load(Ordering::Relaxed));
+
         let sub = tracing_subscriber::registry()
             .with(filter)
             .with(json_filter)
             .with(FileAppendLayer::new())
-            .with(fmt_layer);
+            .with(CLILayer {})
+            .with(fmt_layer.with_filter(print_fmts_filter));
 
         #[cfg(feature = "tokio-console")]
         let sub = sub.with(console_subscriber::spawn());
@@ -125,6 +129,7 @@ impl TraceController {
     /// Get a handle to the trace controller.
     pub fn redirect() -> Self {
         let (tui_tx, tui_rx) = unbounded();
+        TUI_ACTIVE.store(true, Ordering::Relaxed);
         TUI_TX.set(tui_tx.clone()).unwrap();
         Self { tui_rx }
     }
@@ -135,14 +140,20 @@ impl TraceController {
         let log = self.tui_rx.next().await.expect("tracer should never die");
         ServeUpdate::TracingLog { log }
     }
+}
 
-    fn initialize_for_serve() {
-        let filter = EnvFilter::new("error,dx=trace,dioxus-cli=trace,manganis-cli-support=trace");
-
-        tracing_subscriber::registry()
-            .with(filter)
-            .with(CLILayer {})
-            .init();
+impl Drop for TraceController {
+    fn drop(&mut self) {
+        TUI_ACTIVE.store(false, Ordering::Relaxed);
+
+        // re-emit any remaining messages
+        while let Ok(Some(msg)) = self.tui_rx.try_next() {
+            let contents = match msg.content {
+                TraceContent::Text(text) => text,
+                TraceContent::Cargo(msg) => msg.message.to_string(),
+            };
+            tracing::error!("{}", contents);
+        }
     }
 }
 
@@ -231,6 +242,10 @@ where
         event: &tracing::Event<'_>,
         _ctx: tracing_subscriber::layer::Context<'_, S>,
     ) {
+        if !TUI_ACTIVE.load(Ordering::Relaxed) {
+            return;
+        }
+
         let mut visitor = CollectVisitor::new();
         event.record(&mut visitor);