Skip to content

Commit 5165678

Browse files
anpCQ Bot
anp
authored and
CQ Bot
committed
[starnix][ffx] Move adb proxy code to a subcommand.
Makes it possible to keep the current workflow available while iterating on a direct connection workflow. Bug: 342237307 Change-Id: If3a684863f8babd6bed2d302d23423a1cfc5346b Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1052923 Reviewed-by: Kevin Lindkvist <lindkvist@google.com> Commit-Queue: Adam Perry <adamperry@google.com> Fuchsia-Auto-Submit: Adam Perry <adamperry@google.com> Reviewed-by: Steven Grady <slgrady@google.com>
1 parent 158f2b1 commit 5165678

File tree

5 files changed

+178
-134
lines changed

5 files changed

+178
-134
lines changed

‎src/developer/ffx/tests/cli-goldens/goldens/ffx/starnix/adb.golden‎

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"name": "adb",
33
"description": "Bridge from host adb to adbd running inside starnix",
44
"examples": [
5-
"ffx starnix adb"
5+
"ffx starnix adb proxy"
66
],
77
"flags": [
88
{
@@ -13,30 +13,6 @@
1313
"description": "display usage information",
1414
"hidden": false
1515
},
16-
{
17-
"kind": {
18-
"Option": {
19-
"arg_name": "moniker"
20-
}
21-
},
22-
"optionality": "optional",
23-
"long": "--moniker",
24-
"short": "m",
25-
"description": "the moniker of the container running adbd (defaults to looking for a container in the current session)",
26-
"hidden": false
27-
},
28-
{
29-
"kind": {
30-
"Option": {
31-
"arg_name": "port"
32-
}
33-
},
34-
"optionality": "optional",
35-
"long": "--port",
36-
"short": "p",
37-
"description": "which port to serve the adb server on",
38-
"hidden": false
39-
},
4016
{
4117
"kind": {
4218
"Option": {
@@ -48,14 +24,6 @@
4824
"short": null,
4925
"description": "path to the adb client command",
5026
"hidden": false
51-
},
52-
{
53-
"kind": "Switch",
54-
"optionality": "optional",
55-
"long": "--no-autoconnect",
56-
"short": null,
57-
"description": "disable automatically running \"adb connect\"",
58-
"hidden": false
5927
}
6028
],
6129
"notes": [],

‎src/developer/ffx/tests/cli-goldens/goldens/ffx/starnix/adb/proxy.golden‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
{
2+
"name": "proxy",
3+
"description": "Bridge from host adb to adbd running inside starnix",
4+
"examples": [],
5+
"flags": [
6+
{
7+
"kind": "Switch",
8+
"optionality": "optional",
9+
"long": "--help",
10+
"short": null,
11+
"description": "display usage information",
12+
"hidden": false
13+
},
14+
{
15+
"kind": {
16+
"Option": {
17+
"arg_name": "moniker"
18+
}
19+
},
20+
"optionality": "optional",
21+
"long": "--moniker",
22+
"short": "m",
23+
"description": "the moniker of the container running adbd (defaults to looking for a container in the current session)",
24+
"hidden": false
25+
},
26+
{
27+
"kind": {
28+
"Option": {
29+
"arg_name": "port"
30+
}
31+
},
32+
"optionality": "optional",
33+
"long": "--port",
34+
"short": "p",
35+
"description": "which port to serve the adb server on",
36+
"hidden": false
37+
},
38+
{
39+
"kind": "Switch",
40+
"optionality": "optional",
41+
"long": "--no-autoconnect",
42+
"short": null,
43+
"description": "disable automatically running \"adb connect\"",
44+
"hidden": false
45+
}
46+
],
47+
"notes": [],
48+
"commands": [],
49+
"positionals": [],
50+
"error_codes": []
51+
}

‎src/developer/ffx/tests/cli-goldens/goldens/starnix_filelist.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
ffx/starnix.golden
22
ffx/starnix/adb.golden
3+
ffx/starnix/adb/proxy.golden
34
ffx/starnix/console.golden
45
ffx/starnix/resume.golden
56
ffx/starnix/suspend.golden

‎src/developer/ffx/tools/starnix/src/adb.rs‎

Lines changed: 124 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
use anyhow::{Context, Result};
66
use argh::{ArgsInfo, FromArgs};
77
use async_net::{Ipv4Addr, TcpListener, TcpStream};
8-
use fho::{Connector, SimpleWriter};
8+
use fho::Connector;
99
use fidl_fuchsia_developer_remotecontrol as rc;
1010
use fidl_fuchsia_starnix_container as fstarcontainer;
1111
use fuchsia_async as fasync;
@@ -24,6 +24,35 @@ use crate::common::*;
2424

2525
const ADB_DEFAULT_PORT: u32 = 5555;
2626

27+
#[derive(ArgsInfo, FromArgs, Debug, PartialEq)]
28+
#[argh(
29+
subcommand,
30+
name = "adb",
31+
example = "ffx starnix adb proxy",
32+
description = "Bridge from host adb to adbd running inside starnix"
33+
)]
34+
pub struct StarnixAdbCommand {
35+
/// path to the adb client command
36+
#[argh(option, default = "String::from(\"adb\")")]
37+
adb: String,
38+
#[argh(subcommand)]
39+
subcommand: AdbSubcommand,
40+
}
41+
42+
#[derive(ArgsInfo, FromArgs, Debug, PartialEq)]
43+
#[argh(subcommand)]
44+
enum AdbSubcommand {
45+
Proxy(AdbProxyArgs),
46+
}
47+
48+
impl StarnixAdbCommand {
49+
pub async fn run(&self, rcs_connector: &Connector<rc::RemoteControlProxy>) -> Result<()> {
50+
match &self.subcommand {
51+
AdbSubcommand::Proxy(args) => args.run_proxy(&self.adb, rcs_connector).await,
52+
}
53+
}
54+
}
55+
2756
async fn serve_adb_connection(mut stream: TcpStream, bridge_socket: fidl::Socket) -> Result<()> {
2857
let mut bridge = fidl::AsyncSocket::from_socket(bridge_socket);
2958
let (breader, mut bwriter) = (&mut bridge).split();
@@ -41,32 +70,6 @@ async fn serve_adb_connection(mut stream: TcpStream, bridge_socket: fidl::Socket
4170
copy_result.map(|_| ()).map_err(|e| e.into())
4271
}
4372

44-
#[derive(ArgsInfo, FromArgs, Debug, PartialEq)]
45-
#[argh(
46-
subcommand,
47-
name = "adb",
48-
example = "ffx starnix adb",
49-
description = "Bridge from host adb to adbd running inside starnix"
50-
)]
51-
pub struct StarnixAdbCommand {
52-
/// the moniker of the container running adbd
53-
/// (defaults to looking for a container in the current session)
54-
#[argh(option, short = 'm')]
55-
pub moniker: Option<String>,
56-
57-
/// which port to serve the adb server on
58-
#[argh(option, short = 'p', default = "find_open_port(5556)")]
59-
pub port: u16,
60-
61-
/// path to the adb client command
62-
#[argh(option, default = "String::from(\"adb\")")]
63-
pub adb: String,
64-
65-
/// disable automatically running "adb connect"
66-
#[argh(switch)]
67-
pub no_autoconnect: bool,
68-
}
69-
7073
fn find_open_port(start: u16) -> u16 {
7174
let mut addr = SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), start);
7275

