Remove support for forced-odrefresh
We now always use the async interface.
This doesn't remove as much as I'd hoped; we need to wait until we
always run odrefresh in the VM for the big wins.
Bug: 209572296
Test: Still builds
Change-Id: I4054aae788c85ba20ae050fd2f77cea1eed16870
diff --git a/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl b/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl
index e73963d..ec5f2f5 100644
--- a/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl
+++ b/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl
@@ -55,15 +55,4 @@
* a reference to the ICompilationTask until compilation completes or is cancelled.
*/
ICompilationTask startAsyncOdrefresh(ICompilationTaskCallback callback);
-
- /**
- * Run odrefresh in a test instance of CompOS until completed or failed.
- *
- * This compiles BCP extensions and system server, even if the system artifacts are up to date,
- * and writes the results to a test directory to avoid disrupting any real artifacts in
- * existence.
- *
- * TODO(205750213): Change the API to async.
- */
- byte startTestOdrefresh();
}
diff --git a/compos/composd/src/compilation_task.rs b/compos/composd/src/compilation_task.rs
index f82a3e9..871c4fb 100644
--- a/compos/composd/src/compilation_task.rs
+++ b/compos/composd/src/compilation_task.rs
@@ -26,6 +26,8 @@
use std::sync::{Arc, Mutex};
use std::thread;
+// TODO: Delete
+
#[derive(Clone)]
pub struct CompilationTask {
running_task: Arc<Mutex<Option<RunningTask>>>,
diff --git a/compos/composd/src/odrefresh.rs b/compos/composd/src/odrefresh.rs
index 2cefab0..f06a4b2 100644
--- a/compos/composd/src/odrefresh.rs
+++ b/compos/composd/src/odrefresh.rs
@@ -16,23 +16,15 @@
//! Handle the details of executing odrefresh to generate compiled artifacts.
-use crate::fd_server_helper::FdServerConfig;
+// TODO: Delete
+
use anyhow::{bail, Context, Result};
-use compos_aidl_interface::aidl::com::android::compos::ICompOsService::ICompOsService;
-use compos_aidl_interface::binder::Strong;
use compos_common::odrefresh::{ExitCode, ODREFRESH_PATH};
use compos_common::timeouts::{need_extra_time, EXTENDED_TIMEOUTS};
use compos_common::VMADDR_CID_ANY;
-use rustutils::system_properties;
use shared_child::SharedChild;
-use std::fs::{File, OpenOptions};
-use std::os::unix::fs::OpenOptionsExt;
-use std::os::unix::io::AsRawFd;
-use std::path::Path;
use std::process::Command;
-const ART_APEX_DATA: &str = "/data/misc/apexdata/com.android.art";
-
pub struct Odrefresh {
child: SharedChild,
}
@@ -82,43 +74,3 @@
self.child.kill().context("Killing odrefresh process failed")
}
}
-
-pub fn run_in_vm(service: Strong<dyn ICompOsService>, target_dir_name: &str) -> Result<ExitCode> {
- let staging_dir = open_dir(composd_native::palette_create_odrefresh_staging_directory()?)?;
- let system_dir = open_dir(Path::new("/system"))?;
- let output_dir = open_dir(Path::new(ART_APEX_DATA))?;
-
- // Spawn a fd_server to serve the FDs.
- let fd_server_config = FdServerConfig {
- ro_dir_fds: vec![system_dir.as_raw_fd()],
- rw_dir_fds: vec![staging_dir.as_raw_fd(), output_dir.as_raw_fd()],
- ..Default::default()
- };
- let fd_server_raii = fd_server_config.into_fd_server()?;
-
- let zygote_arch = system_properties::read("ro.zygote")?;
- let exit_code = service.odrefresh(
- system_dir.as_raw_fd(),
- output_dir.as_raw_fd(),
- staging_dir.as_raw_fd(),
- target_dir_name,
- &zygote_arch,
- )?;
-
- drop(fd_server_raii);
- if let Some(exit_code) = ExitCode::from_i32(exit_code.into()) {
- Ok(exit_code)
- } else {
- bail!("odrefresh exited with {}", exit_code)
- }
-}
-
-/// Returns an owned FD of the directory. It currently returns a `File` as a FD owner, but
-/// it's better to use `std::os::unix::io::OwnedFd` once/if it becomes standard.
-fn open_dir(path: &Path) -> Result<File> {
- OpenOptions::new()
- .custom_flags(libc::O_DIRECTORY)
- .read(true) // O_DIRECTORY can only be opened with read
- .open(path)
- .with_context(|| format!("Failed to open {:?} directory as path fd", path))
-}
diff --git a/compos/composd/src/odrefresh_task.rs b/compos/composd/src/odrefresh_task.rs
index 262021c..9b70248 100644
--- a/compos/composd/src/odrefresh_task.rs
+++ b/compos/composd/src/odrefresh_task.rs
@@ -14,19 +14,28 @@
* limitations under the License.
*/
+//! Handle running odrefresh in the VM, with an async interface to allow cancellation
+
+use crate::fd_server_helper::FdServerConfig;
use crate::instance_starter::CompOsInstance;
-use crate::odrefresh;
use android_system_composd::aidl::android::system::composd::{
ICompilationTask::ICompilationTask, ICompilationTaskCallback::ICompilationTaskCallback,
};
use android_system_composd::binder::{Interface, Result as BinderResult, Strong};
-use anyhow::Result;
+use anyhow::{bail, Context, Result};
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::ICompOsService;
use compos_common::odrefresh::ExitCode;
use log::{error, warn};
+use rustutils::system_properties;
+use std::fs::{File, OpenOptions};
+use std::os::unix::fs::OpenOptionsExt;
+use std::os::unix::io::AsRawFd;
+use std::path::Path;
use std::sync::{Arc, Mutex};
use std::thread;
+const ART_APEX_DATA: &str = "/data/misc/apexdata/com.android.art";
+
#[derive(Clone)]
pub struct OdrefreshTask {
running_task: Arc<Mutex<Option<RunningTask>>>,
@@ -43,6 +52,12 @@
}
}
+struct RunningTask {
+ callback: Strong<dyn ICompilationTaskCallback>,
+ #[allow(dead_code)] // Keeps the CompOS VM alive
+ comp_os: Arc<CompOsInstance>,
+}
+
impl OdrefreshTask {
/// Return the current running task, if any, removing it from this CompilationTask.
/// Once removed, meaning the task has ended or been canceled, further calls will always return
@@ -67,7 +82,7 @@
fn start_thread(self, service: Strong<dyn ICompOsService>, target_dir_name: String) {
thread::spawn(move || {
- let exit_code = odrefresh::run_in_vm(service, &target_dir_name);
+ let exit_code = run_in_vm(service, &target_dir_name);
let task = self.take();
// We don't do the callback if cancel has already happened.
@@ -91,8 +106,42 @@
}
}
-struct RunningTask {
- callback: Strong<dyn ICompilationTaskCallback>,
- #[allow(dead_code)] // Keeps the CompOS VM alive
- comp_os: Arc<CompOsInstance>,
+fn run_in_vm(service: Strong<dyn ICompOsService>, target_dir_name: &str) -> Result<ExitCode> {
+ let staging_dir = open_dir(composd_native::palette_create_odrefresh_staging_directory()?)?;
+ let system_dir = open_dir(Path::new("/system"))?;
+ let output_dir = open_dir(Path::new(ART_APEX_DATA))?;
+
+ // Spawn a fd_server to serve the FDs.
+ let fd_server_config = FdServerConfig {
+ ro_dir_fds: vec![system_dir.as_raw_fd()],
+ rw_dir_fds: vec![staging_dir.as_raw_fd(), output_dir.as_raw_fd()],
+ ..Default::default()
+ };
+ let fd_server_raii = fd_server_config.into_fd_server()?;
+
+ let zygote_arch = system_properties::read("ro.zygote")?;
+ let exit_code = service.odrefresh(
+ system_dir.as_raw_fd(),
+ output_dir.as_raw_fd(),
+ staging_dir.as_raw_fd(),
+ target_dir_name,
+ &zygote_arch,
+ )?;
+
+ drop(fd_server_raii);
+ if let Some(exit_code) = ExitCode::from_i32(exit_code.into()) {
+ Ok(exit_code)
+ } else {
+ bail!("odrefresh exited with {}", exit_code)
+ }
+}
+
+/// Returns an owned FD of the directory. It currently returns a `File` as a FD owner, but
+/// it's better to use `std::os::unix::io::OwnedFd` once/if it becomes standard.
+fn open_dir(path: &Path) -> Result<File> {
+ OpenOptions::new()
+ .custom_flags(libc::O_DIRECTORY)
+ .read(true) // O_DIRECTORY can only be opened with read
+ .open(path)
+ .with_context(|| format!("Failed to open {:?} directory as path fd", path))
}
diff --git a/compos/composd/src/service.rs b/compos/composd/src/service.rs
index 6ce462c..23c411b 100644
--- a/compos/composd/src/service.rs
+++ b/compos/composd/src/service.rs
@@ -19,7 +19,6 @@
use crate::compilation_task::CompilationTask;
use crate::instance_manager::InstanceManager;
-use crate::odrefresh;
use crate::odrefresh_task::OdrefreshTask;
use crate::util::to_binder_result;
use android_system_composd::aidl::android::system::composd::{
@@ -71,11 +70,6 @@
check_permissions()?;
to_binder_result(self.do_start_async_odrefresh(callback))
}
-
- fn startTestOdrefresh(&self) -> binder::Result<i8> {
- check_permissions()?;
- to_binder_result(self.do_odrefresh_for_test())
- }
}
impl IsolatedCompilationService {
@@ -113,17 +107,6 @@
Ok(BnCompilationTask::new_binder(task, BinderFeatures::default()))
}
-
- fn do_odrefresh_for_test(&self) -> Result<i8> {
- let compos = self
- .instance_manager
- .start_test_instance()
- .context("Starting CompOS for odrefresh test")?;
- let service = compos.get_service();
-
- let exit_code = odrefresh::run_in_vm(service, "test-artifacts")?;
- Ok(exit_code as i8)
- }
}
fn check_permissions() -> binder::Result<()> {
diff --git a/compos/composd_cmd/composd_cmd.rs b/compos/composd_cmd/composd_cmd.rs
index 37d5378..41e2b1a 100644
--- a/compos/composd_cmd/composd_cmd.rs
+++ b/compos/composd_cmd/composd_cmd.rs
@@ -34,9 +34,11 @@
fn main() -> Result<()> {
let app = clap::App::new("composd_cmd").arg(
- clap::Arg::with_name("command").index(1).takes_value(true).required(true).possible_values(
- &["staged-apex-compile", "forced-compile-test", "forced-odrefresh", "async-odrefresh"],
- ),
+ clap::Arg::with_name("command")
+ .index(1)
+ .takes_value(true)
+ .required(true)
+ .possible_values(&["staged-apex-compile", "forced-compile-test", "async-odrefresh"]),
);
let args = app.get_matches();
let command = args.value_of("command").unwrap();
@@ -46,7 +48,6 @@
match command {
"staged-apex-compile" => run_staged_apex_compile()?,
"forced-compile-test" => run_forced_compile_for_test()?,
- "forced-odrefresh" => run_forced_odrefresh_for_test()?,
"async-odrefresh" => run_async_odrefresh_for_test()?,
_ => panic!("Unexpected command {}", command),
}
@@ -155,11 +156,3 @@
}
}
}
-
-fn run_forced_odrefresh_for_test() -> Result<()> {
- let service = wait_for_interface::<dyn IIsolatedCompilationService>("android.system.composd")
- .context("Failed to connect to composd service")?;
- let compilation_result = service.startTestOdrefresh().context("Compilation failed")?;
- println!("odrefresh exit code: {:?}", compilation_result);
- Ok(())
-}