From fb7c5682758ef57fd3621dfc83a6382185ea353d Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 15 Nov 2024 23:23:45 -0500 Subject: [PATCH] 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 --- .cargo/config.toml | 3 +- Cargo.lock | 12 ++++ Cargo.toml | 4 ++ packages/cli/Cargo.toml | 1 + packages/cli/src/build/bundle.rs | 10 +-- packages/cli/src/build/request.rs | 91 +++++++++++++++++++----- packages/cli/src/cli/build.rs | 4 +- packages/cli/src/cli/bundle.rs | 2 +- packages/cli/src/cli/target.rs | 113 +++++++++++++++++++++++++----- packages/cli/src/dioxus_crate.rs | 9 +-- packages/cli/src/logging.rs | 45 ++++++++---- 11 files changed, 230 insertions(+), 64 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index 1c37730a1..a61121287 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -10,4 +10,5 @@ opt-level = 2 [profile.dioxus-android] inherits = "dev" -opt-level = 2 +opt-level = 1 +debug = 0 diff --git a/Cargo.lock b/Cargo.lock index 234fc4527..60d44c2d3 100644 --- a/Cargo.lock +++ b/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", ] diff --git a/Cargo.toml b/Cargo.toml index eb25a6fc7..cb930aeef 100644 --- a/Cargo.toml +++ b/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] diff --git a/packages/cli/Cargo.toml b/packages/cli/Cargo.toml index a712b2473..9ca1ac79a 100644 --- a/packages/cli/Cargo.toml +++ b/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 } diff --git a/packages/cli/src/build/bundle.rs b/packages/cli/src/build/bundle.rs index 7e3e30182..e4c6fde53 100644 --- a/packages/cli/src/build/bundle.rs +++ b/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(), }, diff --git a/packages/cli/src/build/request.rs b/packages/cli/src/build/request.rs index d2ee32269..c2cd4bb9c 100644 --- a/packages/cli/src/build/request.rs +++ b/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> { 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 diff --git a/packages/cli/src/cli/build.rs b/packages/cli/src/cli/build.rs index 4a9ae3802..78bc4f4b7 100644 --- a/packages/cli/src/cli/build.rs +++ b/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() ); diff --git a/packages/cli/src/cli/bundle.rs b/packages/cli/src/cli/bundle.rs index 2db93e6b5..5bee1c924 100644 --- a/packages/cli/src/cli/bundle.rs +++ b/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()), diff --git a/packages/cli/src/cli/target.rs b/packages/cli/src/cli/target.rs index 3a9052b27..dad739dd8 100644 --- a/packages/cli/src/cli/target.rs +++ b/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 { + // 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() } } diff --git a/packages/cli/src/dioxus_crate.rs b/packages/cli/src/dioxus_crate.rs index ea7fca5f2..09260520d 100644 --- a/packages/cli/src/dioxus_crate.rs +++ b/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()) } } diff --git a/packages/cli/src/logging.rs b/packages/cli/src/logging.rs index 8562b1f6f..fca7bcdb8 100644 --- a/packages/cli/src/logging.rs +++ b/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> = OnceCell::new(); pub static VERBOSITY: OnceCell = 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"); +impl Drop for TraceController { + fn drop(&mut self) { + TUI_ACTIVE.store(false, Ordering::Relaxed); - tracing_subscriber::registry() - .with(filter) - .with(CLILayer {}) - .init(); + // 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);