@@ -88,86 +91,109 @@ fn find_open_port(start: u16) -> u16 {
8891
}
8992
}
9093

91-
pub async fn starnix_adb(
92-
command: &StarnixAdbCommand,
93-
rcs_connector: &Connector<rc::RemoteControlProxy>,
94-
_writer: SimpleWriter,
95-
) -> Result<()> {
96-
let reconnect = || async {
97-
let rcs_proxy = connect_to_rcs(rcs_connector).await?;
98-
anyhow::Ok((
99-
connect_to_contoller(&rcs_proxy, command.moniker.clone()).await?,
100-
Arc::new(AtomicBool::new(false)),
101-
))
102-
};
103-
let mut controller_proxy = reconnect().await?;
104-
105-
let mut signals = Signals::new(&[SIGINT]).unwrap();
106-
let handle = signals.handle();
107-
let signal_thread = std::thread::spawn(move || {
108-
if let Some(signal) = signals.forever().next() {
109-
assert_eq!(signal, SIGINT);
110-
eprintln!("Caught interrupt. Shutting down starnix adb bridge...");
111-
std::process::exit(0);
112-
}
113-
});
114-
115-
let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, command.port))
116-
.await
117-
.expect("cannot bind to adb address");
118-
let listen_address = listener.local_addr().expect("cannot get adb server address");
119-
120-
if !command.no_autoconnect {
121-
// It's necessary to run adb connect on a separate thread so it doesn't block the async
122-
// executor running the socket listener.
123-
let adb_path = command.adb.clone();
124-
std::thread::spawn(move || {
125-
let mut adb_command = Command::new(&adb_path);
126-
adb_command.arg("connect").arg(listen_address.to_string());
127-
match adb_command.status() {
128-
Ok(_) => {}
129-
Err(io_err) if io_err.kind() == ErrorKind::NotFound => {
130-
panic!("Could not find adb binary named `{adb_path}`. If your adb is not in your $PATH, use the --adb flag to specify where to find it.");
131-
}
132-
Err(io_err) => {
133-
panic!("Failed to run `${adb_command:?}`: {io_err:?}");
134-
}
94+
#[derive(ArgsInfo, FromArgs, Debug, PartialEq)]
95+
#[argh(
96+
subcommand,
97+
name = "proxy",
98+
description = "Bridge from host adb to adbd running inside starnix"
99+
)]
100+
struct AdbProxyArgs {
101+
/// the moniker of the container running adbd
102+
/// (defaults to looking for a container in the current session)
103+
#[argh(option, short = 'm')]
104+
pub moniker: Option<String>,
105+
106+
/// which port to serve the adb server on
107+
#[argh(option, short = 'p', default = "find_open_port(5556)")]
108+
pub port: u16,
109+
110+
/// disable automatically running "adb connect"
111+
#[argh(switch)]
112+
pub no_autoconnect: bool,
113+
}
114+
115+
impl AdbProxyArgs {
116+
async fn run_proxy(
117+
&self,
118+
adb: &str,
119+
rcs_connector: &Connector<rc::RemoteControlProxy>,
120+
) -> Result<()> {
121+
let reconnect = || async {
122+
let rcs_proxy = connect_to_rcs(rcs_connector).await?;
123+
anyhow::Ok((
124+
connect_to_contoller(&rcs_proxy, self.moniker.clone()).await?,
125+
Arc::new(AtomicBool::new(false)),
126+
))
127+
};
128+
let mut controller_proxy = reconnect().await?;
129+
130+
let mut signals = Signals::new(&[SIGINT]).unwrap();
131+
let handle = signals.handle();
132+
let signal_thread = std::thread::spawn(move || {
133+
if let Some(signal) = signals.forever().next() {
134+
assert_eq!(signal, SIGINT);
135+
eprintln!("Caught interrupt. Shutting down starnix adb bridge...");
136+
std::process::exit(0);
135137
}
136138
});
137-
} else {
138-
println!("ADB bridge started. To connect: adb connect {listen_address}");
139-
}
140139

141-
while let Some(stream) = listener.incoming().next().await {
142-
if controller_proxy.1.load(Ordering::SeqCst) {
143-
controller_proxy = reconnect().await?;
140+
let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, self.port))
141+
.await
142+
.expect("cannot bind to adb address");
143+
let listen_address = listener.local_addr().expect("cannot get adb server address");
144+
145+
if !self.no_autoconnect {
146+
// It's necessary to run adb connect on a separate thread so it doesn't block the async
147+
// executor running the socket listener.
148+
let adb_path = adb.to_string();
149+
std::thread::spawn(move || {
150+
let mut adb_command = Command::new(&adb_path);
151+
adb_command.arg("connect").arg(listen_address.to_string());
152+
match adb_command.status() {
153+
Ok(_) => {}
154+
Err(io_err) if io_err.kind() == ErrorKind::NotFound => {
155+
panic!("Could not find adb binary named `{adb_path}`. If your adb is not in your $PATH, use the --adb flag to specify where to find it.");
156+
}
157+
Err(io_err) => {
158+
panic!("Failed to run `${adb_command:?}`: {io_err:?}");
159+
}
160+
}
161+
});
162+
} else {
163+
println!("ADB bridge started. To connect: adb connect {listen_address}");
144164
}
145165

146-
let stream = stream?;
147-
let (sbridge, cbridge) = fidl::Socket::create_stream();
166+
while let Some(stream) = listener.incoming().next().await {
167+
if controller_proxy.1.load(Ordering::SeqCst) {
168+
controller_proxy = reconnect().await?;
169+
}
148170

149-
controller_proxy
150-
.0
151-
.vsock_connect(fstarcontainer::ControllerVsockConnectRequest {
152-
port: Some(ADB_DEFAULT_PORT),
153-
bridge_socket: Some(sbridge),
154-
..Default::default()
171+
let stream = stream?;
172+
let (sbridge, cbridge) = fidl::Socket::create_stream();
173+
174+
controller_proxy
175+
.0
176+
.vsock_connect(fstarcontainer::ControllerVsockConnectRequest {
177+
port: Some(ADB_DEFAULT_PORT),
178+
bridge_socket: Some(sbridge),
179+
..Default::default()
180+
})
181+
.context("connecting to adbd")?;
182+
183+
let reconnect_flag = Arc::clone(&controller_proxy.1);
184+
fasync::Task::spawn(async move {
185+
serve_adb_connection(stream, cbridge)
186+
.await
187+
.unwrap_or_else(|e| println!("serve_adb_connection returned with {:?}", e));
188+
reconnect_flag.store(true, std::sync::atomic::Ordering::SeqCst);
155189
})
156-
.context("connecting to adbd")?;
190+
.detach();
191+
}
157192

158-
let reconnect_flag = Arc::clone(&controller_proxy.1);
159-
fasync::Task::spawn(async move {
160-
serve_adb_connection(stream, cbridge)
161-
.await
162-
.unwrap_or_else(|e| println!("serve_adb_connection returned with {:?}", e));
163-
reconnect_flag.store(true, std::sync::atomic::Ordering::SeqCst);
164-
})
165-
.detach();
193+
handle.close();
194+
signal_thread.join().expect("signal thread to shutdown without panic");
195+
Ok(())
166196
}
167-
168-
handle.close();
169-
signal_thread.join().expect("signal thread to shutdown without panic");
170-
Ok(())
171197
}
172198

173199
#[cfg(test)]

‎src/developer/ffx/tools/starnix/src/lib.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,7 @@ impl FfxMain for StarnixTool {
4747
async fn main(self, writer: Self::Writer) -> Result<()> {
4848
match &self.cmd.subcommand {
4949
StarnixSubCommand::Adb(command) => {
50-
adb::starnix_adb(command, &self.rcs_connector, writer)
51-
.await
52-
.map_err(|e| Error::User(e))
50+
command.run(&self.rcs_connector).await.map_err(|e| Error::User(e))
5351
}
5452
#[cfg(feature = "enable_console_tool")]
5553
StarnixSubCommand::Console(command) => {

0 commit comments

Comments
 (0)